perf symbol: Look for ImageBase in PE file to compute .text offset

Instead of using the file offset in the debug file.

This fixes a regression from 00a3423492bc90be99e529a64f13fdd80a0e8c0a, causing incorrect symbol resolution when debug file have been stripped from non-debug sections (in which case its .text section is empty and doesn't have any file position).

The debug files could also be created with a different file alignment, and have different file positions from the mmap-ed binary, or have the section reordered.

This instead looks for the file image base, using the corresponding bfd *ABS* symbols. As PE symbols only have 4 bytes, it also needs to keep .text section vma high bits.

Signed-off-by Remi Bernon <[email protected]>

---

Hi!

As I'm not updating it often I only recently realized that perf had a regression when using stripped debug info files, and all symbols from PE files are off. This should make things better.

Cheers,

 tools/perf/util/symbol.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

tools/perf/util/symbol.c +16 -4

index 77fc46ca07c0..0fc9a5410739 100644
@@ -1581,10 +1581,6 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
 	if (bfd_get_flavour(abfd) == bfd_target_elf_flavour)
 		goto out_close;
 
-	section = bfd_get_section_by_name(abfd, ".text");
-	if (section)
-		dso->text_offset = section->vma - section->filepos;
-
 	symbols_size = bfd_get_symtab_upper_bound(abfd);
 	if (symbols_size == 0) {
 		bfd_close(abfd);

@@ -1602,6 +1598,22 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
 	if (symbols_count < 0)
 		goto out_free;
 
+	section = bfd_get_section_by_name(abfd, ".text");
+	if (section) {
+		for (i = 0; i < symbols_count; ++i) {
+			if (!strcmp(bfd_asymbol_name(symbols[i]), "__ImageBase") ||
+			    !strcmp(bfd_asymbol_name(symbols[i]), "__image_base__"))
+				break;
+		}
+		if (i < symbols_count) {
+			/* PE symbols can only have 4 bytes, so use .text high bits */
+			dso->text_offset = section->vma - (u32)section->vma;
+			dso->text_offset += (u32)bfd_asymbol_value(symbols[i]);
+		} else {
+			dso->text_offset = section->vma - section->filepos;
+		}
+	}
+
 	qsort(symbols, symbols_count, sizeof(asymbol *), bfd_symbols__cmpvalue);
 
 #ifdef bfd_get_section

Signature · 1 line

2.33.0

Em Thu, Sep 09, 2021 at 09:26:36PM +0200, Remi Bernon escreveu:

Quoted 7 lines

Instead of using the file offset in the debug file.

This fixes a regression from 00a3423492bc90be99e529a64f13fdd80a0e8c0a, causing incorrect symbol resolution when debug file have been stripped from non-debug sections (in which case its .text section is empty and doesn't have any file position).

Quoted 7 lines

The debug files could also be created with a different file alignment, and have different file positions from the mmap-ed binary, or have the section reordered.

This instead looks for the file image base, using the corresponding bfd *ABS* symbols. As PE symbols only have 4 bytes, it also needs to keep .text section vma high bits.

I added a:

Fixes 00a3423492bc90be ("perf symbols: Make dso__load_bfd_symbols() load PE files from debug cache only")

To help [email protected] to pick it, its on my local tree now.

It would be great to get a:

Reviewed-by Nicholas Fraser <[email protected]>

Can we have it, please?

Quoted 24 lines

Em Thu, Sep 09, 2021 at 09:26:36PM +0200, Remi Bernon escreveu:

Quoted 7 lines

Instead of using the file offset in the debug file.

This fixes a regression from 00a3423492bc90be99e529a64f13fdd80a0e8c0a, causing incorrect symbol resolution when debug file have been stripped from non-debug sections (in which case its .text section is empty and doesn't have any file position).

Quoted 7 lines

The debug files could also be created with a different file alignment, and have different file positions from the mmap-ed binary, or have the section reordered.

This instead looks for the file image base, using the corresponding bfd *ABS* symbols. As PE symbols only have 4 bytes, it also needs to keep .text section vma high bits.

I added a:

Fixes 00a3423492bc90be ("perf symbols: Make dso__load_bfd_symbols() load PE files from debug cache only")

To help [email protected] to pick it, its on my local tree now.

It would be great to get a:

Quoted 4 lines

Reviewed-by Nicholas Fraser <[email protected]>

Can we have it, please?

Well, Nicholas doesn't work with us anymore. I've reached them separately but didn't get any answer so I'm thinking they may not be interested.

Em Sun, Sep 12, 2021 at 10:25:26AM +0200, Rémi Bernon escreveu:

Quoted 30 lines · Arnaldo Carvalho de Melo

Quoted 28 lines

Em Thu, Sep 09, 2021 at 09:26:36PM +0200, Remi Bernon escreveu:

Quoted 7 lines

Instead of using the file offset in the debug file.

This fixes a regression from 00a3423492bc90be99e529a64f13fdd80a0e8c0a, causing incorrect symbol resolution when debug file have been stripped from non-debug sections (in which case its .text section is empty and doesn't have any file position).

Quoted 7 lines

The debug files could also be created with a different file alignment, and have different file positions from the mmap-ed binary, or have the section reordered.

This instead looks for the file image base, using the corresponding bfd *ABS* symbols. As PE symbols only have 4 bytes, it also needs to keep .text section vma high bits.

I added a:

Fixes 00a3423492bc90be ("perf symbols: Make dso__load_bfd_symbols() load PE files from debug cache only")

To help [email protected] to pick it, its on my local tree now.

It would be great to get a:

Reviewed-by Nicholas Fraser <[email protected]>

Can we have it, please?

Quoted 2 lines

Well, Nicholas doesn't work with us anymore. I've reached them separately but didn't get any answer so I'm thinking they may not be interested.

No problem, its already upstream now.

Thanks,

- Arnaldo