* [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-27 20:23 ` Leo Yan
2024-08-26 21:35 ` [PATCH V2 4/5] Add map pgoff to python dictionary based on MAPPING_TYPE Steve Clevenger
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
Extract 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.
Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
.../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
index d973c2baed1c..6bf806078f9a 100755
--- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
+++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
@@ -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"]
@@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]':
+ if (dso == "[kernel.kallsyms]"):
+ dso_vm_start = 0
+ map_pgoff = '[unknown]'
+ else:
+ dso_vm_start = int(dso_start)
+ start_addr += map_pgoff
+ stop_addr += map_pgoff
+ map_pgoff = '[unknown]'
+
if (options.objdump_name != None):
# It doesn't need to decrease virtual memory offset for disassembly
# for kernel dso and executable file dso, so in this case we set
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 4/5] Add map pgoff to python dictionary based on MAPPING_TYPE
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter Steve Clevenger
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 2/5] Add dso__is_pie prototype Steve Clevenger
3 siblings, 0 replies; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
Add 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.
Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
.../util/scripting-engines/trace-event-python.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index fb00f3ad6815..8a056c3574ec 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -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,
@@ -898,7 +905,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);
@@ -923,7 +930,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)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 4/5] Add map pgoff to python dictionary based on MAPPING_TYPE Steve Clevenger
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-27 19:55 ` Leo Yan
2024-08-26 21:35 ` [PATCH V2 2/5] Add dso__is_pie prototype Steve Clevenger
3 siblings, 1 reply; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
Use 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.
Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
tools/perf/util/map.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index e1d14936a60d..df7c06fc373e 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -171,8 +171,11 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
assert(!dso__kernel(dso));
map__init(result, start, start + len, pgoff, dso);
+ if (map->pgoff && !no_dso)
+ no_dso = dso__is_pie(dso); // PIE check
+
if (anon || no_dso) {
- map->mapping_type = MAPPING_TYPE__IDENTITY;
+ map__set_mapping_type(map, MAPPING_TYPE__IDENTITY);
/*
* Set memory without DSO as loaded. All map__find_*
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 2/5] Add dso__is_pie prototype
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
` (2 preceding siblings ...)
2024-08-26 21:35 ` [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE Steve Clevenger
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-27 19:29 ` Leo Yan
3 siblings, 1 reply; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
Add prototype to dso__is_pie() global.
Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
tools/perf/util/symbol.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 3fb5d146d9b1..33ea2596ce31 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 1/5] Add dso__is_pie call to identify ELF PIE
[not found] <cover.1724707494.git.scclevenger@os.amperecomputing.com>
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
1 sibling, 0 replies; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
- Decrease indentation, add null pointer checks per Leo Yan review
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.
Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index e398abfd13a0..0e49c1345a67 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -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
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset
[not found] <cover.1724707494.git.scclevenger@os.amperecomputing.com>
2024-08-26 21:35 ` [PATCH V2 1/5] Add dso__is_pie call to identify ELF PIE Steve Clevenger
@ 2024-08-26 21:35 ` Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter Steve Clevenger
` (3 more replies)
1 sibling, 4 replies; 11+ messages in thread
From: Steve Clevenger @ 2024-08-26 21:35 UTC (permalink / raw)
To: james.clark, mike.leach
Cc: suzuki.poulose, leo.yan, ilkka, coresight, linux-perf-users,
linux-arm-kernel
Changes in V2:
- Updated mailing list distribution
Fedora 37 distributed shared binary and executable mapped files show a
zero text section offset. Starting with the Fedora 38 distribution, the
shared binary and executable mapped files show a non-zero text section
offset for some binaries. The text offset parameter is never passed into
the arm-cs-trace-disasm.py script to allow the script to adjust the
start/end address range passed to objdump. This adjustment is required
to correctly offset into the dso text section. Not doing so results in
an incorrect user instruction trace display for Fedora 38 (and later)
user trace output.
Steve Clevenger (5):
Add dso__is_pie call to identify ELF PIE
Add dso__is_pie prototype
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 | 17 +++++-
tools/perf/util/map.c | 5 +-
.../scripting-engines/trace-event-python.c | 13 +++-
tools/perf/util/symbol-elf.c | 60 +++++++++++++++++++
tools/perf/util/symbol.h | 1 +
5 files changed, 90 insertions(+), 6 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 2/5] Add dso__is_pie prototype
2024-08-26 21:35 ` [PATCH V2 2/5] Add dso__is_pie prototype Steve Clevenger
@ 2024-08-27 19:29 ` Leo Yan
0 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2024-08-27 19:29 UTC (permalink / raw)
To: Steve Clevenger, james.clark, mike.leach
Cc: suzuki.poulose, ilkka, coresight, linux-perf-users,
linux-arm-kernel
On 8/26/2024 10:35 PM, Steve Clevenger wrote:
>
> Changes in V2:
> - Updated mailing list distribution
Usually, we record the change history into the cover letter, or put it under
the line starting with `---` in the commit. This can avoid the change history
to be merged into the mainline.
> Add prototype to dso__is_pie() global.
Please squash the patch 02 into 01. Otherwise, the patch 01 will introduce
building regression:
util/symbol-elf.c:669:6: error: no previous prototype for ‘dso__is_pie’
[-Werror=missing-prototypes]
669 | bool dso__is_pie(struct dso *dso)
| ^~~~~~~~~~~
Thanks,
Leo
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> ---
> tools/perf/util/symbol.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 3fb5d146d9b1..33ea2596ce31 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE
2024-08-26 21:35 ` [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE Steve Clevenger
@ 2024-08-27 19:55 ` Leo Yan
2024-08-28 1:41 ` Steve Clevenger
0 siblings, 1 reply; 11+ messages in thread
From: Leo Yan @ 2024-08-27 19:55 UTC (permalink / raw)
To: Steve Clevenger, james.clark, mike.leach
Cc: suzuki.poulose, ilkka, coresight, linux-perf-users,
linux-arm-kernel
On 8/26/2024 10:35 PM, Steve Clevenger wrote:
>
> Changes in V2:
> - Updated mailing list distribution
>
> Use 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.
>
>
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> ---
> tools/perf/util/map.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index e1d14936a60d..df7c06fc373e 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -171,8 +171,11 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> assert(!dso__kernel(dso));
> map__init(result, start, start + len, pgoff, dso);
>
> + if (map->pgoff && !no_dso)
> + no_dso = dso__is_pie(dso); // PIE check
Here the logic is whatever the `pgoff` is (zero or non-zero), we need to
always set the mapping type as MAPPING_TYPE__IDENTITY. It is no need to check
pgoff and is no matter with 'no_dso'. So we can simply check dso__is_pie() and
set IDENTITY flag for the true case.
if (dso__is_pie(dso)) {
map__set_mapping_type(map, MAPPING_TYPE__IDENTITY);
} else if (anon || no_dso) {
...
}
BTW, please rebase the series on the top of the perf-tools-next branch [1], I
saw this patch has merging conflict on it.
Thanks,
Leo
[1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
branch: perf-tools-next
> +
> if (anon || no_dso) {
> - map->mapping_type = MAPPING_TYPE__IDENTITY;
> + map__set_mapping_type(map, MAPPING_TYPE__IDENTITY);
>
> /*
> * Set memory without DSO as loaded. All map__find_*
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter
2024-08-26 21:35 ` [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter Steve Clevenger
@ 2024-08-27 20:23 ` Leo Yan
2024-08-28 1:49 ` Steve Clevenger
0 siblings, 1 reply; 11+ messages in thread
From: Leo Yan @ 2024-08-27 20:23 UTC (permalink / raw)
To: Steve Clevenger, james.clark, mike.leach
Cc: suzuki.poulose, ilkka, coresight, linux-perf-users,
linux-arm-kernel
On 8/26/2024 10:35 PM, Steve Clevenger wrote:
>
> Changes in V2:
> - Updated mailing list distribution
>
> Extract 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.
>
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> ---
> .../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> index d973c2baed1c..6bf806078f9a 100755
> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> @@ -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"]
> @@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]':
> + if (dso == "[kernel.kallsyms]"):
> + dso_vm_start = 0
> + map_pgoff = '[unknown]'
> + else:
> + dso_vm_start = int(dso_start)
> + start_addr += map_pgoff
> + stop_addr += map_pgoff
> + map_pgoff = '[unknown]'
Every sample has its own field `map_pgoff`. So I am confused why we need the
sentence: map_pgoff = '[unknown]'. And you can see with above change, we will
set dso_vm_start delicately with below code section.
How about a simple change like below?
@@ -267,7 +268,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)
If this change does not work, I am curious if anything we missed to handle in
the perf's C code (frontend).
Thanks,
Leo
> if (options.objdump_name != None):
> # It doesn't need to decrease virtual memory offset for disassembly
> # for kernel dso and executable file dso, so in this case we set
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE
2024-08-27 19:55 ` Leo Yan
@ 2024-08-28 1:41 ` Steve Clevenger
0 siblings, 0 replies; 11+ messages in thread
From: Steve Clevenger @ 2024-08-28 1:41 UTC (permalink / raw)
To: Leo Yan, james.clark, mike.leach
Cc: suzuki.poulose, ilkka, coresight, linux-perf-users,
linux-arm-kernel
On 8/27/2024 12:55 PM, Leo Yan wrote:
> On 8/26/2024 10:35 PM, Steve Clevenger wrote:
>>
>> Changes in V2:
>> - Updated mailing list distribution
>>
>> Use 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.
>>
>>
>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
>> ---
>> tools/perf/util/map.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index e1d14936a60d..df7c06fc373e 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -171,8 +171,11 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>> assert(!dso__kernel(dso));
>> map__init(result, start, start + len, pgoff, dso);
>>
>> + if (map->pgoff && !no_dso)
>> + no_dso = dso__is_pie(dso); // PIE check
>
> Here the logic is whatever the `pgoff` is (zero or non-zero), we need to
> always set the mapping type as MAPPING_TYPE__IDENTITY. It is no need to check
> pgoff and is no matter with 'no_dso'. So we can simply check dso__is_pie() and
> set IDENTITY flag for the true case.
>
> if (dso__is_pie(dso)) {
> map__set_mapping_type(map, MAPPING_TYPE__IDENTITY);
> } else if (anon || no_dso) {
> ...
> }
>
> BTW, please rebase the series on the top of the perf-tools-next branch [1], I
> saw this patch has merging conflict on it.
>
> Thanks,
> Leo
Hi Leo,
I combined the dso__is_pie() call onto the end of the existing 'if'
statement you now show as an 'else if'. Please see V3 of the patch series.
Steve
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> branch: perf-tools-next
>> +
>> if (anon || no_dso) {
>> - map->mapping_type = MAPPING_TYPE__IDENTITY;
>> + map__set_mapping_type(map, MAPPING_TYPE__IDENTITY);
>>
>> /*
>> * Set memory without DSO as loaded. All map__find_*
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter
2024-08-27 20:23 ` Leo Yan
@ 2024-08-28 1:49 ` Steve Clevenger
0 siblings, 0 replies; 11+ messages in thread
From: Steve Clevenger @ 2024-08-28 1:49 UTC (permalink / raw)
To: Leo Yan, james.clark, mike.leach
Cc: suzuki.poulose, ilkka, coresight, linux-perf-users,
linux-arm-kernel
On 8/27/2024 1:23 PM, Leo Yan wrote:
> On 8/26/2024 10:35 PM, Steve Clevenger wrote:
>>
>> Changes in V2:
>> - Updated mailing list distribution
>>
>> Extract 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.
>>
>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
>> ---
>> .../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> index d973c2baed1c..6bf806078f9a 100755
>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> @@ -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"]
>> @@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]':
>> + if (dso == "[kernel.kallsyms]"):
>> + dso_vm_start = 0
>> + map_pgoff = '[unknown]'
>> + else:
>> + dso_vm_start = int(dso_start)
>> + start_addr += map_pgoff
>> + stop_addr += map_pgoff
>> + map_pgoff = '[unknown]'
>
> Every sample has its own field `map_pgoff`. So I am confused why we need the
> sentence: map_pgoff = '[unknown]'. And you can see with above change, we will
> set dso_vm_start delicately with below code section.
>
> How about a simple change like below?
>
> @@ -267,7 +268,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)
>
> If this change does not work, I am curious if anything we missed to handle in
> the perf's C code (frontend).
>
> Thanks,
> Leo
Leo,
Sorry, the map_pgoff = '[unknown]' instances are actually leftover debug
artifacts used for print type debug. I resorted to print debug after I
didn't see success attempting mixed-mode debug with Visual Studio Code
on AARCH64. If you have a known working launch configuration, please
share it. See V3 of the patch series. I've rebased to the
perf-tools-next branch.
Steve
>
>> if (options.objdump_name != None):
>> # It doesn't need to decrease virtual memory offset for disassembly
>> # for kernel dso and executable file dso, so in this case we set
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-08-28 1:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1724707494.git.scclevenger@os.amperecomputing.com>
2024-08-26 21:35 ` [PATCH V2 1/5] Add dso__is_pie call to identify ELF PIE Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 0/5] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 5/5] Adjust objdump start/end range per map pgoff parameter Steve Clevenger
2024-08-27 20:23 ` Leo Yan
2024-08-28 1:49 ` Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 4/5] Add map pgoff to python dictionary based on MAPPING_TYPE Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 3/5] Force MAPPING_TYPE__IDENTIY for PIE Steve Clevenger
2024-08-27 19:55 ` Leo Yan
2024-08-28 1:41 ` Steve Clevenger
2024-08-26 21:35 ` [PATCH V2 2/5] Add dso__is_pie prototype Steve Clevenger
2024-08-27 19:29 ` Leo Yan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).