From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id pWITITy0lGIENwkAWB0awg (envelope-from ) for ; Mon, 30 May 2022 08:10:36 -0400 Received: by simark.ca (Postfix, from userid 112) id 766351E221; Mon, 30 May 2022 08:10:36 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=r18+rBO1; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 602AB1E00D for ; Mon, 30 May 2022 08:10:35 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 994AD383D823 for ; Mon, 30 May 2022 12:10:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 994AD383D823 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1653912634; bh=qgHUcKqf6OkzSyQplnac0eHhRdIvTdvecRTnT6yUw08=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=r18+rBO1Zhqi8UYj/VoUiUJy015VWFcPdpa96pCi2mpKoTfKjQB6aZg52wtQJ18er f8HkwZ/SrCtrXm0Rt/6l8nzqAkDYmPO+EYlzGikZPOBROOm6k7PXm51JBw5nReg0Sr BoUHaGT99N4C/cIqrh8Q+bvPKWh5NAx0jfWV+YqU= Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id EE46F38F861F for ; Mon, 30 May 2022 12:10:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EE46F38F861F Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 2C7021F88F for ; Mon, 30 May 2022 12:10:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 17B7413A84 for ; Mon, 30 May 2022 12:10:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id l3SiBB+0lGIWCwAAMHmgww (envelope-from ) for ; Mon, 30 May 2022 12:10:07 +0000 Date: Mon, 30 May 2022 14:10:05 +0200 To: gdb-patches@sourceware.org Subject: [PATCH][gdb/tdep] Detect get_pc_thunk call in i386 prologue Message-ID: <20220530121003.GA16697@delia.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "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
push %ebp │ │ 0x8048407 mov %esp,%ebp │ │ 0x8048409 push %ecx │ │ 0x804840a lea 0x8(%ebp),%ecx │ │b+ 0x804840d 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
push %ebp │ │ 0x52e mov %esp,%ebp │ │ 0x530 push %ecx │ │ 0x531 lea 0x8(%ebp),%ecx │ │ 0x534 call 0x571 <__x86.get_pc_thunk.ax> │ │ 0x539 add $0x1ac7,%eax │ │b+ 0x53e 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
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
call 0x11b8 <__x86.get_pc_thunk.ax> │ │ 0x1182 add $0x2e7e,%eax │ │ 0x1187 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 call 0x56556375 <__x86.get_pc_thunk.ax> │ │ 0x56556365 add $0x2c9b,%eax │ │B+> 0x5655636a 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 call 0x56556340 <__x86.get_pc_thunk.ax> │ │ 0x56556330 add $0x2cd0,%eax │ │B+> 0x56556335 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" {