Re: Another bug for __builtin_object_size? (Or expected behavior)

[Qing Zhao via Gcc-patches]

> On Aug 17, 2023, at 5:32 PM, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
> 
> On 2023-08-17 17:25, Qing Zhao wrote:
>>> It's not exactly the same issue, the earlier discussion was about choosing 
>>> sizes in the same pass while the current one is about choosing between 
>>> passes, but I agree it "rhymes".  This is what I was alluding to originally 
>>> (for OST_MINIMUM use MIN_EXPR if both passes returned a pass) but I haven't 
>>> thought about it hard enough to be 100% confident that it's the better 
>>> solution, especially for OST_MAXIMUM.
>> We have two different sources to get SIZE information for the subobject:
>> 1. From the TYPESIZE information embedded in the IR;
>> 2. From the initialization information propagated from data flow, this 
>> includes both malloc call and the DECL_INIT.
>> We need to choose between these two when both available, (these two 
>> information could be
>>  in the same pass as we discussed before, or in different passes which is 
>> shown in this discussion).
>> I think that the MIN_EXPR might be the right choice (especially for 
>> OST_MAXIMUM) -:)
> 
> It's worth a shot I guess.  We could emit something like the following in 
> early_object_sizes_execute_one:
> 
>  sz = (__bos(o->sub, ost) == unknown
>        ? early_size
>        : MIN_EXPR (__bos(o->sub, ost), early_size));
> 
> and see if it sticks.

I came up with the following change for tree-object-size.cc:

diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index a62af0500563..e1b2008c6dcc 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -2016,10 +2016,22 @@ do_valueize (tree t)
   return t;
 }
 
-/* Process a __builtin_object_size or __builtin_dynamic_object_size call in
-   CALL early for subobjects before any object information is lost due to
-   optimization.  Insert a MIN or MAX expression of the result and
-   __builtin_object_size at I so that it may be processed in the second pass.
+/* Process a __builtin_object_size or __builtin_dynamic_object_size call
+   early for subobjects before any object information is lost due to
+   optimization.
+
+   We have two different sources to get the size information for subobjects:
+   A. The TYPE information of the subobject in the IR;
+   B. The initialization information propagated through data flow.
+   In the early pass, only A is available.
+   B might be available in the second pass.
+
+   If both A and B are available, we should choose the minimum one between
+   these two.
+
+   Insert a MIN expression of the result from the early pass and the original
+   __builtin_object_size call at I so that it may be processed in the second 
pass.
+
    __builtin_dynamic_object_size is treated like __builtin_object_size here
    since we're only looking for constant bounds.  */
 
@@ -2036,7 +2048,7 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, 
gimple *call)
   unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
   tree ptr = gimple_call_arg (call, 0);
 
-  if (object_size_type != 1 && object_size_type != 3)
+  if (object_size_type & OST_SUBOBJECT == 0)
     return;
 
   if (TREE_CODE (ptr) != ADDR_EXPR && TREE_CODE (ptr) != SSA_NAME)
@@ -2050,9 +2062,8 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, 
gimple *call)
 
   tree tem = make_ssa_name (type);
   gimple_call_set_lhs (call, tem);
-  enum tree_code code = object_size_type & OST_MINIMUM ? MAX_EXPR : MIN_EXPR;
   tree cst = fold_convert (type, bytes);
-  gimple *g = gimple_build_assign (lhs, code, tem, cst);
+  gimple *g = gimple_build_assign (lhs, MIN_EXPR, tem, cst);
   gsi_insert_after (i, g, GSI_NEW_STMT);
   update_stmt (call);
 }

Let me know if you see any issue with the change.

thanks.

Qing

> 
> Thanks,
> Sid