From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x130.google.com (mail-lf1-x130.google.com [IPv6:2a00:1450:4864:20::130]) by sourceware.org (Postfix) with ESMTPS id 303C13858C2F for ; Mon, 22 Jan 2024 07:41:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 303C13858C2F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 303C13858C2F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705909315; cv=none; b=bE6hlm1fHxFSSnWL9G4FYSXMAWn5i0yK27ByQYFUAvM62o9nQDyCtF5ofdhICndchS0XbentYXgoIC6lrV71GVOfSVGlPaF0OX8Q7ZrV+3tsfHB4V5+m6ebRsVbt0Fr76uVu2JQR7E8s96nImoW4aOqJeCwJm/J2jwkhHLBhLy4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705909315; c=relaxed/simple; bh=pxOKblPfPEwBJ0ZOn2fmaP9Hu+GLQZm6HOdKn/YB/T4=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Ho7OzjY15YI5KgoUrOL60qBJEu4fva9pN3E0nCm19Fbj41vCYVIqc0P8z4GjtsocVg4T/h39keLTvftwInq4v0jZl8HzctJl89Ofo1c1PeeBzwnsu7Dv2zLvoQ1eVSlM9Bm31fb7jnEj2icAkT8tjNCF+SucyIpqBTvT53Iq+0E= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x130.google.com with SMTP id 2adb3069b0e04-50eab4bf47aso2042991e87.0 for ; Sun, 21 Jan 2024 23:41:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705909311; x=1706514111; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=rHayXe8vct9r6AjXZgVXNr9BCrOJeFicPHK+Cf22bLg=; b=DWIQddQQx8BO2vrwNlh8E2Kyp11s29w9M0dXZtitECpVMLPLGx1bgWUJx3314Zm/rA prXpIms1wMiCfLjC3yILsU4JXdAnqt6T2c89YtWwTNM5qWQouZlAiuwMCE6raemakkRQ duVKDUUHsWW5gcQO6QeaVS+8kxfiRaV3Cgh+oCNOEbLHy1g0bfiAuuxNJ7BH8F1JlJHP FjcZpuuMRWXyoZmmSUocZYDh5vrCLEXFOz3eRSqb5WjaIN3xSxFlCHqocwKfHZmGrlOK V4QpZahR0XpP9fX+iniy+3XSF/fBNT7vhihGVtZfS6Kmxc33xMgZaw76GM7Whcy0EsXk F/jg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705909311; x=1706514111; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rHayXe8vct9r6AjXZgVXNr9BCrOJeFicPHK+Cf22bLg=; b=Y2U7GnmAKH8q+0CyHkPSi5ou6TFC3u90n2zyPefZrpkSZ0U97uU9KzDHvOL7+h4Tcy 8m7yGHywG4jliXK87rJW7sRti3MFcBJz1HZJEMjM8Kt33Xo4aBO5C4M8ylACTEb8xIjN N1Kk9zyZDQ3pe8M93maDrrwwI5bxaT9wXQHxLRwJJfxcQ8YF38JO9/SQcF3pAcqxKpo3 glacxnGcm+Jobt5RY7Es9mYUAwA7dXNh3ZSj2EWpUp8veCpMbpH18nInj4fvb7tzXcqe 1S4kn3PEET3SR29rZZ7xFF/v3ML/zbMbHjhpb/GuvZtWuMW3do1CpJ47IkQ9z6EFVwsM ev7g== X-Gm-Message-State: AOJu0YwWPz6TkEzs7rb3JBnUwM8xtFpmw0K8CG6DzH1kDSTIcBDtdzB3 +4wRBjmVluV+Vsr54un7vlEMhYgRqcjvddQpt/H/9Yh4PdBhrR/oJPr1MZRwFWtUOIO2sCavI3s PmaO5rYS+XLQPJJUF+MbcapdDuBOXYDJ8 X-Google-Smtp-Source: AGHT+IFYvnO0wypc3IKejyrOEUYmZxrwiyJRpdJUUbNOvDpP88LjbHnEvsMvSxrMOMnWxYa0zb3eHt6qLGoYwa9xY00= X-Received: by 2002:a05:6512:3f23:b0:50f:c24:39f8 with SMTP id y35-20020a0565123f2300b0050f0c2439f8mr2749228lfa.6.1705909311223; Sun, 21 Jan 2024 23:41:51 -0800 (PST) MIME-Version: 1.0 References: <50C8811D-9C2A-4FFB-9FC5-D24C5A76F868@oracle.com> <70C34042-B741-4697-9524-396CB9D40DF8@gmail.com> <4D91B027-0B34-4DB4-9338-1B9DE6E2EB00@oracle.com> <63DEF4CB-18DA-48C5-842B-B743EF365B24@oracle.com> <97EDB4A5-1A68-4D46-A060-F550AEB2812A@oracle.com> In-Reply-To: <97EDB4A5-1A68-4D46-A060-F550AEB2812A@oracle.com> From: Richard Biener Date: Mon, 22 Jan 2024 08:40:36 +0100 Message-ID: Subject: Re: HELP: Questions on unshare_expr To: Qing Zhao Cc: "jakub@redhat.com" , gcc Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Jan 19, 2024 at 5:26=E2=80=AFPM Qing Zhao wr= ote: > > > > > On Jan 19, 2024, at 4:30 AM, Richard Biener wrote: > > > > On Thu, Jan 18, 2024 at 3:46=E2=80=AFPM Qing Zhao wrote: > >> > >> > >> > >>> On Jan 17, 2024, at 1:43 AM, Richard Biener wrote: > >>> > >>> On Wed, Jan 17, 2024 at 7:42=E2=80=AFAM Richard Biener > >>> wrote: > >>>> > >>>> On Tue, Jan 16, 2024 at 9:26=E2=80=AFPM Qing Zhao wrote: > >>>>> > >>>>> > >>>>> > >>>>>> On Jan 15, 2024, at 4:31 AM, Richard Biener wrote: > >>>>>> > >>>>>>> All my questions for unshare_expr relate to a LTO bug that I cur= rently 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 =E2=80=98ssa_name=E2= =80=99 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) > >>>>>>> > >>>>>>> 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 fai= ls 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=3D97172 > >>>>> ICE: tree code =E2=80=98ssa_name=E2=80=99 is not supported in LTO s= treams 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 =E2=80=9Cunshare_expr=E2=80=9D, =E2=80=9Csave_expr=E2=80= =9D, and the combination of these two, I suspect that > >>>>> the current gcc cannot handle the combination of these two correctl= y for my case. > >>>>> > >>>>> My testing case is: > >>>>> > >>>>> #include > >>>>> 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 =3D (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2]= [n1])); > >>>>> f->n =3D m; > >>>>> f->p[m][n2][n1]=3D1; > >>>>> return; > >>>>> } > >>>>> > >>>>> int main(int argc, char *argv[]) > >>>>> { > >>>>> setup_and_test_vla (10, 11, 20); > >>>>> return 0; > >>>>> } > >>>>> > >>>>> Failed with > >>>>> my_gcc -Os -fsanitize=3Dbounds -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 Ja= kub in this > >>>>> email thread, and I think the code should be fine. > >>>>> > >>>>> The code is following: > >>>>> > >>>>> =3D=3D=3D=3D=3D > >>>>> 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 ar= ray, > >>>>> 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 arra= y, tree *offset) > >>>>> 514 { > >>>>> 515 if (!is_access_with_size_p (array)) > >>>>> 516 return NULL_TREE; > >>>>> 517 tree bound =3D get_bound_from_access_with_size (array); > >>>>> 518 /* The type of the call to .ACCESS_WITH_SIZE is a pointer typ= e to > >>>>> 519 the element of the array. */ > >>>>> 520 tree element_size =3D TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (a= rray))); > >>>>> 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 =3D save_expr (*offset); > >>>>> 526 tree index =3D fold_build2 (EXACT_DIV_EXPR, > >>>>> 527 sizetype, *offset, > >>>>> 528 unshare_expr (element_size)); > >>>>> 529 /* Create a "(T *) 0" tree node to describe the original arra= y 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 ADD= R_EXPR's > >>>>> 535 first operand is the original COMPONENT_REF. */ > >>>>> 536 tree ref =3D CALL_EXPR_ARG (array, 0); > >>>>> 537 tree array_type > >>>>> 538 =3D unshare_expr (TREE_TYPE (TREE_OPERAND (TREE_OPERAND(ref= , 0), 0))); > >>>>> 539 tree zero_with_type =3D build_int_cst (build_pointer_type (ar= ray_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 } > >>>>> > >>>>> =3D=3D=3D=3D=3D > >>>>> > >>>>> 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 5= 26, I added > >>>>> unshare_expr. > >>>>> > >>>>> However, I still see the guilty IR as in gdb: > >>>>> > >>>>> unit-size > >>>>> side-effects > >>>>> arg:0 > >>>>> > >>>>> arg:0 > >>>>> nothrow > >>>>> def_stmt > >>>>> version:12 in-free-list> > >>>>> arg:1 > >>>>> nothrow > >>>>> def_stmt > >>>>> version:13 in-free-list>> > >>>>> arg:1 > > >>>>> > >>>>> > >>>>> 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=3Dbound= s. Deleting -fsanitize=3Dbounds 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 ro= utine =E2=80=9Cpointer_int_sum=E2=80=9D 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 =3D 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 =3D save_expr (ptrop); > >> 3342 size_exp =3D 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(r= esult_type)), is returned directly as > >> the size_exp, > >> > >> (gdb) call debug_tree(size_exp) > >> >> type >> size > >> unit-size > >> align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-ty= pe 0xfffff57c0000 precision:64 min max > > >> side-effects > >> arg:0 > >> side-effects > >> arg:0 > >> side-effects > >> arg:0 > >> side-effects arg:0 >> > >> arg:1 > >> side-effects > >> arg:0 > >> side-effects arg:0 >>> > >> arg:1 constant 4>> > >> > >> > >> Without unshare_expr to this size_exp, the above TYPE_SIZE_UNIT node c= ontaining SAVE_EXPRs > >> is gimpflified to expressions with SSA_NAME during gimplification. (T= his 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 gi= mpflication of SAVE_EXPRs, I dare not > >> to modify that part of the code. > >> > >> At this moment, I will add unshare_expr to the routine =E2=80=9Cpointe= r_int_sum=E2=80=9D 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 =3D alloc_buf (10); > p2->array[11] =3D 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 =3D alloc_buf (10); > > struct annotated * p2 =3D alloc_buf (10); > *(.ACCESS_WITH_SIZE ((int *) &p2->array, &p2->foo, 1, 32, -1) + 44) =3D= 0; > return; > } > > Do you see any obvious IR issue in the above? Do I miss to generate any D= ECL_EXPR in the above IR? It's an interesting question - I don't see where the gimplifier would need to access DECL/TYPE_SIZE so the mistake, if any, should be that you need to unshare the size expressions you are using as argument to .ACCESS_WITH_SIZE? Mind, you are replacing an ARRAY_REF with a pointer indirection as well - IMO we shouldn't replace accesses this way but instead make it possible for analysis to discover the base/size values? > Thanks. > > Qing > > > I compared it with the following testing case without the =E2=80=9Ccounte= d-by=E2=80=9D annotation > and use an user-defined =E2=80=9Caccess_with_size=E2=80=9D 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 =3D alloc_buf (10); > access_with_size ((int *) &p2->array, &p2->foo, 1, 32, -1)[11] =3D 0; > return; > } > [opc@qinzhao-ol8u3-x86 108896]$ cat test_1.c.005t.original > > ;; Function bar (null) > ;; enabled by -tree-original > > > { > struct annotated * p2 =3D alloc_buf (10); > > struct annotated * p2 =3D alloc_buf (10); > *(access_with_size ((int *) &p2->array, &p2->foo, 1, 32, -1) + 44) =3D = 0; > return; > } > > > > > > >> Let me know if you have any comment and suggestion. > >> > >> Thanks a lot. > >> > >> Qing > >> > >>>> > >>>>> > >>>>> Qing > >>>>> > >>>>>> > >>>>>>> Thanks a lot for the help. > >>>>>>> > >>>>>>> Qing > >