[PATCH] Fix the damage done by my other patch from yesterday to strlenopt-49.c

Richard Biener rguenther@suse.de
Mon Jul 30 17:33:00 GMT 2018


On July 30, 2018 4:41:19 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>
>
>On 07/30/18 15:03, Richard Biener wrote:
>> On Mon, 30 Jul 2018, Bernd Edlinger wrote:
>> 
>>> Hi,
>>>
>>> this is how I would like to handle the over length strings issue in
>the C FE.
>>> If the string constant is exactly the right length and ends in one
>explicit
>>> NUL character, shorten it by one character.
>>>
>>> I thought Martin would be working on it,  but as this is a really
>simple fix,
>>> I would dare to send it to gcc-patches anyway, hope you don't
>mind...
>>>
>>> The patch is relative to the other patch here:
>https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>> 
>> I'll leave this to FE maintainers but can I ask you to verify the
>> (other) FEs do not leak this kind of invalid initializers to the
>> middle-end?  I suggest to put this verification in
>> output_constructor which otherwise happily truncates initializers
>> with excess size.  There's also gimplification which might elide
>> a = { "abcd", "cdse" }; to  a.x = "abcd"; a.y = "cdse"; but
>> hopefully there the GIMPLE verifier (verify_gimple_assign_single)
>> verifies this - well, it only dispatches to useless_type_conversion_p
>> (lhs_type, rhs1_type) for this case, but non-flexarrays should be
>> handled fine there.
>> 
>> Richard.
>> 
>
>In the moment I would already be happy if all STRING_CSTs would
>be zero terminated.
>
>However Go does not create zero-terminated STRING_CSTs, @Ian sorry,
>could you look at changing this to include the terminating NUL char?
>
>Index: gcc/go/go-gcc.cc
>===================================================================
>--- gcc/go/go-gcc.cc	(revision 263068)
>+++ gcc/go/go-gcc.cc	(working copy)
>@@ -1394,7 +1394,7 @@ Gcc_backend::string_constant_expression(const
>std:
>  					      TYPE_QUAL_CONST);
>    tree string_type = build_array_type(const_char_type, index_type);
>    TYPE_STRING_FLAG(string_type) = 1;
>-  tree string_val = build_string(val.length(), val.data());
>+  tree string_val = build_string(val.length() + 1, val.data());
>    TREE_TYPE(string_val) = string_type;
>  
>    return this->make_expression(string_val);
>
>
>A am pretty sure that the C++ FE allows overlength initializers
>with -permissive.  They should be hedged in string_constant IMHO,
>however with the patch I am still holding back on Jeff's request
>I ran over a string constant in tree-ssa-strlen.c
>(get_min_string_length)
>that had a terminating NUL char but the index range type did not
>include the string terminator.  One just needs to be careful here.
>
>A quick survey shows that Fortran creates C strings with range
>1..n, which puts the pr86532 address computation again in question.
>Remember, you asked for array_ref_element_size but not for
>array_ref_low_bound, and Jeff acked the patch in this state.

The (earlier) changes in this area are unfortunately in a quite messy state. It would be nice to roll back completely and attack this in smaller and more obvious issues. 

Richard. 

>
>
>Thanks
>Bernd.



More information about the Gcc-patches mailing list