| Message ID | 20181211101411.7067-7-tdevries@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | Handle .gnu_debugaltlink | expand |
On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries <tdevries@suse.de> wrote: > > Factor out the common handling of DW_AT_abstract_origin and > DW_AT_specification from read_function_entry and read_referenced_name. > > 2018-12-10 Tom de Vries <tdevries@suse.de> > > * dwarf.c (read_referenced_name_1): New function. Factor out of ... > (read_referenced_name): ... here, and ... > (read_function_entry): ... here. > +static const char *read_referenced_name (struct dwarf_data *, struct unit *, > + uint64_t, backtrace_error_callback, > + void *); > + We don't need this declaration. Only add a static declaration if there is a forward reference. > +/* Read the name of a function from a DIE referenced by ATTR with VAL. */ > + > +static const char * > +read_referenced_name_1 (struct dwarf_data *ddata, struct unit *u, > + struct attr *attr, struct attr_val *val, > + backtrace_error_callback error_callback, void *data) > +{ Let's not use a name ending in "_1". That is a cop-out. Pick a meaningful name. Perhaps read_referenced_name_from_attr. Ian
On 16-01-19 02:15, Ian Lance Taylor wrote: > On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries <tdevries@suse.de> wrote: >> >> Factor out the common handling of DW_AT_abstract_origin and >> DW_AT_specification from read_function_entry and read_referenced_name. >> >> 2018-12-10 Tom de Vries <tdevries@suse.de> >> >> * dwarf.c (read_referenced_name_1): New function. Factor out of ... >> (read_referenced_name): ... here, and ... >> (read_function_entry): ... here. > >> +static const char *read_referenced_name (struct dwarf_data *, struct unit *, >> + uint64_t, backtrace_error_callback, >> + void *); >> + > > We don't need this declaration. Only add a static declaration if > there is a forward reference. > > We need this static declaration because there's a forward reference to read_referenced_name in read_referenced_name_1. >> +/* Read the name of a function from a DIE referenced by ATTR with VAL. */ >> + >> +static const char * >> +read_referenced_name_1 (struct dwarf_data *ddata, struct unit *u, >> + struct attr *attr, struct attr_val *val, >> + backtrace_error_callback error_callback, void *data) >> +{ > > Let's not use a name ending in "_1". That is a cop-out. Pick a > meaningful name. Perhaps read_referenced_name_from_attr. Used read_referenced_name_from_attr. Thanks, - Tom [libbacktrace] Factor out read_referenced_name_from_attr Factor out the common handling of DW_AT_abstract_origin and DW_AT_specification from read_function_entry and read_referenced_name. 2018-12-10 Tom de Vries <tdevries@suse.de> * dwarf.c (read_referenced_name_from_attr): New function. Factor out of ... (read_referenced_name): ... here, and ... (read_function_entry): ... here. --- libbacktrace/dwarf.c | 89 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c index 03365e05778..2a58d938de6 100644 --- a/libbacktrace/dwarf.c +++ b/libbacktrace/dwarf.c @@ -2111,6 +2111,43 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata, return 0; } +static const char *read_referenced_name (struct dwarf_data *, struct unit *, + uint64_t, backtrace_error_callback, + void *); + +/* Read the name of a function from a DIE referenced by ATTR with VAL. */ + +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) +{ + switch (attr->name) + { + case DW_AT_abstract_origin: + case DW_AT_specification: + break; + default: + return NULL; + } + + if (attr->form == DW_FORM_ref_addr + || attr->form == DW_FORM_ref_sig8) + { + /* This refers to an abstract origin defined in + some other compilation unit. We can handle + this case if we must, but it's harder. */ + return NULL; + } + + 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 NULL; +} + /* 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. */ @@ -2194,24 +2231,14 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u, case DW_AT_specification: /* Second name preference: override DW_AT_name, don't override DW_AT_linkage_name. */ - if (abbrev->attrs[i].form == DW_FORM_ref_addr - || abbrev->attrs[i].form == DW_FORM_ref_sig8) - { - /* This refers to a specification defined in some other - compilation unit. We can handle this case if we - must, but it's harder. */ - break; - } - if (val.encoding == ATTR_VAL_UINT - || val.encoding == ATTR_VAL_REF_UNIT) - { - const char *name; + { + const char *name; - name = read_referenced_name (ddata, u, val.u.uint, - error_callback, data); - if (name != NULL) - ret = name; - } + name = read_referenced_name_from_attr (ddata, u, &abbrev->attrs[i], + &val, error_callback, data); + if (name != NULL) + ret = name; + } break; default: @@ -2436,24 +2463,16 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata, DW_AT_linkage_name. */ if (have_linkage_name) break; - if (abbrev->attrs[i].form == DW_FORM_ref_addr - || abbrev->attrs[i].form == DW_FORM_ref_sig8) - { - /* This refers to an abstract origin defined in - some other compilation unit. We can handle - this case if we must, but it's harder. */ - break; - } - if (val.encoding == ATTR_VAL_UINT - || val.encoding == ATTR_VAL_REF_UNIT) - { - const char *name; - - name = read_referenced_name (ddata, u, val.u.uint, - error_callback, data); - if (name != NULL) - function->name = name; - } + { + const char *name; + + name + = read_referenced_name_from_attr (ddata, u, + &abbrev->attrs[i], &val, + error_callback, data); + if (name != NULL) + function->name = name; + } break; case DW_AT_name:
On Wed, Jan 16, 2019 at 8:37 AM Tom de Vries <tdevries@suse.de> wrote: > > On 16-01-19 02:15, Ian Lance Taylor wrote: > > On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries <tdevries@suse.de> wrote: > >> > >> Factor out the common handling of DW_AT_abstract_origin and > >> DW_AT_specification from read_function_entry and read_referenced_name. > >> > >> 2018-12-10 Tom de Vries <tdevries@suse.de> > >> > >> * dwarf.c (read_referenced_name_1): New function. Factor out of ... > >> (read_referenced_name): ... here, and ... > >> (read_function_entry): ... here. > > > >> +static const char *read_referenced_name (struct dwarf_data *, struct unit *, > >> + uint64_t, backtrace_error_callback, > >> + void *); > >> + > > > > We don't need this declaration. Only add a static declaration if > > there is a forward reference. > > > > > > We need this static declaration because there's a forward reference to > read_referenced_name in read_referenced_name_1. Whoops, sorry. This patch is OK. Thanks. Ian
diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c index 2483295beb4..99e5f4c3f51 100644 --- a/libbacktrace/dwarf.c +++ b/libbacktrace/dwarf.c @@ -2111,6 +2111,42 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata, return 0; } +static const char *read_referenced_name (struct dwarf_data *, struct unit *, + uint64_t, backtrace_error_callback, + void *); + +/* Read the name of a function from a DIE referenced by ATTR with VAL. */ + +static const char * +read_referenced_name_1 (struct dwarf_data *ddata, struct unit *u, + struct attr *attr, struct attr_val *val, + backtrace_error_callback error_callback, void *data) +{ + switch (attr->name) + { + case DW_AT_abstract_origin: + case DW_AT_specification: + break; + default: + return NULL; + } + + if (attr->form == DW_FORM_ref_addr + || attr->form == DW_FORM_ref_sig8) + { + /* This refers to an abstract origin defined in + some other compilation unit. We can handle + this case if we must, but it's harder. */ + return NULL; + } + + 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 NULL; +} + /* 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. */ @@ -2194,24 +2230,14 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u, case DW_AT_specification: /* Second name preference: override DW_AT_name, don't override DW_AT_linkage_name. */ - if (abbrev->attrs[i].form == DW_FORM_ref_addr - || abbrev->attrs[i].form == DW_FORM_ref_sig8) - { - /* This refers to a specification defined in some other - compilation unit. We can handle this case if we - must, but it's harder. */ - break; - } - if (val.encoding == ATTR_VAL_UINT - || val.encoding == ATTR_VAL_REF_UNIT) - { - const char *name; + { + const char *name; - name = read_referenced_name (ddata, u, val.u.uint, + name = read_referenced_name_1 (ddata, u, &abbrev->attrs[i], &val, error_callback, data); - if (name != NULL) - ret = name; - } + if (name != NULL) + ret = name; + } break; default: @@ -2436,24 +2462,15 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata, DW_AT_linkage_name. */ if (have_linkage_name) break; - if (abbrev->attrs[i].form == DW_FORM_ref_addr - || abbrev->attrs[i].form == DW_FORM_ref_sig8) - { - /* This refers to an abstract origin defined in - some other compilation unit. We can handle - this case if we must, but it's harder. */ - break; - } - if (val.encoding == ATTR_VAL_UINT - || val.encoding == ATTR_VAL_REF_UNIT) - { - const char *name; + { + const char *name; - name = read_referenced_name (ddata, u, val.u.uint, - error_callback, data); - if (name != NULL) - function->name = name; - } + name + = read_referenced_name_1 (ddata, u, &abbrev->attrs[i], + &val, error_callback, data); + if (name != NULL) + function->name = name; + } break; case DW_AT_name: