* [PATCH] gas: x86: ginsn: handle sub-QWORD ALU with imm and MOV ops correctly
@ 2024-02-02 7:25 Indu Bhagat
2024-02-05 10:02 ` Jan Beulich
0 siblings, 1 reply; 2+ messages in thread
From: Indu Bhagat @ 2024-02-02 7:25 UTC (permalink / raw)
To: binutils; +Cc: Indu Bhagat
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.
---
gas/config/tc-i386.c | 23 ++++
gas/testsuite/gas/scfi/x86_64/ginsn-add-1.l | 62 ++++-----
gas/testsuite/gas/scfi/x86_64/ginsn-add-1.s | 2 +
.../gas/scfi/x86_64/ginsn-dw2-regnum-1.l | 122 ++++++++++--------
.../gas/scfi/x86_64/ginsn-dw2-regnum-1.s | 36 ++++--
5 files changed, 149 insertions(+), 96 deletions(-)
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 2e578e2b0d7..b854c08908c 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5454,6 +5454,9 @@ x86_ginsn_addsub_reg_mem (const symbolS *insn_end_sym)
if (i.mem_operands)
return ginsn;
+ /* Skip detection of 8/16/32-bit op size; 'add/sub reg, reg/mem' ops always
+ make the dest reg untraceable for SCFI. */
+
/* op reg, reg/mem. */
src1_dw2_regnum = ginsn_dw2_regnum (i.op[0].regs);
/* Of interest only when second opnd is not memory. */
@@ -5492,6 +5495,9 @@ x86_ginsn_addsub_mem_reg (const symbolS *insn_end_sym)
if (i.mem_operands && !i.base_reg && !i.index_reg)
return ginsn;
+ /* Skip detection of 8/16/32-bit op size; 'add/sub reg/mem, reg' ops always
+ make the dest reg untraceable for SCFI. */
+
/* op reg/mem, %reg. */
dw2_regnum = ginsn_dw2_regnum (i.op[1].regs);
@@ -5572,6 +5578,11 @@ x86_ginsn_alu_imm (const symbolS *insn_end_sym)
if (i.mem_operands == 1)
return ginsn;
+ /* 8/16/32-bit op size makes the destination reg untraceable for SCFI.
+ Deal with this via the x86_ginsn_unhandled () code path. */
+ if (i.suffix != QWORD_MNEM_SUFFIX)
+ return ginsn;
+
gas_assert (i.imm_operands == 1);
src_imm = i.op[0].imms->X_add_number;
/* The second operand may be a register or indirect access. For SCFI, only
@@ -5618,6 +5629,12 @@ x86_ginsn_move (const symbolS *insn_end_sym)
if (i.mem_operands == 1 && !i.base_reg && !i.index_reg)
return ginsn;
+ /* 8/16/32-bit op size makes the destination reg untraceable for SCFI.
+ Handle mov reg, reg only. mov to or from a memory operand will make
+ dest reg, when present, untraceable, irrespective of the op size. */
+ if (i.reg_operands == 2 && i.suffix != QWORD_MNEM_SUFFIX)
+ return ginsn;
+
gas_assert (i.tm.opcode_space == SPACE_BASE);
if (opcode == 0x8b || opcode == 0x8a)
{
@@ -6014,6 +6031,12 @@ x86_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
switch (opcode)
{
+
+ /* Add opcodes 0x0/0x2 and sub opcodes 0x28/0x2a (with opcode_space
+ SPACE_BASE) are 8-bit ops. While they are relevant for SCFI
+ correctness, skip handling them here and use the x86_ginsn_unhandled
+ code path to generate GINSN_TYPE_OTHER when necessary. */
+
case 0x1: /* add reg, reg/mem. */
case 0x29: /* sub reg, reg/mem. */
if (i.tm.opcode_space != SPACE_BASE)
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.l b/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.l
index ff078d4d883..5190e45699d 100644
--- a/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.l
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.l
@@ -12,38 +12,42 @@ GAS LISTING .*
7 0000 54 push %rsp
7 ginsn: SUB %r7, 8, %r7
7 ginsn: STORE %r7, \[%r7\+0\]
- 8 0001 4889E5 movq %rsp, %rbp
- 8 ginsn: MOV %r7, %r6
- 9
- 10 0004 48010425 addq %rax, symbol
- 10 00000000
- 11 000c 03042500 add symbol, %eax
- 11 000000
- 12 0013 670320 add \(%eax\), %esp
- 12 ginsn: ADD \[%r0\+0\], %r7, %r7
- 13 0016 67012405 add %esp, \(,%eax\)
- 13 00000000
- 14 001e 67032405 add \(,%eax\), %esp
+ 8 0001 01C5 add %eax, %ebp
+ 8 ginsn: ADD %r0, %r6, %r6
+ 9 0003 4889E5 movq %rsp, %rbp
+ 9 ginsn: MOV %r7, %r6
+ 10
+ 11 0006 48010425 addq %rax, symbol
+ 11 00000000
+ 12 000e 03042500 add symbol, %eax
+ 12 000000
+ 13 0015 670320 add \(%eax\), %esp
+ 13 ginsn: ADD \[%r0\+0\], %r7, %r7
+ 14 0018 67012405 add %esp, \(,%eax\)
14 00000000
- 14 ginsn: ADD \[%r0\+0\], %r7, %r7
- 15
- 16 0026 4801C3 addq %rax, %rbx
- 16 ginsn: ADD %r0, %r3, %r3
- 17 0029 01C3 add %eax, %ebx
+ 15 0020 67032405 add \(,%eax\), %esp
+ 15 00000000
+ 15 ginsn: ADD \[%r0\+0\], %r7, %r7
+ 16
+ 17 0028 4801C3 addq %rax, %rbx
17 ginsn: ADD %r0, %r3, %r3
- 18
- 19 002b 4883D408 adc \$8, %rsp
+ 18 002b 01C3 add %eax, %ebx
+ 18 ginsn: ADD %r0, %r3, %r3
+ 19 002d 83C408 add \$8, %esp
19 ginsn: OTH 0, 0, %r7
20
- 21 002f 488345F0 addq \$1, -16\(%rbp\)
- 21 01
+ 21 0030 4883D408 adc \$8, %rsp
+ 21 ginsn: OTH 0, 0, %r7
22
- 23 0034 4803D8 \{load\} addq %rax, %rbx
- 23 ginsn: ADD %r0, %r3, %r3
+ 23 0034 488345F0 addq \$1, -16\(%rbp\)
+ 23 01
24
- 25 0037 C3 ret
- 25 ginsn: RET
- 26 .LFE0:
- 26 ginsn: SYM .LFE0
- 27 .size foo, .-foo
- 27 ginsn: SYM FUNC_END
+ 25 0039 4803D8 {load} addq %rax, %rbx
+ 25 ginsn: ADD %r0, %r3, %r3
+ 26
+ 27 003c C3 ret
+ 27 ginsn: RET
+ 28 .LFE0:
+ 28 ginsn: SYM .LFE0
+ 29 .size foo, .-foo
+ 29 ginsn: SYM FUNC_END
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.s b/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.s
index f9398369531..f6bf5a0a5b8 100644
--- a/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.s
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-add-1.s
@@ -5,6 +5,7 @@
.type foo, @function
foo:
push %rsp
+ add %eax, %ebp
movq %rsp, %rbp
addq %rax, symbol
@@ -15,6 +16,7 @@ foo:
addq %rax, %rbx
add %eax, %ebx
+ add $8, %esp
adc $8, %rsp
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.l b/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.l
index f242e1f2171..8a8edd5742d 100644
--- a/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.l
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.l
@@ -1,69 +1,85 @@
GAS LISTING .*
-.*# Testcase for DWARF regnum ginsn API
+ 1 # Testcase for DWARF regnum ginsn API
2 .text
3 .globl foo
4 .type foo, @function
4 ginsn: SYM FUNC_BEGIN
5 foo:
5 ginsn: SYM foo
- 6 0000 0408 add \$8, %al
- 7 0002 80C108 add \$8, %cl
- 7 ginsn: ADD %r2, 8, %r2
- 8 0005 80C208 add \$8, %dl
- 8 ginsn: ADD %r1, 8, %r1
- 9 0008 80C408 add \$8, %ah
- 9 ginsn: ADD %r0, 8, %r0
- 10 000b 80C508 add \$8, %ch
- 10 ginsn: ADD %r2, 8, %r2
- 11 000e 80C608 add \$8, %dh
- 11 ginsn: ADD %r1, 8, %r1
- 12 0011 80C708 add \$8, %bh
- 12 ginsn: ADD %r3, 8, %r3
- 13
- 14 0014 4080C008 add \$8, %axl
- 14 ginsn: ADD %r0, 8, %r0
- 15 0018 4080C408 add \$8, %spl
- 15 ginsn: ADD %r7, 8, %r7
+ 6 0000 55 push %rbp
+ 6 ginsn: SUB %r7, 8, %r7
+ 6 ginsn: STORE %r6, \[%r7\+0\]
+ 7 0001 4889E5 mov %rsp, %rbp
+ 7 ginsn: MOV %r7, %r6
+ 8
+ 9 0004 8A40FE mov -0x2\(%rax\), %al
+ 9 ginsn: MOV \[%r0\+-2\], %r0
+ 10 0007 8A58FE mov -0x2\(%rax\), %bl
+ 10 ginsn: MOV \[%r0\+-2\], %r3
+ 11 000a 8A50FE mov -0x2\(%rax\), %dl
+ 11 ginsn: MOV \[%r0\+-2\], %r1
+ 12 000d 8A60FE mov -0x2\(%rax\), %ah
+ 12 ginsn: MOV \[%r0\+-2\], %r0
+ 13 0010 8A68FE mov -0x2\(%rax\), %ch
+ 13 ginsn: MOV \[%r0\+-2\], %r2
+ 14 0013 8A70FE mov -0x2\(%rax\), %dh
+ 14 ginsn: MOV \[%r0\+-2\], %r1
+ 15 0016 8A78FE mov -0x2\(%rax\), %bh
+ 15 ginsn: MOV \[%r0\+-2\], %r3
16
- 17 001c 6683C008 add \$8, %ax
- 17 ginsn: ADD %r0, 8, %r0
- 18 0020 664183C0 add \$8, %r8w
- 18 08
- 18 ginsn: ADD %r8, 8, %r8
- 19 0025 6683C408 add \$8, %sp
- 19 ginsn: ADD %r7, 8, %r7
- 20
- 21 0029 83C008 add \$8, %eax
- 21 ginsn: ADD %r0, 8, %r0
- 22 002c 4183C008 add \$8, %r8d
- 22 ginsn: ADD %r8, 8, %r8
- 23 0030 81C40040 add \$16384, %esp
- 23 0000
- 23 ginsn: ADD %r7, 16384, %r7
- 24
- 25 0036 4883C508 add \$8, %rbp
- 25 ginsn: ADD %r6, 8, %r6
- 26
- 27 003a 488D05FE lea -0x2\(%rip\), %rax
- 27 FFFFFF
- 27 ginsn: ADD %r4, -2, %r0
- 28 0041 67488905 mov %rax, 0x2\(%eip\)
- 28 02000000
- 28 ginsn: MOV %r0, \[%r4\+2\]
- 29 0049 67488B05 mov -0x2\(%eip\), %rax
- 29 FEFFFFFF
- 29 ginsn: MOV \[%r4\+-2\], %r0
+ 17 0019 408A40FE mov -0x2\(%rax\), %axl
+ 17 ginsn: MOV \[%r0\+-2\], %r0
+ 18 001d 408A60FE mov -0x2\(%rax\), %spl
+ 18 ginsn: MOV \[%r0\+-2\], %r7
+ 19
+ 20 0021 668B40FE mov -0x2\(%rax\), %ax
+ 20 ginsn: MOV \[%r0\+-2\], %r0
+ 21 0025 66448B40 mov -0x2\(%rax\), %r8w
+ 21 FE
+ 21 ginsn: MOV \[%r0\+-2\], %r8
+ 22 002a 668B60FE mov -0x2\(%rax\), %sp
+ 22 ginsn: MOV \[%r0\+-2\], %r7
+ 23
+ 24 002e 4080C408 add \$8, %spl
+ 24 ginsn: OTH 0, 0, %r7
+ 25
+ 26 0032 8B40FE mov -0x2\(%rax\), %eax
+ 26 ginsn: MOV \[%r0\+-2\], %r0
+ 27 0035 448B40FE mov -0x2\(%rax\), %r8d
+ 27 ginsn: MOV \[%r0\+-2\], %r8
+ 28
+ 29 0039 81C40040 add \$16384, %esp
+ 29 0000
+ 29 ginsn: OTH 0, 0, %r7
30
- 31 0051 C3 ret
- 31 ginsn: RET
+ 31 003f 4883C508 add \$8, %rbp
+ 31 ginsn: ADD %r6, 8, %r6
+ 32
+ 33 0043 488D05FE lea -0x2\(%rip\), %rax
\fGAS LISTING .*
- 32 .LFE0:
- 32 ginsn: SYM .LFE0
- 33 .size foo, .-foo
- 33 ginsn: SYM FUNC_END
+ 33 FFFFFF
+ 33 ginsn: ADD %r4, -2, %r0
+ 34 004a 67488905 mov %rax, 0x2\(%eip\)
+ 34 02000000
+ 34 ginsn: MOV %r0, \[%r4\+2\]
+ 35 0052 67488B05 mov -0x2\(%eip\), %rax
+ 35 FEFFFFFF
+ 35 ginsn: MOV \[%r4\+-2\], %r0
+ 36
+ 37 005a 4889EC mov %rbp, %rsp
+ 37 ginsn: MOV %r6, %r7
+ 38 005d 5D pop %rbp
+ 38 ginsn: LOAD \[%r7\+0\], %r6
+ 38 ginsn: ADD %r7, 8, %r7
+ 39 005e C3 ret
+ 39 ginsn: RET
+ 40 .LFE0:
+ 40 ginsn: SYM .LFE0
+ 41 .size foo, .-foo
+ 41 ginsn: SYM FUNC_END
#pass
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.s b/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.s
index 383c60f635b..09bf42e6e19 100644
--- a/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.s
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.s
@@ -3,23 +3,29 @@
.globl foo
.type foo, @function
foo:
- add $8, %al
- add $8, %cl
- add $8, %dl
- add $8, %ah
- add $8, %ch
- add $8, %dh
- add $8, %bh
-
- add $8, %axl
+ push %rbp
+ mov %rsp, %rbp
+
+ mov -0x2(%rax), %al
+ mov -0x2(%rax), %bl
+ mov -0x2(%rax), %dl
+ mov -0x2(%rax), %ah
+ mov -0x2(%rax), %ch
+ mov -0x2(%rax), %dh
+ mov -0x2(%rax), %bh
+
+ mov -0x2(%rax), %axl
+ mov -0x2(%rax), %spl
+
+ mov -0x2(%rax), %ax
+ mov -0x2(%rax), %r8w
+ mov -0x2(%rax), %sp
+
add $8, %spl
- add $8, %ax
- add $8, %r8w
- add $8, %sp
+ mov -0x2(%rax), %eax
+ mov -0x2(%rax), %r8d
- add $8, %eax
- add $8, %r8d
add $16384, %esp
add $8, %rbp
@@ -28,6 +34,8 @@ foo:
mov %rax, 0x2(%eip)
mov -0x2(%eip), %rax
+ mov %rbp, %rsp
+ pop %rbp
ret
.LFE0:
.size foo, .-foo
--
2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] gas: x86: ginsn: handle sub-QWORD ALU with imm and MOV ops correctly
2024-02-02 7:25 [PATCH] gas: x86: ginsn: handle sub-QWORD ALU with imm and MOV ops correctly Indu Bhagat
@ 2024-02-05 10:02 ` Jan Beulich
0 siblings, 0 replies; 2+ messages in thread
From: Jan Beulich @ 2024-02-05 10:02 UTC (permalink / raw)
To: Indu Bhagat; +Cc: binutils
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-02-05 10:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-02 7:25 [PATCH] gas: x86: ginsn: handle sub-QWORD ALU with imm and MOV ops correctly Indu Bhagat
2024-02-05 10:02 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).