On 2025-06-20 11:26, Qing Zhao wrote:
the mentioned code change:
+ else if (TREE_CODE (rhs) == MEM_REF
+ && POINTER_TYPE_P (TREE_TYPE (rhs))
+ && TREE_CODE (TREE_OPERAND (rhs, 0)) == SSA_NAME)
+ reexamine = merge_object_sizes (osi, var, TREE_OPERAND (rhs, 0));
is mainly for the following IR pattern that is common when the object size is
queried
for a pointer with “counted_by” attribute:
_1 = .ACCESS_WITH_SIZE (_3, _4, 1, 0, -1, 0B);
_5 = *_1; ===========> this is the GIMPLE_ASSIGN stmt that the above
code handles.
_6 = __builtin_dynamic_object_size (_5, 1);
Yes, this is a minimal and necessary change for using the counted_by of
pointers in __bdos.
Hope this is clear.
I will add comments to the above change.
Thanks for the clarification.
Actually, I wonder if this is incomplete. We should be able to get the object
size even if TREE_OPERAND (rhs, 0) is not an SSA_NAME, like we do in
addr_object_size:
```
if (TREE_CODE (pt_var) == MEM_REF)
{
tree sz, wholesize;
if (!osi || (object_size_type & OST_SUBOBJECT) != 0
|| TREE_CODE (TREE_OPERAND (pt_var, 0)) != SSA_NAME)
{
compute_builtin_object_size (TREE_OPERAND (pt_var, 0),
object_size_type & ~OST_SUBOBJECT, &sz);
wholesize = sz;
}
else
{
tree var = TREE_OPERAND (pt_var, 0);
if (osi->pass == 0)
collect_object_sizes_for (osi, var);
if (bitmap_bit_p (computed[object_size_type],
SSA_NAME_VERSION (var)))
{
sz = object_sizes_get (osi, SSA_NAME_VERSION (var));
wholesize = object_sizes_get (osi, SSA_NAME_VERSION (var), true);
}
else
sz = wholesize = size_unknown (object_size_type);
}
if (!size_unknown_p (sz, object_size_type))
sz = size_for_offset (sz, TREE_OPERAND (pt_var, 1), wholesize);
if (!size_unknown_p (sz, object_size_type)
&& (TREE_CODE (sz) != INTEGER_CST
|| compare_tree_int (sz, offset_limit) < 0))
{
pt_var_size = sz;
pt_var_wholesize = wholesize;
}
}
```
Maybe refactor this to handle MEM_REF and update addr_object_size (and
collect_object_sizes_for for the gimple_assign_single_p case) to use it?
However, that's a more general change and we'd probably need a new test case to
validate it.
Yes, I guess that this might be a general improvement to the current __bdos. I
can do it later in a separate patch. Do you have a good testing case for this?
I don't, it was just a theoretical possibility I came up with while
looking at your patch.
I won't block on this though, your minimal change is a step forward, maybe
just add a comment about this?
Yeah, I will add comments to the current change in tree-object-size.cc
<http://tree-object-size.cc/> and also update the testing cases per you
suggested.
Thank you!
Sid
