arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

Changes in V5:
- In symbol-elf.c, branch to exit_close label if open file.
- In trace_event_python.c, correct indentation. set_sym_in_dict call parameter "map_pgoff" renamed as "addr_map_pgoff" to match local naming.

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                  | 61 +++++++++++++++++++
 tools/perf/util/symbol.h                      |  1 +
 5 files changed, 80 insertions(+), 8 deletions(-)

2.25.1

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.

---
 tools/perf/scripts/python/arm-cs-trace-disasm.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 7aff02d84ffb..e8cf5d80d850 100755
@@ -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

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.

---
 tools/perf/util/map.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index e781c8d56a9a..c846faec177b 100644
@@ -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

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.

---
 .../util/scripting-engines/trace-event-python.c     | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

index 6971dd6c231f..7b96504d5406 100644
@@ -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", "addr_map_pgoff");
 	}
 
 	if (sample->flags)

2.25.1

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 | 61 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h     |  1 +
 2 files changed, 62 insertions(+)

index e398abfd13a0..babe47976922 100644
@@ -662,6 +662,67 @@ 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_close;	// false
+
+		data = (Elf_Data *) elf_getdata(scn, NULL);
+		if (!data || !data->d_buf)
+			goto exit_close;	// 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
+	}
+
+exit_close:
+	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

index 3fb5d146d9b1..33ea2596ce31 100644
@@ -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

For the series:

Hi Steve,

Later when you respin a new version patches, it is better to amend the review and ACK tags you have received. (Just remind, b4 is your friend for helping do these things).

Thanks,
Leo

Got it. Thanks, Leo. By the way, I noticed when I rebased to perf-tools-next the kernel version was 6.11-rc3. Is there any chance this patch gets into 6.11?

Steve

On 8/29/24 18:39, Steve Clevenger wrote:>

Linus' master branch now is 6.11-rc5, it is not far away from the 6.11.

I think we still have much chance to get it done before it. Added maintainers in case any missing.

Thanks,
Leo

Hi Leo,

That's great. Unfortuantely, I need to submit a V6 for patch 4/4, which is the arm-cs-trace-disasm.py script. Kernel instruction trace requires zeroing map_pgoff. perf makes most offset decisions, but this one is in the script. It's a one liner.

I know you're a b4 fan, but I've never used it. If I'm able to make progress with it I'll use it to resubmit. Otherwise more of the same.

Steve

Steve

Hi Steve,

On 8/31/2024 8:19 PM, Steve Clevenger wrote:

[...]

Agreed the kernel symbols should be zero map offset.

Thanks for correcting this.

For using the b4 command:

$ b4 am <Message-ID>

You can find the message ID in the lore webpage, e.g. for this series, in the cover letter link [1], click the 'raw' hyper link, then you can fine the Message ID is:
[email protected]

Run 'b4 am [email protected]' for downloading the patches (with automatic amending ACK and review tags).

As result, you will get the file:
v5_20240828_scclevenger_arm_cs_trace_disasm_py_perf_must_accommodate_non_zero_dso_text_offset.mbx

$ git am v5_20240828_scclevenger_arm_cs_trace_disasm_py_perf_must_accommodate_non_zero_dso_text_offset.mbx

Alternatively, if you are using new b4 version, you can directly use the command 'b4 shazam <Message-ID>' for downloading series and applying it on your local branch in one go.

I tried the b4 command for your series but failed, I can see you used git send-email but the series was not discontinuous so b4 command cannot fetch the whole series.

You can just work on you existed patches - don't forget to append my review tags for patches 01 ~ 03. For the patch 04, after you send out I will review it.

Thanks,
Leo

[1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#mc76180d8076cf3414ba81e7c379cda93297aa4a9