On 08/17/18 06:46, Jeff Law wrote: > On 08/05/2018 04:28 AM, Bernd Edlinger wrote: >> Hi, >> >> I would like to do a minor tweak to the patch. >> While staring at the other patch I realized that I should >> better pass size and not thissize to the check >> function, instead of making use of how thissize is >> computed using MIN above. This means that this condition >> >> + if ((unsigned)len != size && (unsigned)len != size + elts) >> + return false; >> >> should better and more readable be: >> >> + if (size < (unsigned)len && size != len - elts) >> + return false; >> >> Furthermore I wanted to have the check function static, >> which I missed to do previously. >> >> For completeness, these are again the precondition patches >> for this patch: >> >> [PATCH] Handle overlength strings in the C FE >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00142.html > I believe Joseph ack'd this already: > > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00655.html > >
Yes. >> >> [PATCH] Make GO string literals properly NUL terminated >> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01931.html > Ian didn't object, deferring to the larger scoped review. My > understanding is this should be a nop for Go. Given it takes us closer > to have a canonical form, I'll go ahead and ACK for the trunk. > > Yes, it should be a no-op, eventually the strings will go into the string merge section, with the follow-up patch. Everything works nicely, as I always bootstrap/regtest with GO. >> >> [PATCH] [Ada] Make middle-end string literals NUL terminated >> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01929.html > This is OK. > > >> >> [PATCH] Create internally nul terminated string literals in fortan FE >> https://gcc.gnu.org/ml/fortran/2018-08/msg00000.html > This is OK too. THough it is unclear why we're not fixing > gfc_build_string_const vs introducing gfc_build_hollerith_string_const. > Are hollerith strings naturally not terminated in the fortran front-end > and thus need special handling, while other calls to > gfc_build_string_const are always with naturally nul terminated strings? > > I think all other strings are null terminated, except the hollerith strings. > > >> >> >> Bootstrapped, as usual. >> Is it OK for trunk? > So as a toplevel comment. While there may be some interaction with > Martin's code to detect and warn for unterminated strings passed to > strlen and other cases, I think the overall goal of canonicalizing our > internal representation of STRING_CST along with a verification step is > a good thing and it may make some of Martin's work easier. > > At this point are the prereqs of the varasm verification patch all ACK'd? > Yes, there is a JIT code gen bug, that was caught by the assertion in my patch: [PATCH] Fix not properly nul-terminated string constants in JIT https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00370.html That is a simple bug, which means, that the JIT FE was never able to assemble strings > 200 character length. Fixing a bug like that makes me I wonder, if there is any test coverage for JIT except the gcc test suite itself. Thanks Bernd.
