[PATCH, V2] gas: gcfg: fix handling of non-local direct jmps in gcfg

[Jan Beulich]

On 28.03.2024 08:59, Indu Bhagat wrote:
> [Changes in V2]
>   - Add function level comments for ginsn_direct_local_jump_p.
>   - Add a jmp to locally defined label in the testcase to make it more
>     complete.  Also, add a jmp to label defined in another function.
>     The testcase now generates an additional warning (as expected):
>     "Warning: GINSN: found unreachable code in func 'bar'"

For this - can't you simply omit the unreachable pair of insns?

Other than that, looks plausible, so feel free to put in.

>   - Use formatting style consistent with other testcases.
> [End of changes in V2]

As a minor remark, this entire section would better go ...

> 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: gcfg: address review comments for local jump issue
> 
> ChangeLog:
> 	* gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d
> 	* gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l
> 	* gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s
> ---
>  gas/ginsn.c                                   | 47 +++++++++++--------
>  gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.d    | 32 +++++++++++++
>  gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.l    |  3 ++
>  gas/testsuite/gas/scfi/x86_64/scfi-cfg-3.s    | 43 +++++++++++++++++
>  gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp |  2 +
>  5 files changed, 108 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

... below here, perhaps with another --- separator. This way it's clear
that the revlog isn't going to be part of the commit message.

Jan