[binutils-gdb] gas: correctly deal with line comments when not preprocessing

Jan Beulich jbeulich@sourceware.org
Fri Aug 2 07:45:03 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3615e5db8d07da71110143ee37f604235dd088a6

commit 3615e5db8d07da71110143ee37f604235dd088a6
Author: Jan Beulich <jbeulich@suse.com>
Date:   Fri Aug 2 09:43:58 2024 +0200

    gas: correctly deal with line comments when not preprocessing
    
    Internal naming of functions / data as well as commentary mixes lines
    and statements. It is presumably this confusion which has led to the
    wrong use of ignore_rest_of_line() when dealing with line comments in
    read_a_source_file(). We shall not (silently) produce different output
    depending on whether -f is passed (for suitable input).
    
    Introduce two new helper macros, intended to be used in favor of open-
    coded accesses to is_end_of_line[]. To emphasize the difference, convert
    ignore_rest_of_line() right away, including adjustments to its comments.
    
    Since most targets have # in line_comment_chars[], add a target-
    independent test for that, plus an x86-only one also checking for non-#
    to work as intended.

Diff:
---
 gas/read.c                            | 13 ++++++++-----
 gas/read.h                            |  8 ++++++++
 gas/testsuite/gas/all/gas.exp         |  4 ++++
 gas/testsuite/gas/all/line-comment.d  | 12 ++++++++++++
 gas/testsuite/gas/all/line-comment.s  |  4 ++++
 gas/testsuite/gas/i386/i386.exp       |  1 +
 gas/testsuite/gas/i386/line-comment.d | 13 +++++++++++++
 gas/testsuite/gas/i386/line-comment.s |  6 ++++++
 8 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/gas/read.c b/gas/read.c
index 9a26a9a5b7a..adb4759a96b 100644
--- a/gas/read.c
+++ b/gas/read.c
@@ -1324,8 +1324,11 @@ read_a_source_file (const char *name)
 	      s = input_line_pointer;
 	      if (!startswith (s, "APP\n"))
 		{
-		  /* We ignore it.  */
-		  ignore_rest_of_line ();
+		  /* We ignore it.  Note: Not ignore_rest_of_line ()!  */
+		  while (s <= buffer_limit)
+		    if (is_end_of_line (*s++))
+		      break;
+		  input_line_pointer = s;
 		  continue;
 		}
 	      bump_line_counters ();
@@ -3974,7 +3977,7 @@ demand_empty_rest_of_line (void)
   /* Return pointing just after end-of-line.  */
 }
 
-/* Silently advance to the end of line.  Use this after already having
+/* Silently advance to the end of a statement.  Use this after already having
    issued an error about something bad.  Like demand_empty_rest_of_line,
    this function may leave input_line_pointer one after buffer_limit;
    Don't call it from within expression parsing code in an attempt to
@@ -3984,9 +3987,9 @@ void
 ignore_rest_of_line (void)
 {
   while (input_line_pointer <= buffer_limit)
-    if (is_end_of_line[(unsigned char) *input_line_pointer++])
+    if (is_end_of_stmt (*input_line_pointer++))
       break;
-  /* Return pointing just after end-of-line.  */
+  /* Return pointing just after end-of-statement.  */
 }
 
 /* Sets frag for given symbol to zero_address_frag, except when the
diff --git a/gas/read.h b/gas/read.h
index 2441d560bbd..3cdd408168f 100644
--- a/gas/read.h
+++ b/gas/read.h
@@ -58,6 +58,14 @@ extern bool input_from_string;
 #define is_name_ender(c) \
   ( lex_type[(unsigned char) (c)] & LEX_END_NAME   )
 
+/* The distinction of "line" and "statement" sadly is blurred by unhelpful
+   naming of e.g. the underlying array.  Most users really mean "end of
+   statement".  Going forward only these wrappers are supposed to be used.  */
+#define is_end_of_stmt(c) \
+  (is_end_of_line[(unsigned char) (c)])
+#define is_end_of_line(c) \
+  (is_end_of_line[(unsigned char) (c)] == 1)
+
 #ifndef is_a_char
 #define CHAR_MASK	(0xff)
 #define NOT_A_CHAR	(CHAR_MASK+1)
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 24376a728d8..d42d8df3ceb 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -490,6 +490,10 @@ if { ![istarget "tic4x*-*-*"] && ![istarget "tic54x*-*-*"] } {
     run_list_test const-1 "-alm"
 }
 
+if { ![istarget "tic30-*-*"] } {
+    run_dump_test "line-comment"
+}
+
 run_list_test macro "-alm"
 
 run_list_test pr20312
diff --git a/gas/testsuite/gas/all/line-comment.d b/gas/testsuite/gas/all/line-comment.d
new file mode 100644
index 00000000000..9531ba800ec
--- /dev/null
+++ b/gas/testsuite/gas/all/line-comment.d
@@ -0,0 +1,12 @@
+#as:
+#as: -f
+#objdump: -dw --disassemble-zeroes
+#name: generic line comment
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <(line_comment|\.text)>:
+ +0+:	.*
+#pass
diff --git a/gas/testsuite/gas/all/line-comment.s b/gas/testsuite/gas/all/line-comment.s
new file mode 100644
index 00000000000..6f74bde81a8
--- /dev/null
+++ b/gas/testsuite/gas/all/line-comment.s
@@ -0,0 +1,4 @@
+	.text
+line_comment:
+	#.warning;$@!|.error
+	.nop
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index 590a55d0c81..3e707b3d0c6 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -106,6 +106,7 @@ if [gas_32_check] then {
     run_list_test "equ-2" "-al"
     run_list_test "equ-bad"
     run_list_test "comments" "-almn"
+    run_dump_test "line-comment"
     run_dump_test "curly"
     run_dump_test "separator"
     run_dump_test "divide"
diff --git a/gas/testsuite/gas/i386/line-comment.d b/gas/testsuite/gas/i386/line-comment.d
new file mode 100644
index 00000000000..4d989a44819
--- /dev/null
+++ b/gas/testsuite/gas/i386/line-comment.d
@@ -0,0 +1,13 @@
+#as:
+#as: -f
+#objdump: -dw
+#name: i386 line comment
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <line_comment>:
+ +[a-f0-9]+:	90[ 	]+nop
+ +[a-f0-9]+:	c3[ 	]+ret
+#pass
diff --git a/gas/testsuite/gas/i386/line-comment.s b/gas/testsuite/gas/i386/line-comment.s
new file mode 100644
index 00000000000..e0d6e9ac12a
--- /dev/null
+++ b/gas/testsuite/gas/i386/line-comment.s
@@ -0,0 +1,6 @@
+	.text
+line_comment:
+	nop
+	#aaa;aas
+	/aad;aam
+	ret


More information about the Binutils-cvs mailing list