diff mbox series

[COMMITTED] gas: change meaning of ; in the BPF assembler

Message ID 20231128140525.29734-1-jose.marchesi@oracle.com
State Unresolved
Headers
Series [COMMITTED] gas: change meaning of ; in the BPF assembler |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Jose E. Marchesi Nov. 28, 2023, 2:05 p.m. UTC 
  The BPF assembler in clang uses semi-colon (;) to separate statements,
not to be begin line comments.  This patch adapts the GNU assembler
accordingly.

Testsuite and documentation updated accordingly.

2023-11-28  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* config/tc-bpf.c: Semicolon does not start a comment, but
	separates multiple commands on a single line.
	* testsuite/gas/bpf/alu-pseudoc.s: Adapt test accordingly.
	* testsuite/gas/bpf/spacing-pseudoc.s: Likewise.
	* testsuite/gas/bpf/offset16-overflow.s: Likewise.
	* testsuite/gas/bpf/jump-relax-jump.s: Likewise.
	* testsuite/gas/bpf/jump-relax-ja.s: Likewise.
	* testsuite/gas/bpf/imm32-overflow.s: Likewise.
	* testsuite/gas/bpf/disp32-overflow.s: Likewise.
	* testsuite/gas/bpf/disp16-overflow-relax.s: Likewise.
	* testsuite/gas/bpf/disp16-overflow.s: Likewise.
	* doc/c-bpf.texi (BPF Special Characters): Update.
---
 gas/ChangeLog                                 | 15 +++++++++++++++
 gas/config/tc-bpf.c                           |  4 ++--
 gas/doc/c-bpf.texi                            | 13 ++++++++++---
 gas/testsuite/gas/bpf/alu-pseudoc.s           |  2 +-
 gas/testsuite/gas/bpf/disp16-overflow-relax.s |  8 ++++----
 gas/testsuite/gas/bpf/disp16-overflow.s       |  4 ++--
 gas/testsuite/gas/bpf/disp32-overflow.s       |  4 ++--
 gas/testsuite/gas/bpf/imm32-overflow.s        |  4 ++--
 gas/testsuite/gas/bpf/jump-relax-ja.s         | 18 +++++++++---------
 gas/testsuite/gas/bpf/jump-relax-jump.s       | 12 ++++++------
 gas/testsuite/gas/bpf/offset16-overflow.s     |  4 ++--
 gas/testsuite/gas/bpf/spacing-pseudoc.s       |  4 ++--
 12 files changed, 57 insertions(+), 35 deletions(-)

Comments

Jan Beulich Nov. 28, 2023, 2:58 p.m. UTC | #1 
On 28.11.2023 15:05, Jose E. Marchesi wrote:
> The BPF assembler in clang uses semi-colon (;) to separate statements,
> not to be begin line comments.  This patch adapts the GNU assembler
> accordingly.
> 
> Testsuite and documentation updated accordingly.

Isn't this a behavioral change which also warrants a NEWS entry?

Jan
Jose E. Marchesi Nov. 28, 2023, 3:27 p.m. UTC | #2 
> On 28.11.2023 15:05, Jose E. Marchesi wrote:
>> The BPF assembler in clang uses semi-colon (;) to separate statements,
>> not to be begin line comments.  This patch adapts the GNU assembler
>> accordingly.
>> 
>> Testsuite and documentation updated accordingly.
>
> Isn't this a behavioral change which also warrants a NEWS entry?

What about this:

diff --git a/gas/NEWS b/gas/NEWS
index 643f0e6700e..6bfab08c5b8 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -37,6 +37,10 @@
 * Add support for various T-Head extensions (XTheadVector, XTheadZvlsseg
   and XTheadZvamo) from version 2.3.0 of the T-Head ISA manual.
 
+* The BPF assembler now uses semi-colon (;) to separate statements, and
+  therefore they cannot longer be used to begin line comments. This matches the
+  behavior of the clang/LLVM BPF assembler.
+
 Changes in 2.41:
 
 * Add support for the KVX instruction set.
Jan Beulich Nov. 28, 2023, 4:23 p.m. UTC | #3 
On 28.11.2023 16:27, Jose E. Marchesi wrote:
> 
>> On 28.11.2023 15:05, Jose E. Marchesi wrote:
>>> The BPF assembler in clang uses semi-colon (;) to separate statements,
>>> not to be begin line comments.  This patch adapts the GNU assembler
>>> accordingly.
>>>
>>> Testsuite and documentation updated accordingly.
>>
>> Isn't this a behavioral change which also warrants a NEWS entry?
> 
> What about this:

Reads okay to me, thanks.

Jan

> diff --git a/gas/NEWS b/gas/NEWS
> index 643f0e6700e..6bfab08c5b8 100644
> --- a/gas/NEWS
> +++ b/gas/NEWS
> @@ -37,6 +37,10 @@
>  * Add support for various T-Head extensions (XTheadVector, XTheadZvlsseg
>    and XTheadZvamo) from version 2.3.0 of the T-Head ISA manual.
>  
> +* The BPF assembler now uses semi-colon (;) to separate statements, and
> +  therefore they cannot longer be used to begin line comments. This matches the
> +  behavior of the clang/LLVM BPF assembler.
> +
>  Changes in 2.41:
>  
>  * Add support for the KVX instruction set.
Jose E. Marchesi Nov. 28, 2023, 4:53 p.m. UTC | #4 
> On 28.11.2023 16:27, Jose E. Marchesi wrote:
>> 
>>> On 28.11.2023 15:05, Jose E. Marchesi wrote:
>>>> The BPF assembler in clang uses semi-colon (;) to separate statements,
>>>> not to be begin line comments.  This patch adapts the GNU assembler
>>>> accordingly.
>>>>
>>>> Testsuite and documentation updated accordingly.
>>>
>>> Isn't this a behavioral change which also warrants a NEWS entry?
>> 
>> What about this:
>
> Reads okay to me, thanks.

Pushed.

> Jan
>
>> diff --git a/gas/NEWS b/gas/NEWS
>> index 643f0e6700e..6bfab08c5b8 100644
>> --- a/gas/NEWS
>> +++ b/gas/NEWS
>> @@ -37,6 +37,10 @@
>>  * Add support for various T-Head extensions (XTheadVector, XTheadZvlsseg
>>    and XTheadZvamo) from version 2.3.0 of the T-Head ISA manual.
>>  
>> +* The BPF assembler now uses semi-colon (;) to separate statements, and
>> +  therefore they cannot longer be used to begin line comments. This matches the
>> +  behavior of the clang/LLVM BPF assembler.
>> +
>>  Changes in 2.41:
>>  
>>  * Add support for the KVX instruction set.
diff mbox series

Patch

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 9414f7df678..c5e781d275c 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,18 @@ 
+2023-11-28  Jose E. Marchesi  <jose.marchesi@oracle.com>
+
+	* config/tc-bpf.c: Semicolon does not start a comment, but
+	separates multiple commands on a single line.
+	* testsuite/gas/bpf/alu-pseudoc.s: Adapt test accordingly.
+	* testsuite/gas/bpf/spacing-pseudoc.s: Likewise.
+	* testsuite/gas/bpf/offset16-overflow.s: Likewise.
+	* testsuite/gas/bpf/jump-relax-jump.s: Likewise.
+	* testsuite/gas/bpf/jump-relax-ja.s: Likewise.
+	* testsuite/gas/bpf/imm32-overflow.s: Likewise.
+	* testsuite/gas/bpf/disp32-overflow.s: Likewise.
+	* testsuite/gas/bpf/disp16-overflow-relax.s: Likewise.
+	* testsuite/gas/bpf/disp16-overflow.s: Likewise.
+	* doc/c-bpf.texi (BPF Special Characters): Update.
+
 2023-11-23  YunQiang Su  <yunqiang.su@cipunited.com>
 
 	* testsuite/gas/mips/elf-rel.s: Use addiu in place of addi.
diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
index b6b1db47427..00567545de8 100644
--- a/gas/config/tc-bpf.c
+++ b/gas/config/tc-bpf.c
@@ -56,9 +56,9 @@  struct bpf_insn
   expressionS *relaxed_exp;
 };
 
-const char comment_chars[]        = ";#";
+const char comment_chars[]        = "#";
 const char line_comment_chars[]   = "#";
-const char line_separator_chars[] = "`";
+const char line_separator_chars[] = ";`";
 const char EXP_CHARS[]            = "eE";
 const char FLT_CHARS[]            = "fFdD";
 
diff --git a/gas/doc/c-bpf.texi b/gas/doc/c-bpf.texi
index 25ae231d19c..7ff07e91764 100644
--- a/gas/doc/c-bpf.texi
+++ b/gas/doc/c-bpf.texi
@@ -68,11 +68,18 @@  the host endianness is used.
 
 @cindex line comment character, BPF
 @cindex BPF line comment character
-The presence of a @samp{;} or a @samp{#} on a line indicates the start
-of a comment that extends to the end of the current line.
+The presence of a @samp{#} on a line indicates the start of a comment
+that extends to the end of the current line.
+
+@cindex block comments, BPF
+@cindex BPF block comments
+The presence of the @samp{/*} sequence indicates the beginning of a
+block (multi-line) comment, whose contents span until the next
+@samp{*/} sequence.  It is not possible to nest block comments.
 
 @cindex statement separator, BPF
-Statements and assembly directives are separated by newlines.
+Statements and assembly directives are separated by newlines and
+@samp{;} characters.
 
 @node BPF Registers
 @section BPF Registers
diff --git a/gas/testsuite/gas/bpf/alu-pseudoc.s b/gas/testsuite/gas/bpf/alu-pseudoc.s
index 3d60d4f7a77..04d2b76e3f7 100644
--- a/gas/testsuite/gas/bpf/alu-pseudoc.s
+++ b/gas/testsuite/gas/bpf/alu-pseudoc.s
@@ -61,5 +61,5 @@ 
         r1 = bswap16 r1
         r2 = bswap32 r2
         r3 = bswap64 r3
-        ;; The following is to be interpreted as a mov, not lddw.
+        /* The following is to be interpreted as a mov, not lddw. */
         r2 = 666ll
diff --git a/gas/testsuite/gas/bpf/disp16-overflow-relax.s b/gas/testsuite/gas/bpf/disp16-overflow-relax.s
index 05d505caaae..7ac969a854a 100644
--- a/gas/testsuite/gas/bpf/disp16-overflow-relax.s
+++ b/gas/testsuite/gas/bpf/disp16-overflow-relax.s
@@ -1,4 +1,4 @@ 
-        jeq %r1,%r2,2147483647  ; Overflows.
-        jlt %r3,%r4,2147483648  ; Overflows.
-        jge %r5,10,-2147483648  ; Overflows.
-        ja -2147483649          ; Overflows.
+        jeq %r1,%r2,2147483647  /* Overflows. */
+        jlt %r3,%r4,2147483648  /* Overflows. */
+        jge %r5,10,-2147483648  /* Overflows. */
+        ja -2147483649          /* Overflows. */
diff --git a/gas/testsuite/gas/bpf/disp16-overflow.s b/gas/testsuite/gas/bpf/disp16-overflow.s
index 4a8fd9fcf1c..c46590e4959 100644
--- a/gas/testsuite/gas/bpf/disp16-overflow.s
+++ b/gas/testsuite/gas/bpf/disp16-overflow.s
@@ -1,4 +1,4 @@ 
         ja 32767
-        jeq %r1,%r2,65536       ; Overflows
+        jeq %r1,%r2,65536       /* Overflows */
         jlt %r3,%r4,-32768
-        jge %r5,10,-32769       ; Overflows
+        jge %r5,10,-32769       /* Overflows */
diff --git a/gas/testsuite/gas/bpf/disp32-overflow.s b/gas/testsuite/gas/bpf/disp32-overflow.s
index 11128a20dff..eb8b445588b 100644
--- a/gas/testsuite/gas/bpf/disp32-overflow.s
+++ b/gas/testsuite/gas/bpf/disp32-overflow.s
@@ -1,4 +1,4 @@ 
         call -2147483648
-        call -2147483649        ; This overflows.
+        call -2147483649        /* This overflows.  */
         call 4294967295
-        call 4294967296         ; This overflows.
+        call 4294967296         /* This overflows.  */
diff --git a/gas/testsuite/gas/bpf/imm32-overflow.s b/gas/testsuite/gas/bpf/imm32-overflow.s
index b2ab43d2e7a..1aac58bd0ea 100644
--- a/gas/testsuite/gas/bpf/imm32-overflow.s
+++ b/gas/testsuite/gas/bpf/imm32-overflow.s
@@ -1,4 +1,4 @@ 
         add %r1, 2147483647
-        or %r2, 4294967296         ; This overflows.
+        or %r2, 4294967296         /* This overflows.  */
         xor %r3, 4294967295
-        sub %r4, 4294967296        ; This overflows.
+        sub %r4, 4294967296        /* This overflows.  */
diff --git a/gas/testsuite/gas/bpf/jump-relax-ja.s b/gas/testsuite/gas/bpf/jump-relax-ja.s
index 1faf67909eb..f164176bbd5 100644
--- a/gas/testsuite/gas/bpf/jump-relax-ja.s
+++ b/gas/testsuite/gas/bpf/jump-relax-ja.s
@@ -1,16 +1,16 @@ 
-        ;; The following two instructions have constant targets that
-        ;; fix in the JA 16-bit signed displacement operand.  These
-        ;; are not relaxed.
+        /* The following two instructions have constant targets that
+           fix in the JA 16-bit signed displacement operand.  These
+           are not relaxed.  */
 1:      ja -32768
         ja 32767
-        ;; The following instruction refers to a defined symbol that
-        ;; is on reach, so it should not be relaxed.
+        /* The following instruction refers to a defined symbol that
+           is on reach, so it should not be relaxed.  */
         ja 1b
-        ;; The following instruction has an undefined symbol as a
-        ;; target.  It is not to be relaxed.
+        /* The following instruction has an undefined symbol as a
+           target.  It is not to be relaxed.  */
         ja undefined + 10
-        ;; The following instructions refer to a defined symbol that
-        ;; is not on reach.  They shall be relaxed to a JAL.
+        /* The following instructions refer to a defined symbol that
+           is not on reach.  They shall be relaxed to a JAL.  */
         ja tail
         tail = .text + 262160
         ja tail
diff --git a/gas/testsuite/gas/bpf/jump-relax-jump.s b/gas/testsuite/gas/bpf/jump-relax-jump.s
index 3ee7c873320..5ea61109bca 100644
--- a/gas/testsuite/gas/bpf/jump-relax-jump.s
+++ b/gas/testsuite/gas/bpf/jump-relax-jump.s
@@ -1,12 +1,12 @@ 
-        ;; The following two instructions have constant targets that
-        ;; fix in the jump 16-bit signed displacement operand.
+        /* The following two instructions have constant targets that
+           fix in the jump 16-bit signed displacement operand.  */
 1:      jeq %r1, %r2, -32768
         jlt %r1, %r2, 32767
-        ;; The following instruction refers to a defined symbol that
-        ;; is on reach, so it should not be relaxed.
+        /* The following instruction refers to a defined symbol that
+           is on reach, so it should not be relaxed.  */
         jle %r1, %r2, 1b
-        ;; The following instructions refer to a defined symbol that
-        ;; is not on reach.  They shall be relaxed.
+        /* The following instructions refer to a defined symbol that
+           is not on reach.  They shall be relaxed.  */
         jeq %r1, %r2, tail
         tail = .text + 262160
         jgt %r1, %r2, tail
diff --git a/gas/testsuite/gas/bpf/offset16-overflow.s b/gas/testsuite/gas/bpf/offset16-overflow.s
index ebd8e05c09e..2bcb63baf8d 100644
--- a/gas/testsuite/gas/bpf/offset16-overflow.s
+++ b/gas/testsuite/gas/bpf/offset16-overflow.s
@@ -1,4 +1,4 @@ 
         ldxh %r2, [%r1 + 65535]
-        ldxw %r2, [%r1 + 65536]  ; This overflows
+        ldxw %r2, [%r1 + 65536]  /* This overflows.  */
         stxw [%r2 - 32768], %r1
-        stxdw [%r2 - 32769], %r1  ; This overflows
+        stxdw [%r2 - 32769], %r1  /* This overflows.  */
diff --git a/gas/testsuite/gas/bpf/spacing-pseudoc.s b/gas/testsuite/gas/bpf/spacing-pseudoc.s
index 3c19d9a5073..5aff93dcf33 100644
--- a/gas/testsuite/gas/bpf/spacing-pseudoc.s
+++ b/gas/testsuite/gas/bpf/spacing-pseudoc.s
@@ -1,5 +1,5 @@ 
-        ;; This test checks that flexible spacing is supported in the
-        ;; pseudoc syntax.
+        /* This test checks that flexible spacing is supported in the
+           pseudoc syntax.  */
         r4 = 0xdeadbeefll
         r4 = 0xdeadbeef ll
         goto +1