On 08/25/2016 01:02 PM, Jeff Law wrote:
On 08/21/2016 02:29 PM, Martin Sebor wrote:
On 08/20/2016 01:26 AM, Jakub Jelinek wrote:
On Fri, Aug 19, 2016 at 04:30:47PM -0600, Martin Sebor wrote:
The patch looks bigger than it actually is because:
1) It modifies the return type of the function to bool rather than
unsigned HOST_WIDE_INT representing the object size (this was
necessary to avoid having its callers misinterpret zero as
unknown when it means zero bytes).
Can you explain why do you need this? I don't understand why do you
need to
differentiate between unknown and maximum (or minimum for modes 2 and
3 that
nobody actually uses in real-world), the builtin after all returns the
same
value for both. If you want to know if the compiler knows the size
precisely, you can request both mode 0 (or 1) and 2 (or 3) and
compare, if
the values are the same, it is the exact size, if there is a range,
then you
have minimum and maximum (and, if minimum is 0, but maximum non-zero,
you
really don't know minimum, if maximum is -1, then you really don't
know the
maximum (no object should be better that big). For the return value, I
don't see how you could reliably differentiate between the two even
if it
made for whatever strange reason sense - for SSA_NAMEs etc. you have
just
recorded the sizes, not also a flag whether it is unknown or known.
The change makes it possible to fold into constants even at -O0 type
2 and 3 calls to the built-in with zero-sized objects.
This refers to the tests in builtin-object-size-17.c, particularly when
compiled at -O0, right?
That's right.
We generally haven't worried much about trying to optimize -O0 under the
theory that the resulting code should as closely match the source as
closely as possible -- including leaving dead/unreachable code in the
resulting .o. That dead/unreachable code may in fact be useful during a
debugging session when the developer manually changes the state into a
"can't happen" state and expects the previously dead/unreachable code to
execute.
Sure, I understand that. It just seemed that if the function folded
non-zero sizes it should also fold the zero. Otherwise the zero size
would not be available to callers like -Wformat-length (at -O0),
causing them to miss potentially important instances of buffer overflow.
But that's a bit of a stretch in my mind and doesn't really apply in
this case due to how __b_o_s is expanded. Ultimately we end up with
conditionals with compile-time constant arguments, so there's no way for
the programmer to twiddle things (ok, maybe on a target where the
compare and branch are separate, the developer could put a break on the
branch, change the condition codes... but that's *really* a stretch in
my mind)
So I don't see any reason to object to this part of the patch --
essentially it falls under the "don't generate stupid code at -O0" in my
mind.
Great, thanks.
Martin
