[PATCH] gas: gcfg: fix handling of non-local direct jmps in gcfg
Indu Bhagat
indu.bhagat@oracle.com
Fri Mar 22 22:22:40 GMT 2024
Ping
On 3/7/24 22:35, Indu Bhagat wrote:
> The ginsn infrastructure in GAS includes the ability to create a GCFG
> (ginsn CFG). A GCFG is currently used for SCFI passes.
>
> This patch fixes the following invalid assumptions / code blocks:
> - The function ginsn_direct_local_jump_p () was erroneously _not_
> checking whether the symbol is locally defined (i.e., within the
> scope of the code block for which GCFG is desired). Fix the code
> to do so.
> - Similarly, the GCFG creation code, in gcfg_build () itself had an
> assumption that a GINSN_TYPE_JUMP to a non-local symbol will not be
> seen. The latter can indeed be seen, and in fact, needs to be treated
> the same way as an exit from the function in terms of control-flow.
>
> gas/
> * ginsn.c (ginsn_direct_local_jump_p): Check if the symbol
> is local to the code block or function being assembled.
> (add_bb_at_ginsn): Remove buggy assumption.
> (frch_ginsn_data_append): Direct jmps do not disqualify a stream
> of ginsns from GCFG creation.
>
> gas/testsuite/
> * gas/scfi/x86_64/scfi-cfg-3.d: New test.
> * gas/scfi/x86_64/scfi-cfg-3.l: New test.
> * gas/scfi/x86_64/scfi-cfg-3.s: New test.
> * gas/scfi/x86_64/scfi-x86-64.exp: Add new test.
> ---
> gas/ginsn.c | 43 +++++++++++--------
> gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d | 24 +++++++++++
> gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l | 2 +
> gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s | 21 +++++++++
> gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp | 2 +
> 5 files changed, 73 insertions(+), 19 deletions(-)
> create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
> create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
> create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
>
> diff --git a/gas/ginsn.c b/gas/ginsn.c
> index 661f51d23c5..9f8e2979ab2 100644
> --- a/gas/ginsn.c
> +++ b/gas/ginsn.c
> @@ -447,13 +447,19 @@ ginsn_indirect_jump_p (ginsnS *ginsn)
> static bool
> ginsn_direct_local_jump_p (ginsnS *ginsn)
> {
> - bool ret_p = false;
> + bool local_p = false;
> + const symbolS *taken_label;
> +
> if (!ginsn)
> - return ret_p;
> + return local_p;
>
> - ret_p |= (ginsn->type == GINSN_TYPE_JUMP
> - && ginsn->src[0].type == GINSN_SRC_SYMBOL);
> - return ret_p;
> + if (ginsn->type == GINSN_TYPE_JUMP
> + && ginsn->src[0].type == GINSN_SRC_SYMBOL)
> + {
> + taken_label = ginsn->src[0].sym;
> + local_p |= (label_ginsn_map_find (taken_label) != NULL);
> + }
> + return local_p;
> }
>
> static char *
> @@ -785,16 +791,13 @@ add_bb_at_ginsn (const symbolS *func, gcfgS *gcfg, ginsnS *ginsn, gbbS *prev_bb,
> || ginsn->type == GINSN_TYPE_JUMP_COND
> || ginsn->type == GINSN_TYPE_RETURN)
> {
> - /* Indirect Jumps or direct jumps to symbols non-local to the
> - function must not be seen here. The caller must have already
> - checked for that. */
> + /* Indirect jumps must not be seen here. The caller must have
> + already checked for that. */
> gas_assert (!ginsn_indirect_jump_p (ginsn));
> - if (ginsn->type == GINSN_TYPE_JUMP)
> - gas_assert (ginsn_direct_local_jump_p (ginsn));
>
> - /* Direct Jumps. May include conditional or unconditional change of
> - flow. What is important for CFG creation is that the target be
> - local to function. */
> + /* Handle direct jumps. For unconditional direct jumps, where the
> + target is not local to the function, treat them later as similar
> + to an exit from function (in the else block). */
> if (ginsn->type == GINSN_TYPE_JUMP_COND
> || ginsn_direct_local_jump_p (ginsn))
> {
> @@ -822,10 +825,14 @@ add_bb_at_ginsn (const symbolS *func, gcfgS *gcfg, ginsnS *ginsn, gbbS *prev_bb,
> /* Add the bb for the fall through path. */
> find_or_make_bb (func, gcfg, ginsn->next, prev_bb, errp);
> }
> - else if (ginsn->type == GINSN_TYPE_RETURN)
> + else
> {
> - /* We'll come back to the ginsns following GINSN_TYPE_RETURN
> - from another path if they are indeed reachable code. */
> + gas_assert (ginsn->type == GINSN_TYPE_RETURN
> + || (ginsn->type == GINSN_TYPE_JUMP
> + && !ginsn_direct_local_jump_p (ginsn)));
> + /* We'll come back to the ginsns following GINSN_TYPE_RETURN or
> + other (non-local) unconditional jmps from another path if they
> + are indeed reachable code. */
> break;
> }
>
> @@ -1083,9 +1090,7 @@ frch_ginsn_data_append (ginsnS *ginsn)
> {
> temp->id = ++id;
>
> - if (ginsn_indirect_jump_p (temp)
> - || (ginsn->type == GINSN_TYPE_JUMP
> - && !ginsn_direct_local_jump_p (temp)))
> + if (ginsn_indirect_jump_p (temp))
> frchain_now->frch_ginsn_data->gcfg_apt_p = false;
>
> if (listing & LISTING_GINSN_SCFI)
> diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
> new file mode 100644
> index 00000000000..b373653c748
> --- /dev/null
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
> @@ -0,0 +1,24 @@
> +#as: --scfi=experimental -W
> +#as:
> +#objdump: -Wf
> +#name: Synthesize CFI in presence of control flow 3
> +#...
> +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+0014 0+001c FDE cie=00000000 pc=0000000000000000..000000000000001c
> + DW_CFA_nop
> +#...
> +
> +#pass
> diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
> new file mode 100644
> index 00000000000..5ff6048bef0
> --- /dev/null
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
> @@ -0,0 +1,2 @@
> +.*Assembler messages:
> +.*8: Warning: SCFI ignores most user-specified CFI directives
> diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
> new file mode 100644
> index 00000000000..ac63af892cb
> --- /dev/null
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
> @@ -0,0 +1,21 @@
> +# Testcase with jmp to function instead of a call.
> +# The CFG creation process is not expected to warn about
> +# missing foo_handler_v1 or xstrdup.
> + .text
> + .globl foo_handler
> + .type foo_handler, @function
> +foo_handler:
> + .cfi_startproc
> + movl current_style(%rip), %eax
> + cmpl $-1, %eax
> + je .L5
> + testb $4, %al
> + je .L3
> + jmp foo_handler_v1
> +.L3:
> + xorl %eax, %eax
> + ret
> +.L5:
> + jmp xstrdup
> + .cfi_endproc
> + .size foo_handler, .-foo_handler
> 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 5324af386f8..c004d99b4ac 100644
> --- a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
> @@ -78,6 +78,8 @@ if { ([istarget "x86_64-*-*"] && ![istarget "x86_64-*-linux*-gnux32"]) } then {
> run_list_test "scfi-cfg-1" "--scfi=experimental --warn"
> run_dump_test "scfi-cfg-2"
> run_list_test "scfi-cfg-2" "--scfi=experimental --warn"
> + run_dump_test "scfi-cfg-3"
> + run_list_test "scfi-cfg-3" "--scfi=experimental --warn"
> run_dump_test "scfi-asm-marker-1"
> run_list_test "scfi-asm-marker-1" "--scfi=experimental --warn"
> run_dump_test "scfi-asm-marker-2"
More information about the Binutils
mailing list