[PATCH] Make strlen range computations more conservative

[Jeff Law]

On 07/25/2018 01:08 AM, Richard Biener wrote:
>>
>> So ISTM that you really need a stronger justification using the
>> standards compliance and/or real world code that is made less safe by
>> keeping string lengths as accurate as possible.
> 
> Note you cannot solely look at what the C standard says.  Instead
> you have to see where the middle-end lessens that constraints since
> these functions are not only called from C FE context.
Agreed.  I guess my point WRT standards was to start with C/C++ and see
if they rule in/out either case.

If (for example) we could make a determination that either standard
didn't allow us to narrow the length of a string after a cast in the way
the code currently does, then that would make the current code simply
wrong and we should either revert those bits or use Bernd's patches in
that space.

If the language standards don't give us guidance of that nature, then we
look at common uses as well as our own GIMPLE/GENERIC (unwritten)
semantics to guide us one way or the other.


> 
> So arguing from a C language lawyer point here is pointless.  You
> have to argue from a GENERIC language lawyer point which is going
> to be impossible since apart from the implementation (which
> includes how all existing GCC FEs _and_ the middle-end uses it)
> there is no formal specification available.  Yes, in most cases
> we say we match C language behavior and constraints but in some
> cases we are clearly and definitely less strict and that has
> followup consequences in other areas.
> 
> While exploiting all fine details of the C language might get
> you more constraints on things like string lengths you have to
> apply some common sense and middle-end knowledge - which means
> from my side trusting hunch feelings whether something is
> possibly not safe.
I'm absolutely not suggesting we want to exploit all the finer language
details here.  The language standards are just one piece of the puzzle IMHO.


> 
> As for patches in this area I would really love to see _smaller_
> changes.  And I'd like to see changes that make it clear
> what cases where supposed to be handled and _not_ including
> other cases "by accident".  See especially my comments on this patch
> from Bernd.
Agreed.


> 
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>> I'd like to ask we hold on this until I return from PTO (Aug 1) so that
>> we can discuss the best thing to do here for each class of change.
>>
>> I think you, Martin, Richi and myself should hash through the technical
>> issues raised by the patch.  Obviously others can chime in, but I think
>> the 4 of us probably need to drive the discussion.
> 
> I think with respect to patches to fix issues in previous patches
> at this point a better option might be to revert the patches causing
> the issues and start from scratch in a more defined manner.
I wouldn't object to that.  I think there's enough concerns here that we
ought to slow down and re-visit the reasoning behind each change and
make sure it's sensible.  If the best way to do that is revert, break
the patches down and resubmit, then let's do that.

Mostly what I wanted to do was give us time to hash through the changes
and decide what the best option was without having two developers
changing, then reverting bits of each other's work.  To that end, again,
I'm open to reverting Martin's work and having it resubmitted as we has
through each issue.

My general preference is to lean towards more accurate length
information (mostly to drive accuracy in warnings elsewhere), but that
obviously has to be within the constraints of the language standards,
common practice, as well as our own internal semantics.  If we need to
pull back in some spaces, that's fine, but we should clearly document
when/how we're doing that and if possible make it consistent throughout
the compiler if at all possible.

I'm also open to the concept of splitting this stuff up a bit in the
sense of what we'll use for optimization vs what we'll use to increase
accuracy of our other warnings such as sprintf).



> 
> Giving recent (temporary) regressions in the testsuite it feels like
> Martin is going too fast.
ACK.

jeff