Fix range end handling of inlined subroutines

Message ID AM6PR03MB51707894C46730F4D2DD88A8E41E0@AM6PR03MB5170.eurprd03.prod.outlook.com
State New, archived
Headers

Commit Message

Bernd Edlinger Feb. 9, 2020, 9:07 p.m. UTC
  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.


Thanks
Bernd.
  

Comments

Andrew Burgess Feb. 10, 2020, 9:48 p.m. UTC | #1
* Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-02-09 21:07:32 +0000]:

> 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.

I've been busy with other projects for the last few days, but plan to
look at this series tomorrow.

Thanks,
Andrew


> 
> 
> Thanks
> Bernd.

> From d15f3346feb1ed5cbbc14e708f3f6b58d88bc0fe Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Sun, 9 Feb 2020 21:13:17 +0100
> Subject: [PATCH] Fix range end handling of inlined subroutines
> 
> Since the is_stmt is now handled, it became
> possible to locate dubious is_stmt line entries
> at the end of an inlined function, in the same
> subfile.
> 
> If any is-stmt line is followed by a non-is-stmt,
> at the same PC as the inlined subroutine ends,
> clear the is-stmt there (except the line = 0
> end marker, of course).
> 
> 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-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* buildsym.c (buildsym_compunit::record_inline_range_end):
> 	New function.
> 	* buildsym.h (buildsym_compunit::record_inline_range_end): Declare.
> 	* dwarf2read.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-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.cp/next-inline.exp: Adjust test.
> ---
>  gdb/buildsym.c                       | 23 +++++++++++++++++++++++
>  gdb/buildsym.h                       |  2 ++
>  gdb/dwarf2read.c                     | 22 +++++++++++++---------
>  gdb/testsuite/gdb.cp/next-inline.exp |  9 ---------
>  4 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 7fd256f..381a7c8 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -732,6 +732,29 @@ 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)
> +{
> +  struct subfile *subfile;
> +
> +  for (subfile = m_subfiles; subfile != NULL; subfile = subfile->next)
> +    {
> +      if (subfile->line_vector != NULL)
> +	{
> +	  struct linetable_entry *items = subfile->line_vector->item;
> +	  int i;
> +
> +	  for (i = subfile->line_vector->nitems - 1; i > 0; i--)
> +	    if (items[i].pc == end && items[i - 1].pc == end
> +		&& !items[i].is_stmt && items[i - 1].line != 0)
> +	      items[i - 1].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
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index c768a4c..3cf0f8b 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;
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index cbee4ab..6919bf1 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14407,10 +14407,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;
>  
> @@ -14521,10 +14517,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;
>  
> @@ -14564,6 +14556,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;
> @@ -14801,6 +14797,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)
> @@ -14816,7 +14813,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);
>          }
>      }
>  
> @@ -14841,6 +14841,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
>
  

Patch

From d15f3346feb1ed5cbbc14e708f3f6b58d88bc0fe Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Sun, 9 Feb 2020 21:13:17 +0100
Subject: [PATCH] Fix range end handling of inlined subroutines

Since the is_stmt is now handled, it became
possible to locate dubious is_stmt line entries
at the end of an inlined function, in the same
subfile.

If any is-stmt line is followed by a non-is-stmt,
at the same PC as the inlined subroutine ends,
clear the is-stmt there (except the line = 0
end marker, of course).

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-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* buildsym.c (buildsym_compunit::record_inline_range_end):
	New function.
	* buildsym.h (buildsym_compunit::record_inline_range_end): Declare.
	* dwarf2read.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-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.cp/next-inline.exp: Adjust test.
---
 gdb/buildsym.c                       | 23 +++++++++++++++++++++++
 gdb/buildsym.h                       |  2 ++
 gdb/dwarf2read.c                     | 22 +++++++++++++---------
 gdb/testsuite/gdb.cp/next-inline.exp |  9 ---------
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 7fd256f..381a7c8 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -732,6 +732,29 @@  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)
+{
+  struct subfile *subfile;
+
+  for (subfile = m_subfiles; subfile != NULL; subfile = subfile->next)
+    {
+      if (subfile->line_vector != NULL)
+	{
+	  struct linetable_entry *items = subfile->line_vector->item;
+	  int i;
+
+	  for (i = subfile->line_vector->nitems - 1; i > 0; i--)
+	    if (items[i].pc == end && items[i - 1].pc == end
+		&& !items[i].is_stmt && items[i - 1].line != 0)
+	      items[i - 1].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
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index c768a4c..3cf0f8b 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;
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index cbee4ab..6919bf1 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14407,10 +14407,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;
 
@@ -14521,10 +14517,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;
 
@@ -14564,6 +14556,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;
@@ -14801,6 +14797,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)
@@ -14816,7 +14813,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);
         }
     }
 
@@ -14841,6 +14841,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