[6/9,libbacktrace] Factor out read_referenced_name_1

Message ID 20181211101411.7067-7-tdevries@suse.de
State New
Headers show
Series
  • Handle .gnu_debugaltlink
Related show

Commit Message

Tom de Vries Dec. 11, 2018, 10:14 a.m.
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.
---
 libbacktrace/dwarf.c | 83 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 50 insertions(+), 33 deletions(-)

-- 
2.16.4

Comments

Martin Sebor via Gcc-patches Jan. 16, 2019, 1:15 a.m. | #1
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
Tom de Vries Jan. 16, 2019, 4:38 p.m. | #2
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:
Ian Lance Taylor Jan. 16, 2019, 6:26 p.m. | #3
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

Patch

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: