On Fri, Sep 20, 2024 at 12:40:26PM -0400, Siddhesh Poyarekar wrote:
When wholesize != size, there is a reasonable opportunity for static
object sizes also to be computed using size_for_offset, so use that.
gcc/ChangeLog:
* tree-object-size.cc (plus_stmt_object_size): Call
SIZE_FOR_OFFSET for some negative offset cases.
* testsuite/gcc.dg/builtin-object-size-3.c (test9): Adjust test.
* testsuite/gcc.dg/builtin-object-size-4.c (test8): Likewise.
---
gcc/testsuite/gcc.dg/builtin-object-size-3.c | 6 +++---
gcc/testsuite/gcc.dg/builtin-object-size-4.c | 6 +++---
gcc/tree-object-size.cc | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
index 3f58da3d500..ec2c62c9640 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
@@ -574,7 +574,7 @@ test9 (unsigned cond)
if (__builtin_object_size (&p[-4], 2) != (cond ? 6 : 10))
FAIL ();
#else
- if (__builtin_object_size (&p[-4], 2) != 0)
+ if (__builtin_object_size (&p[-4], 2) != 6)
FAIL ();
#endif
@@ -585,7 +585,7 @@ test9 (unsigned cond)
if (__builtin_object_size (p, 2) != ((cond ? 2 : 6) + cond))
FAIL ();
#else
- if (__builtin_object_size (p, 2) != 0)
+ if (__builtin_object_size (p, 2) != 2)
FAIL ();
#endif
@@ -598,7 +598,7 @@ test9 (unsigned cond)
!= sizeof (y) - __builtin_offsetof (struct A, c) - 8 + cond)
FAIL ();
#else
- if (__builtin_object_size (p, 2) != 0)
+ if (__builtin_object_size (p, 2) != sizeof (y) - __builtin_offsetof (struct
A, c) - 8)
FAIL ();
#endif
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
index b3eb36efb74..7bcd24c4150 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
@@ -482,7 +482,7 @@ test8 (unsigned cond)
if (__builtin_object_size (&p[-4], 3) != (cond ? 6 : 10))
FAIL ();
#else
- if (__builtin_object_size (&p[-4], 3) != 0)
+ if (__builtin_object_size (&p[-4], 3) != 6)
FAIL ();
#endif
@@ -493,7 +493,7 @@ test8 (unsigned cond)
if (__builtin_object_size (p, 3) != ((cond ? 2 : 6) + cond))
FAIL ();
#else
- if (__builtin_object_size (p, 3) != 0)
+ if (__builtin_object_size (p, 3) != 2)
FAIL ();
#endif
@@ -505,7 +505,7 @@ test8 (unsigned cond)
if (__builtin_object_size (p, 3) != sizeof (y.c) - 8 + cond)
FAIL ();
#else
- if (__builtin_object_size (p, 3) != 0)
+ if (__builtin_object_size (p, 3) != sizeof (y.c) - 8)
FAIL ();
#endif
}
The testcase changes look reasonable to me.
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 6544730e153..f8fae0cbc82 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -1527,7 +1527,7 @@ plus_stmt_object_size (struct object_size_info *osi, tree
var, gimple *stmt)
if (size_unknown_p (bytes, 0))
;
else if ((object_size_type & OST_DYNAMIC)
- || compare_tree_int (op1, offset_limit) <= 0)
+ || bytes != wholesize || compare_tree_int (op1, offset_limit) <=
0)
bytes = size_for_offset (bytes, op1, wholesize);
/* In the static case, with a negative offset, the best estimate for
minimum size is size_unknown but for maximum size, the wholesize is a
The coding conventions say that in cases like this where the whole condition
doesn't fit on a single line, each ||/&& operand should be on a separate
line.
So, the patch should be adding || bytes != wholesize on a separate line.
That said, there is a pre-existing problem, the tree direct comparisons
(bytes != wholesize here, && wholesize != sz in size_for_offset (note,
again, it should be on a separate line), maybe others).
We do INTEGER_CST caching, either using small array for small values or
hash table for larger ones, so INTEGER_CSTs with the same value of the
same type should be pointer equal unless they are TREE_OVERFLOW or similar,
but for anything else, unless you guarantee that in the "same" case
you assign the same tree to size/wholesize rather than say
perform size_binop twice, I'd expect instead comparisons with
operand_equal_p or something similar.
Though, because this patch is solely for the __builtin_object_size case
and the sizes in that case should be solely INTEGER_CSTs, I guess this patch
is ok with the formatting nit fix (and ideally the one in size_for_offset
too).
