* [PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue
@ 2022-05-30 12:10 Tom de Vries
2022-06-08 9:13 ` Tom de Vries
0 siblings, 1 reply; 2+ messages in thread
From: Tom de Vries @ 2022-05-30 12:10 UTC (permalink / raw)
To: gdb-patches
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" {
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue
2022-05-30 12:10 [PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue Tom de Vries
@ 2022-06-08 9:13 ` Tom de Vries
0 siblings, 0 replies; 2+ messages in thread
From: Tom de Vries @ 2022-06-08 9:13 UTC (permalink / raw)
To: gdb-patches
On 5/30/22 14:10, Tom de Vries wrote:
> Fix the regressions by updating the test-cases to accept the new output.
These regressions were those that I ran into with a first version of the
patch, which handled only __x86.get_pc_thunk.ax.
This version handles bx and dx in addition, which triggers an additional
regression (indeed another -O2 test-case):
...
FAIL: gdb.cp/step-and-next-inline.exp: no_header: not in inline 1
...
Anyway, a quick investigation shows that there are still more regs to
handle:
...
$ objdump -d /lib/libc.so.6 | grep get_pc_thunk | awk '{print $9}' | sort -u
<__x86.get_pc_thunk.ax>
<__x86.get_pc_thunk.bp>
<__x86.get_pc_thunk.bx>
<__x86.get_pc_thunk.cx>
<__x86.get_pc_thunk.di>
<__x86.get_pc_thunk.dx>
<__x86.get_pc_thunk.si>
...
so with a more complete version of this patch, still more -O2
regressions may show up.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-06-08 9:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-30 12:10 [PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue Tom de Vries
2022-06-08 9:13 ` Tom de Vries
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).