public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug tdep/30028] New: [gdb/tdep, x86_64] Epilogue unwinder preferred over dwarf unwinder based on gcc 4.4
@ 2023-01-20 14:06 vries at gcc dot gnu.org
  2023-01-20 14:13 ` [Bug tdep/30028] " vries at gcc dot gnu.org
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2023-01-20 14:06 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30028

            Bug ID: 30028
           Summary: [gdb/tdep, x86_64] Epilogue unwinder preferred over
                    dwarf unwinder based on gcc 4.4
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tdep
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider test-case gdb.base/unwind-on-each-insn.exp.

On x86_64-linux, it passes.

Now, let's revert the fix in commit 49d7cd733a7 ("Change calculation of
frame_id by amd64 epilogue unwinder"):
...
$ git show 49d7cd733a7f1b87aa1d40318b3d7c2b65aca5ac gdb/amd64-tdep.c >
tmp.patch
$ patch -p1 -R < tmp.patch 
patching file gdb/amd64-tdep.c
Hunk #1 succeeded at 2945 (offset 8 lines).
Hunk #2 succeeded at 2994 (offset 8 lines).
...

Now we have:
...
FAIL: gdb.base/unwind-on-each-insn.exp: instruction 5: $fba_value == $foo_fba
FAIL: gdb.base/unwind-on-each-insn.exp: instruction 5: check frame-id matches
...

OK, so the test-case acts as regression test for this commit, that seems as
intended.

For context, let's look at the foo insns:
...
00000000004004bc <foo>:
  4004bc:       55                      push   %rbp
  4004bd:       48 89 e5                mov    %rsp,%rbp
  4004c0:       90                      nop
  4004c1:       5d                      pop    %rbp
  4004c2:       c3                      ret
...

This is a minimal version of the failure:
...
$ gdb -q -batch outputs/gdb.base/unwind-on-each-insn/unwind-on-each-insn \
  -ex "b *foo" \
  -ex run \
  -ex "si 3" \
  -ex "info frame" \
  -ex si \
  -ex "info frame"
Breakpoint 1 at 0x4004bc

Breakpoint 1, 0x00000000004004bc in foo ()
0x00000000004004c1 in foo ()
Stack level 0, frame at 0x7fffffffdae0:
 rip = 0x4004c1 in foo; saved rip = 0x4004b5
 called by frame at 0x7fffffffdaf0
 Arglist at 0x7fffffffdad0, args: 
 Locals at 0x7fffffffdad0, Previous frame's sp is 0x7fffffffdae0
 Saved registers:
  rbp at 0x7fffffffdad0, rip at 0x7fffffffdad8
0x00000000004004c2 in foo ()
Stack level 0, frame at 0x7fffffffdad8:
 rip = 0x4004c2 in foo; saved rip = 0x4004b5
 called by frame at 0x7fffffffdaf0
 Arglist at 0x7fffffffdae0, args: 
 Locals at 0x7fffffffdae0, Previous frame's sp is 0x7fffffffdae0
 Saved registers:
  rip at 0x7fffffffdad8
...

The problem is that the "frame at" lists a different address at the last insn.

However, we also have this information available:
...
00000068 000000000000001c 0000003c FDE cie=00000030
pc=00000000004004bc..00000000004004c3     
   LOC           CFA      rbp   ra
00000000004004bc rsp+8    u     c-8
00000000004004bd rsp+16   c-16  c-8
00000000004004c0 rbp+16   c-16  c-8
00000000004004c2 rsp+8    c-16  c-8
...

Let's see if it is correct.

At 0x00000000004004c1 we have:
...
(gdb) p $rbp+16
$1 = (void *) 0x7fffffffdae0
...

At 0x00000000004004c2 we have:
...
(gdb) p $rsp+8
$2 = (void *) 0x7fffffffdae0
...
Well, that looks correct.

So, why is this info not used?

The amd64 epilogue unwinder (the one containing the bug), has the highest
priority:
...
  /* Hook the function epilogue frame unwinder.  This unwinder is               
     appended to the list first, so that it supercedes the other                
     unwinders in function epilogues.  */                                       
  frame_unwind_prepend_unwinder (gdbarch, &amd64_epilogue_frame_unwind);        
...

Let's try to see what happens if we disable it:
...
0x00000000004004c2 in foo ()
Stack level 0, frame at 0x7fffffffdbcc:
...
Hmm, that's also no good, it seems that the i386_epilogue_frame_unwind kicked
in.

I don't under stand why this would be active for x86_64 code, maybe that is a
bug in itself. Anyway, why does this one have this high priority? Well, the
code says:
...

  /* Hook the function epilogue frame unwinder.  This unwinder is               
     appended to the list first, so that it supercedes the DWARF                
     unwinder in function epilogues (where the DWARF unwinder                   
     currently fails).  */
  frame_unwind_append_unwinder (gdbarch, &i386_epilogue_frame_unwind);
...

Hmm, so the "currently" refers to July 2009.

In the submission discussion I find this (
https://sourceware.org/pipermail/gdb-patches/2009-July/066792.html ):
...
> I also think you should add a comment about the specific ordering of
> this unwinder.  It has to come before the dwarf2 unwinder because GCC
> doesn't provide proper CFI for the epilogue, right?

Right - I would like to have a way to suppress this unwinder, maybe
based on the producer string like other recognized dwarf2-frame
quirks, but we can worry about that later.  I hope it will be
unnecessary with GCC 4.5.
...

So, if I understand correctly, the priority decision was done based on gcc 4.4.

So maybe it's time to revisit this.

I suppose the producer test is doable, but we'll need to decide about trusted
vs untrusted versions, which atm I have no clue about.

A minor benefit of preferring dwarf is that behaviour will be more similar to
targets that do not have epilogue unwinders.

Anyway, disabling i386_epilogue_frame_unwind gives me the desired:
...
0x00000000004004c2 in foo ()
Stack level 0, frame at 0x7fffffffdad0:
...
and indeed, the test-case passes again.

At the very least, I would like a maintenance command that tells gdb to prefer
dwarf over the epilogue unwinder, in order to be able to test the actual dwarf.
 That could also be used to proactively spot bugs in the epilogue unwinder.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-02-20 13:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20 14:06 [Bug tdep/30028] New: [gdb/tdep, x86_64] Epilogue unwinder preferred over dwarf unwinder based on gcc 4.4 vries at gcc dot gnu.org
2023-01-20 14:13 ` [Bug tdep/30028] " vries at gcc dot gnu.org
2023-01-20 14:23 ` vries at gcc dot gnu.org
2023-01-20 14:47 ` vries at gcc dot gnu.org
2023-01-20 16:02 ` vries at gcc dot gnu.org
2023-01-20 16:24 ` vries at gcc dot gnu.org
2023-01-20 16:43 ` vries at gcc dot gnu.org
2023-01-20 17:46 ` vries at gcc dot gnu.org
2023-01-21  7:49 ` vries at gcc dot gnu.org
2023-01-21  7:49 ` [Bug tdep/30028] [gdb/tdep, x86_64, i386] " vries at gcc dot gnu.org
2023-02-20 13:51 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).