[PATCH] gas: x86: ginsn: handle sub-QWORD ALU with imm and MOV ops correctly

[Jan Beulich]

On 02.02.2024 08:25, Indu Bhagat wrote:
> PR gas/31326
> SCFI must handle non QWORD ALU with imm and MOV ops correctly
> 
> As per the x86 ISA manual:
>   - 32-bit operands generate a 32-bit result, zero-extended to a 64-bit
>     result in the destination general-purpose register.
>   - 8-bit and 16-bit operands generate an 8-bit or 16-bit result. The
>     upper 56 bits or 48 bits (respectively) of the destination
>     general-purpose register are not modified by the operation.
> 
> Unlike previously thought, sub-QWORD ALU/imm and MOV ops do have
> implications on SCFI.  SCFI/ginsn machinery does not track operation size
> in the ginsn representation.  But given that these sub-QWORD ops update
> only a portion of a 64-bit destination register, for SCFI purposes, this
> needs to be deemed as an untraceable update (when the destination is
> REG_SP / REG_FP). Although in most cases, sub-QWORD ops are not expected
> for stack management, but the SCFI machinery must behave correctly, when
> such ops are indeed present.
> 
> As mentioned earlier, ginsn representation does not carry operation size
> information.  To resolve the issue raised in PR gas/31326, an option is
> to force the generation of GINSN_TYPE_OTHER for all cases when there is
> a 8/16/32 bit op.  But this may dilute the utility of ginsn for other
> use-cases, when they pop up in future.
> 
> The current approach is less disruptive than above in that it generates
> GINSN_TYPE_OTHER for all cases only when:
>   - there is a 8/16/32 bit op, and
>   - the 64-bit op is otherwise traceable.
> 
> In other words this means:
>  - For add/sub ops where dest is reg and src is reg/mem: these always
>    make dest reg untraceable; So, the current handling is unchanged.  We
>    simply skip detecting 8/16/32-bit ops.
>  - An x86 pop instruction is translated to a load ginsn followed by a stack
>    increment add op.  A load op always makes dest reg untraceable.
>    Hence, if the pop instruction is sub-QWORD, we continue to (skip
>    detecting 8/16/32-bit op, and) generate the load instruction as usual.
>    This means that if input asm does have save and restore of unequal sized
>    registers, gas/SCFI will not detect nor warn.
>  - For ALU imm or MOV reg,reg, however, a GINSN_TYPE_OTHER is generated
>    when a 8/16/32-bit op is seen.
> 
> gas/
> 	PR gas/31326
> 	* config/tc-i386.c (x86_ginsn_addsub_reg_mem): Add a code
> 	comment.
> 	(x86_ginsn_addsub_mem_reg): Likewise.
> 	(x86_ginsn_alu_imm): Detect sub-QWORD opsize and exit early.
> 	(x86_ginsn_move): Likewise.
> 	(x86_ginsn_new): Add comment for 8-bit add/sub opcodes (in
>         opcode_space SPACE_BASE) about skipped handling.
> 
> gas/testsuite/:
> 	PR gas/31326
> 	* gas/scfi/x86_64/ginsn-add-1.l: Update.
> 	* gas/scfi/x86_64/ginsn-add-1.s: Add some sub-QWORD add ops.
> 	* gas/scfi/x86_64/ginsn-dw2-regnum-1.l: Update.
> 	* gas/scfi/x86_64/ginsn-dw2-regnum-1.s: Use mov ops instead of
> 	add to invoke and test the ginsn_dw2_regnum code path.

While I'm not fully convinced this moves us in the right direction, it
still looks okay to me, so feel free to put in. Going forward I may then
be proven wrong, once the whole thing ends up more complete and not
experimental anymore.

Jan