[PING] Re: [PATCH 1/3] Add backtrace_decl_line to libbacktrace

Andi Kleen <[email protected]> writes:

I wanted to ping this patchkit.


-Andi

> From: Andi Kleen <[email protected]>
>
> autofdo uses relative line numbers to the dwarf declaration line.
> This is different from the first line of the program.
>
> In order to use libbacktrace for this track the decl_line and
> report it out in moredata mode.
>
> libbacktrace/ChangeLog:
>
> 	* backtrace.h (struct backtrace_moredata): Add
> 	backtrace_decl_line.
> 	* dwarf.c (struct function): Add decl_line.
> 	(call_callback): Handle decl_line.
> 	(read_referenced_name): Pass decl_line.
> 	(read_referenced_name_from_attr): Handle decl_line.
> 	(read_function_entry): Dito.
> 	(report_inlined_functions): Dito.
> 	(dwarf_lookup_pc): Dito.
> 	(dwarf_fileline): Dito.
> 	* internal.h (BACKTRACE_MOREDATA_VERSION): Bump to 4.
> ---
>  libbacktrace/backtrace.h |  5 +++-
>  libbacktrace/dwarf.c     | 62 ++++++++++++++++++++++++++--------------
>  libbacktrace/internal.h  |  2 +-
>  3 files changed, 46 insertions(+), 23 deletions(-)
>
> diff --git a/libbacktrace/backtrace.h b/libbacktrace/backtrace.h
> index 9ca12730a7c..5b076183ca0 100644
> --- a/libbacktrace/backtrace.h
> +++ b/libbacktrace/backtrace.h
> @@ -245,7 +245,7 @@ struct backtrace_moredata
>       possible that future versions of libbacktrace will add new fields
>       and increment the version number accordingly. There is no plan to
>       remove fields from this struct.  Thus the current value of the
> -     version field will be 3.  */
> +     version field will be 4.  */
>    int backtrace_version;
>    /* The DATA value passed to whatever function is calling the callback
>       (backtrace_full, backtrace_pcinfo, or backtrace_syminfo).  */
> @@ -253,6 +253,9 @@ struct backtrace_moredata
>    /* The DWARF discriminator.  This is zero if there is none. See
>       https://wiki.dwarfstd.org/Path_Discriminators.md.  */
>    unsigned int backtrace_discriminator;
> +  /* The first line number of the declaration of the function to compute
> +     relative line numbers.  */
> +  unsigned int backtrace_decl_line;
>  };
>  
>  #ifdef __cplusplus
> diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
> index 0d29443d7aa..0ea344f18b4 100644
> --- a/libbacktrace/dwarf.c
> +++ b/libbacktrace/dwarf.c
> @@ -268,6 +268,8 @@ struct function
>    /* If this is an inlined function, the discriminator of the call
>       site.  */
>    unsigned int caller_disc;
> +  /* The line number of the declaration.  */
> +  unsigned int decl_line;
>    /* Map PC ranges to inlined functions.  */
>    struct function_addrs *function_addrs;
>    size_t function_addrs_count;
> @@ -737,7 +739,7 @@ read_initial_length (struct dwarf_buf *buf, int *is_dwarf64)
>  static int
>  call_callback (struct backtrace_state *state, backtrace_full_callback callback,
>  	       void *data, uintptr_t pc, const char *filename, int lineno,
> -	       const char *function, unsigned int disc)
> +	       const char *function, unsigned int disc, unsigned int decl_line)
>  {
>    if (!state->moredata)
>      return callback (data, pc, filename, lineno, function);
> @@ -749,6 +751,7 @@ call_callback (struct backtrace_state *state, backtrace_full_callback callback,
>        md.backtrace_version = BACKTRACE_MOREDATA_VERSION;
>        md.backtrace_data = data;
>        md.backtrace_discriminator = disc;
> +      md.backtrace_decl_line = decl_line;
>        return callback ((void *) &md, pc, filename, lineno, function);
>      }
>  }
> @@ -3245,7 +3248,7 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata,
>  
>  static const char *read_referenced_name (struct dwarf_data *, struct unit *,
>  					 uint64_t, backtrace_error_callback,
> -					 void *);
> +					 void *, unsigned int *decl_line);
>  
>  /* Read the name of a function from a DIE referenced by ATTR with VAL.  */
>  
> @@ -3253,7 +3256,7 @@ static const char *
>  read_referenced_name_from_attr (struct dwarf_data *ddata, struct unit *u,
>  				struct attr *attr, struct attr_val *val,
>  				backtrace_error_callback error_callback,
> -				void *data)
> +				void *data, unsigned int *decl_line)
>  {
>    switch (attr->name)
>      {
> @@ -3276,12 +3279,12 @@ read_referenced_name_from_attr (struct dwarf_data *ddata, struct unit *u,
>  	return NULL;
>  
>        uint64_t offset = val->u.uint - unit->low_offset;
> -      return read_referenced_name (ddata, unit, offset, error_callback, data);
> +      return read_referenced_name (ddata, unit, offset, error_callback, data, decl_line);
>      }
>  
>    if (val->encoding == ATTR_VAL_UINT
>        || val->encoding == ATTR_VAL_REF_UNIT)
> -    return read_referenced_name (ddata, u, val->u.uint, error_callback, data);
> +    return read_referenced_name (ddata, u, val->u.uint, error_callback, data, decl_line);
>  
>    if (val->encoding == ATTR_VAL_REF_ALT_INFO)
>      {
> @@ -3293,7 +3296,7 @@ read_referenced_name_from_attr (struct dwarf_data *ddata, struct unit *u,
>  
>        uint64_t offset = val->u.uint - alt_unit->low_offset;
>        return read_referenced_name (ddata->altlink, alt_unit, offset,
> -				   error_callback, data);
> +				   error_callback, data, decl_line);
>      }
>  
>    return NULL;
> @@ -3301,12 +3304,13 @@ read_referenced_name_from_attr (struct dwarf_data *ddata, struct unit *u,
>  
>  /* Read the name of a function from a DIE referenced by a
>     DW_AT_abstract_origin or DW_AT_specification tag.  OFFSET is within
> -   the same compilation unit.  */
> +   the same compilation unit. DECL_LINE is a pointer to output the
> +   decl_line.  */
>  
>  static const char *
>  read_referenced_name (struct dwarf_data *ddata, struct unit *u,
>  		      uint64_t offset, backtrace_error_callback error_callback,
> -		      void *data)
> +		      void *data, unsigned int *decl_line)
>  {
>    struct dwarf_buf unit_buf;
>    uint64_t code;
> @@ -3398,12 +3402,18 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u,
>  	    const char *name;
>  
>  	    name = read_referenced_name_from_attr (ddata, u, &abbrev->attrs[i],
> -						   &val, error_callback, data);
> +						   &val, error_callback, data,
> +						   decl_line);
>  	    if (name != NULL)
>  	      ret = name;
>  	  }
>  	  break;
>  
> +	case DW_AT_decl_line:
> +	  if (decl_line != NULL && val.encoding == ATTR_VAL_UINT)
> +	    *decl_line = val.u.uint;
> +	  break;
> +
>  	default:
>  	  break;
>  	}
> @@ -3556,6 +3566,11 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
>  		    function->caller_disc = val.u.uint;
>  		  break;
>  
> +		case DW_AT_decl_line:
> +		  if (val.encoding == ATTR_VAL_UINT)
> +		    function->decl_line = val.u.uint;
> +		  break;
> +
>  		case DW_AT_call_line:
>  		  if (val.encoding == ATTR_VAL_UINT)
>  		    function->caller_lineno = val.u.uint;
> @@ -3573,7 +3588,8 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
>  		    name
>  		      = read_referenced_name_from_attr (ddata, u,
>  							&abbrev->attrs[i], &val,
> -							error_callback, data);
> +							error_callback, data,
> +							&function->decl_line);
>  		    if (name != NULL)
>  		      function->name = name;
>  		  }
> @@ -3804,7 +3820,8 @@ report_inlined_functions (struct backtrace_state *state, uintptr_t pc,
>  			  struct function *function,
>  			  backtrace_full_callback callback, void *data,
>  			  const char **filename, int *lineno,
> -			  unsigned int *disc)
> +			  unsigned int *disc,
> +			  unsigned int *decl_line)
>  {
>    struct function_addrs *p;
>    struct function_addrs *match;
> @@ -3857,13 +3874,13 @@ report_inlined_functions (struct backtrace_state *state, uintptr_t pc,
>  
>    /* Report any calls inlined into this one.  */
>    ret = report_inlined_functions (state, pc, inlined, callback, data,
> -				  filename, lineno, disc);
> +				  filename, lineno, disc, decl_line);
>    if (ret != 0)
>      return ret;
>  
>    /* Report this inlined call.  */
>    ret = call_callback (state, callback, data, pc, *filename, *lineno,
> -		       inlined->name, *disc);
> +		       inlined->name, *disc, *decl_line);
>    if (ret != 0)
>      return ret;
>  
> @@ -3872,6 +3889,7 @@ report_inlined_functions (struct backtrace_state *state, uintptr_t pc,
>    *filename = inlined->caller_filename;
>    *lineno = inlined->caller_lineno;
>    *disc = inlined->caller_disc;
> +  *decl_line = inlined->decl_line;
>  
>    return 0;
>  }
> @@ -3899,6 +3917,7 @@ dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata,
>    const char *filename;
>    int lineno;
>    unsigned int disc;
> +  unsigned int decl_line;
>    int ret;
>  
>    *found = 1;
> @@ -4040,7 +4059,7 @@ dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata,
>        if (new_data)
>  	return dwarf_lookup_pc (state, ddata, pc, callback, error_callback,
>  				data, found);
> -      return call_callback (state, callback, data, pc, NULL, 0, NULL, 0);
> +      return call_callback (state, callback, data, pc, NULL, 0, NULL, 0, 0);
>      }
>  
>    /* Search for PC within this unit.  */
> @@ -4088,14 +4107,14 @@ dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata,
>  	}
>  
>        return call_callback (state, callback, data, pc, entry->u->abs_filename,
> -			    0, NULL, 0);
> +			    0, NULL, 0, 0);
>      }
>  
>    /* Search for function name within this unit.  */
>  
>    if (entry->u->function_addrs_count == 0)
>      return call_callback (state, callback, data, pc, ln->filename, ln->lineno,
> -			  NULL, ln->disc);
> +			  NULL, ln->disc, 0);
>  
>    p = ((struct function_addrs *)
>         bsearch (&pc, entry->u->function_addrs,
> @@ -4104,7 +4123,7 @@ dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata,
>  		function_addrs_search));
>    if (p == NULL)
>      return call_callback (state, callback, data, pc, ln->filename, ln->lineno,
> -			  NULL, ln->disc);
> +			  NULL, ln->disc, 0);
>  
>    /* Here pc >= p->low && pc < (p + 1)->low.  The function_addrs are
>       sorted by low, so if pc > p->low we are at the end of a range of
> @@ -4129,21 +4148,22 @@ dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata,
>      }
>    if (fmatch == NULL)
>      return call_callback (state, callback, data, pc, ln->filename, ln->lineno,
> -			  NULL, ln->disc);
> +			  NULL, ln->disc, 0);
>  
>    function = fmatch->function;
>  
>    filename = ln->filename;
>    lineno = ln->lineno;
>    disc = ln->disc;
> +  decl_line = function->decl_line;
>  
>    ret = report_inlined_functions (state, pc, function, callback, data,
> -				  &filename, &lineno, &disc);
> +				  &filename, &lineno, &disc, &decl_line);
>    if (ret != 0)
>      return ret;
>  
>    return call_callback (state, callback, data, pc, filename, lineno,
> -			function->name, disc);
> +			function->name, disc, decl_line);
>  }
>  
>  
> @@ -4193,7 +4213,7 @@ dwarf_fileline (struct backtrace_state *state, uintptr_t pc,
>  
>    /* FIXME: See if any libraries have been dlopen'ed.  */
>  
> -  return call_callback (state, callback, data, pc, NULL, 0, NULL, 0);
> +  return call_callback (state, callback, data, pc, NULL, 0, NULL, 0, 0);
>  }
>  
>  /* Initialize our data structures from the DWARF debug info for a
> diff --git a/libbacktrace/internal.h b/libbacktrace/internal.h
> index 355d2a353fe..26b41fb7f7f 100644
> --- a/libbacktrace/internal.h
> +++ b/libbacktrace/internal.h
> @@ -428,6 +428,6 @@ extern int backtrace_uncompress_lzma (struct backtrace_state *,
>  				      size_t *uncompressed_size);
>  
>  /* Current expected backtrace_moredata version.  */
> -#define BACKTRACE_MOREDATA_VERSION (3)
> +#define BACKTRACE_MOREDATA_VERSION (4)
>  
>  #endif