On 07/19/18 20:11, Jeff Law wrote: > On 07/19/2018 12:04 PM, Bernd Edlinger wrote: >> Hi, >> >> >> this fixes a few minor nits, which I spotted while >> looking at the string folding functions: >> >> string_constant: Remove impossible check: TREE_CODE (arg) >> can't be COMPONENT_REF and MEM_REF at the same time. > Shouldn't they all be != tests? Though clearly this code isn't being > tested by anything. >
I think a COMPONENT_REF would be possible for strlen(&a.b). It looks like that removing this check will be the best option. Unless Martin has a different idea of course. > >> >> c_strlen: maxelts is (signed) HOST_WIDE_INT, therefore >> use tree_to_shwi. > One could argue maxelts should be unsigned. > I would agree, but a few lines later I see: /* Offset from the beginning of the string in elements. */ HOST_WIDE_INT eltoff; /* We have a known offset into the string. Start searching there for a null character if we can represent it as a single HOST_WIDE_INT. */ if (byteoff == 0) eltoff = 0; else if (! tree_fits_shwi_p (byteoff)) eltoff = -1; else eltoff = tree_to_shwi (byteoff) / eltsize; /* If the offset is known to be out of bounds, warn, and call strlen at runtime. */ if (eltoff < 0 || eltoff > maxelts) { This would be doing signed/unsigned comparisons then. Maybe that is the reason why ? >> >> c_getstr: tree_to_uhwi needs to be protected by >> tree_fits_uhwi_p. > Looks correct to me. > > >> >> BTW: c_getstr uses string_constant which appears to be >> able to extract wide char string initializers, but c_getstr >> does not seem to be prepared for wide char strings: >> >> else if (string[string_length - 1] != '\0') >> { >> /* Support only properly NUL-terminated strings but handle >> consecutive strings within the same array, such as the six >> substrings in "1\0002\0003". */ >> return NULL; >> } > Seems like a goof to me. > Well, maybe this could be a gcc_assert instead. Normal strings should always be zero-terminated, even wide character strings. Anyways probably for another time. Bernd.