public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284
@ 2024-01-26  9:19 Indu Bhagat
  2024-01-26  9:19 ` [PATCH,V3 1/2] x86: testsuite: scfi: adjust COFI testcase Indu Bhagat
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Indu Bhagat @ 2024-01-26  9:19 UTC (permalink / raw)
  To: binutils; +Cc: Indu Bhagat

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

 gas/ginsn.c                                   |  4 +-
 gas/testsuite/gas/scfi/x86_64/ginsn-cofi-1.l  | 38 +++++++++++++++++++
 .../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/scfi/x86_64/scfi-unsupported-cfg-1.l  |  2 +-
 gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp |  3 +-
 7 files changed, 42 insertions(+), 17 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

-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [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

end of thread, other threads:[~2024-01-26 11:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH,V3 0/2] Testcase refactoring and fix PR gas/31284 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).