* [PATCH,V3 1/2] x86: testsuite: scfi: adjust COFI testcase
2024-01-26 9:19 [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 Indu Bhagat
@ 2024-01-26 9:19 ` Indu Bhagat
2024-01-26 9:19 ` [PATCH,V3 2/2] gas: scfi: untraceable control flow should be a hard error Indu Bhagat
2024-01-26 11:57 ` [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 Jan Beulich
2 siblings, 0 replies; 4+ messages in thread
From: Indu Bhagat @ 2024-01-26 9:19 UTC (permalink / raw)
To: binutils; +Cc: Indu Bhagat
[No change from V2]
[V1 - Non existent. Sent first patch series tagged as V2 by mistake.]
The testcase for change of flow instructions in its current shape is not
doing much: it checks that SCFI issues an appropriate warning. The same
warning is covered by another testcase (scfi-unsupported-cfg-1); It is
better to test the ginsn translation instead, for these 'change of flow
instructions'.
gas/testsuite/
* gas/scfi/x86_64/scfi-cofi-1.s: Moved to...
* gas/scfi/x86_64/ginsn-cofi-1.s: ...here.
* gas/scfi/x86_64/scfi-x86-64.exp: Adjust tests.
* gas/scfi/x86_64/scfi-cofi-1.d: Removed.
* gas/scfi/x86_64/scfi-cofi-1.l: Removed.
* gas/scfi/x86_64/ginsn-cofi-1.l: New test.
---
gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l | 36 +++++++++++++++++++
.../x86_64/{scfi-cofi-1.s => ginsn-cofi-1.s} | 4 ---
gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d | 5 ---
gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l | 3 --
gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp | 3 +-
5 files changed, 37 insertions(+), 14 deletions(-)
create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
rename gas/testsuite/gas/scfi/x86_64/{scfi-cofi-1.s => ginsn-cofi-1.s} (84%)
delete mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d
delete mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
new file mode 100644
index 00000000000..fee76f9cc9b
--- /dev/null
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
@@ -0,0 +1,36 @@
+GAS LISTING .*
+
+
+ 1 # Testcase with a variety of "change of flow instructions"
+ 2 #
+ 3 # This test does not have much going on wrt synthesis of CFI;
+ 4 # it just aims to ensure x8_64 -> ginsn decoding behaves
+ 5 # gracefully for these "change of flow instructions"
+ 6 .text
+ 7 .globl foo
+ 8 .type foo, @function
+ 8 ginsn: SYM FUNC_BEGIN
+ 9 foo:
+ 9 ginsn: SYM foo
+ 10 0000 4801D0 addq %rdx, %rax
+ 10 ginsn: ADD %r1, %r0, %r0
+ 11 0003 E200 loop foo
+ 11 ginsn: JCC
+ 12 0005 3EFFE0 notrack jmp \*%rax
+ 12 ginsn: JMP %r0,
+ 13 0008 41FFD0 call \*%r8
+ 13 ginsn: CALL
+ 14 000b 67E305 jecxz .L179
+ 14 ginsn: JCC
+ 15 000e FF6730 jmp \*48\(%rdi\)
+ 15 ginsn: JMP %r5,
+ 16 0011 7000 jo .L179
+ 16 ginsn: JCC
+ 17 .L179:
+ 17 ginsn: SYM .L179
+ 18 0013 C3 ret
+ 18 ginsn: RET
+ 19 .LFE0:
+ 19 ginsn: SYM .LFE0
+ 20 .size foo, .-foo
+ 20 ginsn: SYM FUNC_END
diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.s b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s
similarity index 84%
rename from gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.s
rename to gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s
index 0ea32d4bbe6..0a63910e046 100644
--- a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.s
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.s
@@ -1,7 +1,5 @@
# Testcase with a variety of "change of flow instructions"
#
-# Must be run with -W so it remains warning free.
-#
# This test does not have much going on wrt synthesis of CFI;
# it just aims to ensure x8_64 -> ginsn decoding behaves
# gracefully for these "change of flow instructions"
@@ -9,7 +7,6 @@
.globl foo
.type foo, @function
foo:
- .cfi_startproc
addq %rdx, %rax
loop foo
notrack jmp *%rax
@@ -19,6 +16,5 @@ foo:
jo .L179
.L179:
ret
- .cfi_endproc
.LFE0:
.size foo, .-foo
diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d b/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d
deleted file mode 100644
index 53cc124d860..00000000000
--- a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d
+++ /dev/null
@@ -1,5 +0,0 @@
-#as: --scfi=experimental -W
-#objdump: -Wf
-#name: Synthesize CFI for add insn
-
-#pass
diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l b/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l
deleted file mode 100644
index 61c29da2d9a..00000000000
--- a/gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l
+++ /dev/null
@@ -1,3 +0,0 @@
-.*Assembler messages:
-.*12: Warning: SCFI ignores most user-specified CFI directives
-.*24: Warning: Untraceable control flow for func 'foo'; Skipping SCFI
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 d32cb290d92..9c76974fefe 100644
--- a/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
@@ -29,6 +29,7 @@ if { ([istarget "x86_64-*-*"] && ![istarget "x86_64-*-linux*-gnux32"]) } then {
run_list_test "ginsn-lea-1" "--scfi=experimental -ali"
run_list_test "ginsn-pop-1" "--scfi=experimental -ali"
run_list_test "ginsn-push-1" "--scfi=experimental -ali"
+ run_list_test "ginsn-cofi-1" "--scfi=experimental -ali -W"
run_dump_test "scfi-cfi-label-1"
run_list_test "scfi-cfi-label-1" "--scfi=experimental --warn"
@@ -68,8 +69,6 @@ if { ([istarget "x86_64-*-*"] && ![istarget "x86_64-*-linux*-gnux32"]) } then {
run_dump_test "scfi-pushsection-2"
run_list_test "scfi-pushsection-2" "--scfi=experimental --warn"
- run_dump_test "scfi-cofi-1"
- run_list_test "scfi-cofi-1" "--scfi=experimental --warn"
run_dump_test "scfi-sub-1"
run_list_test "scfi-sub-1" "--scfi=experimental --warn"
run_dump_test "scfi-sub-2"
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH,V3 2/2] gas: scfi: untraceable control flow should be a hard error
2024-01-26 9:19 [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 Indu Bhagat
2024-01-26 9:19 ` [PATCH,V3 1/2] x86: testsuite: scfi: adjust COFI testcase Indu Bhagat
@ 2024-01-26 9:19 ` Indu Bhagat
2024-01-26 11:57 ` [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 Jan Beulich
2 siblings, 0 replies; 4+ messages in thread
From: Indu Bhagat @ 2024-01-26 9:19 UTC (permalink / raw)
To: binutils; +Cc: Indu Bhagat
[Changes from V2]
- Remove string "SCFI:" altogether from the error message
- Adjust the commit message
[End of changes from V2]
[V1 - Non existent. Sent first patch series tagged as V2 by mistake.]
PR gas/31284
Currently, if an indirect jump is seen, GCFG (a CFG of ginsns) cannot be
created, and the SCFI machinery bails out with a warning:
"Warning: Untraceable control flow for func 'foo'; Skipping SCFI"
It is, however, better suited if this is a hard error. Change it to a
hard error. Also change the message to skip mentioning "SCFI", because
the error itself may also useful when ginsns are used for other passes
(distinct from SCFI) involving GCFG, like a pass to detect if there is
unreachable code. Hence, simply say:
"Error: untraceable control flow for func 'foo'"
gas/
PR gas/31284
* ginsn.c (ginsn_data_end): Use as_bad instead of as_warn.
gas/testsuite/
PR gas/31284
* gas/scfi/x86_64/ginsn-cofi-1.l: Adjust to the expected output
in case of errors.
* gas/scfi/x86_64/scfi-unsupported-cfg-1.l: Error not Warning.
---
gas/ginsn.c | 4 ++--
gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l | 18 ++++++++++--------
.../gas/scfi/x86_64/scfi-unsupported-cfg-1.l | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/gas/ginsn.c b/gas/ginsn.c
index 5f6a67ce4f2..661f51d23c5 100644
--- a/gas/ginsn.c
+++ b/gas/ginsn.c
@@ -1161,8 +1161,8 @@ ginsn_data_end (const symbolS *label)
/* Build the cfg of ginsn(s) of the function. */
if (!frchain_now->frch_ginsn_data->gcfg_apt_p)
{
- as_warn (_("Untraceable control flow for func '%s'; Skipping SCFI"),
- S_GET_NAME (func));
+ as_bad (_("untraceable control flow for func '%s'"),
+ S_GET_NAME (func));
goto end;
}
diff --git a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
index fee76f9cc9b..ab6b50d47e8 100644
--- a/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
+++ b/gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l
@@ -1,3 +1,5 @@
+.*: Assembler messages:
+.*:20: Error: untraceable control flow for func 'foo'
GAS LISTING .*
@@ -12,23 +14,23 @@ GAS LISTING .*
8 ginsn: SYM FUNC_BEGIN
9 foo:
9 ginsn: SYM foo
- 10 0000 4801D0 addq %rdx, %rax
+ 10 \?\?\?\? 4801D0 addq %rdx, %rax
10 ginsn: ADD %r1, %r0, %r0
- 11 0003 E200 loop foo
+ 11 \?\?\?\? E200 loop foo
11 ginsn: JCC
- 12 0005 3EFFE0 notrack jmp \*%rax
+ 12 \?\?\?\? 3EFFE0 notrack jmp \*%rax
12 ginsn: JMP %r0,
- 13 0008 41FFD0 call \*%r8
+ 13 \?\?\?\? 41FFD0 call \*%r8
13 ginsn: CALL
- 14 000b 67E305 jecxz .L179
+ 14 \?\?\?\? 67E305 jecxz .L179
14 ginsn: JCC
- 15 000e FF6730 jmp \*48\(%rdi\)
+ 15 \?\?\?\? FF6730 jmp \*48\(%rdi\)
15 ginsn: JMP %r5,
- 16 0011 7000 jo .L179
+ 16 \?\?\?\? 7000 jo .L179
16 ginsn: JCC
17 .L179:
17 ginsn: SYM .L179
- 18 0013 C3 ret
+ 18 \?\?\?\? C3 ret
18 ginsn: RET
19 .LFE0:
19 ginsn: SYM .LFE0
diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.l b/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.l
index 1e138a102fe..c59ba93df45 100644
--- a/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.l
+++ b/gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.l
@@ -1,3 +1,3 @@
.*Assembler messages:
.*50: Warning: SCFI ignores most user-specified CFI directives
-.*52: Warning: Untraceable control flow for func 'foo'; Skipping SCFI
+.*52: Error: untraceable control flow for func 'foo'
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284
2024-01-26 9:19 [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 Indu Bhagat
2024-01-26 9:19 ` [PATCH,V3 1/2] x86: testsuite: scfi: adjust COFI testcase Indu Bhagat
2024-01-26 9:19 ` [PATCH,V3 2/2] gas: scfi: untraceable control flow should be a hard error Indu Bhagat
@ 2024-01-26 11:57 ` Jan Beulich
2 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2024-01-26 11:57 UTC (permalink / raw)
To: Indu Bhagat; +Cc: binutils
On 26.01.2024 10:19, Indu Bhagat wrote:
> Hi,
>
> Sending V3 after addressing review comments on "PATCH 2/2 gas: scfi:
> untraceable control flow should be a hard error".
>
> I sent the first version of the series erroneously as V2.
> https://sourceware.org/pipermail/binutils/2024-January/132085.html
>
> Once approved, OK to apply to 2.42 as well ?
>
> Thanks,
> Indu Bhagat (2):
> x86: testsuite: scfi: adjust COFI testcase
> gas: scfi: untraceable control flow should be a hard error
Okay.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread