This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patch] Fix 64-bit->32-bit vDSO reporting
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Fri, 12 Oct 2012 16:13:27 +0200
- Subject: Re: [patch] Fix 64-bit->32-bit vDSO reporting
Hi Roland,
jankratochvil/auxv32
On Thu, 11 Oct 2012 00:27:35 +0200, Roland McGrath wrote:
> What about just looking at the first 8 bytes of the auxv data? The first
> element is never AT_NULL, so it will have a nonzero a_type. No a_type
> value will ever be more than UINT32_MAX. So for big-endian, the first four
> bytes being zero says it's 64-bit. For little-endian, it's not so easy
> without assuming that the first a_val is nonzero. That's actually reliable
> enough in practice, but it feels wrong. Maybe there is something better
> for little-endian that I'm not thinking of.
[...]
On Thu, 11 Oct 2012 22:20:19 +0200, Roland McGrath wrote:
> I was hoping you might think up some clever trick for the little-endian
> case,
32-bit lit x86 | 32 AT_SYSINFO 0xf772a420 | 20 00 00 00 20 a4 72 f7
64-bit lit x86 | 33 AT_SYSINFO_EHDR 0x7fff69fff000 | 21 00 00 00 00 00 00 00 00 f0 ff 69 ff 7f 00 00
32-bit big ppc | 22 AT_IGNOREPPC 22 | 00 00 00 16 00 00 00 16
64-bit big ppc | 22 AT_IGNOREPPC 22 | 00 00 00 00 00 00 00 16 00 00 00 00 00 00 00 16
32-bit big 390 | 33 AT_SYSINFO_EHDR 0x7d715000 | 00 00 00 21 7d 71 50 00
64-bit big 390 | 33 AT_SYSINFO_EHDR 0x3fffd72d000 | 00 00 00 00 00 00 00 21 00 00 03 ff fd 72 d0 00
32-bit lit arm | 16 AT_HWCAP 0xe8d7 | 10 00 00 00 d7 e8 00 00
00 00 00 00 => 64-bit big
00 00 XX XX => 32-bit big
00 00 00 00 => 64-bit little (XXX 1st tag parameter is never 0)
=> 32-bit little
As long we permit first a_val to be zero there is probably no solution.
We could detect it also from the format of /proc/PID/maps, for 64-bit
inferiors there will always be some of the modules at >= 4GB.
But that also nobody guarantees.
I do not find opening /proc/PID/exe such a performance loss but it is true
I have chosen elfutils for its performance so it is not fair to make it worse.
Implemented trying to decode it both ways each time, it is IMO zero-cost as it
is only more userland computations without involving more syscalls. If it
gets ambiguous it will fallback to /proc/PID/exe; but that should never happen.
Thanks,
Jan
commit 929132df6fd8a272278173b5a3150cdbf45ac356
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Wed Oct 10 20:42:30 2012 +0200
libdwfl/
linux-proc-maps.c: Include system.h.
(PROCEXEFMT, get_pid_class): New.
(grovel_auxv): Detect 32-bit vs. 64-bit auxv, possibly call get_pid_class.
Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 189d3b7..a3aa49d 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,10 @@
+2012-10-12 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * linux-proc-maps.c: Include system.h.
+ (PROCEXEFMT, get_pid_class): New.
+ (grovel_auxv): Detect 32-bit vs. 64-bit auxv, possibly call
+ get_pid_class.
+
2012-10-10 Jan Kratochvil <jan.kratochvil@redhat.com>
* dwfl_segment_report_module.c (dwfl_segment_report_module):
diff --git a/libdwfl/linux-proc-maps.c b/libdwfl/linux-proc-maps.c
index 4fbe90d..3c3f1e7 100644
--- a/libdwfl/linux-proc-maps.c
+++ b/libdwfl/linux-proc-maps.c
@@ -39,13 +39,42 @@
#include <unistd.h>
#include <assert.h>
#include <endian.h>
+#include "system.h"
#define PROCMAPSFMT "/proc/%d/maps"
#define PROCMEMFMT "/proc/%d/mem"
#define PROCAUXVFMT "/proc/%d/auxv"
+#define PROCEXEFMT "/proc/%d/exe"
+/* Return ELFCLASS64 or ELFCLASS32 for the main ELF executable. Return
+ ELFCLASSNONE for an error. */
+
+static unsigned char
+get_pid_class (pid_t pid)
+{
+ char *fname;
+ if (asprintf (&fname, PROCEXEFMT, pid) < 0)
+ return ELFCLASSNONE;
+
+ int fd = open64 (fname, O_RDONLY);
+ free (fname);
+ if (fd < 0)
+ return ELFCLASSNONE;
+
+ unsigned char buf[EI_CLASS + 1];
+ ssize_t nread = pread_retry (fd, &buf, sizeof buf, 0);
+ close (fd);
+ if (nread != sizeof (buf) || buf[EI_MAG0] != ELFMAG0
+ || buf[EI_MAG1] != ELFMAG1 || buf[EI_MAG2] != ELFMAG2
+ || buf[EI_MAG3] != ELFMAG3
+ || (buf[EI_CLASS] != ELFCLASS64 && buf[EI_CLASS] != ELFCLASS32))
+ return ELFCLASSNONE;
+
+ return buf[EI_CLASS];
+}
+
/* Search /proc/PID/auxv for the AT_SYSINFO_EHDR tag. */
static int
@@ -60,61 +89,72 @@ grovel_auxv (pid_t pid, Dwfl *dwfl, GElf_Addr *sysinfo_ehdr)
if (fd < 0)
return errno == ENOENT ? 0 : errno;
- ssize_t nread;
- do
+ GElf_Addr sysinfo_ehdr64 = 0, sysinfo_ehdr32 = 0;
+ GElf_Addr segment_align64 = dwfl->segment_align;
+ GElf_Addr segment_align32 = dwfl->segment_align;
+ off_t offset = 0;
+ for (;;)
{
union
{
- char buffer[sizeof (long int) * 2 * 64];
- Elf64_auxv_t a64[sizeof (long int) * 2 * 64 / sizeof (Elf64_auxv_t)];
- Elf32_auxv_t a32[sizeof (long int) * 2 * 32 / sizeof (Elf32_auxv_t)];
+ Elf64_auxv_t a64;
+ Elf32_auxv_t a32[2];
} d;
- nread = read (fd, &d, sizeof d);
- if (nread > 0)
+ eu_static_assert (sizeof d.a64 == sizeof d.a32);
+ ssize_t nread = pread_retry (fd, &d.a64, sizeof d.a64, offset);
+ if (nread < 0)
+ return errno;
+ for (unsigned a32i = 0; a32i < nread / sizeof d.a32[0]; a32i++)
{
- switch (sizeof (long int))
- {
- case 4:
- for (size_t i = 0; (char *) &d.a32[i] < &d.buffer[nread]; ++i)
- if (d.a32[i].a_type == AT_SYSINFO_EHDR)
- {
- *sysinfo_ehdr = d.a32[i].a_un.a_val;
- if (dwfl->segment_align > 1)
- {
- nread = 0;
- break;
- }
- }
- else if (d.a32[i].a_type == AT_PAGESZ
- && dwfl->segment_align <= 1)
- dwfl->segment_align = d.a32[i].a_un.a_val;
- break;
- case 8:
- for (size_t i = 0; (char *) &d.a64[i] < &d.buffer[nread]; ++i)
- if (d.a64[i].a_type == AT_SYSINFO_EHDR)
- {
- *sysinfo_ehdr = d.a64[i].a_un.a_val;
- if (dwfl->segment_align > 1)
- {
- nread = 0;
- break;
- }
- }
- else if (d.a64[i].a_type == AT_PAGESZ
- && dwfl->segment_align <= 1)
- dwfl->segment_align = d.a64[i].a_un.a_val;
+ const Elf32_auxv_t *a32 = d.a32 + a32i;
+ switch (a32->a_type)
+ {
+ case AT_SYSINFO_EHDR:
+ sysinfo_ehdr32 = a32->a_un.a_val;
break;
- default:
- abort ();
+ case AT_PAGESZ:
+ segment_align32 = a32->a_un.a_val;
break;
- }
+ }
}
+ if ((size_t) nread < sizeof d.a64)
+ break;
+ const Elf64_auxv_t *a64 = &d.a64;
+ switch (a64->a_type)
+ {
+ case AT_SYSINFO_EHDR:
+ sysinfo_ehdr64 = a64->a_un.a_val;
+ break;
+ case AT_PAGESZ:
+ segment_align64 = a64->a_un.a_val;
+ break;
+ }
+ offset += nread;
}
- while (nread > 0);
close (fd);
- return nread < 0 ? errno : 0;
+ bool valid64 = sysinfo_ehdr64 || segment_align64 != dwfl->segment_align;
+ bool valid32 = sysinfo_ehdr32 || segment_align32 != dwfl->segment_align;
+
+ if (! valid64 && ! valid32)
+ return 0;
+
+ unsigned char pid_class = ELFCLASSNONE;
+ if (valid64 && valid32)
+ pid_class = get_pid_class (pid);
+
+ if (pid_class == ELFCLASS64 || (valid64 && ! valid32))
+ {
+ *sysinfo_ehdr = sysinfo_ehdr64;
+ dwfl->segment_align = segment_align64;
+ }
+ if (pid_class == ELFCLASS32 || (! valid64 && valid32))
+ {
+ *sysinfo_ehdr = sysinfo_ehdr32;
+ dwfl->segment_align = segment_align32;
+ }
+ return 0;
}
static int