Message ID | 20220120104553.GK2646553@tucnak |
---|---|
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 5A86F3858411 for <patchwork@sourceware.org>; Thu, 20 Jan 2022 10:47:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5A86F3858411 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1642675662; bh=+SmHGkdlLLAHtqtv31AkphC5VWFplLNUKAqB5G3HPz4=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=fPpC5uO/5/GvsI0mTObCAVw3cE1maKBiH8gGMYh4CeTkLwLS7S1rnb/ZZxWtOvS/R PecpdiMvxsD97puXGz4nfmPz7JOEl92X9NAdXEOowL0bWitbpsG7drL57jH6UR6gzG NP5WWx/eA3EtIKwCadtdSEO9Pqf3d+lcDd80xWbs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 25B433857803 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 10:46:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 25B433857803 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-608-HqklxIaYOCuHvRirJC_r5g-1; Thu, 20 Jan 2022 05:45:57 -0500 X-MC-Unique: HqklxIaYOCuHvRirJC_r5g-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F259684DA44; Thu, 20 Jan 2022 10:45:56 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.40.192.74]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 88F3E7AB4D; Thu, 20 Jan 2022 10:45:55 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 20KAjrRE054131 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 20 Jan 2022 11:45:54 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 20KAjrNm054130; Thu, 20 Jan 2022 11:45:53 +0100 Date: Thu, 20 Jan 2022 11:45:53 +0100 To: Jason Merrill <jason@redhat.com>, Richard Biener <rguenther@suse.de> Subject: [PATCH] dwarf2out: Fix -gsplit-dwarf on riscv [PR103874] Message-ID: <20220120104553.GK2646553@tucnak> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jakub Jelinek <jakub@redhat.com> Cc: gcc-patches@gcc.gnu.org Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
dwarf2out: Fix -gsplit-dwarf on riscv [PR103874]
|
|
Commit Message
Jakub Jelinek
Jan. 20, 2022, 10:45 a.m. UTC
Hi! riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently due to some aggressive linker optimizations). As the following testcase shows, we mishandle in index_rnglists the !HAVE_AS_LEB128 && !have_multiple_function_sections case. output_rnglists does roughly: FOR_EACH_VEC_SAFE_ELT (ranges_table, i, r) { ... if (block_num > 0) { ... if (HAVE_AS_LEB128) { if (!have_multiple_function_sections) { // code not using r->*_entry continue; } // code that sometimes doesn't use r->*_entry, // sometimes r->begin_entry } else if (dwarf_split_debug_info) { // code that uses both r->begin_entry and r->end_entry } else { // code not using r->*_entry } } else if (block_num < 0) { if (!have_multiple_function_sections) gcc_unreachable (); ... } } and index_rnglists is what sets up those r->{begin,end}_entry members. The code did an early if (!have_multiple_function_sections) continue; which is fine for the HAVE_AS_LEB128 case, because r->*_entry is not used in that case, but not for !HAVE_AS_LEB128 that uses it anyway. Fixed thusly, tested on the testcase with x86_64 -> riscv64 cross, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2022-01-20 Jakub Jelinek <jakub@redhat.com> PR debug/103874 * dwarf2out.cc (index_rnglists): For !HAVE_AS_LEB128 and block_num > 0, index entry even if !have_multiple_function_sections. * gcc.dg/debug/dwarf2/pr103874.c: New test. Jakub
Comments
On Thu, 20 Jan 2022, Jakub Jelinek wrote: > Hi! > > riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently > due to some aggressive linker optimizations). > As the following testcase shows, we mishandle in index_rnglists the > !HAVE_AS_LEB128 && !have_multiple_function_sections case. > > output_rnglists does roughly: > FOR_EACH_VEC_SAFE_ELT (ranges_table, i, r) > { > ... > if (block_num > 0) > { > ... > if (HAVE_AS_LEB128) > { > if (!have_multiple_function_sections) > { > // code not using r->*_entry > continue; > } > // code that sometimes doesn't use r->*_entry, > // sometimes r->begin_entry > } > else if (dwarf_split_debug_info) > { > // code that uses both r->begin_entry and r->end_entry > } > else > { > // code not using r->*_entry > } > } > else if (block_num < 0) > { > if (!have_multiple_function_sections) > gcc_unreachable (); > ... > } > } > and index_rnglists is what sets up those r->{begin,end}_entry members. > The code did an early if (!have_multiple_function_sections) continue; > which is fine for the HAVE_AS_LEB128 case, because r->*_entry is not > used in that case, but not for !HAVE_AS_LEB128 that uses it anyway. > > Fixed thusly, tested on the testcase with x86_64 -> riscv64 cross, > bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Thanks, Richard. > 2022-01-20 Jakub Jelinek <jakub@redhat.com> > > PR debug/103874 > * dwarf2out.cc (index_rnglists): For !HAVE_AS_LEB128 and > block_num > 0, index entry even if !have_multiple_function_sections. > > * gcc.dg/debug/dwarf2/pr103874.c: New test. > > --- gcc/dwarf2out.cc.jj 2022-01-18 11:58:59.000000000 +0100 > +++ gcc/dwarf2out.cc 2022-01-19 13:30:08.936008194 +0100 > @@ -12094,9 +12094,10 @@ index_rnglists (void) > if (r->label && r->idx != DW_RANGES_IDX_SKELETON) > r->idx = rnglist_idx++; > > - if (!have_multiple_function_sections) > - continue; > int block_num = r->num; > + if ((HAVE_AS_LEB128 || block_num < 0) > + && !have_multiple_function_sections) > + continue; > if (HAVE_AS_LEB128 && (r->label || r->maybe_new_sec)) > base = false; > if (block_num > 0) > --- gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c.jj 2022-01-19 13:35:25.485631843 +0100 > +++ gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c 2022-01-19 13:36:53.608413534 +0100 > @@ -0,0 +1,12 @@ > +/* PR debug/103874 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -g -gsplit-dwarf -dA -Wno-implicit-function-declaration" } */ > + > +void > +foo (void) > +{ > + { > + bar (); > + baz (); > + } > +} > > Jakub > >
On Thu, 20 Jan 2022 02:45:53 PST (-0800), gcc-patches@gcc.gnu.org wrote: > Hi! > > riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently > due to some aggressive linker optimizations). I don't really understand the rest of this, but we do have a subset of LEB128 (constant expressions only). I'm not sure exactly what the requirements are here, but one could imagine extending our assembler support to cover them -- we might never have full support for LEB128 expressions (because of linker relaxation), but we might be able to make more stuff work. I'm not sure if that helps or hurts, though, as we'll still be a special case. > As the following testcase shows, we mishandle in index_rnglists the > !HAVE_AS_LEB128 && !have_multiple_function_sections case. > > output_rnglists does roughly: > FOR_EACH_VEC_SAFE_ELT (ranges_table, i, r) > { > ... > if (block_num > 0) > { > ... > if (HAVE_AS_LEB128) > { > if (!have_multiple_function_sections) > { > // code not using r->*_entry > continue; > } > // code that sometimes doesn't use r->*_entry, > // sometimes r->begin_entry > } > else if (dwarf_split_debug_info) > { > // code that uses both r->begin_entry and r->end_entry > } > else > { > // code not using r->*_entry > } > } > else if (block_num < 0) > { > if (!have_multiple_function_sections) > gcc_unreachable (); > ... > } > } > and index_rnglists is what sets up those r->{begin,end}_entry members. > The code did an early if (!have_multiple_function_sections) continue; > which is fine for the HAVE_AS_LEB128 case, because r->*_entry is not > used in that case, but not for !HAVE_AS_LEB128 that uses it anyway. > > Fixed thusly, tested on the testcase with x86_64 -> riscv64 cross, > bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2022-01-20 Jakub Jelinek <jakub@redhat.com> > > PR debug/103874 > * dwarf2out.cc (index_rnglists): For !HAVE_AS_LEB128 and > block_num > 0, index entry even if !have_multiple_function_sections. > > * gcc.dg/debug/dwarf2/pr103874.c: New test. > > --- gcc/dwarf2out.cc.jj 2022-01-18 11:58:59.000000000 +0100 > +++ gcc/dwarf2out.cc 2022-01-19 13:30:08.936008194 +0100 > @@ -12094,9 +12094,10 @@ index_rnglists (void) > if (r->label && r->idx != DW_RANGES_IDX_SKELETON) > r->idx = rnglist_idx++; > > - if (!have_multiple_function_sections) > - continue; > int block_num = r->num; > + if ((HAVE_AS_LEB128 || block_num < 0) > + && !have_multiple_function_sections) > + continue; > if (HAVE_AS_LEB128 && (r->label || r->maybe_new_sec)) > base = false; > if (block_num > 0) > --- gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c.jj 2022-01-19 13:35:25.485631843 +0100 > +++ gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c 2022-01-19 13:36:53.608413534 +0100 > @@ -0,0 +1,12 @@ > +/* PR debug/103874 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -g -gsplit-dwarf -dA -Wno-implicit-function-declaration" } */ > + > +void > +foo (void) > +{ > + { > + bar (); > + baz (); > + } > +} > > Jakub
On Thu, Jan 20, 2022 at 01:13:45PM -0800, Palmer Dabbelt wrote: > On Thu, 20 Jan 2022 02:45:53 PST (-0800), gcc-patches@gcc.gnu.org wrote: > > riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently > > due to some aggressive linker optimizations). > > I don't really understand the rest of this, but we do have a subset of > LEB128 (constant expressions only). I'm not sure exactly what the > requirements are here, but one could imagine extending our assembler support > to cover them -- we might never have full support for LEB128 expressions > (because of linker relaxation), but we might be able to make more stuff > work. > > I'm not sure if that helps or hurts, though, as we'll still be a special > case. HAVE_AS_LEB128 really needs to be able to handle both constants and difference of two labels in the same section. Most targets resolve something like that in the assembler as constant which they encode into sleb128 or uleb128 and put into the section that uses those directives (typically debugging sections). If a target performs aggressive linker relaxation, then probably some relocation would need to be added (but one that can encode the two symbols for the difference, so perhaps two relocations that must be consecutive or something similar) and resolve that by the linker. Though, that would mean that even in the debugging section offsets wouldn't be fixed during assembly... Jakub
On Thu, 20 Jan 2022 13:20:34 PST (-0800), gcc-patches@gcc.gnu.org wrote: > On Thu, Jan 20, 2022 at 01:13:45PM -0800, Palmer Dabbelt wrote: >> On Thu, 20 Jan 2022 02:45:53 PST (-0800), gcc-patches@gcc.gnu.org wrote: >> > riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently >> > due to some aggressive linker optimizations). >> >> I don't really understand the rest of this, but we do have a subset of >> LEB128 (constant expressions only). I'm not sure exactly what the >> requirements are here, but one could imagine extending our assembler support >> to cover them -- we might never have full support for LEB128 expressions >> (because of linker relaxation), but we might be able to make more stuff >> work. >> >> I'm not sure if that helps or hurts, though, as we'll still be a special >> case. > > HAVE_AS_LEB128 really needs to be able to handle both constants and > difference of two labels in the same section. > Most targets resolve something like that in the assembler as constant which > they encode into sleb128 or uleb128 and put into the section that uses > those directives (typically debugging sections). > If a target performs aggressive linker relaxation, then probably some > relocation would need to be added (but one that can encode the two symbols > for the difference, so perhaps two relocations that must be consecutive or > something similar) and resolve that by the linker. Though, that would mean > that even in the debugging section offsets wouldn't be fixed during > assembly... Differences are the hard case for RISC-V, as they can grow numerically. That could then cause the LEB to grow in byte size, possibly violating one of our linker relaxation invariants. The only way I've come up with to support these would be to pad the LEBs, and I'm not sure if that's legal. Not sure if I'm missing something, though.
On Thu, 20 Jan 2022 13:33:35 PST (-0800), Palmer Dabbelt wrote: > On Thu, 20 Jan 2022 13:20:34 PST (-0800), gcc-patches@gcc.gnu.org wrote: >> On Thu, Jan 20, 2022 at 01:13:45PM -0800, Palmer Dabbelt wrote: >>> On Thu, 20 Jan 2022 02:45:53 PST (-0800), gcc-patches@gcc.gnu.org wrote: >>> > riscv*-*-* are the only modern targets that !HAVE_AS_LEB128 (apparently >>> > due to some aggressive linker optimizations). >>> >>> I don't really understand the rest of this, but we do have a subset of >>> LEB128 (constant expressions only). I'm not sure exactly what the >>> requirements are here, but one could imagine extending our assembler support >>> to cover them -- we might never have full support for LEB128 expressions >>> (because of linker relaxation), but we might be able to make more stuff >>> work. >>> >>> I'm not sure if that helps or hurts, though, as we'll still be a special >>> case. >> >> HAVE_AS_LEB128 really needs to be able to handle both constants and >> difference of two labels in the same section. >> Most targets resolve something like that in the assembler as constant which >> they encode into sleb128 or uleb128 and put into the section that uses >> those directives (typically debugging sections). >> If a target performs aggressive linker relaxation, then probably some >> relocation would need to be added (but one that can encode the two symbols >> for the difference, so perhaps two relocations that must be consecutive or >> something similar) and resolve that by the linker. Though, that would mean >> that even in the debugging section offsets wouldn't be fixed during >> assembly... > > Differences are the hard case for RISC-V, as they can grow numerically. > That could then cause the LEB to grow in byte size, possibly violating > one of our linker relaxation invariants. The only way I've come up with > to support these would be to pad the LEBs, and I'm not sure if that's > legal. > > Not sure if I'm missing something, though. Andrew points out that label differences within the same section can't increase, so this might be a lot more manageable than I thought it was.
--- gcc/dwarf2out.cc.jj 2022-01-18 11:58:59.000000000 +0100 +++ gcc/dwarf2out.cc 2022-01-19 13:30:08.936008194 +0100 @@ -12094,9 +12094,10 @@ index_rnglists (void) if (r->label && r->idx != DW_RANGES_IDX_SKELETON) r->idx = rnglist_idx++; - if (!have_multiple_function_sections) - continue; int block_num = r->num; + if ((HAVE_AS_LEB128 || block_num < 0) + && !have_multiple_function_sections) + continue; if (HAVE_AS_LEB128 && (r->label || r->maybe_new_sec)) base = false; if (block_num > 0) --- gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c.jj 2022-01-19 13:35:25.485631843 +0100 +++ gcc/testsuite/gcc.dg/debug/dwarf2/pr103874.c 2022-01-19 13:36:53.608413534 +0100 @@ -0,0 +1,12 @@ +/* PR debug/103874 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -g -gsplit-dwarf -dA -Wno-implicit-function-declaration" } */ + +void +foo (void) +{ + { + bar (); + baz (); + } +}