This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Check the STRING_CSTs in varasm.c
On 08/17/18 06:46, Jeff Law wrote:
> On 08/05/2018 04:28 AM, Bernd Edlinger wrote:
>> 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
> I believe Joseph ack'd this already:
>> [PATCH] Make GO string literals properly NUL terminated
> 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
> This is OK.
>> [PATCH] Create internally nul terminated string literals in fortan FE
> 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] Fix not properly nul-terminated string constants in JIT
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.