[committed] libstdc++: Replace operator>>(istream&, char*) [LWG 2499]

[Jonathan Wakely]

On 06/08/20 12:12 +0200, Jakub Jelinek wrote:
>On Wed, Aug 05, 2020 at 04:31:08PM -0600, Martin Sebor via Gcc-patches wrote:
>> I've always found the second argument to __builtin_object_size
>> confusing for types above 1.  I don't see anything wrong in
>> the diff but I believe the most useful results are with type 1
>> for string functions and type 0 for raw memory functions like
>> memcpy (that's what _FORTIFY_SOURCE uses for the two sets of
>> functions).  In type 2 when the result is zero it means one of
>> two things: either the size of the array couldn't be determined
>> or it really is zero.  That's less than helpful in cases like:
>>
>>   char a[8];
>>   strcpy (a + 8, s);
>>
>> where it prevents detecting the buffer overflow.
>
>I don't know what is confusing about it.

Personally I find the docs very confusing.

"The second bit determines if maximum or minimum of remaining bytes is
computed. "

OK, so is it maximum when the bit is set of maximum when the bit is
clear?

To answer that question I have to go back to the middle of the
previous paragraph and carefully parse it. 

"the returned number is the maximum of remaining byte counts in those
objects if type & 2 is 0 and minimum if nonzero."

This part talks about "if type & 2 is 0" and "nonzero", could we be
consistent and talk about a bit being clear/set, or use bitwise
operator notation, but not flip between the two? And use zero/nonzero
rather than 0/nonzero?

The inconsistency in presentation increases the mental load of parsing
it. I'll propose a patch for those docs when I get time.


>With the 0/1 arguments bos returns an upper bound for the object size
>(and the don't know value is the maximum in that case, i.e. (size_t)-1),
>while with 2/3 arguments bos returns an lower bound for the object size
>(and thus the don't know value is the minimum value, i.e. 0).
>The 2/3 modes are obviously not something you want to use in strcpy etc.
>implementation, in those cases you want to abort the program only when
>it is guaranteed to be invalid, i.e. when it will certainly overflow
>the available size in any case, while with the 2/3 modes it would abort already
>if there is a possibility the object might not be big enough.

For my case I'm not aborting, I'm deciding whether to use the result
from __builtin_object_size or just assume the array is as large as the
entire address space (which is the old behaviour).

I think Martin's right that I should use 0. Technically I could
probably use 1, because for struct S { char buf1[2]; char buf2[2]; };
it would be undefined to write 4 bytes into it, but it "worked" with
previous versions and so I'm choosing to let it keep "working". This
doesn't need to be 100% safe, because the API has been replaced by a
safer one for C++20 anyway.

>One can e.g. use both modes to check if the object is known to have exactly
>a particular size, when
>__builtin_object_size (ptr, 0) == __builtin_object_size (ptr, 2)
>and the bos returns say 25, then you know it is exactly 25 bytes.
>E.g. if one has:
>  ptr = flag ? malloc (32) : malloc (64);
>  x[0] = __builtin_object_size (ptr, 0);
>  x[1] = __builtin_object_size (ptr, 2);
>then x[0] will be 64 as the maximum and x[1] to 32 as the minimum (of course
>unless flag can be folded to constant, then both would be the same depending
>on to which constant it is folded).
>
>	Jakub