This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Make strlen range computations more conservative
On 08/23/18 00:34, Jeff Law wrote:
> On 08/22/2018 11:22 AM, Bernd Edlinger wrote:
>> On 08/22/18 18:05, Martin Sebor wrote:
>>> On 08/21/2018 10:05 PM, Bernd Edlinger wrote:
>>>> On 08/22/18 00:25, Jeff Law wrote:
>>>>> On 08/21/2018 02:43 AM, Richard Biener wrote:
>>>>>> On Mon, 20 Aug 2018, Bernd Edlinger wrote:
>>>>> [ snip. ]
>>>>>
>>>>>>> Yes, I found some peanuts on my way.
>>>>>>>
>>>>>>> For instance this fix for PR middle-end/86121 survives bootstrap on
>>>>>>> it's own, and fixes one xfail.
>>>>>>>
>>>>>>> Is it OK for trunk?
>>>>>>
>>>>>> Yes, that's OK for trunk.
>>>>> Agreed. Seems like a nice independent bugfix and I don't think it
>>>>> adversely affects anything else under current discussion. In fact, not
>>>>> folding here makes it easier to warn about incorrect code elsewhere.
>>>>>
>>>>>>
>>>>>>>> Btw, I don't think we want sth like
>>>>>>>> flag_assume_zero_terminated_char_arrays or even make it default at
>>>>>>>> -Ofast.
>>>>>>>>
>>>>>>>
>>>>>>> Yes, I agree. Is there a consensus about this?
>>>>>>
>>>>>> Well, it's my own opinion of course. Show me a benchmark that
>>>>>> improves with -fassume-zero-terminated-char-arrays. Certainly
>>>>>> for security reasons it sounds a dangerous thing (and the documentation
>>>>>> needs a more thorough description of what it really means).
>>>>> I certainly don't want to see a flag. We've already got way too many;
>>>>> adding another for marginal behavior just seems wrong.
>>>>>
>>>>>>
>>>>>>> If yes, I go ahead and remove that option again.
>>>>>>>
>>>>>>> BTW: I needed this option in a few test cases, that insist in checking the
>>>>>>> optimizer to eliminate stuff, based on the VRP info. (6 +/-1 or so).
>>>>>>
>>>>>> Any example?
>>>>>>
>>>>>>> But we can as well remove those test cases.
>>>>> Bernd, if there are specific tests that you want to see removed, we
>>>>> should discuss them.
>>>>>
>>>>
>>>> The test cases are:
>>>> gcc.dg/strlenopt-36.c
>>>
>>> There are plenty of valid test cases in this test. For example:
>>>
>>> extern char a7[7];
>>> if (strlen (a7) >= 7) // fold to false
>>> abort ();
>>>
>>> Even if we wanted to accommodate common definitions the array
>>> declarations could be changed to static and the tests would
>>> be useful:
>>>
>>> static char a7[7];
>>>
>>> There is no valid program where the if condition could be true.
>>>
>>>> gcc.dg/strlenopt-40.c
>>>
>>> There are even more completely uncontroversial test cases here,
>>> such as:
>>>
>>> if (strlen (i < 0 ? "123" : "4321") > 4) // fold to false
>>> abort ();
>>>
>>
>> I see, the trouble is that the test case mixes valid cases with
>> cases that depend on type info in GIMPLE.
> I believe Martin's point is that there are tests within those files that
> are still valid. We don't want to zap the entire test unless all the
> subtests are invalid. We need to look at each sub-test and determine if
> it's valid or not.
>
I can try to just keep test cases like the above, and delete
what doen't work, especially in this test case that seems doable.
Bernd.