> On Jan 17, 2024, at 1:43 AM, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> On Wed, Jan 17, 2024 at 7:42 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
>>
>> On Tue, Jan 16, 2024 at 9:26 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>>
>>>
>>>
>>>> On Jan 15, 2024, at 4:31 AM, Richard Biener <richard.guent...@gmail.com>
>>>> wrote:
>>>>
>>>>> All my questions for unshare_expr relate to a LTO bug that I currently
>>>>> stuck with
>>>>> when using .ACCESS_WITH_SIZE in bound sanitizer (only with -flto, without
>>>>> -flto, no issue):
>>>>>
>>>>> [opc@qinzhao-aarch64-ol8 gcc]$ sh t
>>>>> during IPA pass: modref
>>>>> t.c:20:1: internal compiler error: tree code ‘ssa_name’ is not supported
>>>>> in LTO streams
>>>>> 0x14c3993 lto_write_tree
>>>>> ../../latest-gcc-write/gcc/lto-streamer-out.cc:561
>>>>> 0x14c3aeb lto_output_tree_1
>>>>>
>>>>> And the value of the tree node that triggered the ICE is:
>>>>> (gdb) call debug_tree(expr)
>>>>> <ssa_name 0xfffff5761e60 type <error_mark 0xfffff56c0e58>
>>>>> nothrow
>>>>> def_stmt
>>>>> version:13 in-free-list>
>>>>>
>>>>> Is there any good way to debug LTO bug?
>>>>
>>>> This happens usually when you have a VLA type and its type fields are not
>>>> properly gimplified which usually happens because the frontend fails to
>>>> insert a gimplification point for it (a DECL_EXPR).
>>>
>>> I found an old gcc bug
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97172
>>> ICE: tree code ‘ssa_name’ is not supported in LTO streams since
>>> r11-3303-g6450f07388f9fe57
>>>
>>> Which is very similar to the bug I am having right now.
>>>
>>> After further study, I suspect that the issue I am having right now with
>>> the LTO streaming also
>>> relate to “unshare_expr”, “save_expr”, and the combination of these two, I
>>> suspect that
>>> the current gcc cannot handle the combination of these two correctly for my
>>> case.
>>>
>>> My testing case is:
>>>
>>> #include <stdlib.h>
>>> void __attribute__((__noinline__)) setup_and_test_vla (int n1, int n2, int
>>> m)
>>> {
>>> struct foo {
>>> int n;
>>> int p[][n2][n1] __attribute__((counted_by(n)));
>>> } *f;
>>>
>>> f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2][n1]));
>>> f->n = m;
>>> f->p[m][n2][n1]=1;
>>> return;
>>> }
>>>
>>> int main(int argc, char *argv[])
>>> {
>>> setup_and_test_vla (10, 11, 20);
>>> return 0;
>>> }
>>>
>>> Failed with
>>> my_gcc -Os -fsanitize=bounds -flto
>>>
>>> If changing either n1 or n2 to a constant, the testing passed.
>>> If deleting -flto, the testing passed too.
>>>
>>> I double checked my code per the suggestions provided by you and Jakub in
>>> this
>>> email thread, and I think the code should be fine.
>>>
>>> The code is following:
>>>
>>> =====
>>> 504 /* Instrument array bounds for INDIRECT_REFs whose pointers are
>>> 505 POINTER_PLUS_EXPRs of calls to .ACCESS_WITH_SIZE. We create special
>>> 506 builtins that gets expanded in the sanopt pass, and make an array
>>> 507 dimension of it. ARRAY is the pointer to the base of the array,
>>> 508 which is a call to .ACCESS_WITH_SIZE, *OFFSET is the offset to the
>>> 509 beginning of array.
>>> 510 Return NULL_TREE if no instrumentation is emitted. */
>>> 511
>>> 512 tree
>>> 513 ubsan_instrument_bounds_indirect_ref (location_t loc, tree array, tree
>>> *offset)
>>> 514 {
>>> 515 if (!is_access_with_size_p (array))
>>> 516 return NULL_TREE;
>>> 517 tree bound = get_bound_from_access_with_size (array);
>>> 518 /* The type of the call to .ACCESS_WITH_SIZE is a pointer type to
>>> 519 the element of the array. */
>>> 520 tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array)));
>>> 521 gcc_assert (bound);
>>> 522
>>> 523 /* Given the offset, and the size of each element, the index can be
>>> 524 computed as: offset/element_size. */
>>> 525 *offset = save_expr (*offset);
>>> 526 tree index = fold_build2 (EXACT_DIV_EXPR,
>>> 527 sizetype, *offset,
>>> 528 unshare_expr (element_size));
>>> 529 /* Create a "(T *) 0" tree node to describe the original array type.
>>> 530 We get the original array type from the first argument of the call
>>> to
>>> 531 .ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, num_bytes, -1).
>>> 532
>>> 533 Originally, REF is a COMPONENT_REF with the original array type,
>>> 534 it was converted to a pointer to an ADDR_EXPR, and the ADDR_EXPR's
>>> 535 first operand is the original COMPONENT_REF. */
>>> 536 tree ref = CALL_EXPR_ARG (array, 0);
>>> 537 tree array_type
>>> 538 = unshare_expr (TREE_TYPE (TREE_OPERAND (TREE_OPERAND(ref, 0), 0)));
>>> 539 tree zero_with_type = build_int_cst (build_pointer_type (array_type),
>>> 0);
>>> 540 return build_call_expr_internal_loc (loc, IFN_UBSAN_BOUNDS,
>>> 541 void_type_node, 3,
>>> zero_with_type,
>>> 542 index, bound);
>>> 543 }
>>>
>>> =====
>>>
>>> Inside gdb, the guilty IR failed in LTO streaming is from the above line
>>> 520:
>>> TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array))),
>>>
>>> When I use this tree node as an operand of the expression at line 526, I
>>> added
>>> unshare_expr.
>>>
>>> However, I still see the guilty IR as in gdb:
>>>
>>> unit-size <mult_expr 0xfffff5aabf90 type <integer_type
>>> 0xfffff57c0000 sizetype>
>>> side-effects
>>> arg:0 <mult_expr 0xfffff5aabf68 type <integer_type
>>> 0xfffff57c0000 sizetype>
>>>
>>> arg:0 <ssa_name 0xfffff5761e18 type <error_mark
>>> 0xfffff56c0e58>
>>> nothrow
>>> def_stmt
>>> version:12 in-free-list>
>>> arg:1 <ssa_name 0xfffff5761e60 type <error_mark
>>> 0xfffff56c0e58>
>>> nothrow
>>> def_stmt
>>> version:13 in-free-list>>
>>> arg:1 <integer_cst 0xfffff56c10c8 constant 4>>
>>>
>>>
>>> I have been stuck with this bug for quite some time.
>>> Any help is helpful.
>>
>> The above hasn't been gimplified correctly, you'd instead see
>> a D.1234 in there, not an expression with SSA names. That happens
>> when the frontend fails to emit a DECL_EXPR for a decl with this
>> type.
>
> .. which then also results in missing unsharing of this expression
> (so the SSA names leak in)
Thanks a lot for the hints.
One correction first, the LTO bug is not related to -fsanitize=bounds.
Deleting -fsanitize=bounds still can
repeat the failure.
After further debugging into the gimplification phase related with the
SAVE_EXPR, I finally locate the place
where the unshareing of the expression is missing. This is in the routine
“pointer_int_sum” of c-family/c-common.cc:
3330 {
3331 if (!complain && !COMPLETE_TYPE_P (TREE_TYPE (result_type)))
3332 return error_mark_node;
3333 size_exp = size_in_bytes_loc (loc, TREE_TYPE (result_type));
3334 /* Wrap the pointer expression in a SAVE_EXPR to make sure it
3335 is evaluated first when the size expression may depend
3336 on it for VM types. */
3337 if (TREE_SIDE_EFFECTS (size_exp)
3338 && TREE_SIDE_EFFECTS (ptrop)
3339 && variably_modified_type_p (TREE_TYPE (ptrop), NULL))
3340 {
3341 ptrop = save_expr (ptrop);
3342 size_exp = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop,
size_exp);
3343 }
3344 }
In the above, at line 3333, the tree node, TYPE_SIZE_UNIT
(TREE_TYPE(result_type)), is returned directly as
the size_exp,
(gdb) call debug_tree(size_exp)
<mult_expr 0xfffff5a6f910
type <integer_type 0xfffff57c0000 sizetype public unsigned DI
size <integer_cst 0xfffff56c0e70 constant 64>
unit-size <integer_cst 0xfffff56c0e88 constant 8>
align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0xfffff57c0000 precision:64 min <integer_cst 0xfffff56c0ea0 0> max <integer_cst
0xfffff56d05e0 18446744073709551615>>
side-effects
arg:0 <mult_expr 0xfffff5a6f8e8 type <integer_type 0xfffff57c0000 sizetype>
side-effects
arg:0 <nop_expr 0xfffff56dc540 type <integer_type 0xfffff57c0000
sizetype>
side-effects
arg:0 <save_expr 0xfffff56dc4c0 type <integer_type 0xfffff57c05e8
int>
side-effects arg:0 <parm_decl 0xfffff76b6f80 n1>>>
arg:1 <nop_expr 0xfffff56dc600 type <integer_type 0xfffff57c0000
sizetype>
side-effects
arg:0 <save_expr 0xfffff56dc580 type <integer_type 0xfffff57c05e8
int>
side-effects arg:0 <parm_decl 0xfffff76b7000 n2>>>>
arg:1 <integer_cst 0xfffff56c10c8 type <integer_type 0xfffff57c0000
sizetype> constant 4>>
Without unshare_expr to this size_exp, the above TYPE_SIZE_UNIT node containing
SAVE_EXPRs
is gimpflified to expressions with SSA_NAME during gimplification. (This is
unaccepted by LTO).
Adding an unshare_expr (size_exp) resolved this problem.
Although I still think that there might be potential issue with the
gimpflication of SAVE_EXPRs, I dare not
to modify that part of the code.
At this moment, I will add unshare_expr to the routine “pointer_int_sum” to
workaround this issue.
Let me know if you have any comment and suggestion.
Thanks a lot.
Qing
>>
>>>
>>> Qing
>>>
>>>>
>>>>> Thanks a lot for the help.
>>>>>
>>>>> Qing
