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. > Let me know if you have any comment and suggestion. > > Thanks a lot. > > Qing > > >> > >>> > >>> Qing > >>> > >>>> > >>>>> Thanks a lot for the help. > >>>>> > >>>>> Qing > >
