Re: [PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1
On Wed, Jan 16, 2019 at 8:37 AM Tom de Vries wrote: > > On 16-01-19 02:15, Ian Lance Taylor wrote: > > On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries 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 > >> > >> * 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
Re: [PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1
On 16-01-19 02:15, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries 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
>>
>> * 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
* 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, >attrs[i],
+ , 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,
Re: [PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1
On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries 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
>
> * 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
[PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1
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
* dwarf.c (read_referenced_name_1): New function. Factor out of ...
(read_referenced_name): ... here, and ...
(read_function_entry): ... here.
---
libbacktrace/dwarf.c | 83 +++-
1 file changed, 50 insertions(+), 33 deletions(-)
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, >attrs[i], ,
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, >attrs[i],
+ , error_callback, data);
+ if (name != NULL)
+ function->name = name;
+ }
break;
case DW_AT_name:
--
2.16.4
