arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset
v4 cover arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset
Changes in V4:
- In trace-event-python.c, fixed perf-tools-next merge problem.
Changes in V3:
- Rebased to linux-perf-tools branch.
- Squash symbol-elf.c and symbol.h into same commit.
- In map.c, merge dso__is_pie() call into existing if statement.
- In arm-cs-trace-disasm.py, remove debug artifacts.
Changes in V2:
- In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer checks per Leo Yan review.
- Updated mailing list distribution
Steve Clevenger (4):
Add dso__is_pie call to identify ELF PIE
Force MAPPING_TYPE__IDENTIY for PIE
Add map pgoff to python dictionary based on MAPPING_TYPE Adjust objdump start/end range per map pgoff parameter
.../scripts/python/arm-cs-trace-disasm.py | 9 ++- tools/perf/util/map.c | 4 +- .../scripting-engines/trace-event-python.c | 13 +++- tools/perf/util/symbol-elf.c | 60 +++++++++++++++++++ tools/perf/util/symbol.h | 1 + 5 files changed, 79 insertions(+), 8 deletions(-)
Signature · 1 line
2.25.1
PATCH v4 1/4 Add dso__is_pie call to identify ELF PIE
Add dso__is_pie global to read the .dynamic section DT_FLAGS_1 entry for the DF_1_PIE flag. This identifies position executable code.
--- tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)
tools/perf/util/symbol-elf.c +60 -0
@@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,return err; } +/* +* Check dynamic section DT_FLAGS_1 for a Position Independent +* Executable (PIE). +*/ +bool dso__is_pie (struct dso *dso) +{ +Elf *elf =NULL ; +Elf_Scn *scn =NULL ; +GElf_Ehdr ehdr; +GElf_Shdr shdr; +bool is_pie = false; +char dso_path[PATH_MAX ]; +int fd =-1 ; + +if (!dso || (elf_version (EV_CURRENT ) ==EV_NONE )) + goto exit;// false + +dso__build_id_filename (dso, dso_path,sizeof (dso_path), false); + + fd =open (dso_path,O_RDONLY ); + +if (fd <0 ) { +pr_debug ("%s: cannot read cached %s.\n" , __func__, dso_path); + goto exit;// false + } + + elf =elf_begin (fd,ELF_C_READ ,NULL ); +gelf_getehdr (elf, &ehdr); + +if (ehdr.e_type ==ET_DYN ) { +Elf_Data *data; +GElf_Dyn *entry; +int n_entries = shdr.sh_size /sizeof (GElf_Dyn); + + scn =elf_section_by_name (elf, &ehdr, &shdr,".dynamic" ,NULL ); +if (!scn) + goto exit;// false + + data = (Elf_Data *)elf_getdata (scn,NULL ); +if (!data || !data->d_buf) + goto exit;// false + +// check DT_FLAGS_1 +for (int i =0 ; i < n_entries; i++) { + entry = ((GElf_Dyn *) data->d_buf) + i; +if (entry->d_tag ==DT_FLAGS_1 ) { +if ((entry->d_un.d_val &DF_1_PIE ) !=0 ) { + is_pie = true; +break ; + } + } + }// end for + } + +elf_end (elf); +close (fd); +exit: +return is_pie; +} +/* * We need to check if we have a .dynsym, so that we can handle the * .plt, synthesizing its symbols, that aren't on the symtabs (be it
tools/perf/util/symbol.h +1 -0
@@ -127,6 +127,7 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym);void dso__delete_symbol (struct dso *dso,struct symbol *sym); +bool dso__is_pie (struct dso *dso);struct symbol *dso__find_symbol (struct dso *dso,u64 addr);struct symbol *dso__find_symbol_nocache (struct dso *dso,u64 addr);
Signature · 1 line
2.25.1
Re: [PATCH V4 1/4] Add dso__is_pie call to identify ELF PIE
Quoted 45 lines · Steve Clevenger
Add
dso__is_pieglobal to read the .dynamic sectionDT_FLAGS_1entry for theDF_1_PIEflag. This identifies position executable code.Signed-off-by Steve Clevenger <[email protected]>--- tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)tools/perf/util/symbol-elf.c +27
-0@@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,return err; } +/* +* Check dynamic section DT_FLAGS_1 for a Position Independent +* Executable (PIE). +*/ +bool dso__is_pie (struct dso *dso) +{ +Elf *elf =NULL ; +Elf_Scn *scn =NULL ; +GElf_Ehdr ehdr; +GElf_Shdr shdr; +bool is_pie = false; +char dso_path[PATH_MAX ]; +int fd =-1 ; + +if (!dso || (elf_version (EV_CURRENT ) ==EV_NONE )) + goto exit;// false + +dso__build_id_filename (dso, dso_path,sizeof (dso_path), false); + + fd =open (dso_path,O_RDONLY ); + +if (fd <0 ) { +pr_debug ("%s: cannot read cached %s.\n" , __func__, dso_path); + goto exit;// false + } + + elf =elf_begin (fd,ELF_C_READ ,NULL );
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 701
Quoted 10 lines
tools/perf/util/symbol-elf.c +10
-0+gelf_getehdr (elf, &ehdr); + +if (ehdr.e_type ==ET_DYN ) { +Elf_Data *data; +GElf_Dyn *entry; +int n_entries = shdr.sh_size /sizeof (GElf_Dyn); + + scn =elf_section_by_name (elf, &ehdr, &shdr,".dynamic" ,NULL ); +if (!scn) + goto exit;// false
It should goto a new tag (e.g. exit_failed) so can invoke elf_end(elf) and close(fd).
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 705
Quoted 4 lines
tools/perf/util/symbol-elf.c +4
-0+ + data = (Elf_Data *)elf_getdata (scn,NULL ); +if (!data || !data->d_buf) + goto exit;// false
Ditto.
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 718
Quoted 13 lines
tools/perf/util/symbol-elf.c +13
-0+ +// check DT_FLAGS_1 +for (int i =0 ; i < n_entries; i++) { + entry = ((GElf_Dyn *) data->d_buf) + i; +if (entry->d_tag ==DT_FLAGS_1 ) { +if ((entry->d_un.d_val &DF_1_PIE ) !=0 ) { + is_pie = true; +break ; + } + } + }// end for + } +
Put 'exit_failed' tag at here.
With the changes:
Quoted 24 lines
tools/perf/util/symbol-elf.c +6
-0+elf_end (elf); +close (fd); +exit: +return is_pie; +} +/* * We need to check if we have a .dynsym, so that we can handle the * .plt, synthesizing its symbols, that aren't on the symtabs (be it tools/perf/util/symbol.h +1
-1@@ -127,6 +127,7 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym);void dso__delete_symbol (struct dso *dso,struct symbol *sym); +bool dso__is_pie (struct dso *dso);struct symbol *dso__find_symbol (struct dso *dso,u64 addr);struct symbol *dso__find_symbol_nocache (struct dso *dso,u64 addr); --2.25.1
Re: [PATCH V4 1/4] Add dso__is_pie call to identify ELF PIE
Quoted 85 lines · Leo Yan
Quoted 45 lines · Steve Clevenger
Add
dso__is_pieglobal to read the .dynamic sectionDT_FLAGS_1entry for theDF_1_PIEflag. This identifies position executable code.Signed-off-by Steve Clevenger <[email protected]>--- tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)tools/perf/util/symbol-elf.c +27
-0@@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,return err; } +/* +* Check dynamic section DT_FLAGS_1 for a Position Independent +* Executable (PIE). +*/ +bool dso__is_pie (struct dso *dso) +{ +Elf *elf =NULL ; +Elf_Scn *scn =NULL ; +GElf_Ehdr ehdr; +GElf_Shdr shdr; +bool is_pie = false; +char dso_path[PATH_MAX ]; +int fd =-1 ; + +if (!dso || (elf_version (EV_CURRENT ) ==EV_NONE )) + goto exit;// false + +dso__build_id_filename (dso, dso_path,sizeof (dso_path), false); + + fd =open (dso_path,O_RDONLY ); + +if (fd <0 ) { +pr_debug ("%s: cannot read cached %s.\n" , __func__, dso_path); + goto exit;// false + } + + elf =elf_begin (fd,ELF_C_READ ,NULL );tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 701
Quoted 10 lines
tools/perf/util/symbol-elf.c +10
-0+gelf_getehdr (elf, &ehdr); + +if (ehdr.e_type ==ET_DYN ) { +Elf_Data *data; +GElf_Dyn *entry; +int n_entries = shdr.sh_size /sizeof (GElf_Dyn); + + scn =elf_section_by_name (elf, &ehdr, &shdr,".dynamic" ,NULL ); +if (!scn) + goto exit;// false It should goto a new tag (e.g.
exit_failed) so can invokeelf_end(elf)and close(fd).tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 705
Quoted 4 lines
tools/perf/util/symbol-elf.c +4
-0+ + data = (Elf_Data *)elf_getdata (scn,NULL ); +if (!data || !data->d_buf) + goto exit;// false Ditto.
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 718
Quoted 13 lines
tools/perf/util/symbol-elf.c +13
-0+ +// check DT_FLAGS_1 +for (int i =0 ; i < n_entries; i++) { + entry = ((GElf_Dyn *) data->d_buf) + i; +if (entry->d_tag ==DT_FLAGS_1 ) { +if ((entry->d_un.d_val &DF_1_PIE ) !=0 ) { + is_pie = true; +break ; + } + } + }// end for + } +Put '
exit_failed' tag at here.With the changes:
Quoted 8 lines
Reviewed-by Leo Yan <[email protected]>tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 724
Quoted 6 lines
tools/perf/util/symbol-elf.c +6
-0+elf_end (elf); +close (fd); +exit: +return is_pie; +} +
Hi Leo,
It's been a long time since I've seen goto's used as a rule rather than an exception. I see it introduced the problem you've mentioned. Is it ok to simply update the cover letter (patch 0), and this one (patch 1) as V5? If I don't hear from you, I'll resubmit all.
Thanks,
Steve
Quoted 18 lines
Quoted 18 lines
tools/perf/util/symbol-elf.c +0
-0/* * We need to check if we have a .dynsym, so that we can handle the * .plt, synthesizing its symbols, that aren't on the symtabs (be it tools/perf/util/symbol.h +1
-1@@ -127,6 +127,7 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym);void dso__delete_symbol (struct dso *dso,struct symbol *sym); +bool dso__is_pie (struct dso *dso);struct symbol *dso__find_symbol (struct dso *dso,u64 addr);struct symbol *dso__find_symbol_nocache (struct dso *dso,u64 addr); --2.25.1
Re: [PATCH V4 1/4] Add dso__is_pie call to identify ELF PIE
Quoted 98 lines · Steve Clevenger
Quoted 85 lines · Leo Yan
Quoted 45 lines · Steve Clevenger
Add
dso__is_pieglobal to read the .dynamic sectionDT_FLAGS_1entry for theDF_1_PIEflag. This identifies position executable code.Signed-off-by Steve Clevenger <[email protected]>--- tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)tools/perf/util/symbol-elf.c +27
-0@@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,return err; } +/* +* Check dynamic section DT_FLAGS_1 for a Position Independent +* Executable (PIE). +*/ +bool dso__is_pie (struct dso *dso) +{ +Elf *elf =NULL ; +Elf_Scn *scn =NULL ; +GElf_Ehdr ehdr; +GElf_Shdr shdr; +bool is_pie = false; +char dso_path[PATH_MAX ]; +int fd =-1 ; + +if (!dso || (elf_version (EV_CURRENT ) ==EV_NONE )) + goto exit;// false + +dso__build_id_filename (dso, dso_path,sizeof (dso_path), false); + + fd =open (dso_path,O_RDONLY ); + +if (fd <0 ) { +pr_debug ("%s: cannot read cached %s.\n" , __func__, dso_path); + goto exit;// false + } + + elf =elf_begin (fd,ELF_C_READ ,NULL );tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 701
Quoted 10 lines
tools/perf/util/symbol-elf.c +10
-0+gelf_getehdr (elf, &ehdr); + +if (ehdr.e_type ==ET_DYN ) { +Elf_Data *data; +GElf_Dyn *entry; +int n_entries = shdr.sh_size /sizeof (GElf_Dyn); + + scn =elf_section_by_name (elf, &ehdr, &shdr,".dynamic" ,NULL ); +if (!scn) + goto exit;// false It should goto a new tag (e.g.
exit_failed) so can invokeelf_end(elf)and close(fd).tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 705
Quoted 4 lines
tools/perf/util/symbol-elf.c +4
-0+ + data = (Elf_Data *)elf_getdata (scn,NULL ); +if (!data || !data->d_buf) + goto exit;// false Ditto.
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 718
Quoted 13 lines
tools/perf/util/symbol-elf.c +13
-0+ +// check DT_FLAGS_1 +for (int i =0 ; i < n_entries; i++) { + entry = ((GElf_Dyn *) data->d_buf) + i; +if (entry->d_tag ==DT_FLAGS_1 ) { +if ((entry->d_un.d_val &DF_1_PIE ) !=0 ) { + is_pie = true; +break ; + } + } + }// end for + } +Put '
exit_failed' tag at here.With the changes:
Quoted 8 lines
Reviewed-by Leo Yan <[email protected]>tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 724
Quoted 6 lines
tools/perf/util/symbol-elf.c +6
-0+elf_end (elf); +close (fd); +exit: +return is_pie; +} +Hi Leo,
Quoted 2 lines
It's been a long time since I've seen goto's used as a rule rather than an exception. I see it introduced the problem you've mentioned.
I am not sure if I understand this. In the Linux code, it is quite common to use goto to handle failure cases (for releasing resources).
Quoted 2 lines
Is it ok to simply update the cover letter (patch 0), and this one (patch 1) as V5? If I don't hear from you, I'll resubmit all.
I assume you also need to address patch 03? It is good to send the all patches in one go, this would be easier for maintainers to pick up the whole series (e.g. using b4).
Thanks,
Leo
Re: [PATCH V4 1/4] Add dso__is_pie call to identify ELF PIE
In reply to Steve ClevengerQuoted 98 lines · Steve Clevenger
Quoted 85 lines · Leo Yan
Quoted 45 lines · Steve Clevenger
Add
dso__is_pieglobal to read the .dynamic sectionDT_FLAGS_1entry for theDF_1_PIEflag. This identifies position executable code.Signed-off-by Steve Clevenger <[email protected]>--- tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)tools/perf/util/symbol-elf.c +27
-0@@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,return err; } +/* +* Check dynamic section DT_FLAGS_1 for a Position Independent +* Executable (PIE). +*/ +bool dso__is_pie (struct dso *dso) +{ +Elf *elf =NULL ; +Elf_Scn *scn =NULL ; +GElf_Ehdr ehdr; +GElf_Shdr shdr; +bool is_pie = false; +char dso_path[PATH_MAX ]; +int fd =-1 ; + +if (!dso || (elf_version (EV_CURRENT ) ==EV_NONE )) + goto exit;// false + +dso__build_id_filename (dso, dso_path,sizeof (dso_path), false); + + fd =open (dso_path,O_RDONLY ); + +if (fd <0 ) { +pr_debug ("%s: cannot read cached %s.\n" , __func__, dso_path); + goto exit;// false + } + + elf =elf_begin (fd,ELF_C_READ ,NULL );tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 701
Quoted 10 lines
tools/perf/util/symbol-elf.c +10
-0+gelf_getehdr (elf, &ehdr); + +if (ehdr.e_type ==ET_DYN ) { +Elf_Data *data; +GElf_Dyn *entry; +int n_entries = shdr.sh_size /sizeof (GElf_Dyn); + + scn =elf_section_by_name (elf, &ehdr, &shdr,".dynamic" ,NULL ); +if (!scn) + goto exit;// false It should goto a new tag (e.g.
exit_failed) so can invokeelf_end(elf)and close(fd).tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 705
Quoted 4 lines
tools/perf/util/symbol-elf.c +4
-0+ + data = (Elf_Data *)elf_getdata (scn,NULL ); +if (!data || !data->d_buf) + goto exit;// false Ditto.
tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 718
Quoted 13 lines
tools/perf/util/symbol-elf.c +13
-0+ +// check DT_FLAGS_1 +for (int i =0 ; i < n_entries; i++) { + entry = ((GElf_Dyn *) data->d_buf) + i; +if (entry->d_tag ==DT_FLAGS_1 ) { +if ((entry->d_un.d_val &DF_1_PIE ) !=0 ) { + is_pie = true; +break ; + } + } + }// end for + } +Put '
exit_failed' tag at here.With the changes:
Quoted 8 lines
Reviewed-by Leo Yan <[email protected]>tools/perf/util/symbol-elf.c static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, · line 724
Quoted 6 lines
tools/perf/util/symbol-elf.c +6
-0+elf_end (elf); +close (fd); +exit: +return is_pie; +} +Hi Leo,
Quoted 2 lines
It's been a long time since I've seen goto's used as a rule rather than an exception. I see it introduced the problem you've mentioned.
I am not sure if I understand this. In the Linux code, it is quite common to use goto to handle failure cases (for releasing resources).
Quoted 2 lines
Is it ok to simply update the cover letter (patch 0), and this one (patch 1) as V5? If I don't hear from you, I'll resubmit all.
I assume you also need to address patch 03? It is good to send the all patches in one go, this would be easier for maintainers to pick up the whole series (e.g. using b4).
Thanks,
Leo
PATCH v4 2/4 Force MAPPING_TYPE__IDENTIY for PIE
In reply to Steve ClevengerUse dso__is_pie() to check whether the DSO file is a Position Independent Executable (PIE). If PIE, change the MAPPING_TYPE to MAPPING_TYPE__IDENTITY so a zero map pgoff (text offset) is passed into the script.
--- tools/perf/util/map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/map.c +2 -2
@@ -173,8 +173,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,assert (!dso__kernel (dso));map__init (result, start, start + len, pgoff, dso, prot, flags); -if (anon || no_dso) { - map->mapping_type =MAPPING_TYPE__IDENTITY ; +if (anon || no_dso ||dso__is_pie (dso)) { +map__set_mapping_type (map,MAPPING_TYPE__IDENTITY );/* * Set memory without DSO as loaded. All map__find_*
Signature · 1 line
2.25.1
Re: [PATCH V4 2/4] Force MAPPING_TYPE__IDENTIY for PIE
Quoted 7 lines · Steve Clevenger
Use
dso__is_pie()to check whether the DSO file is a Position Independent Executable (PIE). If PIE, change theMAPPING_TYPEtoMAPPING_TYPE__IDENTITYso a zero map pgoff (text offset) is passed into the script.Signed-off-by Steve Clevenger <[email protected]>
Quoted 22 lines
--- tools/perf/util/map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)tools/perf/util/map.c +2
-3@@ -173,8 +173,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,assert (!dso__kernel (dso));map__init (result, start, start + len, pgoff, dso, prot, flags); -if (anon || no_dso) { - map->mapping_type =MAPPING_TYPE__IDENTITY ; +if (anon || no_dso ||dso__is_pie (dso)) { +map__set_mapping_type (map,MAPPING_TYPE__IDENTITY );/* * Set memory without DSO as loaded. All map__find_* -- 2.25.1
PATCH v4 3/4 Add map pgoff to python dictionary based on MAPPING_TYPE
In reply to Steve ClevengerAdd map_pgoff parameter to python dictionary so it can be seen by the python script, arm-cs-trace-disasm.py. map_pgoff is forced to zero in the dictionary if file type is MAPPING_TYPE__IDENTITY. Otherwise, the map_pgoff value is directly added to the dictionary.
--- .../util/scripting-engines/trace-event-python.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
tools/perf/util/scripting-engines/trace-event-python.c +10 -3
@@ -798,7 +798,8 @@ static int set_regs_in_dict(PyObject *dict,static void set_sym_in_dict (PyObject *dict,struct addr_location *al,const char *dso_field,const char *dso_bid_field,const char *dso_map_start,const char *dso_map_end, -const char *sym_field,const char *symoff_field) +const char *sym_field,const char *symoff_field, +const char *map_pgoff) {char sbuild_id[SBUILD_ID_SIZE ];
@@ -814,6 +815,12 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al, PyLong_FromUnsignedLong(map__start(al->map)));pydict_set_item_string_decref (dict, dso_map_end,PyLong_FromUnsignedLong (map__end (al->map))); +if (al->map->mapping_type ==MAPPING_TYPE__DSO ) +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (al->map->pgoff)); +else +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (0 )); }if (al->sym) { pydict_set_item_string_decref(dict, sym_field,
@@ -900,7 +907,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,pydict_set_item_string_decref (dict,"comm" ,_PyUnicode_FromString (thread__comm_str (al->thread)));set_sym_in_dict (dict, al,"dso" ,"dso_bid" ,"dso_map_start" ,"dso_map_end" , -"symbol" ,"symoff" ); +"symbol" ,"symoff" ,"map_pgoff" );pydict_set_item_string_decref (dict,"callchain" , callchain);
@@ -925,7 +932,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, PyBool_FromLong(1));set_sym_in_dict (dict_sample, addr_al,"addr_dso" ,"addr_dso_bid" ,"addr_dso_map_start" ,"addr_dso_map_end" , -"addr_symbol" ,"addr_symoff" ); +"addr_symbol" ,"addr_symoff" ,"map_pgoff" ); }if (sample->flags)
Signature · 1 line
2.25.1
Re: [PATCH V4 3/4] Add map pgoff to python dictionary based on MAPPING_TYPE
Quoted 26 lines · Steve Clevenger
Add
map_pgoffparameter to python dictionary so it can be seen by the python script,arm-cs-trace-disasm.py.map_pgoffis forced to zero in the dictionary if file type isMAPPING_TYPE__IDENTITY. Otherwise, themap_pgoffvalue is directly added to the dictionary.Signed-off-by Steve Clevenger <[email protected]>--- .../util/scripting-engines/trace-event-python.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)tools/perf/util/scripting-engines/trace-event-python.c +2
-1@@ -798,7 +798,8 @@ static int set_regs_in_dict(PyObject *dict,static void set_sym_in_dict (PyObject *dict,struct addr_location *al,const char *dso_field,const char *dso_bid_field,const char *dso_map_start,const char *dso_map_end, -const char *sym_field,const char *symoff_field) +const char *sym_field,const char *symoff_field, +const char *map_pgoff) {char sbuild_id[SBUILD_ID_SIZE ];
tools/perf/util/scripting-engines/trace-event-python.c static void set_sym_in_dict(PyObject *dict, struct addr_location *al, · line 823
Quoted 9 lines
tools/perf/util/scripting-engines/trace-event-python.c +6
-0@@ -814,6 +815,12 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al, PyLong_FromUnsignedLong(map__start(al->map)));pydict_set_item_string_decref (dict, dso_map_end,PyLong_FromUnsignedLong (map__end (al->map))); +if (al->map->mapping_type ==MAPPING_TYPE__DSO ) +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (al->map->pgoff)); +else +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (0 ));
Indention is inconsistent. Please keep the same format.
Quoted 13 lines
tools/perf/util/scripting-engines/trace-event-python.c +1
-1}if (al->sym) { pydict_set_item_string_decref(dict, sym_field,@@ -900,7 +907,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,pydict_set_item_string_decref (dict,"comm" ,_PyUnicode_FromString (thread__comm_str (al->thread)));set_sym_in_dict (dict, al,"dso" ,"dso_bid" ,"dso_map_start" ,"dso_map_end" , -"symbol" ,"symoff" ); +"symbol" ,"symoff" ,"map_pgoff" );pydict_set_item_string_decref (dict,"callchain" , callchain);
tools/perf/util/scripting-engines/trace-event-python.c static PyObject *get_perf_sample_dict(struct perf_sample *sample, · line 935
Quoted 5 lines
tools/perf/util/scripting-engines/trace-event-python.c +1
-1@@ -925,7 +932,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, PyBool_FromLong(1));set_sym_in_dict (dict_sample, addr_al,"addr_dso" ,"addr_dso_bid" ,"addr_dso_map_start" ,"addr_dso_map_end" , -"addr_symbol" ,"addr_symoff" ); +"addr_symbol" ,"addr_symoff" ,"map_pgoff" );
This dict variable is for data address parsing. For alignment the naming, here can change it as "addr_map_pgoff"?
Please note, from my understanding, this renaming will not impact the sequential change as the python script should only use the dso dict. Please confirm at your side.
With above changes:
Quoted 6 lines
tools/perf/util/scripting-engines/trace-event-python.c +0
-1}if (sample->flags) --2.25.1
Re: [PATCH V4 3/4] Add map pgoff to python dictionary based on MAPPING_TYPE
Quoted 26 lines · Leo Yan
Quoted 25 lines · Steve Clevenger
Add
map_pgoffparameter to python dictionary so it can be seen by the python script,arm-cs-trace-disasm.py.map_pgoffis forced to zero in the dictionary if file type isMAPPING_TYPE__IDENTITY. Otherwise, themap_pgoffvalue is directly added to the dictionary.Signed-off-by Steve Clevenger <[email protected]>--- .../util/scripting-engines/trace-event-python.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)tools/perf/util/scripting-engines/trace-event-python.c +2
-1@@ -798,7 +798,8 @@ static int set_regs_in_dict(PyObject *dict,static void set_sym_in_dict (PyObject *dict,struct addr_location *al,const char *dso_field,const char *dso_bid_field,const char *dso_map_start,const char *dso_map_end, -const char *sym_field,const char *symoff_field) +const char *sym_field,const char *symoff_field, +const char *map_pgoff) {char sbuild_id[SBUILD_ID_SIZE ];
Quoted 12 lines
tools/perf/util/scripting-engines/trace-event-python.c static void set_sym_in_dict(PyObject *dict, struct addr_location *al, · line 823
Quoted 10 lines
tools/perf/util/scripting-engines/trace-event-python.c +6
-0@@ -814,6 +815,12 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al, PyLong_FromUnsignedLong(map__start(al->map)));pydict_set_item_string_decref (dict, dso_map_end,PyLong_FromUnsignedLong (map__end (al->map))); +if (al->map->mapping_type ==MAPPING_TYPE__DSO ) +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (al->map->pgoff)); +else +pydict_set_item_string_decref (dict, map_pgoff, +PyLong_FromUnsignedLongLong (0 ));Indention is inconsistent. Please keep the same format.
Corrected.
Quoted 23 lines
Quoted 14 lines
tools/perf/util/scripting-engines/trace-event-python.c +1
-1}if (al->sym) { pydict_set_item_string_decref(dict, sym_field,@@ -900,7 +907,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,pydict_set_item_string_decref (dict,"comm" ,_PyUnicode_FromString (thread__comm_str (al->thread)));set_sym_in_dict (dict, al,"dso" ,"dso_bid" ,"dso_map_start" ,"dso_map_end" , -"symbol" ,"symoff" ); +"symbol" ,"symoff" ,"map_pgoff" );pydict_set_item_string_decref (dict,"callchain" , callchain);tools/perf/util/scripting-engines/trace-event-python.c static PyObject *get_perf_sample_dict(struct perf_sample *sample, · line 935
Quoted 5 lines
tools/perf/util/scripting-engines/trace-event-python.c +1
-1@@ -925,7 +932,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, PyBool_FromLong(1));set_sym_in_dict (dict_sample, addr_al,"addr_dso" ,"addr_dso_bid" ,"addr_dso_map_start" ,"addr_dso_map_end" , -"addr_symbol" ,"addr_symoff" ); +"addr_symbol" ,"addr_symoff" ,"map_pgoff" );This dict variable is for data address parsing. For alignment the naming, here can change it as "
addr_map_pgoff"?
Quoted 3 lines
Please note, from my understanding, this renaming will not impact the sequential change as the python script should only use the dso dict. Please confirm at your side.
Renamed as "addr_map_pgoff" to match local call convention. The dso dictionary is not affected. See V5 patch series.
Quoted 11 lines
With above changes:
Reviewed-by Leo Yan <[email protected]>Quoted 6 lines
tools/perf/util/scripting-engines/trace-event-python.c +0
-1}if (sample->flags) --2.25.1
PATCH v4 4/4 Adjust objdump start/end range per map pgoff parameter
In reply to Steve ClevengerExtract map_pgoff parameter from the dictionary, and adjust start/end range passed to objdump based on the value.
The start_addr/stop_addr address checks are changed to print a warning only if verbose == True. This script repeatedly sees a zero value passed in for
start_addr = cpu_data[str(cpu) + 'addr']
These zero values are not a new problem. The start_addr/stop_addr warning clutters the instruction trace output, hence this change.
--- tools/perf/scripts/python/arm-cs-trace-disasm.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
tools/perf/scripts/python/arm-cs-trace-disasm.py +6 -3
@@ -187,6 +187,7 @@ def process_event(param_dict): dso_start = get_optional(param_dict, "dso_map_start") dso_end = get_optional(param_dict, "dso_map_end") symbol = get_optional(param_dict, "symbol") + map_pgoff = get_optional(param_dict, "map_pgoff") cpu = sample["cpu"] ip = sample["ip"]
@@ -249,11 +250,13 @@ def process_event(param_dict): return if (start_addr < int(dso_start) or start_addr > int(dso_end)): - print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) return if (stop_addr < int(dso_start) or stop_addr > int(dso_end)): - print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) return if (options.objdump_name != None):
@@ -267,7 +270,7 @@ def process_event(param_dict): dso_fname = get_dso_file_path(dso, dso_bid) if path.exists(dso_fname): - print_disam(dso_fname, dso_vm_start, start_addr, stop_addr) + print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr + map_pgoff) else: print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
Signature · 1 line
2.25.1
Re: [PATCH V4 4/4] Adjust objdump start/end range per map pgoff parameter
Quoted 9 lines · Steve Clevenger
Extract
map_pgoffparameter from the dictionary, and adjust start/end range passed to objdump based on the value.The start_addr/stop_addr address checks are changed to print a warning only if verbose == True. This script repeatedly sees a zero value passed in for
start_addr = cpu_data[str(cpu) + 'addr']
Quoted 4 lines
These zero values are not a new problem. The start_addr/stop_addr warning clutters the instruction trace output, hence this change.
Signed-off-by Steve Clevenger <[email protected]>
Quoted 44 lines
--- tools/perf/scripts/python/arm-cs-trace-disasm.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)tools/perf/scripts/python/arm-cs-trace-disasm.py +6
-4@@ -187,6 +187,7 @@ def process_event(param_dict): dso_start = get_optional(param_dict, "dso_map_start") dso_end = get_optional(param_dict, "dso_map_end") symbol = get_optional(param_dict, "symbol") + map_pgoff = get_optional(param_dict, "map_pgoff") cpu = sample["cpu"] ip = sample["ip"]@@ -249,11 +250,13 @@ def process_event(param_dict): return if (start_addr < int(dso_start) or start_addr > int(dso_end)): - print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) return if (stop_addr < int(dso_start) or stop_addr > int(dso_end)): - print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) return if (options.objdump_name != None):@@ -267,7 +270,7 @@ def process_event(param_dict): dso_fname = get_dso_file_path(dso, dso_bid) if path.exists(dso_fname): - print_disam(dso_fname, dso_vm_start, start_addr, stop_addr) + print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr + map_pgoff) else: print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr)) --2.25.1