public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alex Coplan <alex.coplan@arm.com>
To: binutils@sourceware.org
Subject: [PATCH] gas: Fix internal error on long local labels
Date: Wed, 5 Aug 2020 14:51:05 +0100	[thread overview]
Message-ID: <20200805135105.vtbqtd5lqcdrqzkm@arm.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

Hello,

Currently, on an input such as "88888888888:", GAS hits a signed integer
overflow and (likely) an assertion failure. I see:

$ echo "88888888888:" | bin/aarch64-none-elf-as
{standard input}: Assembler messages:
{standard input}:1: Internal error in fb_label_name at ../gas/symbols.c:2049.
Please report this bug.

To fix this issue, I've taken two steps:

1. Change the type used for processing local labels in
   read_a_source_file() from int to long, to allow representing more
   local labels, and also since all uses of this variable (temp) are
   actually of type long.

2. Detect if we would overflow and bail out with an error message
   instead of actually overflowing and hitting the assertion in
   fb_label_name().

Testing:
 * Regression tested on a broad selection of targets.

OK for master?

Thanks,
Alex

---

gas/ChangeLog:

2020-08-05  Alex Coplan  <alex.coplan@arm.com>

	* read.c (read_a_source_file): Use long for local labels, detect
	overflow and raise an error for overly-long labels.
	* testsuite/gas/all/gas.exp: Add local-label-overflow test.
	* testsuite/gas/all/local-label-overflow.d: New test.
	* testsuite/gas/all/local-label-overflow.l: Error output.
	* testsuite/gas/all/local-label-overflow.s: Input.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2891 bytes --]

diff --git a/gas/read.c b/gas/read.c
index f192cc16d57..64021e62a40 100644
--- a/gas/read.c
+++ b/gas/read.c
@@ -40,6 +40,8 @@
 #include "dw2gencfi.h"
 #include "wchar.h"
 
+#include <limits.h>
+
 #ifndef TC_START_LABEL
 #define TC_START_LABEL(STR, NUL_CHAR, NEXT_CHAR) (NEXT_CHAR == ':')
 #endif
@@ -816,7 +818,7 @@ read_a_source_file (const char *name)
   char nul_char;
   char next_char;
   char *s;		/* String of symbol, '\0' appended.  */
-  int temp;
+  long temp;
   pseudo_typeS *pop;
 
 #ifdef WARN_COMMENTS
@@ -1212,10 +1214,21 @@ read_a_source_file (const char *name)
 	      /* Read the whole number.  */
 	      while (ISDIGIT (*input_line_pointer))
 		{
-		  temp = (temp * 10) + *input_line_pointer - '0';
+		  const char digit = *input_line_pointer - '0';
+		  if (temp > ((LONG_MAX - digit) / 10))
+		    {
+		      as_bad (_("local label too large near %s"), backup);
+		      temp = -1;
+		      break;
+		    }
+		  temp = (temp * 10) + digit;
 		  ++input_line_pointer;
 		}
 
+	      /* Overflow: stop processing the label.  */
+	      if (temp == -1)
+		continue;
+
 	      if (LOCAL_LABELS_DOLLAR
 		  && *input_line_pointer == '$'
 		  && *(input_line_pointer + 1) == ':')
@@ -1224,7 +1237,7 @@ read_a_source_file (const char *name)
 
 		  if (dollar_label_defined (temp))
 		    {
-		      as_fatal (_("label \"%d$\" redefined"), temp);
+		      as_fatal (_("label \"%ld$\" redefined"), temp);
 		    }
 
 		  define_dollar_label (temp);
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 5496d0bf7a7..a0158f38dde 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -102,6 +102,7 @@ if { ![istarget "bfin-*-*"] } then {
 }
 gas_test_error "assign-bad.s" "" "== assignment for symbol already set"
 run_dump_test assign-bad-recursive
+run_dump_test local-label-overflow
 
 run_dump_test simple-forward
 run_dump_test forward
diff --git a/gas/testsuite/gas/all/local-label-overflow.d b/gas/testsuite/gas/all/local-label-overflow.d
new file mode 100644
index 00000000000..1786a45743a
--- /dev/null
+++ b/gas/testsuite/gas/all/local-label-overflow.d
@@ -0,0 +1,6 @@
+#source: local-label-overflow.s
+#error_output: local-label-overflow.l
+#skip: hppa*-*-*
+#skip: ia64-*-vms
+#skip: mmix-*-*
+#skip: sh-*-pe
diff --git a/gas/testsuite/gas/all/local-label-overflow.l b/gas/testsuite/gas/all/local-label-overflow.l
new file mode 100644
index 00000000000..be24355bbd4
--- /dev/null
+++ b/gas/testsuite/gas/all/local-label-overflow.l
@@ -0,0 +1,2 @@
+[^:]*: Assembler messages:
+.*: Error: local label too large near 888888888888888888888888888:
diff --git a/gas/testsuite/gas/all/local-label-overflow.s b/gas/testsuite/gas/all/local-label-overflow.s
new file mode 100644
index 00000000000..e5569217f44
--- /dev/null
+++ b/gas/testsuite/gas/all/local-label-overflow.s
@@ -0,0 +1 @@
+8888888888888888888888888888:

             reply	other threads:[~2020-08-05 13:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 13:51 Alex Coplan [this message]
2020-08-06 13:56 ` Alan Modra
2020-08-06 16:48   ` Alex Coplan
2020-08-07 13:44     ` H.J. Lu
2020-08-17 17:22       ` Alex Coplan
2020-08-17 17:34         ` H.J. Lu
2020-09-01 10:43           ` Alex Coplan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200805135105.vtbqtd5lqcdrqzkm@arm.com \
    --to=alex.coplan@arm.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).