From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id DFB553839830 for ; Mon, 2 Aug 2021 13:31:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DFB553839830 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B97401FFA0; Mon, 2 Aug 2021 13:31:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1627911093; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=13DPxvZ64uTyrETvP9iHZh1Aa22j2IIktqAWQAGmk2g=; b=iPZNhe/B24baUpeWci9qEy4HyHjej18/cMyNq8w3d2lktx+BS17LakrQx7WUi2m2r7PNpC eShjeJc0CY16qnZ+C2wRRuqck7fH81uVU2iUzA17BNk/LO5behTiiTMpvIREQELUoPDaSc wlgbqs6vRsmQoiEs5OYIvm/Er9lwReE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1627911093; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=13DPxvZ64uTyrETvP9iHZh1Aa22j2IIktqAWQAGmk2g=; b=Y93RXbY/FSYKY+2E2dZ+eJKX+XKacdVjf+mZba4OkChFTiPURoMJA3jAWn2zPPIRAJ9HA8 S1dcTtjPlMRuwADw== Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 91470A3BB6; Mon, 2 Aug 2021 13:31:33 +0000 (UTC) Date: Mon, 2 Aug 2021 15:31:33 +0200 (CEST) From: Richard Biener To: Bernd Edlinger cc: "gcc-patches@gcc.gnu.org" , Eric Botcazou , Arnaud Charlet Subject: Re: [PATCH 1/2] Fix debug info for ignored decls at start of assembly In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, 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 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Aug 2021 13:31:36 -0000 On Fri, 30 Jul 2021, Bernd Edlinger wrote: > > > On 7/29/21 9:23 AM, Richard Biener wrote: > > On Wed, 28 Jul 2021, Bernd Edlinger wrote: > > > >> On 7/28/21 2:51 PM, Richard Biener wrote: > >>> On Mon, 26 Jul 2021, Bernd Edlinger wrote: > >>> > >>>> Ignored functions decls that are compiled at the start of > >>>> the assembly have bogus line numbers until the first .file > >>>> directive, as reported in PR101575. > >>>> > >>>> The work around for this issue is to emit a dummy .file > >>>> directive when the first function is DECL_IGNORED_P, when > >>>> that is not already done, mostly for -fdwarf-4. > >>> > >>> I wonder if it makes sense to unconditionally announce the > >>> TU with a .file directive at the beginning. ISTR this is > >>> what we now do with -gdwarf-5. > >>> > >> > >> Yes, that would work, even when the file name is not guessed > >> correctly. > >> > >> Initially I had "" unconditionally here, and it did > >> not really hurt, except that it is visible with readelf. > > > > I think I'd prefer that, since if we don't announce a .file > > before the first assembler statement but ask gas to produce > > line info it might be tempted to create line info referencing > > the possibly temporary filename of the assembler file which > > is undesirable from a build reproducability point. > > > > Yeah, I understand. > > Meanwhile I found a simple C test case without ignored functions > > $ cat test1.c > asm("nop"); > int main () > { > return 0; > } > > $ gcc -g test1.c > $ readelf --debug-dump=decodedline a.out > Contents of the .debug_line section: > > CU: ./test1.c: > File name Line number Starting address View Stmt > test1.c 5 0x401106 x > test1.c 3 0x401107 x > test1.c 4 0x40110b x > test1.c 5 0x401110 x > test1.c - 0x401112 > > even with the proposed patch, so I agree it is incomplete. > > I tried the gdb test case and compile it with different LTO > options, but the gen_AT_string was always valid, in some > cases a lto debug section together with a couple .file > directives was output before the .file 0. > So I'd like to use the file name from gen_AT_string, since > it's most of the time accurate, and avoids unnecessary confusion > on the readers of the produced debug info. > > So I'd propose the attached patch instead. > Is it OK for trunk? Works for me - please give others a chance to comment though. Thanks, Richard. > > > Richard. > > > >>> Note get_AT_string (comp_unit_die (), DW_AT_name) doesn't > >>> work with LTO, you'll get then. > >>> > >> > >> Yeah, that's why I wanted to restrict that to the case where > >> it's absolutely necessary. > >> > >>> Is the dwarf assembler bug reported/fixed? Can you include > >>> a reference please? > >>> > >> > >> I've just added a bug report, it's unlikely to be fixed IMHO: > >> https://sourceware.org/bugzilla/show_bug.cgi?id=28149 > >> > >> I will add that to the patch description: > >> > >> Ignored functions decls that are compiled at the start of > >> the assembly have bogus line numbers until the first .file > >> directive, as reported in PR101575. > >> > >> The corresponding binutils bug report is > >> https://sourceware.org/bugzilla/show_bug.cgi?id=28149 > >> > >> The work around for this issue is to emit a dummy .file > >> directive when the first function is DECL_IGNORED_P, when > >> that is not already done, mostly for -fdwarf-4. > >> > >> > >> Thanks > >> Bernd. > >> > >>> Thanks, > >>> Richard. > >>> > >>>> 2021-07-24 Bernd Edlinger > >>>> > >>>> PR ada/101575 > >>>> * dwarf2out.c (dwarf2out_begin_prologue): Move init > >>>> of fde->ignored_debug to dwarf2out_set_ignored_loc. > >>>> (dwarf2out_set_ignored_loc): This is now also called > >>>> when no .loc statement is to be generated, in that case > >>>> we emit a dummy .file statement when needed. > >>>> * final.c (final_start_function_1, > >>>> final_scan_insn_1): Call debug_hooks->set_ignored_loc > >>>> for all DECL_IGNORED_P functions. > >>>> --- > >>>> gcc/dwarf2out.c | 29 +++++++++++++++++++++++++---- > >>>> gcc/final.c | 5 ++--- > >>>> 2 files changed, 27 insertions(+), 7 deletions(-) > >>>> > >>>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > >>>> index 884f1e1..8de0d6f 100644 > >>>> --- a/gcc/dwarf2out.c > >>>> +++ b/gcc/dwarf2out.c > >>>> @@ -1115,7 +1115,6 @@ dwarf2out_begin_prologue (unsigned int line ATTRIBUTE_UNUSED, > >>>> fde->dw_fde_current_label = dup_label; > >>>> fde->in_std_section = (fnsec == text_section > >>>> || (cold_text_section && fnsec == cold_text_section)); > >>>> - fde->ignored_debug = DECL_IGNORED_P (current_function_decl); > >>>> in_text_section_p = fnsec == text_section; > >>>> > >>>> /* We only want to output line number information for the genuine dwarf2 > >>>> @@ -28546,10 +28545,32 @@ dwarf2out_set_ignored_loc (unsigned int line, unsigned int column, > >>>> { > >>>> dw_fde_ref fde = cfun->fde; > >>>> > >>>> - fde->ignored_debug = false; > >>>> - set_cur_line_info_table (function_section (fde->decl)); > >>>> + if (filename) > >>>> + { > >>>> + set_cur_line_info_table (function_section (fde->decl)); > >>>> + > >>>> + dwarf2out_source_line (line, column, filename, 0, true); > >>>> + } > >>>> + else > >>>> + { > >>>> + fde->ignored_debug = true; > >>>> + > >>>> + /* Work around for PR101575: output a dummy .file directive. */ > >>>> + if (in_first_function_p > >>>> + && debug_info_level >= DINFO_LEVEL_TERSE > >>>> + && dwarf_debuginfo_p () > >>>> +#if defined(HAVE_AS_GDWARF_5_DEBUG_FLAG) && defined(HAVE_AS_WORKING_DWARF_N_FLAG) > >>>> + && dwarf_version <= 4 > >>>> +#endif > >>>> + && output_asm_line_debug_info ()) > >>>> + { > >>>> + const char *filename0 = get_AT_string (comp_unit_die (), DW_AT_name); > >>>> > >>>> - dwarf2out_source_line (line, column, filename, 0, true); > >>>> + if (filename0 == NULL) > >>>> + filename0 = ""; > >>>> + maybe_emit_file (lookup_filename (filename0)); > >>>> + } > >>>> + } > >>>> } > >>>> > >>>> /* Record the beginning of a new source file. */ > >>>> diff --git a/gcc/final.c b/gcc/final.c > >>>> index ac6892d..82a5767 100644 > >>>> --- a/gcc/final.c > >>>> +++ b/gcc/final.c > >>>> @@ -1725,7 +1725,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen, > >>>> if (!dwarf2_debug_info_emitted_p (current_function_decl)) > >>>> dwarf2out_begin_prologue (0, 0, NULL); > >>>> > >>>> - if (DECL_IGNORED_P (current_function_decl) && last_linenum && last_filename) > >>>> + if (DECL_IGNORED_P (current_function_decl)) > >>>> debug_hooks->set_ignored_loc (last_linenum, last_columnnum, last_filename); > >>>> > >>>> #ifdef LEAF_REG_REMAP > >>>> @@ -2205,8 +2205,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, > >>>> } > >>>> else if (!DECL_IGNORED_P (current_function_decl)) > >>>> debug_hooks->switch_text_section (); > >>>> - if (DECL_IGNORED_P (current_function_decl) && last_linenum > >>>> - && last_filename) > >>>> + if (DECL_IGNORED_P (current_function_decl)) > >>>> debug_hooks->set_ignored_loc (last_linenum, last_columnnum, > >>>> last_filename); > >>>> > >>>> > >>> > >> > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)