[PATCH v3 1/3] Introduce a new line table flag is_weak

Andrew Burgess andrew.burgess@embecosm.com
Fri Sep 10 09:00:14 GMT 2021


Hi Bernd,

When preparing multi-patch series with git format-patch it is really
helpful if you can use the --thread option, this will keep all of the
mail together inside most folks email clients.

Thanks,
Andrew


* Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-09-05 21:15:23 +0200]:

> This introduces a new line table flag is_weak.
> The line entries at the end of a subroutine range,
> use this to indicate that they may possibly
> be part of the previous subroutine.
> 
> 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_weak = 1.
> 
> Additionally this adds a "fake" end sequence to the
> record_line function, that is line number -1.
> That will be used in the next patch.
> 
> Finally this adds a handling for empty ranges to
> record_block_range.  Currently this function is
> not called with empty ranges, but that will be used
> in the next patch.
> 
> There should be no functional changes after this commit.





> 
> gdb:
> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* buildsym.c (buildsym_compunit::record_block_range): Store inline
> 	range end PC values.  Make empty inline ranges one byte long.
> 	(buildsym_compunit::record_line): Update the comment about the
> 	end of sequence marker.  Mark previous lines at the same PC
> 	as weak for a fake end sequence marker.
> 	(buildsym_compunit::patch_inline_end_pos): New helper function.
> 	(buildsym_compunit::end_symtab_with_blockvector): Patch line table.
> 	* buildsym.h (buildsym_compunit::m_inline_end_vector): New data item.
> 	* jit.c (jit_symtab_line_mapping_add_impl): Initialize is_weak.
> 	* symmisc.c (dump_symtab_1,
> 	maintenance_print_one_line_table): Handle is_weak.
> 	* symtab.h (linetable_entry::is_weak,
> 	symtab_and_line::is_weak): New data items.
> 	* xcoffread.c (arrange_linetable): Initialize is_weak.
> 
> gdb/testsuite:
> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.dwarf2/dw2-ranges-base.exp: Adjust test, handle WEAK.
> 	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: Likewise.
> ---
>  gdb/buildsym.c                                     | 103 ++++++++++++++++++---
>  gdb/buildsym.h                                     |   3 +
>  gdb/jit.c                                          |   1 +
>  gdb/symmisc.c                                      |  10 +-
>  gdb/symtab.h                                       |   4 +
>  .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     |   4 +-
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp       |   6 +-
>  gdb/xcoffread.c                                    |   1 +
>  8 files changed, 111 insertions(+), 21 deletions(-)
> 

> From c213b2f7dc961649e36431963d9f6b221e92f797 Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Tue, 3 Nov 2020 18:41:43 +0100
> Subject: [PATCH v3 1/3] Introduce a new line table flag is_weak
> 
> This introduces a new line table flag is_weak.
> The line entries at the end of a subroutine range,
> use this to indicate that they may possibly
> be part of the previous subroutine.
> 
> 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_weak = 1.
> 
> Additionally this adds a "fake" end sequence to the
> record_line function, that is line number -1.
> That will be used in the next patch.
> 
> Finally this adds a handling for empty ranges to
> record_block_range.  Currently this function is
> not called with empty ranges, but that will be used
> in the next patch.
> 
> There should be no functional changes after this commit.
> 
> gdb:
> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* buildsym.c (buildsym_compunit::record_block_range): Store inline
> 	range end PC values.  Make empty inline ranges one byte long.
> 	(buildsym_compunit::record_line): Update the comment about the
> 	end of sequence marker.  Mark previous lines at the same PC
> 	as weak for a fake end sequence marker.
> 	(buildsym_compunit::patch_inline_end_pos): New helper function.
> 	(buildsym_compunit::end_symtab_with_blockvector): Patch line table.
> 	* buildsym.h (buildsym_compunit::m_inline_end_vector): New data item.
> 	* jit.c (jit_symtab_line_mapping_add_impl): Initialize is_weak.
> 	* symmisc.c (dump_symtab_1,
> 	maintenance_print_one_line_table): Handle is_weak.
> 	* symtab.h (linetable_entry::is_weak,
> 	symtab_and_line::is_weak): New data items.
> 	* xcoffread.c (arrange_linetable): Initialize is_weak.
> 
> gdb/testsuite:
> 2021-01-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.dwarf2/dw2-ranges-base.exp: Adjust test, handle WEAK.
> 	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: Likewise.
> ---
>  gdb/buildsym.c                                     | 103 ++++++++++++++++++---
>  gdb/buildsym.h                                     |   3 +
>  gdb/jit.c                                          |   1 +
>  gdb/symmisc.c                                      |  10 +-
>  gdb/symtab.h                                       |   4 +
>  .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     |   4 +-
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp       |   6 +-
>  gdb/xcoffread.c                                    |   1 +
>  8 files changed, 111 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 0f7449f..7c0153c 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -425,6 +425,16 @@ struct block *
>        || end_inclusive + 1 != BLOCK_END (block))
>      m_pending_addrmap_interesting = true;
>  
> +  if (block_inlined_p (block))
> +    {
> +      m_inline_end_vector.push_back (end_inclusive + 1);
> +      if (end_inclusive + 1 == start)
> +	{
> +	  end_inclusive = start;
> +	  m_pending_addrmap_interesting = true;
> +	}
> +    }
> +
>    if (m_pending_addrmap == nullptr)
>      m_pending_addrmap = addrmap_create_mutable (&m_pending_addrmap_obstack);
>  
> @@ -692,19 +702,16 @@ struct blockvector *
>  		      * sizeof (struct linetable_entry))));
>      }
>  
> -  /* Normally, we treat lines as unsorted.  But the end of sequence
> -     marker is special.  We sort line markers at the same PC by line
> -     number, so end of sequence markers (which have line == 0) appear
> -     first.  This is right if the marker ends the previous function,
> -     and there is no padding before the next function.  But it is
> -     wrong if the previous line was empty and we are now marking a
> -     switch to a different subfile.  We must leave the end of sequence
> -     marker at the end of this group of lines, not sort the empty line
> -     to after the marker.  The easiest way to accomplish this is to
> -     delete any empty lines from our table, if they are followed by
> -     end of sequence markers.  All we lose is the ability to set
> -     breakpoints at some lines which contain no instructions
> -     anyway.  */
> +  /* The end of sequence marker is special.  We need to delete any
> +     previous lines at the same PC, otherwise these lines may cause
> +     problems since they might be at the same address as the following
> +     function.  For instance suppose a function calls abort there is no
> +     reason to emit a ret after that point (no joke).
> +     So the label may be at the same address where the following
> +     function begins.  There is also a fake end of sequence marker (-1)
> +     that we emit internally when switching between different CUs
> +     In this case, duplicate line table entries shall not be deleted.
> +     We simply set the is_weak marker in this case.  */
>    if (line == 0)
>      {
>        struct linetable_entry *last = nullptr;
> @@ -720,14 +727,80 @@ struct blockvector *
>        if (last == nullptr || last->line == 0)
>  	return;
>      }
> +  else if (line == -1)
> +    {
> +      line = 0;
> +      e = subfile->line_vector->item + subfile->line_vector->nitems;
> +      while (e > subfile->line_vector->item)
> +	{
> +	  e--;
> +	  if (e->pc != pc)
> +	    break;
> +	  e->is_weak = 1;
> +	}
> +    }
>  
>    e = subfile->line_vector->item + subfile->line_vector->nitems++;
>    e->line = line;
>    e->is_stmt = is_stmt ? 1 : 0;
> +  e->is_weak = 0;
>    e->pc = pc;
>  }
>  
>  
> +/* 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_weak = 1 as well.  */
> +  do
> +    {
> +      /* We stop at the first line entry with a different address,
> +	 or when we see an end of sequence marker.  */
> +      a--;
> +      if (items[a].pc != end || items[a].line == 0)
> +	break;
> +
> +      items[a].is_weak = 1;
> +    }
> +  while (a > 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 +1038,10 @@ struct compunit_symtab *
>  			      subfile->line_vector->item
>  			      + subfile->line_vector->nitems,
>  			      lte_is_less_than);
> +
> +	  for (int i = 0; i < m_inline_end_vector.size (); 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 b35c26d..bacf308 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -397,6 +397,9 @@ struct buildsym_compunit
>  
>    /* Pending symbols that are local to the lexical context.  */
>    struct pending *m_local_symbols = nullptr;
> +
> +  /* Pending inline end range addresses.  */
> +  std::vector<CORE_ADDR> m_inline_end_vector;
>  };
>  
>  
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 666262e..c0e8a09 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -463,6 +463,7 @@ struct gdb_object
>        stab->linetable->item[i].pc = (CORE_ADDR) map[i].pc;
>        stab->linetable->item[i].line = map[i].line;
>        stab->linetable->item[i].is_stmt = 1;
> +      stab->linetable->item[i].is_weak = 0;
>      }
>  }
>  
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index e38ceb6..7905a6f 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -271,6 +271,8 @@ static void print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
>  	  fputs_filtered (paddress (gdbarch, l->item[i].pc), outfile);
>  	  if (l->item[i].is_stmt)
>  	    fprintf_filtered (outfile, "\t(stmt)");
> +	  if (l->item[i].is_weak)
> +	    fprintf_filtered (outfile, "\t(weak)");
>  	  fprintf_filtered (outfile, "\n");
>  	}
>      }
> @@ -987,11 +989,12 @@ static void print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
>        /* Leave space for 6 digits of index and line number.  After that the
>  	 tables will just not format as well.  */
>        struct ui_out *uiout = current_uiout;
> -      ui_out_emit_table table_emitter (uiout, 4, -1, "line-table");
> +      ui_out_emit_table table_emitter (uiout, 5, -1, "line-table");
>        uiout->table_header (6, ui_left, "index", _("INDEX"));
>        uiout->table_header (6, ui_left, "line", _("LINE"));
>        uiout->table_header (18, ui_left, "address", _("ADDRESS"));
> -      uiout->table_header (1, ui_left, "is-stmt", _("IS-STMT"));
> +      uiout->table_header (4, ui_left, "stmt", _("STMT"));
> +      uiout->table_header (4, ui_left, "weak", _("WEAK"));
>        uiout->table_body ();
>  
>        for (int i = 0; i < linetable->nitems; ++i)
> @@ -1007,7 +1010,8 @@ static void print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
>  	    uiout->field_string ("line", _("END"));
>  	  uiout->field_core_addr ("address", objfile->arch (),
>  				  item->pc);
> -	  uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
> +	  uiout->field_string ("stmt", item->is_stmt ? "Y" : "");
> +	  uiout->field_string ("weak", item->is_weak ? "Y" : "");
>  	  uiout->text ("\n");
>  	}
>      }
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index fd8dd62..ddaafb0 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1323,6 +1323,8 @@ struct linetable_entry
>  
>    /* True if this PC is a good location to place a breakpoint for LINE.  */
>    unsigned is_stmt : 1;
> +  /* True if this PC is at a subroutine range end.  */
> +  unsigned is_weak : 1;
>  
>    /* The address for this entry.  */
>    CORE_ADDR pc;
> @@ -1910,6 +1912,8 @@ struct symtab_and_line
>    /* If the line number information is valid, then this indicates if this
>       line table entry had the is-stmt flag set or not.  */
>    bool is_stmt = false;
> +  /* True if this PC is at a subroutine range end.  */
> +  bool is_weak = false;
>  
>    /* The probe associated with this symtab_and_line.  */
>    probe *prob = NULL;
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
> index 42cfd0b..be5c29d 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
> @@ -86,10 +86,10 @@ if ![runto_main] {
>  
>  set test "END with address 1 eliminated"
>  gdb_test_multiple "maint info line-table $srcfile$" $test {
> -    -re -wrap "END *0x0*1 *Y \r\n.*" {
> +    -re -wrap "END *0x0*1 *Y *\r\n.*" {
>  	fail $gdb_test_name
>      }
> -    -re -wrap "END *$hex *Y " {
> +    -re -wrap "END *$hex *Y *" {
>  	pass $gdb_test_name
>      }
>  }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> index d55b7fd..a79643f 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> @@ -154,7 +154,7 @@ set prev -1
>  set seq_count 0
>  gdb_test_multiple "maint info line-table gdb.dwarf2/dw2-ranges-base.c" \
>      "count END markers in line table" {
> -	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\(\[ \t\]+Y\)? *\r\n" {
> +	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\(\[ \t\]+Y\)?(\[ \t\]+Y\)? *\r\n" {
>  	    if { $prev != -1 } {
>  		gdb_assert "$prev == 1" \
>  		    "prev of normal entry at $seq_count is end marker"
> @@ -163,7 +163,7 @@ gdb_test_multiple "maint info line-table gdb.dwarf2/dw2-ranges-base.c" \
>  	    incr seq_count
>  	    exp_continue
>  	}
> -	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\(\[ \t\]+Y\)? *\r\n" {
> +	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\(\[ \t\]+Y\)?\(\[ \t\]+Y\)? *\r\n" {
>  	    if { $prev != -1 } {
>  		gdb_assert "$prev == 0" \
>  		    "prev of end marker at $seq_count is normal entry"
> @@ -179,7 +179,7 @@ gdb_test_multiple "maint info line-table gdb.dwarf2/dw2-ranges-base.c" \
>  	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
>  	    exp_continue
>  	}
> -	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\[ \t\]+IS-STMT *\r\n" {
> +	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\[ \t\]+STMT\[ \t\]+WEAK *\r\n" {
>  	    exp_continue
>  	}
>      }
> diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
> index a854d4d..1409971 100644
> --- a/gdb/xcoffread.c
> +++ b/gdb/xcoffread.c
> @@ -449,6 +449,7 @@ struct find_targ_sec_arg
>  	    }
>  	  fentry[function_count].line = ii;
>  	  fentry[function_count].is_stmt = 1;
> +	  fentry[function_count].is_weak = 0;
>  	  fentry[function_count].pc = oldLineTb->item[ii].pc;
>  	  ++function_count;
>  
> -- 
> 1.9.1
> 



More information about the Gdb-patches mailing list