tools/perf: Fix to get the DW_AT_decl_file and DW_AT_call_file as unsinged data

PATCH Applied Masami Hiramatsu (Google) 3 participants 4 messages Nov 5, 2022 – Nov 8, 2022 perf events

lore

Share & tools

lore

Masami Hiramatsu (Google)

author

2022-11-05 03:01 UTC #0 lore raw

PATCH 1/1 tools/perf: Fix to get the DW_AT_decl_file and DW_AT_call_file as unsinged data

From: Masami Hiramatsu (Google) <[email protected]>

Dwarf version 5 standard Sec 2.14 says that

Any debugging information entry representing the declaration of an object, module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and DW_AT_decl_column attributes, each of whose value is an unsigned integer constant.

So it should be an unsigned integer data. Also, even though the standard doesn't clearly say the DW_AT_call_file is signed or unsigned, the elfutils (eu-readelf) interprets it as unsigned integer data and it is natural to handle it as unsigned integer data as same as DW_AT_decl_file. This changes the DW_AT_call_file as unsigned integer data too.

Fixes 3f4460a28fb2 ("perf probe: Filter out redundant inline-instances")

Cc [email protected]

Signed-off-by Masami Hiramatsu (Google) <[email protected]>

---
 tools/perf/util/dwarf-aux.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

tools/perf/util/dwarf-aux.c +4 ~~-17~~

index 30b36b525681..b07414409771 100644
@@ -315,19 +315,6 @@ static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
 	return 0;
 }
 
-/* Get attribute and translate it as a sdata */
-static int die_get_attr_sdata(Dwarf_Die *tp_die, unsigned int attr_name,
-			      Dwarf_Sword *result)
-{
-	Dwarf_Attribute attr;
-
-	if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
-	    dwarf_formsdata(&attr, result) != 0)
-		return -ENOENT;
-
-	return 0;
-}
-
 /**
  * die_is_signed_type - Check whether a type DIE is signed or not
  * @tp_die: a DIE of a type

@@ -467,9 +454,9 @@ int die_get_data_member_location(Dwarf_Die *mb_die, Dwarf_Word *offs)
 /* Get the call file index number in CU DIE */
 static int die_get_call_fileno(Dwarf_Die *in_die)
 {
-	Dwarf_Sword idx;
+	Dwarf_Word idx;
 
-	if (die_get_attr_sdata(in_die, DW_AT_call_file, &idx) == 0)
+	if (die_get_attr_udata(in_die, DW_AT_call_file, &idx) == 0)
 		return (int)idx;
 	else
 		return -ENOENT;

@@ -478,9 +465,9 @@ static int die_get_call_fileno(Dwarf_Die *in_die)
 /* Get the declared file index number in CU DIE */
 static int die_get_decl_fileno(Dwarf_Die *pdie)
 {
-	Dwarf_Sword idx;
+	Dwarf_Word idx;
 
-	if (die_get_attr_sdata(pdie, DW_AT_decl_file, &idx) == 0)
+	if (die_get_attr_udata(pdie, DW_AT_decl_file, &idx) == 0)
 		return (int)idx;
 	else
 		return -ENOENT;

Namhyung Kim

maintainer Acked-by on v1 1/1

2022-11-07 21:12 UTC #1 lore raw

Hi Masami,

Quoted 16 lines · Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>
Dwarf version 5 standard Sec 2.14 says that
Any debugging information entry representing the declaration of an object, module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and DW_AT_decl_column attributes, each of whose value is an unsigned integer constant.
So it should be an unsigned integer data. Also, even though the standard doesn't clearly say the DW_AT_call_file is signed or unsigned, the elfutils (eu-readelf) interprets it as unsigned integer data and it is natural to handle it as unsigned integer data as same as DW_AT_decl_file. This changes the DW_AT_call_file as unsigned integer data too.

Quoted 3 lines

Fixes 3f4460a28fb2 ("perf probe: Filter out redundant inline-instances")
Cc [email protected]
Signed-off-by Masami Hiramatsu (Google) <[email protected]>

Acked-by Namhyung Kim <[email protected]>

Thanks,
Namhyung

Quoted 53 lines

---
 tools/perf/util/dwarf-aux.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

tools/perf/util/dwarf-aux.c +4 ~~-17~~

index 30b36b525681..b07414409771 100644
@@ -315,19 +315,6 @@ static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
        return 0;
 }
 
-/* Get attribute and translate it as a sdata */
-static int die_get_attr_sdata(Dwarf_Die *tp_die, unsigned int attr_name,
-                             Dwarf_Sword *result)
-{
-       Dwarf_Attribute attr;
-
-       if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
-           dwarf_formsdata(&attr, result) != 0)
-               return -ENOENT;
-
-       return 0;
-}
-
 /**
  * die_is_signed_type - Check whether a type DIE is signed or not
  * @tp_die: a DIE of a type

@@ -467,9 +454,9 @@ int die_get_data_member_location(Dwarf_Die *mb_die, Dwarf_Word *offs)
 /* Get the call file index number in CU DIE */
 static int die_get_call_fileno(Dwarf_Die *in_die)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(in_die, DW_AT_call_file, &idx) == 0)
+       if (die_get_attr_udata(in_die, DW_AT_call_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

@@ -478,9 +465,9 @@ static int die_get_call_fileno(Dwarf_Die *in_die)
 /* Get the declared file index number in CU DIE */
 static int die_get_decl_fileno(Dwarf_Die *pdie)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(pdie, DW_AT_decl_file, &idx) == 0)
+       if (die_get_attr_udata(pdie, DW_AT_decl_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

Arnaldo Carvalho de Melo

maintainer Applied

2022-11-08 21:08 UTC #2 lore raw

Em Mon, Nov 07, 2022 at 01:09:00PM -0800, Namhyung Kim escreveu:

Quoted 20 lines

Hi Masami,
Quoted 16 lines · Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) <[email protected]>
Dwarf version 5 standard Sec 2.14 says that
Any debugging information entry representing the declaration of an object, module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and DW_AT_decl_column attributes, each of whose value is an unsigned integer constant.
So it should be an unsigned integer data. Also, even though the standard doesn't clearly say the DW_AT_call_file is signed or unsigned, the elfutils (eu-readelf) interprets it as unsigned integer data and it is natural to handle it as unsigned integer data as same as DW_AT_decl_file. This changes the DW_AT_call_file as unsigned integer data too.

Quoted 5 lines

Quoted 3 lines
Fixes 3f4460a28fb2 ("perf probe: Filter out redundant inline-instances")
Cc [email protected]
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
Acked-by Namhyung Kim <[email protected]>

Thanks, applied.

- Arnaldo

Quoted 57 lines

Thanks,
Namhyung

Quoted 53 lines

---
 tools/perf/util/dwarf-aux.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

tools/perf/util/dwarf-aux.c +4 ~~-17~~

index 30b36b525681..b07414409771 100644
@@ -315,19 +315,6 @@ static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
        return 0;
 }
 
-/* Get attribute and translate it as a sdata */
-static int die_get_attr_sdata(Dwarf_Die *tp_die, unsigned int attr_name,
-                             Dwarf_Sword *result)
-{
-       Dwarf_Attribute attr;
-
-       if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
-           dwarf_formsdata(&attr, result) != 0)
-               return -ENOENT;
-
-       return 0;
-}
-
 /**
  * die_is_signed_type - Check whether a type DIE is signed or not
  * @tp_die: a DIE of a type

@@ -467,9 +454,9 @@ int die_get_data_member_location(Dwarf_Die *mb_die, Dwarf_Word *offs)
 /* Get the call file index number in CU DIE */
 static int die_get_call_fileno(Dwarf_Die *in_die)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(in_die, DW_AT_call_file, &idx) == 0)
+       if (die_get_attr_udata(in_die, DW_AT_call_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

@@ -478,9 +465,9 @@ static int die_get_call_fileno(Dwarf_Die *in_die)
 /* Get the declared file index number in CU DIE */
 static int die_get_decl_fileno(Dwarf_Die *pdie)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(pdie, DW_AT_decl_file, &idx) == 0)
+       if (die_get_attr_udata(pdie, DW_AT_decl_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

Signature · 1 line

- Arnaldo

Masami Hiramatsu (Google)

author

2022-11-08 23:35 UTC #3 lore raw

Quoted 21 lines · On Tue, 8 Nov 2022 18:08:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Nov 07, 2022 at 01:09:00PM -0800, Namhyung Kim escreveu:
Quoted 20 lines
Hi Masami,
Quoted 16 lines · Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) <[email protected]>
Dwarf version 5 standard Sec 2.14 says that
Any debugging information entry representing the declaration of an object, module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and DW_AT_decl_column attributes, each of whose value is an unsigned integer constant.
So it should be an unsigned integer data. Also, even though the standard doesn't clearly say the DW_AT_call_file is signed or unsigned, the elfutils (eu-readelf) interprets it as unsigned integer data and it is natural to handle it as unsigned integer data as same as DW_AT_decl_file. This changes the DW_AT_call_file as unsigned integer data too.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Fixes 3f4460a28fb2 ("perf probe: Filter out redundant inline-instances")
Cc [email protected]
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
Acked-by Namhyung Kim <[email protected]>
Thanks, applied.

Thanks Namhyung and Arnaldo!

Quoted 65 lines

- Arnaldo

Quoted 57 lines

Thanks,
Namhyung

Quoted 53 lines

---
 tools/perf/util/dwarf-aux.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

tools/perf/util/dwarf-aux.c +4 ~~-17~~

index 30b36b525681..b07414409771 100644
@@ -315,19 +315,6 @@ static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
        return 0;
 }
 
-/* Get attribute and translate it as a sdata */
-static int die_get_attr_sdata(Dwarf_Die *tp_die, unsigned int attr_name,
-                             Dwarf_Sword *result)
-{
-       Dwarf_Attribute attr;
-
-       if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
-           dwarf_formsdata(&attr, result) != 0)
-               return -ENOENT;
-
-       return 0;
-}
-
 /**
  * die_is_signed_type - Check whether a type DIE is signed or not
  * @tp_die: a DIE of a type

@@ -467,9 +454,9 @@ int die_get_data_member_location(Dwarf_Die *mb_die, Dwarf_Word *offs)
 /* Get the call file index number in CU DIE */
 static int die_get_call_fileno(Dwarf_Die *in_die)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(in_die, DW_AT_call_file, &idx) == 0)
+       if (die_get_attr_udata(in_die, DW_AT_call_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

@@ -478,9 +465,9 @@ static int die_get_call_fileno(Dwarf_Die *in_die)
 /* Get the declared file index number in CU DIE */
 static int die_get_decl_fileno(Dwarf_Die *pdie)
 {
-       Dwarf_Sword idx;
+       Dwarf_Word idx;
 
-       if (die_get_attr_sdata(pdie, DW_AT_decl_file, &idx) == 0)
+       if (die_get_attr_udata(pdie, DW_AT_decl_file, &idx) == 0)
                return (int)idx;
        else
                return -ENOENT;

Signature · 1 line

- Arnaldo

Signature · 1 line

Masami Hiramatsu (Google) <[email protected]>