[PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Milian Wolff]

On my system with g++ (GCC) 8.2.1 20180831 with GNU gold (GNU Binutils
2.31.1) 1.16, the .eh_frame section does not have type PROGBITS
but rather is using X86_64_UNWIND nowadays:

```
$ echo "int main(){ return 0; }" > test.c
$ gcc test.c
$ readelf --sections a.out | grep .eh_frame
  [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
  [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
```

Without this patch, libdw refuses to use the available unwind
information, leading to broken backtraces while unwinding. With the
patch applied, unwinding works once more in such situations.

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
---
 libdw/dwarf_getcfi_elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libdw/dwarf_getcfi_elf.c b/libdw/dwarf_getcfi_elf.c
index 315cc02f..4bcfe5cd 100644
--- a/libdw/dwarf_getcfi_elf.c
+++ b/libdw/dwarf_getcfi_elf.c
@@ -298,7 +298,7 @@ getcfi_shdr (Elf *elf, const GElf_Ehdr *ehdr)
 	    }
 	  else if (!strcmp (name, ".eh_frame"))
 	    {
-	      if (shdr->sh_type == SHT_PROGBITS)
+	      if (shdr->sh_type == SHT_PROGBITS || shdr->sh_type == SHT_X86_64_UNWIND)
 		return getcfi_scn_eh_frame (elf, ehdr, scn, shdr,
 					    hdr_scn, hdr_vaddr);
 	      else
-- 
2.19.1

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Mark Wielaard]

On Mon, 2018-10-29 at 16:21 +0100, Milian Wolff wrote:
> On my system with g++ (GCC) 8.2.1 20180831 with GNU gold (GNU
> Binutils
> 2.31.1) 1.16, the .eh_frame section does not have type PROGBITS
> but rather is using X86_64_UNWIND nowadays:

Urgh, who thought it would be a good idea to have a architecture
specific (instead of a GNU specific) section type for unwind tables...
And why is there no separate type for .eh_frame_hdr. Now you still need
to check the name anyway...

But, given that we have that now, yes, lets deal with it.

> ```
> $ echo "int main(){ return 0; }" > test.c
> $ gcc test.c
> $ readelf --sections a.out | grep .eh_frame
>   [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
>   [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
> ```
> 
> Without this patch, libdw refuses to use the available unwind
> information, leading to broken backtraces while unwinding. With the
> patch applied, unwinding works once more in such situations.

Three questions:

- What testcase did you use?
  In theory dwarf_getcfi_elf () should fall back to using phdrs and
  find the PT_GNU_EH_FRAME data instead.

- It might be better to change the check to shdr->sh_type != SHT_NOBITS
  The idea is probably that we don't want to look at the data in case
  this is a .debug file which has it removed. This might be better than
  adding a check for X86_64_UNWIND since then we would also need to
  check the arch. Does != SHT_NOBITS work for you?

- What does eu-readelf -S show?
  I think we need a x86_64_section_type_name () ebl hook to show it
  correctly.

Thanks,

Mark

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 3090 bytes --]

On Montag, 5. November 2018 00:04:32 CET Mark Wielaard wrote:
> On Mon, 2018-10-29 at 16:21 +0100, Milian Wolff wrote:
> > On my system with g++ (GCC) 8.2.1 20180831 with GNU gold (GNU
> > Binutils
> > 2.31.1) 1.16, the .eh_frame section does not have type PROGBITS
> 
> > but rather is using X86_64_UNWIND nowadays:
> Urgh, who thought it would be a good idea to have a architecture
> specific (instead of a GNU specific) section type for unwind tables...
> And why is there no separate type for .eh_frame_hdr. Now you still need
> to check the name anyway...
> 
> But, given that we have that now, yes, lets deal with it.
> 
> > ```
> > $ echo "int main(){ return 0; }" > test.c
> > $ gcc test.c
> > $ readelf --sections a.out | grep .eh_frame
> >   [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
> >   [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
> > ```
> > 
> > Without this patch, libdw refuses to use the available unwind
> > information, leading to broken backtraces while unwinding. With the
> > patch applied, unwinding works once more in such situations.
> 
> Three questions:
> 
> - What testcase did you use?
>   In theory dwarf_getcfi_elf () should fall back to using phdrs and
>   find the PT_GNU_EH_FRAME data instead.

For unwinding, I used this full example:

https://paste.kde.org/p5rogntox

Then I compiled with `g++ -g O2` and recorded it with perf (`perf record --
call-graph dwarf`). Finally, I looked at the unwinding results via `perf 
script`. I compiled perf from sources with `NO_LIBUNWIND=1` to have it use 
libdw instead.

Interestingly, when I try to reproduce this on my laptop (i.e. compile even 
the trivial C example), then I cannot reproduce this at all anymore - the 
.eh_frame sections show up as PROGBITS. My desktop at work still shows this 
behavior though (also see below). I can't quite explain this difference...

> - It might be better to change the check to shdr->sh_type != SHT_NOBITS
>   The idea is probably that we don't want to look at the data in case
>   this is a .debug file which has it removed. This might be better than
>   adding a check for X86_64_UNWIND since then we would also need to
>   check the arch. Does != SHT_NOBITS work for you?

Yes, since SHT_NOBITS is not equal to SHT_X86_64_UNWIND :)

> - What does eu-readelf -S show?
>   I think we need a x86_64_section_type_name () ebl hook to show it
>   correctly.

Yes, that looks like it:

```
$ cat test.c
int main() { return 0; }
$ gcc test.c
$ readelf -S a.out | grep eh_frame
  [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
  [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
$ eu-readelf -S a.out | grep eh_frame
[14] .eh_frame            SHT_LOPROC+1 0000000000000670 00000670 000000b4  0 A      
0   0  8
[15] .eh_frame_hdr        SHT_LOPROC+1 0000000000000724 00000724 0000002c  0 A      
0   0  4
```

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

Hi Milian,

On Tue, 2018-11-06 at 00:12 +0100, Milian Wolff wrote:
> On Montag, 5. November 2018 00:04:32 CET Mark Wielaard wrote:
> 
> Interestingly, when I try to reproduce this on my laptop (i.e. compile even 
> the trivial C example), then I cannot reproduce this at all anymore - the 
> .eh_frame sections show up as PROGBITS. My desktop at work still shows this 
> behavior though (also see below). I can't quite explain this difference...

It seems to only happen with a specific combination of gcc and the gold
linker, I could only generate the SHT_X86_64_UNWIND sections only on
fedora 29 with gcc 8.2.1 and gold version 2.31.1-13.fc29 (1.16).

> > - It might be better to change the check to shdr->sh_type != SHT_NOBITS
> >   The idea is probably that we don't want to look at the data in case
> >   this is a .debug file which has it removed. This might be better than
> >   adding a check for X86_64_UNWIND since then we would also need to
> >   check the arch. Does != SHT_NOBITS work for you?
> 
> Yes, since SHT_NOBITS is not equal to SHT_X86_64_UNWIND :)

OK, then lets change your patch to do that as attached.

> > - What does eu-readelf -S show?
> >   I think we need a x86_64_section_type_name () ebl hook to show it
> >   correctly.
> 
> Yes, that looks like it:

And the other attached patch should clean that up.

Thanks,

Mark

[-- Attachment #2: 0001-Also-find-CFI-in-sections-of-type-SHT_X86_64_UNWIND.patch --]
[-- Type: text/x-patch, Size: 1912 bytes --]

From e556deeb6498b9a5fc320c1d9ee23c6fcdcab384 Mon Sep 17 00:00:00 2001
From: Milian Wolff <milian.wolff@kdab.com>
Date: Mon, 29 Oct 2018 16:21:26 +0100
Subject: [PATCH 1/2] Also find CFI in sections of type SHT_X86_64_UNWIND

On my system with g++ (GCC) 8.2.1 20180831 with GNU gold (GNU Binutils
2.31.1) 1.16, the .eh_frame section does not have type PROGBITS
but rather is using X86_64_UNWIND nowadays:

```
$ echo "int main(){ return 0; }" > test.c
$ gcc test.c
$ readelf --sections a.out | grep .eh_frame
  [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
  [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
```

Without this patch, libdw refuses to use the available unwind
information, leading to broken backtraces while unwinding. With the
patch applied, unwinding works once more in such situations.

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdw/ChangeLog          | 4 ++++
 libdw/dwarf_getcfi_elf.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index ebe002c..627fdde 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2018-10-29  Milian Wolff  <milian.wolff@kdab.com>
+
+	* dwarf_getcfi_elf.c (getcfi_shdr): Check sh_type != SHT_NOBITS.
+
 2018-09-13  Mark Wielaard  <mark@klomp.org>
 
 	* dwarf_begin_elf.c (check_section): Drop ehdr argument, add and
diff --git a/libdw/dwarf_getcfi_elf.c b/libdw/dwarf_getcfi_elf.c
index 315cc02..adcaea0 100644
--- a/libdw/dwarf_getcfi_elf.c
+++ b/libdw/dwarf_getcfi_elf.c
@@ -298,7 +298,7 @@ getcfi_shdr (Elf *elf, const GElf_Ehdr *ehdr)
 	    }
 	  else if (!strcmp (name, ".eh_frame"))
 	    {
-	      if (shdr->sh_type == SHT_PROGBITS)
+	      if (shdr->sh_type != SHT_NOBITS)
 		return getcfi_scn_eh_frame (elf, ehdr, scn, shdr,
 					    hdr_scn, hdr_vaddr);
 	      else
-- 
1.8.3.1


[-- Attachment #3: 0002-backends-Add-x86_64-section_type_name-for-SHT_X86_64.patch --]
[-- Type: text/x-patch, Size: 2548 bytes --]

From 3ca96596538cb57d9f23f6f5ddfc8e145e8b2177 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Tue, 6 Nov 2018 12:01:25 +0100
Subject: [PATCH 2/2] backends: Add x86_64 section_type_name for
 SHT_X86_64_UNWIND.

Makes sure that eu-readelf and eu-elflint recognize and show the
x86_64 specific section type correctly.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 backends/ChangeLog       |  5 +++++
 backends/x86_64_init.c   |  3 ++-
 backends/x86_64_symbol.c | 14 +++++++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/backends/ChangeLog b/backends/ChangeLog
index 768c270..e2a0281 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,8 @@
+2018-11-06  Mark Wielaard  <mark@klomp.org>
+
+	* x86_64_symbol.c (x86_64_section_type_name): New function.
+	* x86_64_init.c (x86_64_int): Hook section_type_name.
+
 2018-10-20  Mark Wielaard  <mark@klomp.org>
 
 	* ppc_initreg.c (ppc_set_initial_registers_tid): Use define instead of
diff --git a/backends/x86_64_init.c b/backends/x86_64_init.c
index adfa479..49f6c6c 100644
--- a/backends/x86_64_init.c
+++ b/backends/x86_64_init.c
@@ -1,5 +1,5 @@
 /* Initialization of x86-64 specific backend library.
-   Copyright (C) 2002-2009, 2013 Red Hat, Inc.
+   Copyright (C) 2002-2009, 2013, 2018 Red Hat, Inc.
    Copyright (C) H.J. Lu <hjl.tools@gmail.com>, 2015.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2002.
@@ -55,6 +55,7 @@ x86_64_init (Elf *elf __attribute__ ((unused)),
   eh->name = "AMD x86-64";
   x86_64_init_reloc (eh);
   HOOK (eh, reloc_simple_type);
+  HOOK (eh, section_type_name);
   if (eh->class == ELFCLASS32)
     eh->core_note = x32_core_note;
   else
diff --git a/backends/x86_64_symbol.c b/backends/x86_64_symbol.c
index e07b180..98457bc 100644
--- a/backends/x86_64_symbol.c
+++ b/backends/x86_64_symbol.c
@@ -1,5 +1,5 @@
 /* x86_64 specific symbolic name handling.
-   Copyright (C) 2002, 2005 Red Hat, Inc.
+   Copyright (C) 2002, 2005, 2018 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -59,3 +59,15 @@ x86_64_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type,
       return ELF_T_NUM;
     }
 }
+
+/* Return symbolic representation of section type.  */
+const char *
+x86_64_section_type_name (int type,
+			  char *buf __attribute__ ((unused)),
+			  size_t len __attribute__ ((unused)))
+{
+  if (type == SHT_X86_64_UNWIND)
+    return "X86_64_UNWIND";
+
+  return NULL;
+}
-- 
1.8.3.1

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

On Dienstag, 6. November 2018 12:06:57 CET Mark Wielaard wrote:
> Hi Milian,
> 
> On Tue, 2018-11-06 at 00:12 +0100, Milian Wolff wrote:
> > On Montag, 5. November 2018 00:04:32 CET Mark Wielaard wrote:
> > 
> > Interestingly, when I try to reproduce this on my laptop (i.e. compile
> > even
> > the trivial C example), then I cannot reproduce this at all anymore - the
> > .eh_frame sections show up as PROGBITS. My desktop at work still shows
> > this
> > behavior though (also see below). I can't quite explain this difference...
> 
> It seems to only happen with a specific combination of gcc and the gold
> linker, I could only generate the SHT_X86_64_UNWIND sections only on
> fedora 29 with gcc 8.2.1 and gold version 2.31.1-13.fc29 (1.16).

At least on my system, that doesn't seem to be enough. Both my desktop and my 
laptop are running on ArchLinux with the same versions of GCC and Gold, yet 
one shows this behavior while the other one isn't...

What I noticed is that ccache seems to influence it for me, which makes this 
even stranger:

```
$ which gcc
/usr/lib/ccache/bin/gcc
$ gcc --version
gcc (GCC) 8.2.1 20180831
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc test.c
$ readelf -S a.out | grep eh_frame
  [14] .eh_frame_hdr     PROGBITS         0000000000002004  00002004
  [15] .eh_frame         PROGBITS         0000000000002030  00002030

$ /usr/bin/gcc test.c
$ /usr/bin/gcc --version
gcc (GCC) 8.2.1 20180831
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ /usr/bin/gcc test.c
$ readelf -S a.out | grep eh_frame
  [14] .eh_frame         X86_64_UNWIND    0000000000000670  00000670
  [15] .eh_frame_hdr     X86_64_UNWIND    0000000000000724  00000724
```

Anyhow, that's unrelated to the patches at hand here. See below.

> > > - It might be better to change the check to shdr->sh_type != SHT_NOBITS
> > > 
> > >   The idea is probably that we don't want to look at the data in case
> > >   this is a .debug file which has it removed. This might be better than
> > >   adding a check for X86_64_UNWIND since then we would also need to
> > >   check the arch. Does != SHT_NOBITS work for you?
> > 
> > Yes, since SHT_NOBITS is not equal to SHT_X86_64_UNWIND :)
> 
> OK, then lets change your patch to do that as attached.
> 
> > > - What does eu-readelf -S show?
> > > 
> > >   I think we need a x86_64_section_type_name () ebl hook to show it
> > >   correctly.
> > 
> > Yes, that looks like it:
>
> And the other attached patch should clean that up.

Both patches work for me:

    Tested-by: Milian Wolff <milian.wolff@kdab.com>

Thanks
-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Mark Wielaard]

On Wed, Nov 07, 2018 at 10:02:54AM +0100, Milian Wolff wrote:
> On Dienstag, 6. November 2018 12:06:57 CET Mark Wielaard wrote:
> > It seems to only happen with a specific combination of gcc and the gold
> > linker, I could only generate the SHT_X86_64_UNWIND sections only on
> > fedora 29 with gcc 8.2.1 and gold version 2.31.1-13.fc29 (1.16).
> 
> At least on my system, that doesn't seem to be enough. Both my desktop and my 
> laptop are running on ArchLinux with the same versions of GCC and Gold, yet 
> one shows this behavior while the other one isn't...

Strange indeed. My system doesn't have ccache, it does show the issue.

> Both patches work for me:
> 
>     Tested-by: Milian Wolff <milian.wolff@kdab.com>

Great. I pushed both to master now.

Thanks,

Mark

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 238 bytes --]

Found another issue while testing with a gcc 8.1 and gold setup I
noticed some self-tests failed because elflint doesn't know about
SHT_X86_64_UNWIND section type and cannot match the PT_GNU_EH_FRAME
segment. The attached patch fixes it.

[-- Attachment #2: 0001-elflint-Allow-PT_GNU_EH_FRAME-segment-to-match-SHT_X.patch --]
[-- Type: text/x-diff, Size: 1642 bytes --]

From ad11949bbd22aadd8d0e950ff7defbad7cc271f2 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Sat, 10 Nov 2018 23:33:03 +0100
Subject: [PATCH] elflint: Allow PT_GNU_EH_FRAME segment to match
 SHT_X86_64_UNWIND section.

The gold linker might generate an .eh_frame_hdr with a SHT_X86_64_UNWIND
type instead of a SHT_PROGBITS type.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog | 5 +++++
 src/elflint.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index f1a35798..fc6d414f 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2018-11-10  Mark Wielaard  <mark@klomp.org>
+
+	* elflint.c (check_program_header): Allow PT_GNU_EH_FRAME segment
+	to be matched against SHT_X86_64_UNWIND section.
+
 2018-10-20  Mark Wielaard  <mark@klomp.org>
 
 	* readelf.c (process_elf_file): Use dwelf_elf_begin to open pure_elf.
diff --git a/src/elflint.c b/src/elflint.c
index fa3af4c5..658f2ae2 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -4605,8 +4605,10 @@ program header offset in ELF header and PHDR entry do not match"));
 	      any = true;
 	      shdr = gelf_getshdr (scn, &shdr_mem);
 	      if (shdr != NULL
-		  && shdr->sh_type == (is_debuginfo
-				       ? SHT_NOBITS : SHT_PROGBITS)
+		  && ((is_debuginfo && shdr->sh_type == SHT_NOBITS)
+		      || (! is_debuginfo
+			  && (shdr->sh_type == SHT_PROGBITS
+			      || shdr->sh_type == SHT_X86_64_UNWIND)))
 		  && elf_strptr (ebl->elf, shstrndx, shdr->sh_name) != NULL
 		  && ! strcmp (".eh_frame_hdr",
 			       elf_strptr (ebl->elf, shstrndx, shdr->sh_name)))
-- 
2.19.1

Re: [PATCH] Also find CFI in sections of type SHT_X86_64_UNWIND

[Mark Wielaard]

On Sun, 2018-11-11 at 00:28 +0100, Mark Wielaard wrote:
> Found another issue while testing with a gcc 8.1 and gold setup I
> noticed some self-tests failed because elflint doesn't know about
> SHT_X86_64_UNWIND section type and cannot match the PT_GNU_EH_FRAME
> segment. The attached patch fixes it.

> Subject: [PATCH] elflint: Allow PT_GNU_EH_FRAME segment to match
>  SHT_X86_64_UNWIND section.
> 
> The gold linker might generate an .eh_frame_hdr with a
> SHT_X86_64_UNWIND type instead of a SHT_PROGBITS type.

Pushed to master.