| Message ID | 3103467.5fSG56mABF@fomalhaut |
|---|---|
| State | New |
| Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 85BFF385140A for <patchwork@sourceware.org>; Fri, 13 May 2022 08:21:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 85BFF385140A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1652430073; bh=mb4FKTms5ZgNzqiymoq+9nmVoG6hiJIk1rToYD3nCqA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=IXHPg8bQ1l5mEDj3H0qdOS8+/j0F5eRYBKv09zjcrxOdXxuOGjjH2eN31n8IPEmpt 8ZOCl3AJILF9hLq6BpFmXotUim9t+hzbYLskEElKxOTiNmYMMp6TfuE/JE6I/sAopZ RTVFcJZeIcabFwxjqcryGCoLardecwz7AO7J60AU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by sourceware.org (Postfix) with ESMTPS id D2F42385736E for <gcc-patches@gcc.gnu.org>; Fri, 13 May 2022 08:07:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D2F42385736E Received: by mail-wr1-x430.google.com with SMTP id i5so10297427wrc.13 for <gcc-patches@gcc.gnu.org>; Fri, 13 May 2022 01:07:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=mb4FKTms5ZgNzqiymoq+9nmVoG6hiJIk1rToYD3nCqA=; b=cI4lNSiOnqNngEBvlJXab5LdtqCjcQCwTUW28R//bG6kKlJiyRXfgV2KRNJ4a+eQNq 2ialaLZx22f7RUJPmlbTaHdnrvt9m6y/w0QI4rvx/AueLj9hoO9SLZ7Ecna/fHf79QJ1 KxLYr1blh2ruo3cWyVh9WFwK1DkRBXA2sQyg6b3u+Kb1Cq0mOy88nPT181ock6F805JL vbdxmRMKwAO03ChqWTU4lXXuIACDPTuD/mXBuCQ1RbRtsK47RzvdeP9/acH2Q7X38QEZ jme91ek54oS9FWA4nZtvNLqE/QenNHXBxTe67Dnti/iiuAWvFPl2xnowPJPf48Ntbybt Ug9w== X-Gm-Message-State: AOAM532vAyZOvYmGNfYpIl4KVHoxnAwL3Iurlg5NH8jdVuaGCsCwBC/F yMCSmdUZPX0lR2QaZNGs+eFzYVbO11sdfVyN X-Google-Smtp-Source: ABdhPJwuqNXm4py9v95s6kA2YUa/jDwm23M4b/4HiYrzkgVR+i2aa3ch0KWyTt9uJS2v1Gju3jzNPg== X-Received: by 2002:adf:ec92:0:b0:20a:d261:2cf2 with SMTP id z18-20020adfec92000000b0020ad2612cf2mr2805791wrn.296.1652429263460; Fri, 13 May 2022 01:07:43 -0700 (PDT) Received: from fomalhaut.localnet ([2a01:e0a:8d5:d990:bf38:f508:6f40:de1d]) by smtp.gmail.com with ESMTPSA id e23-20020adf9bd7000000b0020aca418f26sm1507213wrc.1.2022.05.13.01.07.42 for <gcc-patches@gcc.gnu.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 May 2022 01:07:42 -0700 (PDT) X-Google-Original-From: Eric Botcazou <ebotcazou@adacore.com> To: gcc-patches@gcc.gnu.org Subject: [PATCH] Do not use DW_OP_not for TRUTH_NOT_EXPR in conditional expressions Date: Fri, 13 May 2022 10:07:24 +0200 Message-ID: <3103467.5fSG56mABF@fomalhaut> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart5720973.MhkbZ0Pkbq" Content-Transfer-Encoding: 7Bit X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Eric Botcazou via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Eric Botcazou <botcazou@adacore.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
| Series |
Do not use DW_OP_not for TRUTH_NOT_EXPR in conditional expressions
|
|
Commit Message
Eric Botcazou
May 13, 2022, 8:07 a.m. UTC
Hi, DW_OP_not is a bitwise, not a logical NOT, so it computes the wrong result in a DWARF conditional expression. Tested (GCC + GDB° on x86-64/Linux, OK for the mainline? 2022-05-13 Eric Botcazou <ebotcazou@adacore.com> * dwarf2out.c (loc_list_from_tree_1) <COND_EXPR>: Swap the operands if the condition is a TRUTH_NOT_EXPR.
Comments
On Fri, May 13, 2022 at 10:21 AM Eric Botcazou via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi, > > DW_OP_not is a bitwise, not a logical NOT, so it computes the wrong result in > a DWARF conditional expression. > > Tested (GCC + GDB° on x86-64/Linux, OK for the mainline? But this doesn't fix case TRUTH_NOT_EXPR: case BIT_NOT_EXPR: op = DW_OP_not; goto do_unop; I also wonder where we get the TRUTH_NOT_EXPR to expand from? I suspect some non-gimplified global tree? Thanks, Richard. > > 2022-05-13 Eric Botcazou <ebotcazou@adacore.com> > > * dwarf2out.c (loc_list_from_tree_1) <COND_EXPR>: Swap the operands > if the condition is a TRUTH_NOT_EXPR. > > -- > Eric Botcazou
> But this doesn't fix > > case TRUTH_NOT_EXPR: > case BIT_NOT_EXPR: > op = DW_OP_not; > goto do_unop; Nope (I couldn't trigger it after my change). > I also wonder where we get the TRUTH_NOT_EXPR to expand from? I suspect > some non-gimplified global tree? Yes, it's in the TYPE_SIZE of a global type: package P is type Rec (Defined : Boolean) is record case Defined is when false => null; when others => I : Integer; end case; end record; A : access Rec; end P;
On Fri, May 13, 2022 at 10:25:02AM +0200, Richard Biener via Gcc-patches wrote: > On Fri, May 13, 2022 at 10:21 AM Eric Botcazou via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > Hi, > > > > DW_OP_not is a bitwise, not a logical NOT, so it computes the wrong result in > > a DWARF conditional expression. > > > > Tested (GCC + GDB° on x86-64/Linux, OK for the mainline? > > But this doesn't fix > > case TRUTH_NOT_EXPR: > case BIT_NOT_EXPR: > op = DW_OP_not; > goto do_unop; > > I also wonder where we get the TRUTH_NOT_EXPR to expand from? I suspect > some non-gimplified global tree? So shouldn't we expand TRUTH_NOT_EXPR as (push argument) DW_OP_lit0 DW_OP_eq then? Jakub
> But this doesn't fix > > case TRUTH_NOT_EXPR: > case BIT_NOT_EXPR: > op = DW_OP_not; > goto do_unop; Revised patch attached, using Jakub's suggestion. The original (buggy) DWARF procedure for the Ada testcase I previously posted is: .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) .uleb128 0xc # DW_AT_location .byte 0x12 # DW_OP_dup .byte 0x20 # DW_OP_not .byte 0x28 # DW_OP_bra .value 0x4 .byte 0x34 # DW_OP_lit4 .byte 0x2f # DW_OP_skip .value 0x1 .byte 0x30 # DW_OP_lit0 .byte 0x16 # DW_OP_swap .byte 0x13 # DW_OP_drop With Jakub's fix: .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) .uleb128 0xd # DW_AT_location .byte 0x12 # DW_OP_dup .byte 0x30 # DW_OP_lit0 .byte 0x29 # DW_OP_eq .byte 0x28 # DW_OP_bra .value 0x4 .byte 0x34 # DW_OP_lit4 .byte 0x2f # DW_OP_skip .value 0x1 .byte 0x30 # DW_OP_lit0 .byte 0x16 # DW_OP_swap .byte 0x13 # DW_OP_drop With mine: .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) .uleb128 0xb # DW_AT_location .byte 0x12 # DW_OP_dup .byte 0x28 # DW_OP_bra .value 0x4 .byte 0x30 # DW_OP_lit0 .byte 0x2f # DW_OP_skip .value 0x1 .byte 0x34 # DW_OP_lit4 .byte 0x16 # DW_OP_swap .byte 0x13 # DW_OP_drop * dwarf2out.c (loc_list_from_tree_1) <TRUTH_NOT_EXPR>: Do a logical instead of a bitwise negation. <COND_EXPR>: Swap the operands if the condition is TRUTH_NOT_EXPR.
On Mon, May 16, 2022 at 9:06 AM Eric Botcazou <botcazou@adacore.com> wrote: > > > But this doesn't fix > > > > case TRUTH_NOT_EXPR: > > case BIT_NOT_EXPR: > > op = DW_OP_not; > > goto do_unop; > > Revised patch attached, using Jakub's suggestion. The original (buggy) DWARF > procedure for the Ada testcase I previously posted is: > > .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) > .uleb128 0xc # DW_AT_location > .byte 0x12 # DW_OP_dup > .byte 0x20 # DW_OP_not > .byte 0x28 # DW_OP_bra > .value 0x4 > .byte 0x34 # DW_OP_lit4 > .byte 0x2f # DW_OP_skip > .value 0x1 > .byte 0x30 # DW_OP_lit0 > .byte 0x16 # DW_OP_swap > .byte 0x13 # DW_OP_drop > > With Jakub's fix: > > .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) > .uleb128 0xd # DW_AT_location > .byte 0x12 # DW_OP_dup > .byte 0x30 # DW_OP_lit0 > .byte 0x29 # DW_OP_eq > .byte 0x28 # DW_OP_bra > .value 0x4 > .byte 0x34 # DW_OP_lit4 > .byte 0x2f # DW_OP_skip > .value 0x1 > .byte 0x30 # DW_OP_lit0 > .byte 0x16 # DW_OP_swap > .byte 0x13 # DW_OP_drop > > With mine: > > .uleb128 0x8 # (DIE (0x5b) DW_TAG_dwarf_procedure) > .uleb128 0xb # DW_AT_location > .byte 0x12 # DW_OP_dup > .byte 0x28 # DW_OP_bra > .value 0x4 > .byte 0x30 # DW_OP_lit0 > .byte 0x2f # DW_OP_skip > .value 0x1 > .byte 0x34 # DW_OP_lit4 > .byte 0x16 # DW_OP_swap > .byte 0x13 # DW_OP_drop > > > * dwarf2out.c (loc_list_from_tree_1) <TRUTH_NOT_EXPR>: Do a logical > instead of a bitwise negation. > <COND_EXPR>: Swap the operands if the condition is TRUTH_NOT_EXPR. LGTM. Thanks, Richard. > -- > Eric Botcazou
On Mon, May 16, 2022 at 09:45:18AM +0200, Richard Biener via Gcc-patches wrote: > > * dwarf2out.c (loc_list_from_tree_1) <TRUTH_NOT_EXPR>: Do a logical > > instead of a bitwise negation. > > <COND_EXPR>: Swap the operands if the condition is TRUTH_NOT_EXPR. > > LGTM. It won't work for types larger than size of address, it would need to use dwarf_OP (DW_OP_const_type) instead of DW_OP_lit0 in that case. But maybe TRUTH_NOT_EXPR will be never seen for such types and after all, even the loc_list_from_tree_1 INTEGER_CST case doesn't handle that (the RTL case does). So I think at least for now it is ok. Jakub
> It won't work for types larger than size of address, it would need to use > dwarf_OP (DW_OP_const_type) instead of DW_OP_lit0 in that case. > But maybe TRUTH_NOT_EXPR will be never seen for such types and after all, > even the loc_list_from_tree_1 INTEGER_CST case doesn't handle that > (the RTL case does). > So I think at least for now it is ok. Thanks. Any objection to me installing it on the 12 branch as well?
On Mon, May 16, 2022 at 10:47:53AM +0200, Eric Botcazou wrote: > > It won't work for types larger than size of address, it would need to use > > dwarf_OP (DW_OP_const_type) instead of DW_OP_lit0 in that case. > > But maybe TRUTH_NOT_EXPR will be never seen for such types and after all, > > even the loc_list_from_tree_1 INTEGER_CST case doesn't handle that > > (the RTL case does). > > So I think at least for now it is ok. > > Thanks. Any objection to me installing it on the 12 branch as well? Nope. It is ok. Jakub
diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc index 5681b01749a..affa2b5e52e 100644 --- a/gcc/dwarf2out.cc +++ b/gcc/dwarf2out.cc @@ -19497,6 +19497,15 @@ loc_list_from_tree_1 (tree loc, int want_address, list_ret = loc_list_from_tree_1 (TREE_OPERAND (TREE_OPERAND (loc, 0), 0), 0, context); + /* DW_OP_not is a bitwise, not a logical NOT, so avoid it. */ + else if (TREE_CODE (TREE_OPERAND (loc, 0)) == TRUTH_NOT_EXPR) + { + lhs = loc_descriptor_from_tree (TREE_OPERAND (loc, 2), 0, context); + rhs = loc_list_from_tree_1 (TREE_OPERAND (loc, 1), 0, context); + list_ret + = loc_list_from_tree_1 (TREE_OPERAND (TREE_OPERAND (loc, 0), 0), + 0, context); + } else list_ret = loc_list_from_tree_1 (TREE_OPERAND (loc, 0), 0, context); if (list_ret == 0 || lhs == 0 || rhs == 0)