[PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue

Tom de Vries tdevries@suse.de
Mon May 30 12:10:05 GMT 2022


Hi,

Consider test-case gdb.base/gdb11531.exp and target boards unix/-m32 and
unix/-m32/-fPIE/-pie.

With gcc 7.5.0 and unix/-m32, we have a breakpoint at main set here:
...
┌───────────────────────────────────────────────────────────────────────┐
│    0x8048406 <main>        push   %ebp                                │
│    0x8048407 <main+1>      mov    %esp,%ebp                           │
│    0x8048409 <main+3>      push   %ecx                                │
│    0x804840a <main+4>      lea    0x8(%ebp),%ecx                      │
│b+  0x804840d <main+7>      movl   $0x5,0x804a01c                      │
...
because (see skip_prologue_sal for details) first the architecture-specific
prologue skipper skips to 0x804840a, after which the line table is examined:
...
File name            Line number    Starting address    View    Stmt
gdb11531.c                    33           0x8048406               x
gdb11531.c                    34           0x804840d               x
gdb11531.c                    35           0x8048417               x
...
and since the address does not fall exactly at a line entry, the address of
the first following entry is picked: 0x804840d, at line 34.

With gcc 7.5.0 and unix/-m32/-fPIE/-pie, the breakpoint is set here:
...
┌───────────────────────────────────────────────────────────────────────┐
│    0x52d <main>    push   %ebp                                        │
│    0x52e <main+1>  mov    %esp,%ebp                                   │
│    0x530 <main+3>  push   %ecx                                        │
│    0x531 <main+4>  lea    0x8(%ebp),%ecx                              │
│    0x534 <main+7>  call   0x571 <__x86.get_pc_thunk.ax>               │
│    0x539 <main+12> add    $0x1ac7,%eax                                │
│b+  0x53e <main+17> movl   $0x5,0x1c(%eax)                             │
...
because (see skip_prologue_sal for details) first the architecture-specific
prologue skipper skips to 0x531, after which the line table is examined:
...
File name            Line number    Starting address    View    Stmt
gdb11531.c                    33               0x52d               x
gdb11531.c                    34               0x53e               x
gdb11531.c                    35               0x548               x
...
and again since the address does not fall exactly at a line entry, the address
of the first following entry is picked: 0x53e, at line 34.

With gcc 12.1.0 and unix/-m32, we have a breakpoint at main set here:
...
┌───────────────────────────────────────────────────────────────────────┐
│b+  0x804915d <main>        movl   $0x5,0x804c01c                      │
│...
because first the architecture-specific prologue doesn't skip anything, after
which the line table is examined:
...
File name            Line number    Starting address    View    Stmt
gdb11531.c                    33           0x804915d               x
gdb11531.c                    34           0x804915d       1       x
gdb11531.c                    35           0x8049167               x
...
and the second entry matching the address is picked, at line 34.
In more detail, the logic in find_pc_sect_line is such that we find the first
entry that no longer matches the address (at line 35) and then pick the
preceding entry.

With gcc 12.1.0 and unix/-m32/-fPIE/-pie, the breakpoint is set here:
...
┌───────────────────────────────────────────────────────────────────────┐
│b+  0x117d <main>           call   0x11b8 <__x86.get_pc_thunk.ax>      │
│    0x1182 <main+5>         add    $0x2e7e,%eax                        │
│    0x1187 <main+10>        movl   $0x5,0x1c(%eax)                     │
...
because first the architecture-specific prologue doesn't skip anything, after
which the line table is examined:
...
File name            Line number    Starting address    View    Stmt
gdb11531.c                    33              0x117d               x
gdb11531.c                    34              0x1187               x
gdb11531.c                    35              0x1191               x
...
and we pick entry at line 33.

This causes a FAIL because the test-case expect the break at line 34 instead
of line 33:
...
FAIL: gdb.base/gdb11531.exp: watchpoint variable triggers at next
...

Fix this by detecting the get_pc_thunk call and associated add in the
architecture-specific prologue skipping.

It is an open question whether the get_pc_thunk call is indeed part of the
prologue.  F.i., in gcc the corresponding insn is emitted after
NOTE_INSN_PROLOGUE_END, but before NOTE_INSN_FUNCTION_BEG.
OTOH, for both compiler versions, the call is part of the first line entry,
which usually represents the prologue.

This fix causes two related regressions:
...
FAIL: gdb.base/break.exp: run until breakpoint set at \
  small function, optimized file
FAIL: gdb.base/hbreak2.exp: run until hardware breakpoint set at \
  small function, optimized file
...

The problem is that a break at marker4 no longer ends up at a stmt boundary:
...
-Breakpoint 2, marker4 (d=177601976) at break1.c:59^M
+Breakpoint 2, 0x5655636a in marker4 (d=177601976) at break1.c:59^M
...

In other words, the breakpoint ends up here:
...
┌───────────────────────────────────────────────────────────────────────┐
│    0x56556360 <marker4>    call   0x56556375 <__x86.get_pc_thunk.ax>  │
│    0x56556365 <marker4+5>  add    $0x2c9b,%eax                        │
│B+> 0x5655636a <marker4+10> mov    0x4(%esp),%edx                      │
...
because the architecture-specific prologue now skips to 0x5655636a, after
which the line table is examined:
...
break1.c:
File name                            Line number    Starting address    View    Stmt
break1.c                                      59              0x1360               x
break1.c                                      59              0x1360       1       x
break1.c                                      59              0x136a
break1.c                                      59              0x1374
break1.c                                       -              0x1375
...
and the entry at 0x136a is picked, which isn't at a statement boundary.

The line table could be interpreted here such that that gcc when optimizing
doesn't consider the get_pc_thunk call part of the prologue, but instead part
of the first statement.  But if we drop the optimization level to -O1, we have
the exact same code:
...
┌───────────────────────────────────────────────────────────────────────┐
│    0x5655632b <marker4>    call   0x56556340 <__x86.get_pc_thunk.ax>  │
│    0x56556330 <marker4+5>  add    $0x2cd0,%eax                        │
│B+> 0x56556335 <marker4+10> mov    0x4(%esp),%edx                      │
...
but a line table that does have an entry with stmt boundary marker at
address 0x56556335:
...
File name            Line number    Starting address    View    Stmt
break1.c                      59              0x132b               x
break1.c                      59              0x132b       1
break1.c                      59              0x1335               x
break1.c                      59              0x1335       1
break1.c                      59              0x133f
break1.c                      -              0x1340
...

So, I'm interpreting the O2 line table as a bug, filed as
"[debug, i386] sched2 moves get_pc_thunk call past debug_insn" (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105772 ).

Fix the regressions by updating the test-cases to accept the new output.

Note btw that accepting the new input matches an already present comment in
both test-cases:
...
  # Therefore the address after the prologue (where the breakpoint is)
  # has no exactly matching line symbol, and GDB reports the breakpoint
  # as if it were in the middle of a line rather than at the beginning.
...

The test-case break.exp used to accept this input, but that was dropped in
commit 3b377a3aa77 "Drop non-prototype C function header variants: 'break'
test case".  Likewise for hbreak.exp.

Tested on x86_64-linux.

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


Any comments?

Thanks,
- Tom

[gdb/tdep] Detect get_pc_thunk call in i386 prologue

---
 gdb/i386-tdep.c                    | 117 ++++++++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/break.exp   |   2 +-
 gdb/testsuite/gdb.base/hbreak2.exp |   2 +-
 3 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 8501e12e241..c0204531018 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1782,6 +1782,119 @@ i386_analyze_register_saves (CORE_ADDR pc, CORE_ADDR current_pc,
   return pc;
 }
 
+static CORE_ADDR
+i386_skip_call_pc_thunk (struct gdbarch *gdbarch, CORE_ADDR start_pc,
+			 struct i386_frame_cache *cache)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR pc, res_pc;
+  bool have_ax = false;
+  bool have_bx = false;
+  bool have_dx = false;
+  gdb_byte op;
+
+  /* Point to first insn.  */
+  pc = start_pc;
+
+  /* Insn: call <__x86.get_pc_thunk.ax>.  */
+  if (target_read_code (pc + 0, &op, 1))
+    return start_pc;
+  if (op != 0xe8)
+    return start_pc;
+
+  /* Check that last byte of insn is present.  */
+  if (target_read_code (pc + 4, &op, 1))
+    return start_pc;
+
+  /* Get address of call target.  */
+  CORE_ADDR get_pc_thunk_addr
+    = (pc + 5
+       + read_code_integer (pc + 1, 4, byte_order));
+
+  /* Point to second insn.  */
+  pc += 5;
+
+  /* Insn: add $0xdead,%eax.  */
+  if (target_read_code (pc + 0, &op, 1))
+    return start_pc;
+  if (op == 0x05)
+    {
+      have_ax = true;
+
+      /* Check that last byte of insn is present.  */
+      if (target_read_code (pc + 4, &op, 1))
+	return start_pc;
+
+      /* Point after second insn.  */
+      pc += 5;
+    }
+  else if (op == 0x81)
+    {
+      if (target_read_code (pc + 1, &op, 1))
+	return start_pc;
+      if (op == 0xc2)
+	have_dx = true;
+      else if (op == 0xc3)
+	have_bx = true;
+      else
+	return start_pc;
+
+      /* Check that last byte of insn is present.  */
+      if (target_read_code (pc + 5, &op, 1))
+	return start_pc;
+
+      /* Point after second insn.  */
+      pc += 6;
+    }
+  else
+    return start_pc;
+
+  /* We skipped past the get_pc_thunk call and associated add insn, save
+     pc to return it later.  */
+  res_pc = pc;
+
+  /* Now verify that the call target is as expected.  Point to first insn of
+     call target.  */
+  pc = get_pc_thunk_addr;
+
+  /* Insn: mov (%esp),%eax.  */
+  if (target_read_code (pc + 0 , &op, 1))
+    return start_pc;
+  if (op != 0x8b)
+    return start_pc;
+
+  if (target_read_code (pc + 1, &op, 1))
+    return start_pc;
+  if (have_ax && op == 0x04)
+    ;
+  else if (have_bx && op == 0x1c)
+    ;
+  else if (have_dx && op == 0x14)
+    ;
+  else
+    return start_pc;
+
+  if (target_read_code (pc + 2, &op, 1))
+    return start_pc;
+  if (op != 0x24)
+    return start_pc;
+
+  /* Point to second insn of call target.  */
+  pc += 3;
+
+  /* Insn: ret.  */
+  if (target_read_code (pc, &op, 1))
+    return start_pc;
+  if (op != 0xc3)
+    return start_pc;
+
+  /* Force prologue skipping to succeed.  */
+  if (cache->locals < 0)
+    cache->locals = 0;
+
+  return res_pc;
+}
+
 /* Do a full analysis of the prologue at PC and update CACHE
    accordingly.  Bail out early if CURRENT_PC is reached.  Return the
    address where the analysis stopped.
@@ -1821,7 +1934,9 @@ i386_analyze_prologue (struct gdbarch *gdbarch,
   pc = i386_skip_probe (pc);
   pc = i386_analyze_stack_align (pc, current_pc, cache);
   pc = i386_analyze_frame_setup (gdbarch, pc, current_pc, cache);
-  return i386_analyze_register_saves (pc, current_pc, cache);
+  pc = i386_analyze_register_saves (pc, current_pc, cache);
+  pc = i386_skip_call_pc_thunk (gdbarch, pc, cache);
+  return pc;
 }
 
 /* Return PC of first real instruction.  */
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index 4421459154f..38f443f7150 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -818,7 +818,7 @@ set bp_location14 [gdb_get_line_number "set breakpoint 14 here" $srcfile1]
 
 gdb_test_multiple "continue" \
     "run until breakpoint set at small function, optimized file" {
-	-re "Breakpoint $decimal, marker4 \\(d=(d@entry=)?177601976\\) at .*$srcfile1:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
+	-re "Breakpoint $decimal, ($hex in )?marker4 \\(d=(d@entry=)?177601976\\) at .*$srcfile1:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
 	    pass "run until breakpoint set at small function, optimized file (line bp_location14)"
 	}
 	-re "Breakpoint $decimal, factorial \\(.*\\) .*\{\r\n$gdb_prompt" {
diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
index aecf613643d..f176e9f2aa0 100644
--- a/gdb/testsuite/gdb.base/hbreak2.exp
+++ b/gdb/testsuite/gdb.base/hbreak2.exp
@@ -571,7 +571,7 @@ set bp_location14 [gdb_get_line_number "set breakpoint 14 here" $srcfile1]
 
 gdb_test_multiple "continue" \
     "run until hardware breakpoint set at small function, optimized file" {
-	-re "Breakpoint $decimal, marker4 \\(d=(d@entry=)?177601976\\) at .*$srcfile1:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
+	-re "Breakpoint $decimal, ($hex in )?marker4 \\(d=(d@entry=)?177601976\\) at .*$srcfile1:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
 	    pass "run until hardware breakpoint set at small function, optimized file (line bp_location14)"
 	}
 	-re "Breakpoint $decimal, factorial \\(.*\\) .*\{\r\n$gdb_prompt" {


More information about the Gdb-patches mailing list