From patchwork Sat Feb 22 06:38:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 38274 Received: (qmail 122742 invoked by alias); 22 Feb 2020 06:39:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 122723 invoked by uid 89); 22 Feb 2020 06:39:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=dubious, understood, yesterday, tagged X-HELO: EUR04-DB3-obe.outbound.protection.outlook.com Received: from mail-oln040092074011.outbound.protection.outlook.com (HELO EUR04-DB3-obe.outbound.protection.outlook.com) (40.92.74.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 22 Feb 2020 06:38:58 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GfRg+dHfLdu0dNeHVTVl4b4OYlDe5DW9Z4gNQQgI0XJF+6x2qa6drhHW3Ucrp8Cq9NzMSx9HVMA8YJ6wTa+tV/tR+BNfH4EocfG493xfSbzxhcvhHPOmiJwtMEdI3eoNpIu/eh3YROrw6efTTBzsQWx65HHQPRiqTVAPuxUcGtF5jN8zv4kgPhx9s7+5lNDOiItfQrGqvTWMho0mBQk1CHP6+6hIHnR7/zYzV6UEw1z8lV7sO5mJVfm2Us5fOOxv+9ewSR04RaE9+II1H7KJA7EGmQ9JuQkEUNSK8Nqc8ODMsW3n+e+ZxPmSAY7D55LGeywcH9gmiCGtIn8zEO8bGQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=n0wiGW6dZRmAyvE1JVRVlDGjTq69swXpCck+MDSOiaI=; b=BX64R9v1mZ6PE3+Y3actxEvSGpljOpb/AMnAuPtj8cTV2whxxKelkXlY3sjxNLqsVVm21V8aQsEeOxtBWoY3NgvdDBJLjVbSNu4H3R6fiOcXY0ypYHVHp6DQd9hMqJpCAgCpAr291LTzmtKccMOv5+timOnCnAZtWeWCkccPubl7qeEZbBhI1E1jy2tH6I4ZARYoM5tdkbfKH53r0lF4HZj4yfT0BUrbvkwfyQfdQUPvvkFbZBCYZICFVz1S+m6LufIk350nO1RNzPgv5J+TzLgt1jD+B19rD0Qd3/g2QzciIBkoU0E+lt0cyTW+BN1DP/nnA5jWdO2pk13vBtLT1Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from HE1EUR04FT037.eop-eur04.prod.protection.outlook.com (10.152.26.52) by HE1EUR04HT174.eop-eur04.prod.protection.outlook.com (10.152.27.130) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.18; Sat, 22 Feb 2020 06:38:55 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com (10.152.26.53) by HE1EUR04FT037.mail.protection.outlook.com (10.152.26.161) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.18 via Frontend Transport; Sat, 22 Feb 2020 06:38:55 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd]) by AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd%6]) with mapi id 15.20.2729.033; Sat, 22 Feb 2020 06:38:55 +0000 Received: from [192.168.1.101] (92.77.140.102) by ZR0P278CA0040.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:1d::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.18 via Frontend Transport; Sat, 22 Feb 2020 06:38:54 +0000 From: Bernd Edlinger To: Andrew Burgess , "gdb-patches@sourceware.org" Subject: [PATCHv2] Fix range end handling of inlined subroutines Date: Sat, 22 Feb 2020 06:38:55 +0000 Message-ID: References: <94e33268f64060fc887670f4ee5ed524050cbcc7.1580902412.git.andrew.burgess@embecosm.com> In-Reply-To: x-microsoft-original-message-id: <97dd6a04-c3a1-d18f-9a9e-3f46b867e524@hotmail.de> x-ms-exchange-antispam-messagedata: rXXMmRSBjv8UTc0TNxR/G4gSFfspmlGK3/ah1AMFp1ALF62vT+kxN7eQp5hh4zdoM+w93+flJAS+Mcqe/2BILKWX3RkD7I0SP+lrB+DB3jBzTNdYgjonV9eT3Hs0JVhYVnCO9u7YFlurvWAaNV+4yw== x-ms-exchange-transport-forked: True MIME-Version: 1.0 On 2/9/20 10:07 PM, Bernd Edlinger wrote: > Hi, > > this is based on Andrew's patch here: > > https://sourceware.org/ml/gdb-patches/2020-02/msg00092.html > > This and adds a heuristic to fix the case where caller > and callee reside in the same subfile, it uses > the recorded is-stmt bits and locates end of > range infos, including the previously ignored empty > range, and adjusting is-stmt info at this > same pc, when the last item is not-is-stmt, the > previous line table entries are dubious and > we reset the is-stmt bit there. > This fixes the new test case in Andrew's patch. > > It understood, that this is just a heuristic > approach, since we do not have exactly the data, > which is needed to determine at which of the identical > PCs in the line table the subroutine actually ends. > So, this tries to be as conservative as possible, > just changing what is absolutely necessary. > > This patch itself is preliminary, since the is-stmt patch > needs to be rebased after the refactoring of > dwarf2read.c from yesterday, so I will have to rebase > this patch as well, but decided to wait for Andrew. > > So, this is an update to my previous patch above: https://sourceware.org/ml/gdb-patches/2020-02/msg00262.html It improves performance on big data, by using binary search to locate the bogus line table entries. Otherwise it behaves identical to the previous version, and is still waiting for Andrew's patch before it can be applied. Thanks Bernd. From 758b584e1447fce4648a3be3b9af930af20a6fb6 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 9 Feb 2020 21:13:17 +0100 Subject: [PATCHv2] Fix range end handling of inlined subroutines Since the is_stmt is now handled, it becomes possible to locate dubious is_stmt line entries at the end of an inlined function, even if the called inline function is in the same subfile. When there is a sequence of line entries at the same address where an inline range ends, and the last item has is_stmt = 0, we force all previous items to have is_stmt = 0 as well. If the last line at that address has is_stmt = 1, there is no need to change anything, since a step over will always stop at that last line from the same address, which is fine, since it is outside the subroutine. This is about the best we can do at the moment, unless location view information are added to the block ranges debug info structure, and location views are implemented in gdb in general. gdb: 2020-02-22 Bernd Edlinger * buildsym.c (buildsym_compunit::record_inline_range_end, patch_inline_end_pos): New helper functions. (buildsym_compunit::end_symtab_with_blockvector): Patch line table. (buildsym_compunit::~buildsym_compunit): Cleanup m_inline_end_vector. * buildsym.h (buildsym_compunit::record_inline_range_end): Declare. (buildsym_compunit::m_inline_end_vector, buildsym_compunit::m_inline_end_vector_length, buildsym_compunit::m_inline_end_vector_nitems): New data items. * dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process): Don't ignore empty ranges here. (dwarf2_ranges_read): Ignore empty ranges here. (dwarf2_record_block_ranges): Pass end of range PC to record_inline_range_end for inline functions. gdb/testsuite: 2020-02-22 Bernd Edlinger * gdb.cp/next-inline.exp: Adjust test. --- gdb/buildsym.c | 84 ++++++++++++++++++++++++++++++++++++ gdb/buildsym.h | 11 +++++ gdb/dwarf2/read.c | 22 ++++++---- gdb/testsuite/gdb.cp/next-inline.exp | 9 ---- 4 files changed, 108 insertions(+), 18 deletions(-) diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 7fd256f..c93944a 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -113,6 +113,8 @@ buildsym_compunit::~buildsym_compunit () next1 = next->next; xfree ((void *) next); } + + xfree (m_inline_end_vector); } struct macro_table * @@ -732,6 +734,84 @@ buildsym_compunit::record_line (struct subfile *subfile, int line, } +/* Record a PC where a inlined subroutine ends. */ + +void +buildsym_compunit::record_inline_range_end (CORE_ADDR end) +{ + if (m_inline_end_vector == nullptr) + { + m_inline_end_vector_length = INITIAL_LINE_VECTOR_LENGTH; + m_inline_end_vector = (CORE_ADDR *) + xmalloc (sizeof (CORE_ADDR) * m_inline_end_vector_length); + m_inline_end_vector_nitems = 0; + } + else if (m_inline_end_vector_nitems == m_inline_end_vector_length) + { + m_inline_end_vector_length *= 2; + m_inline_end_vector = (CORE_ADDR *) + xrealloc ((char *) m_inline_end_vector, + sizeof (CORE_ADDR) * m_inline_end_vector_length); + } + + m_inline_end_vector[m_inline_end_vector_nitems++] = end; +} + + +/* Patch the is_stmt bits at the given inline end address. + The line table has to be already sorted. */ + +static void +patch_inline_end_pos (struct linetable *table, CORE_ADDR end) +{ + int a = 2, b = table->nitems - 1; + struct linetable_entry *items = table->item; + + /* We need at least two items with pc = end in the table. + The lowest usable items are at pos 0 and 1, the highest + usable items are at pos b - 2 and b - 1. */ + if (a > b || end < items[1].pc || end > items[b - 2].pc) + return; + + /* Look for the first item with pc > end in the range [a,b]. + The previous element has pc = end or there is no match. + We set a = 2, since we need at least two consecutive elements + with pc = end to do anything useful. + We set b = nitems - 1, since we are not interested in the last + element which should be an end of sequence marker with line = 0 + and is_stmt = 1. */ + while (a < b) + { + int c = (a + b) / 2; + + if (end < items[c].pc) + b = c; + else + a = c + 1; + } + + a--; + if (items[a].pc != end || items[a].is_stmt) + return; + + /* When there is a sequence of line entries at the same address + where an inline range ends, and the last item has is_stmt = 0, + we force all previous items to have is_stmt = 0 as well. + Setting breakpoints at those addresses is currently not + supported, since it is unclear if the previous addresses are + part of the subroutine or the calling program. */ + while (a-- > 0) + { + /* We stop at the first line entry with a different address, + or when we see an end of sequence marker. */ + if (items[a].pc != end || items[a].line == 0) + break; + + items[a].is_stmt = 0; + } +} + + /* Subroutine of end_symtab to simplify it. Look for a subfile that matches the main source file's basename. If there is only one, and if the main source file doesn't have any symbol or line number @@ -965,6 +1045,10 @@ buildsym_compunit::end_symtab_with_blockvector (struct block *static_block, subfile->line_vector->item + subfile->line_vector->nitems, lte_is_less_than); + + for (int i = 0; i < m_inline_end_vector_nitems; i++) + patch_inline_end_pos (subfile->line_vector, + m_inline_end_vector[i]); } /* Allocate a symbol table if necessary. */ diff --git a/gdb/buildsym.h b/gdb/buildsym.h index c768a4c..2845789 100644 --- a/gdb/buildsym.h +++ b/gdb/buildsym.h @@ -190,6 +190,8 @@ struct buildsym_compunit void record_line (struct subfile *subfile, int line, CORE_ADDR pc, bool is_stmt); + void record_inline_range_end (CORE_ADDR end); + struct compunit_symtab *get_compunit_symtab () { return m_compunit_symtab; @@ -397,6 +399,15 @@ private: /* Pending symbols that are local to the lexical context. */ struct pending *m_local_symbols = nullptr; + + /* Pending inline end range addresses. */ + CORE_ADDR * m_inline_end_vector = nullptr; + + /* Number of allocated inline end range addresses. */ + int m_inline_end_vector_length = 0; + + /* Number of pending inline end range addresses. */ + int m_inline_end_vector_nitems = 0; }; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 2fa346c..1c4cd8b 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -13571,10 +13571,6 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu, return false; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - range_beginning += base; range_end += base; @@ -13685,10 +13681,6 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu, return 0; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - range_beginning += base; range_end += base; @@ -13728,6 +13720,10 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return, retval = dwarf2_ranges_process (offset, cu, [&] (CORE_ADDR range_beginning, CORE_ADDR range_end) { + /* Empty range entries have no effect. */ + if (range_beginning == range_end) + return; + if (ranges_pst != NULL) { CORE_ADDR lowpc; @@ -13965,6 +13961,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block, struct gdbarch *gdbarch = get_objfile_arch (objfile); struct attribute *attr; struct attribute *attr_high; + bool inlined_subroutine = (die->tag == DW_TAG_inlined_subroutine); attr_high = dwarf2_attr (die, DW_AT_high_pc, cu); if (attr_high) @@ -13980,7 +13977,10 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block, low = gdbarch_adjust_dwarf2_addr (gdbarch, low + baseaddr); high = gdbarch_adjust_dwarf2_addr (gdbarch, high + baseaddr); - cu->get_builder ()->record_block_range (block, low, high - 1); + if (inlined_subroutine) + cu->get_builder ()->record_inline_range_end (high); + if (low < high) + cu->get_builder ()->record_block_range (block, low, high - 1); } } @@ -14005,6 +14005,10 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block, end += baseaddr; start = gdbarch_adjust_dwarf2_addr (gdbarch, start); end = gdbarch_adjust_dwarf2_addr (gdbarch, end); + if (inlined_subroutine) + cu->get_builder ()->record_inline_range_end (end); + if (start == end) + return; cu->get_builder ()->record_block_range (block, start, end - 1); blockvec.emplace_back (start, end); }); diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp index 0b2b22d..11d9e2e 100644 --- a/gdb/testsuite/gdb.cp/next-inline.exp +++ b/gdb/testsuite/gdb.cp/next-inline.exp @@ -42,24 +42,15 @@ proc do_test { use_header } { gdb_test "step" ".*" "step into get_alias_set" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 1" - # It's possible that this first failure (when not using a header - # file) is GCC's fault, though the remaining failures would best - # be fixed by adding location views support (though it could be - # that some easier heuristic could be figured out). Still, it is - # not certain that the first failure wouldn't also be fixed by - # having location view support, so for now it is tagged as such. - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" ".*TREE_TYPE.*" "next step 1" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 2" gdb_test "next" ".*TREE_TYPE.*" "next step 2" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 3" - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" ".*TREE_TYPE.*" "next step 3" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 4" - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" "return 0.*" "next step 4" gdb_test "bt" \ "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ -- 1.9.1