Re: [PATCH 1/2] [PR symtab/17276]: Coalesce line number entries.

[Simon Marchi <simon dot marchi at ericsson dot com>]

Hi Doug,

One comment below.

On 14-08-15 09:00 PM, Doug Evans wrote:
> Hi.
> 
> This is the first of two patches to fix 17276.
> 
> The problem here is that clang can emit multiple line number entries
> for the same line, e.g., for
> 
>   for (i = 0; i < 100000; i++); // line #34
> 
> clang will emit four line number entries:
> 
> The Directory Table:
>   ../../../binutils-gdb/gdb/testsuite/gdb.base
> 
>  The File Name Table:
>   Entry Dir     Time    Size    Name
>   1     1       0       0       watchpoint-reuse-slot.c
> 
>  Line Number Statements:
>   Extended opcode 2: set Address to 0x400600
>   Advance Line by 29 to 30
>   Copy
>   Set column to 3
>   Set prologue_end to true
>   Special opcode 160: advance Address by 11 to 0x40060b and Line by 1 to 31
>   Set column to 8
>   Special opcode 106: advance Address by 7 to 0x400612 and Line by 3 to 34
>   Extended opcode 4: set Discriminator to 4
>   Special opcode 103: advance Address by 7 to 0x400619 and Line by 0 to 34
>   Extended opcode 4: set Discriminator to 2
>   Special opcode 201: advance Address by 14 to 0x400627 and Line by 0 to 34
>   Set column to 27
>   Extended opcode 4: set Discriminator to 3
>   Special opcode 75: advance Address by 5 to 0x40062c and Line by 0 to 34
>   Set column to 3
>   Extended opcode 4: set Discriminator to 0
>   Advance PC by constant 17 to 0x40063d
>   Special opcode 63: advance Address by 4 to 0x400641 and Line by 2 to 36
>   Advance PC by 2 to 0x400643
>   Extended opcode 1: End of Sequence
> 
> This confuses gdb because a pc can be in the middle of the source line,
> and yet at the start of a line number entry.
> 
> This patch is just cleanup, no functional changes.
> It wraps some operations in functions to abstract away the details.
> 
> 2014-08-15  Doug Evans  <dje@google.com>
> 
> 	* buildsym.h (record_line_ftype): New typedef.
> 	(record_line): Use it.
> 	* dwarf2read.c (dwarf_record_line, dwarf_finish_line): New functions.
> 	(dwarf_decode_lines_1): Call them.
> 
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index 57467c7..8ce01b2 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -171,6 +171,10 @@ EXTERN int context_stack_size;
>  
>  EXTERN int within_function;
>  
> +/* The type of the record_line function.  */
> +typedef void (record_line_ftype) (struct subfile *subfile, int line,
> +				  CORE_ADDR pc);
> +
>  
>  
>  #define next_symbol_text(objfile) (*next_symbol_text_func)(objfile)
> @@ -236,7 +240,7 @@ extern struct context_stack *push_context (int desc, CORE_ADDR valu);
>  
>  extern struct context_stack *pop_context (void);
>  
> -extern void record_line (struct subfile *subfile, int line, CORE_ADDR pc);
> +extern record_line_ftype record_line;
>  
>  extern void start_symtab (const char *name, const char *dirname,
>  			  CORE_ADDR start_addr);
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 801f05d..2017821 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -17172,6 +17172,32 @@ noop_record_line (struct subfile *subfile, int line, CORE_ADDR pc)
>    return;
>  }
>  
> +/* Use P_RECORD_LINE to record line number LINE beginning at address ADDRESS
> +   in the line table of subfile SUBFILE.  */
> +
> +static void
> +dwarf_record_line (struct gdbarch *gdbarch, struct subfile *subfile,
> +		   unsigned int line, CORE_ADDR address,
> +		   record_line_ftype p_record_line)
> +{
> +  CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address);
> +
> +  (*p_record_line) (current_subfile, line, addr);
                       ^^^^^^^^^^^^^^^
I am pretty sure current_subfile here should be subfile.

> +}
> +
> +/* Subroutine of dwarf_decode_lines_1 to simplify it.
> +   Mark the end of a set of line number records.
> +   The arguments are the same as for dwarf_record_line.
> +   If SUBFILE is NULL the request is ignored.  */
> +
> +static void
> +dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile,
> +		   CORE_ADDR address, record_line_ftype p_record_line)
> +{
> +  if (subfile != NULL)
> +    dwarf_record_line (gdbarch, subfile, 0, address, p_record_line);
> +}
> +
>  /* Subroutine of dwarf_decode_lines to simplify it.
>     Process the line number information in LH.  */
>  
> @@ -17205,7 +17231,6 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
>        unsigned int line = 1;
>        int is_stmt = lh->default_is_stmt;
>        int end_sequence = 0;
> -      CORE_ADDR addr;
>        unsigned char op_index = 0;
>  
>        if (!decode_for_pst_p && lh->num_file_names >= file)
> @@ -17257,14 +17282,13 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
>  		    {
>  		      if (last_subfile != current_subfile)
>  			{
> -			  addr = gdbarch_addr_bits_remove (gdbarch, address);
> -			  if (last_subfile)
> -			    (*p_record_line) (last_subfile, 0, addr);
> +			  dwarf_finish_line (gdbarch, last_subfile,
> +					     address, p_record_line);
>  			  last_subfile = current_subfile;
>  			}
>  		      /* Append row to matrix using current values.  */
> -		      addr = gdbarch_addr_bits_remove (gdbarch, address);
> -		      (*p_record_line) (current_subfile, line, addr);
> +		      dwarf_record_line (gdbarch, current_subfile,
> +					 line, address, p_record_line);
>  		    }
>  		}
>  	    }
> @@ -17357,13 +17381,12 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
>  		    {
>  		      if (last_subfile != current_subfile)
>  			{
> -			  addr = gdbarch_addr_bits_remove (gdbarch, address);
> -			  if (last_subfile)
> -			    (*p_record_line) (last_subfile, 0, addr);
> +			  dwarf_finish_line (gdbarch, last_subfile,
> +					     address, p_record_line);
>  			  last_subfile = current_subfile;
>  			}
> -		      addr = gdbarch_addr_bits_remove (gdbarch, address);
> -		      (*p_record_line) (current_subfile, line, addr);
> +		      dwarf_record_line (gdbarch, current_subfile,
> +					 line, address, p_record_line);
>  		    }
>  		}
>  	      break;
> @@ -17459,8 +17482,8 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
>            lh->file_names[file - 1].included_p = 1;
>            if (!decode_for_pst_p)
>  	    {
> -	      addr = gdbarch_addr_bits_remove (gdbarch, address);
> -	      (*p_record_line) (current_subfile, 0, addr);
> +	      dwarf_finish_line (gdbarch, current_subfile, address,
> +				 p_record_line);
>  	    }
>          }
>      }
>