> On Jan 19, 2024, at 4:30 AM, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> On Thu, Jan 18, 2024 at 3:46 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>>
>>
>>> 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.
>
> It's only a workaround mind you. The bug is that the frontend fails
> to emit a DECL_EXPR which would
> trigger both unsharing and proper gimplification of the type size.
For a simple testing case:
$ cat test.c
struct annotated {
unsigned int foo;
char b;
int array[] __attribute__((counted_by (foo)));
};
extern struct annotated * alloc_buf (int index);
static void bar ()
{
struct annotated *p2 = alloc_buf (10);
p2->array[11] = 0;
return;
}
The C FE generates the following IR:
[opc@qinzhao-ol8u3-x86 108896]$ cat test.c.005t.original
;; Function bar (null)
;; enabled by -tree-original
{
struct annotated * p2 = alloc_buf (10);
struct annotated * p2 = alloc_buf (10);
*(.ACCESS_WITH_SIZE ((int *) &p2->array, &p2->foo, 1, 32, -1) + 44) = 0;
return;
}
Do you see any obvious IR issue in the above? Do I miss to generate any
DECL_EXPR in the above IR?
Thanks.
Qing
I compared it with the following testing case without the “counted-by”
annotation
and use an user-defined “access_with_size” function, The IR looks like exactly
the same:
$ cat test_1.c
struct annotated {
unsigned int foo;
char b;
int array[];
};
extern struct annotated *alloc_buf (int);
extern int *access_with_size (int * ref, unsigned int * size, int a, int b, int
c);
static void bar ()
{
struct annotated *p2 = alloc_buf (10);
access_with_size ((int *) &p2->array, &p2->foo, 1, 32, -1)[11] = 0;
return;
}
[opc@qinzhao-ol8u3-x86 108896]$ cat test_1.c.005t.original
;; Function bar (null)
;; enabled by -tree-original
{
struct annotated * p2 = alloc_buf (10);
struct annotated * p2 = alloc_buf (10);
*(access_with_size ((int *) &p2->array, &p2->foo, 1, 32, -1) + 44) = 0;
return;
}
>
>> Let me know if you have any comment and suggestion.
>>
>> Thanks a lot.
>>
>> Qing
>>
>>>>
>>>>>
>>>>> Qing
>>>>>
>>>>>>
>>>>>>> Thanks a lot for the help.
>>>>>>>
>>>>>>> Qing
