Discussion:
[PATCH] binutils/readelf: Fix unwind entries of 64-bit hppa object files
Helge Deller
2018-08-14 16:03:29 UTC
Permalink
readelf shows wrong unwind entries when run against a 64-bit hppa2.0
object file (32-bit hppa1.1 object file is OK). This is fixed by
replacing eh_addr_size by the constant 4 in slurp_hppa_unwind_table()
which refers to the number of 32-bit values and not to the size of an
address.

If ok, can someone please commit?

Thanks,
Helge

binutils/
* readelf.c (slurp_hppa_unwind_table): Replace "eh_addr_size" with "4".

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 8a61db6459..97a491801c 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -8049,7 +8049,9 @@ slurp_hppa_unwind_table (Filedata * filedata,

i = rp->r_offset / unw_ent_size;

- switch ((rp->r_offset % unw_ent_size) / eh_addr_size)
+ /* On 32- and 64-bit object files, each unwind entry consists of four
+ 32-bit values (start, end, 2 x flags). */
+ switch ((rp->r_offset % unw_ent_size) / 4)
{
case 0:
aux->table[i].start.section = sym->st_shndx;
Alan Modra
2018-08-15 00:20:23 UTC
Permalink
Post by Helge Deller
* readelf.c (slurp_hppa_unwind_table): Replace "eh_addr_size" with "4".
hppa_process_unwind also uses eh_addr_size. Do you see the correct
number of entries reported? Rather than this patch, I suspect you
should set eh_addr_size in process_section_headers.
--
Alan Modra
Australia Development Lab, IBM
Helge Deller
2018-08-15 07:36:39 UTC
Permalink
Post by Alan Modra
Post by Helge Deller
* readelf.c (slurp_hppa_unwind_table): Replace "eh_addr_size" with "4".
hppa_process_unwind also uses eh_addr_size. Do you see the correct
number of entries reported?
You are right. I didn't noticed, but my old patch reported a wrong
number of unwind entries.
Post by Alan Modra
Rather than this patch, I suspect you should set eh_addr_size in
process_section_headers.
Yes. The patch below fixes both issues.

Thanks!
Helge


diff --git a/binutils/readelf.c b/binutils/readelf.c
index 8a61db6459..3d239d3ab2 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -6090,6 +6090,11 @@ process_section_headers (Filedata * filedata)
break;
}
break;
+
+ case EM_PARISC:
+ /* ELF64 uses 32-bit unwind entries too. */
+ eh_addr_size = 4;
+ break;
}

#define CHECK_ENTSIZE_VALUES(section, i, size32, size64) \
John David Anglin
2018-08-16 13:54:47 UTC
Permalink
Post by Helge Deller
Post by Alan Modra
Post by Helge Deller
* readelf.c (slurp_hppa_unwind_table): Replace "eh_addr_size" with "4".
hppa_process_unwind also uses eh_addr_size. Do you see the correct
number of entries reported?
You are right. I didn't noticed, but my old patch reported a wrong
number of unwind entries.
Post by Alan Modra
Rather than this patch, I suspect you should set eh_addr_size in
process_section_headers.
Yes. The patch below fixes both issues.
I tried the change below on various kernel .o files and a vmlinux
(64-bit) file.  The kernel was built
with -ffunction-sections.  While the change is no doubt an improvement,
"readelf -u" still seems
broken/useless.  The ranges displayed for vmlinux don't seem to
correspond to the addresses
displayed with "objdump -d".  I see lots of "symbol + n" entries in the
readelf output.  The symbol
matching for .o files also doesn't work at all reliably.  Maybe it's
better when -ffunction-stections
isn't used.

Do 64-bit kernel backtraces work?

We need to determine if gas is correctly generating unwind entries. If
it is, I think we have further
issues with readelf.
Post by Helge Deller
Thanks!
Helge
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 8a61db6459..3d239d3ab2 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -6090,6 +6090,11 @@ process_section_headers (Filedata * filedata)
break;
}
break;
+
+ /* ELF64 uses 32-bit unwind entries too. */
+ eh_addr_size = 4;
+ break;
}
#define CHECK_ENTSIZE_VALUES(section, i, size32, size64) \
Thanks,
Dave
--
John David Anglin ***@bell.net
Alan Modra
2018-10-10 03:07:44 UTC
Permalink
Post by Helge Deller
Post by Alan Modra
Post by Helge Deller
* readelf.c (slurp_hppa_unwind_table): Replace "eh_addr_size" with "4".
hppa_process_unwind also uses eh_addr_size. Do you see the correct
number of entries reported?
You are right. I didn't noticed, but my old patch reported a wrong
number of unwind entries.
Post by Alan Modra
Rather than this patch, I suspect you should set eh_addr_size in
process_section_headers.
Yes. The patch below fixes both issues.
I tried the change below on various kernel .o files and a vmlinux (64-bit)
file.  The kernel was built
with -ffunction-sections.  While the change is no doubt an improvement,
"readelf -u" still seems
broken/useless.  The ranges displayed for vmlinux don't seem to correspond
to the addresses
displayed with "objdump -d".  I see lots of "symbol + n" entries in the
readelf output.  The symbol
matching for .o files also doesn't work at all reliably.  Maybe it's better
when -ffunction-stections
isn't used.
Do 64-bit kernel backtraces work?
We need to determine if gas is correctly generating unwind entries. If it
is, I think we have further
issues with readelf.
Huh, yes, confusion reigns. There are two possible unwind sections to
consider, .PARISC.unwind and .eh_frame. As things are at the moment,
it looks to me that eh_addr_size of 4 is needed for .PARISC.unwind and
8 for .eh_frame. I'm basing that on the fact that tc-hppa.c
pa_build_unwind_subspace emits 4-byte entries to .PARISC.unwind, while
DWARF2_FDE_RELOC_SIZE is 8 for hppa64, leading to 8-byte addresses if
you use .cfi directives to generate .eh_frame.

So the proper fix for readelf looks to be the following, which
incidentally makes slurp_hppa_unwind_table self-consistent (it always
reads 4-byte start and end offsets.)

From 43f6cd0588a735c202934789d67b6ed4302f255d Mon Sep 17 00:00:00 2001
From: Alan Modra <***@gmail.com>
Date: Wed, 10 Oct 2018 13:14:56 +1030
Subject: [PATCH 2/4] HPPA64 .PARISC.unwind entries

.PARISC.unwind has 32-bit addresses in both 32-bit ELF and 64-bit ELF.
Well, strictly speaking, the 32-bit "start" and "end" fields are
segment relative offsets. (The 64-bit ABI says so, while the 32-bit
ABI says they are addresses but it appears they are segment relative
offsets in practice. Likely the 32-bit ABI lacks an update.)

* readelf.c (hppa_process_unwind): Don't use eh_addr_size to
calculate number of entries.
(slurp_hppa_unwind_table): Don't use eh_addr_size here either.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 09436ccedc..fd568340df 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,10 @@
+2018-10-10 Helge Deller <***@gmx.de>
+ Alan Modra <***@gmail.com>
+
+ * readelf.c (hppa_process_unwind): Don't use eh_addr_size to
+ calculate number of entries.
+ (slurp_hppa_unwind_table): Don't use eh_addr_size here either.
+
2018-10-10 Alan Modra <***@gmail.com>

* objdump.c (dump_dwarf): Set s12z eh_addr_size to 4.
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 2748664a30..41f55ee4ed 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -8065,7 +8065,7 @@ slurp_hppa_unwind_table (Filedata * filedata,

i = rp->r_offset / unw_ent_size;

- switch ((rp->r_offset % unw_ent_size) / eh_addr_size)
+ switch ((rp->r_offset % unw_ent_size) / 4)
{
case 0:
aux->table[i].start.section = sym->st_shndx;
@@ -8133,7 +8133,7 @@ hppa_process_unwind (Filedata * filedata)
{
if (streq (SECTION_NAME (sec), ".PARISC.unwind"))
{
- unsigned long num_unwind = sec->sh_size / (2 * eh_addr_size + 8);
+ unsigned long num_unwind = sec->sh_size / 16;

printf (ngettext ("\nUnwind section '%s' at offset 0x%lx "
"contains %lu entry:\n",
--
Alan Modra
Australia Development Lab, IBM
Loading...