gas: gcfg: fix handling of non-local direct jmps in gcfg

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.
This commit is contained in:
Indu Bhagat 2024-03-28 11:57:23 -07:00
parent b58829cdef
commit e67388a6a4
5 changed files with 105 additions and 20 deletions

View File

@ -444,16 +444,26 @@ ginsn_indirect_jump_p (ginsnS *ginsn)
return ret_p;
}
/* Return whether the GINSN is an unconditional jump to a label which is
defined locally in the scope of the block of insns, which are currently
being processed for GCFG creation. */
static bool
ginsn_direct_local_jump_p (ginsnS *ginsn)
{
bool ret_p = false;
if (!ginsn)
return ret_p;
bool local_p = false;
const symbolS *taken_label;
ret_p |= (ginsn->type == GINSN_TYPE_JUMP
&& ginsn->src[0].type == GINSN_SRC_SYMBOL);
return ret_p;
if (!ginsn)
return local_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 +795,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 +829,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 +1094,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)

View File

@ -0,0 +1,32 @@
#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+0010 0+001c FDE cie=00000000 pc=0+0000..0+001e
DW_CFA_nop
#...
0+002c 0+0018 0+0030 FDE cie=00000000 pc=0+001e..0+0029
DW_CFA_advance_loc: 1 to 0+001f
DW_CFA_def_cfa_offset: 16
DW_CFA_offset: r6 \(rbp\) at cfa-16
DW_CFA_advance_loc: 3 to 0+0022
DW_CFA_def_cfa_register: r6 \(rbp\)
DW_CFA_nop
#pass

View File

@ -0,0 +1,2 @@
.*Assembler messages:
.*9: Warning: SCFI ignores most user-specified CFI directives

View File

@ -0,0 +1,40 @@
# Testcase with jmp to function instead of a call.
# Also includes a jmp to label locally defined.
# 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
jmp .L2
.L2:
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
# New function with unconditional jump to a label previously defined
# in a different function.
.globl bar
.type bar, @function
bar:
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset %rbp, -16
movq %rsp, %rbp
.cfi_def_cfa_register %rbp
movl $0, %eax
jmp .L2
.cfi_endproc
.size bar, .-bar

View File

@ -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"