From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7850) id 937033849AF3; Wed, 10 Apr 2024 21:06:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 937033849AF3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1712783218; bh=VT6j/ZR8e2WITUV3Pdeb5oUnDhKKr3FJXCxvu0YYXj4=; h=From:To:Subject:Date:From; b=BERvtHSc30rw8bGxCXcDpHOEPfJksrRa9LJt8fKU6/82dD/00XnhFq5k8lDMc1GqV Xfdk3ARWcv2Kw3nVJS6GqfNTXfO9S9PEtkbGf1Kf9hK8nplIlP9JQBEPiCwohhhB+x holhKzXpBkUCjg68hj+E9qUzr+h/YZBudjSA6ms0= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Indu Bhagat To: binutils-cvs@sourceware.org Subject: [binutils-gdb] gas: scfi: bugfixes for SCFI state propagation X-Act-Checkin: binutils-gdb X-Git-Author: Indu Bhagat X-Git-Refname: refs/heads/master X-Git-Oldrev: a1c6a60cc55a17b5f3c8e26f1bc68b4ee2470e54 X-Git-Newrev: f1c5d46cae3fa4a3c41c05c796c69d52eef97067 Message-Id: <20240410210658.937033849AF3@sourceware.org> Date: Wed, 10 Apr 2024 21:06:58 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Df1c5d46cae3f= a4a3c41c05c796c69d52eef97067 commit f1c5d46cae3fa4a3c41c05c796c69d52eef97067 Author: Indu Bhagat Date: Wed Apr 10 13:46:37 2024 -0700 gas: scfi: bugfixes for SCFI state propagation =20 There are two state propagation functions in SCFI machinery - forward and backward flow. The patch addresses two issues: - In forward_flow_scfi_state (), the state being compared in forward = flow must be that at the exit of a prev bb and that at the entry of the next bb. The variable holding the state to be compared was previously (erroneously) stale. - In cmp_scfi_state (), the assumption that two different control flows, leading to the same basic block, cannot have a mismatched notion of CFA base register, is not true. Remove the assertion and instead return err if mismatch. =20 Fixing these issues helps correctly synthesize CFI, when previously SCFI was erroring out for an otherwise valid input asm. =20 gas/ * scfi.c (cmp_scfi_state): Remove assertion and return mismatch in return value as applicable. (forward_flow_scfi_state): Update state object to be the same as the exit state of the prev bb before comparing. =20 gas/testsuite/ * gas/scfi/x86_64/scfi-x86-64.exp: Add new test. * gas/scfi/x86_64/scfi-cfg-5.d: New test. * gas/scfi/x86_64/scfi-cfg-5.l: New test. * gas/scfi/x86_64/scfi-cfg-5.s: New test. Diff: --- gas/scfi.c | 20 ++++++++------ gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.d | 39 +++++++++++++++++++++++= ++++ gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.l | 2 ++ gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.s | 32 ++++++++++++++++++++++ gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp | 2 ++ 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/gas/scfi.c b/gas/scfi.c index 37cc85cfed7..929ea8a73e1 100644 --- a/gas/scfi.c +++ b/gas/scfi.c @@ -155,7 +155,7 @@ cmp_scfi_state (scfi_stateS *state1, scfi_stateS *state= 2) int ret; =20 if (!state1 || !state2) - ret =3D 1; + return 1; =20 /* Skip comparing the scratch[] value of registers. The user visible unwind information is derived from the regs[] from the SCFI state. */ @@ -164,10 +164,12 @@ cmp_scfi_state (scfi_stateS *state1, scfi_stateS *sta= te2) =20 /* For user functions which perform dynamic stack allocation, after swit= ching t REG_FP based CFA tracking, it is perfectly possible to have stack u= sage - in some control flows. However, double-checking that all control flo= ws - have the same idea of CFA tracking before this wont hurt. */ - gas_assert (state1->regs[REG_CFA].base =3D=3D state2->regs[REG_CFA].base= ); - if (state1->regs[REG_CFA].base =3D=3D REG_SP) + in some control flows. Further, the different control flows may even= not + have the same idea of CFA tracking (likely later necessitating genera= tion + of .cfi_remember_state / .cfi_restore_state pair). */ + ret |=3D state1->regs[REG_CFA].base !=3D state2->regs[REG_CFA].base; + + if (!ret && state1->regs[REG_CFA].base =3D=3D REG_SP) ret |=3D state1->stack_size !=3D state2->stack_size; =20 ret |=3D state1->traceable_p !=3D state2->traceable_p; @@ -911,11 +913,11 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state) return ret; } =20 -/* Recursively perform forward flow of the (unwind information) SCFI state +/* Recursively perform forward flow of the (unwind information) SCFI STATE starting at basic block GBB. =20 - The forward flow process propagates the SCFI state at exit of a basic b= lock - to the successor basic block. + The core of forward flow process takes the SCFI state at the entry of a= bb + and updates it incrementally as per the semantics of each ginsn in the = bb. =20 Returns error code, if any. */ =20 @@ -961,6 +963,8 @@ forward_flow_scfi_state (gcfgS *gcfg, gbbS *gbb, scfi_s= tateS *state) bb_for_each_edge(gbb, gedge) { gbb =3D gedge->dst_bb; + /* Ensure that the state is the one from the exit of the prev bb. */ + memcpy (state, prev_bb->exit_state, sizeof (scfi_stateS)); if (gbb->visited) { ret =3D cmp_scfi_state (gbb->entry_state, state); diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.d b/gas/testsuite/gas= /scfi/x86_64/scfi-cfg-5.d new file mode 100644 index 00000000000..5a0a2e1f927 --- /dev/null +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.d @@ -0,0 +1,39 @@ +#as: --scfi=3Dexperimental -W +#as: +#objdump: -Wf +#name: Synthesize CFI in presence of control flow 5 +#... +Contents of the .eh_frame section: + +00000000 0+0014 0+0000 CIE + Version: 1 + Augmentation: "zR" + Code alignment factor: 1 + Data alignment factor: -8 + Return address column: 16 + Augmentation data: 1b + DW_CFA_def_cfa: r7 \(rsp\) ofs 8 + DW_CFA_offset: r16 \(rip\) at cfa-8 + DW_CFA_nop + DW_CFA_nop + +0+0018 0+002c 0000001c FDE cie=3D00000000 pc=3D0+0000..0+0017 + DW_CFA_advance_loc: 1 to 0+0001 + DW_CFA_def_cfa_offset: 16 + DW_CFA_offset: r6 \(rbp\) at cfa-16 + DW_CFA_advance_loc: 3 to 0+0004 + DW_CFA_def_cfa_register: r6 \(rbp\) + DW_CFA_advance_loc: 5 to 0+0009 + DW_CFA_remember_state + DW_CFA_advance_loc: 6 to 0+000f + DW_CFA_def_cfa_register: r7 \(rsp\) + DW_CFA_restore: r6 \(rbp\) + DW_CFA_def_cfa_offset: 8 + DW_CFA_advance_loc: 1 to 0+0010 + DW_CFA_restore_state + DW_CFA_advance_loc: 6 to 0+0016 + DW_CFA_def_cfa_register: r7 \(rsp\) + DW_CFA_restore: r6 \(rbp\) + DW_CFA_def_cfa_offset: 8 + +#pass diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.l b/gas/testsuite/gas= /scfi/x86_64/scfi-cfg-5.l new file mode 100644 index 00000000000..abca835a642 --- /dev/null +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.l @@ -0,0 +1,2 @@ +.*Assembler messages: +.*5: Warning: SCFI ignores most user-specified CFI directives diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.s b/gas/testsuite/gas= /scfi/x86_64/scfi-cfg-5.s new file mode 100644 index 00000000000..80b081ffe07 --- /dev/null +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-5.s @@ -0,0 +1,32 @@ + .text + .globl foo + .type foo, @function +foo: + .cfi_startproc + push %rbp + .cfi_def_cfa_offset 16 + .cfi_offset %rbp, -16 + mov %rsp, %rbp + .cfi_def_cfa_register %rbp + cmpl $-1, %eax + jne .L1 +.L2: + .cfi_remember_state + call bar + pop %rbp + .cfi_def_cfa_register %rsp + .cfi_restore %rbp + .cfi_def_cfa_offset 8 + ret + +.L1: + .cfi_restore_state + testq %rax, %rax + je .L2 + pop %rbp + .cfi_def_cfa_register %rsp + .cfi_restore %rbp + .cfi_def_cfa_offset 8 + ret + .cfi_endproc + .size foo,.-foo diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp b/gas/testsuite/= gas/scfi/x86_64/scfi-x86-64.exp index 63f0d7e4700..a91dc9df9e5 100644 --- a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp +++ b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp @@ -82,6 +82,8 @@ if { ([istarget "x86_64-*-*"] && ![istarget "x86_64-*-li= nux*-gnux32"]) } then { run_list_test "scfi-cfg-3" "--scfi=3Dexperimental --warn" run_dump_test "scfi-cfg-4" run_list_test "scfi-cfg-4" "--scfi=3Dexperimental --warn" + run_dump_test "scfi-cfg-5" + run_list_test "scfi-cfg-5" "--scfi=3Dexperimental --warn" run_dump_test "scfi-asm-marker-1" run_list_test "scfi-asm-marker-1" "--scfi=3Dexperimental --warn" run_dump_test "scfi-asm-marker-2"