One update, last Friday, I merged all my patches for counted-by support (including the Patch to workaround the LTO issue) with the latest trunk, bootstrapped and run the testing, everything is good.
Today, when I disabled the Patch that workaround the LTO issue, surprisingly, I cannot repeat the LTO issue anymore with the latest trunk + my counted-by support patches. I.e., without the LTO workaround, everything works just fine with the latest gcc. I suspected that some change in the latest GCC “fixed” (or hide) the issue. Qing > On Jan 22, 2024, at 9:52 AM, Qing Zhao <qing.z...@oracle.com> wrote: > > > >> On Jan 22, 2024, at 2:40 AM, Richard Biener <richard.guent...@gmail.com> >> wrote: >> >> On Fri, Jan 19, 2024 at 5:26 PM Qing Zhao <qing.z...@oracle.com> wrote: >>> >>> >>> >>>> 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? >> >> It's an interesting question - I don't see where the gimplifier would >> need to access DECL/TYPE_SIZE > > My bad, the above simple example did not expose the LTO error. Just used to > show the IR generated for the array access. > > The LTO error only happens with multi-dimension array, whose inner dimensions > are VLA. And the outermost dimension is flexible array member. Like > following: > > > 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; > } > > And the IR for the routine “setup_and_test_vla” is: > > { > typedef struct foo struct struct foo; > struct foo * f; > > SAVE_EXPR <n1>;, SAVE_EXPR <n2>;;, { > typedef struct foo struct struct foo; > }; > struct foo * f; > f = (struct foo *) malloc (0, SAVE_EXPR <n1>;, SAVE_EXPR <n2>;;, ((long > unsigned int) m * (long unsigned int) ((sizetype) SAVE_EXPR <n1> * (sizetype) > SAVE_EXPR <n2>) + 1) * 4;); > f->n = m; > (*(.ACCESS_WITH_SIZE ((int[0:(sizetype) ((long int) SAVE_EXPR <n2> + > -1)][0:(sizetype) ((long int) SAVE_EXPR <n1> + -1)] *) &f->p, &f->n, 1, 32, > -1) + (sizetype) (((long unsigned int) m * (long unsigned int) ((sizetype) > SAVE_EXPR <n1> * (sizetype) SAVE_EXPR <n2>)) * 4)))[n2][n1] = 1; > return; > } > > > >> so the mistake, if any, should be that you need to unshare the size >> expressions you are using as >> argument to .ACCESS_WITH_SIZE? > > Hm, I tried that in the beginning, but didn’t work. I will check again. > >> Mind, you are replacing an ARRAY_REF >> with a pointer indirection >> as well > > Yes, this is for resolving a very early gimplification issue as I reported > last Nov: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638793.html > > Since no-one responded at that time, I fixed the issue by replacing the > ARRAY_REF > With a pointer indirection: > https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639605.html > > The reason for such change is: return a flexible array member TYPE is not > allowed > by C language (our gimplification follows this rule), so, we have to return a > pointer TYPE instead. > > >> - IMO we shouldn't replace accesses this way but instead make >> it possible for analysis to >> discover the base/size values? > > Yes, if keeping the ARRAY_REF, then bound sanitizer instrumentation will be > much simpler since the INDEX > Information is kept in the TYPE NODE of the ARRAY_REF. > However, due to the above reason, we have to replace the ARRAY_REF with a > pointer INDIRECT_REF. > Such change made the bound sanitizer more difficult due to the INDEX was lost > when the ARRAY_REF was > Converted to the INDIRECT_REF. > > I resolved this issue by adding a new argument to .ACCESS_WITH_SIZE to record > the INDEX when converting > The ARRAY_REF to INDIRECT_REF during C FE. > > Let me know if you have any comment. > > Thanks. > > Qing >> >>> 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
