From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by sourceware.org (Postfix) with ESMTPS id CC1313858C50 for ; Mon, 28 Mar 2022 23:47:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CC1313858C50 Received: by mail-pl1-x630.google.com with SMTP id j8so5912917pll.11 for ; Mon, 28 Mar 2022 16:47:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=1yf9RQiFogTvbE1uHXyMuCsIILEWmj2MF5iYU/ouZXI=; b=g2wBmcepwsqOnayr4s/kzsVL0YOWfW1IEc1iE+Jt4Y1umQ3iblveXJOMNGBTiy84vm vRJoLsQEEq10vtYMMGtHDjio7v6srBb0srdOgKQn5OX2v1XWZ1cGGDPLEezB7ZaUC6cZ m/Mby25WNDEo2RiPbQ5ZhfprgEautXjuF9Tnm7p8c5itOUZcLoOrsgwvn+6C2GwEAgEz Clew0j6rWJUgfunaDPrJ8QANyPYzQTreZbmO0DcuZcEWpR8KtE9ZSSKW+myCO5c/cnZ6 zwfqW8kXMpqdBcZQQ3FKlFjtFEvTwiyt3QT7sPeJepRT5YF8q6VO2A0y9c8UDQdnC+ZM O1Ew== X-Gm-Message-State: AOAM531Nj4DqV/Xd+NmYzB0J41Q+30zndaiZtaPKOVTPiQvNwADAAA4a xCjkXk8D9kNDo7kkJcl1W4yQeyYuUsQ= X-Google-Smtp-Source: ABdhPJzJ7QypwD/+1orLT9OZWVZaebt7CjqvmrR0WYaAUjSVFBFUv9VpbC4KQ+CMsMMJOm9oaFgw1Q== X-Received: by 2002:a17:90a:a018:b0:1c6:5dac:3da with SMTP id q24-20020a17090aa01800b001c65dac03damr1552179pjp.195.1648511238438; Mon, 28 Mar 2022 16:47:18 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id h12-20020a056a00170c00b004fab8f3244esm17493010pfc.28.2022.03.28.16.47.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Mar 2022 16:47:17 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 9AFCD1140450; Tue, 29 Mar 2022 10:17:14 +1030 (ACDT) Date: Tue, 29 Mar 2022 10:17:14 +1030 From: Alan Modra To: Nick Clifton Cc: Jan Beulich , sesse@chromium.org, binutils@sourceware.org, "Steinar H. Gunderson" Subject: Re: [PATCH] Add a trie to map quickly from address range to compilation unit. Message-ID: References: <20220321094030.1256430-1-sesse@google.com> <63191455-2374-5db9-f55e-ddf794c7d88e@redhat.com> <49afcaee-63a8-2d18-64d1-0fc0abfe4669@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <49afcaee-63a8-2d18-64d1-0fc0abfe4669@suse.com> X-Spam-Status: No, score=-3037.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Mar 2022 23:47:22 -0000 On Mon, Mar 28, 2022 at 12:19:52PM +0200, Jan Beulich wrote: > On 25.03.2022 00:30, Alan Modra via Binutils wrote: > > On Thu, Mar 24, 2022 at 09:01:38AM +0100, Steinar H. Gunderson wrote: > >> On Thu, Mar 24, 2022 at 03:52:27PM +1030, Alan Modra wrote: > >>> Huh, I remember looking at this code a while ago and finding it > >>> confusing. I think the code would be clearer, and behave the same on > >>> normal line number info with the following patch: > >> > >> An interesting question is: Do you want to keep searching through > >> compilation units once you've found a match with a line number? > >> Should we go straight to “goto done” then? > > > > This would be reverting commit 240d6706c6a2. In > > https://sourceware.org/bugzilla/show_bug.cgi?id=15935#c3 I came to the > > conclusion that the pr15935 testcase had bogus debug info and closed > > the bug as invalid. The reporter apparently opened another bug, > > https://sourceware.org/bugzilla/show_bug.cgi?id=15994 a month later > > that Nick fixed by making _bfd_dwarf2_find_nearest_line do extra work. > > Which of course is unnecessary with good debug info, but in many cases > > we try to make binutils give the best result even with bad input. I > > don't know the details beyond that. It might have been that the > > compiler producing the bad debug info was one supported by RedHat. > > > > Now we have pr28592 and others complaining that objdump or addr2line > > have significantly slowed. Given that pr15935 dates back to 2013, I > > would presume that people have moved on from whatever broken compiler > > produced bad line info, and that we should indeed revert commit > > 240d6706c6a2. Nick? > > Since I ended up working on that function as well, I did notice another > potentially relevant aspect: The adjustment done back at the time was > only for the case of already processed CUs. The subsequent loop reading > any remaining ones doesn't similarly attempt to find a better match. > IOW the effects of that change can only have been partial anyway. Yes, Steinar noticed that too. Another thing I noticed is that prior to Nick's original patch looking for the best line range match, we used to return a function name result for addresses that matched a DW_TAG_subprogram (and similar) address ramge info. See the comp_unit_find_nearest_line change in the following, a patch to revert commit 240d6706c6a2. PR 28592 PR 15994 PR 15935 * dwarf2.c (lookup_address_in_line_info_table): Return bool rather than a range. (comp_unit_find_nearest_line): Likewise. Return true if function info found without line info. (_bfd_dwarf2_find_nearest_line): Revert range handling code. diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c index 8b5ac6012e1..4beebcd2835 100644 --- a/bfd/dwarf2.c +++ b/bfd/dwarf2.c @@ -2543,13 +2543,12 @@ decode_line_info (struct comp_unit *unit) return NULL; } -/* If ADDR is within TABLE set the output parameters and return the - range of addresses covered by the entry used to fill them out. - Otherwise set * FILENAME_PTR to NULL and return 0. +/* If ADDR is within TABLE set the output parameters and return TRUE, + otherwise set *FILENAME_PTR to NULL and return FALSE. The parameters FILENAME_PTR, LINENUMBER_PTR and DISCRIMINATOR_PTR are pointers to the objects to be filled in. */ -static bfd_vma +static bool lookup_address_in_line_info_table (struct line_info_table *table, bfd_vma addr, const char **filename_ptr, @@ -2608,12 +2607,12 @@ lookup_address_in_line_info_table (struct line_info_table *table, *linenumber_ptr = info->line; if (discriminator_ptr) *discriminator_ptr = info->discriminator; - return seq->last_line->address - seq->low_pc; + return true; } fail: *filename_ptr = NULL; - return 0; + return false; } /* Read in the .debug_ranges section for future reference. */ @@ -4008,14 +4007,11 @@ comp_unit_contains_address (struct comp_unit *unit, bfd_vma addr) } /* If UNIT contains ADDR, set the output parameters to the values for - the line containing ADDR. The output parameters, FILENAME_PTR, - FUNCTION_PTR, and LINENUMBER_PTR, are pointers to the objects - to be filled in. - - Returns the range of addresses covered by the entry that was used - to fill in *LINENUMBER_PTR or 0 if it was not filled in. */ + the line containing ADDR and return TRUE. Otherwise return FALSE. + The output parameters, FILENAME_PTR, FUNCTION_PTR, and + LINENUMBER_PTR, are pointers to the objects to be filled in. */ -static bfd_vma +static bool comp_unit_find_nearest_line (struct comp_unit *unit, bfd_vma addr, const char **filename_ptr, @@ -4023,7 +4019,7 @@ comp_unit_find_nearest_line (struct comp_unit *unit, unsigned int *linenumber_ptr, unsigned int *discriminator_ptr) { - bool func_p; + bool line_p, func_p; if (!comp_unit_maybe_decode_line_info (unit)) return false; @@ -4033,10 +4029,11 @@ comp_unit_find_nearest_line (struct comp_unit *unit, if (func_p && (*function_ptr)->tag == DW_TAG_inlined_subroutine) unit->stash->inliner_chain = *function_ptr; - return lookup_address_in_line_info_table (unit->line_table, addr, - filename_ptr, - linenumber_ptr, - discriminator_ptr); + line_p = lookup_address_in_line_info_table (unit->line_table, addr, + filename_ptr, + linenumber_ptr, + discriminator_ptr); + return line_p || func_p; } /* Check to see if line info is already decoded in a comp_unit. @@ -5187,54 +5184,17 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd, } else { - bfd_vma min_range = (bfd_vma) -1; - const char * local_filename = NULL; - struct funcinfo *local_function = NULL; - unsigned int local_linenumber = 0; - unsigned int local_discriminator = 0; - for (each = stash->f.all_comp_units; each; each = each->next_unit) { - bfd_vma range = (bfd_vma) -1; - found = ((each->arange.high == 0 || comp_unit_contains_address (each, addr)) - && (range = (comp_unit_find_nearest_line - (each, addr, &local_filename, - &local_function, &local_linenumber, - &local_discriminator))) != 0); + && comp_unit_find_nearest_line (each, addr, + filename_ptr, + &function, + linenumber_ptr, + discriminator_ptr)); if (found) - { - /* PRs 15935 15994: Bogus debug information may have provided us - with an erroneous match. We attempt to counter this by - selecting the match that has the smallest address range - associated with it. (We are assuming that corrupt debug info - will tend to result in extra large address ranges rather than - extra small ranges). - - This does mean that we scan through all of the CUs associated - with the bfd each time this function is called. But this does - have the benefit of producing consistent results every time the - function is called. */ - if (range <= min_range) - { - if (filename_ptr && local_filename) - * filename_ptr = local_filename; - if (local_function) - function = local_function; - if (discriminator_ptr && local_discriminator) - * discriminator_ptr = local_discriminator; - if (local_linenumber) - * linenumber_ptr = local_linenumber; - min_range = range; - } - } - } - - if (* linenumber_ptr) - { - found = true; - goto done; + goto done; } } @@ -5259,7 +5219,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd, filename_ptr, &function, linenumber_ptr, - discriminator_ptr) != 0); + discriminator_ptr)); if (found) break; -- Alan Modra Australia Development Lab, IBM