public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>
Subject: [PATCH 2/3] x86: parse_register() must not alter the parsed string
Date: Fri, 31 Mar 2023 12:05:53 +0200	[thread overview]
Message-ID: <e0e3ab15-50da-a92a-bcec-e392f09a9c37@suse.com> (raw)
In-Reply-To: <ae00493d-55a2-da0f-1d97-620424c7136b@suse.com>

This reverts the code change done by 100f993c53a5 ("x86: Check
unbalanced braces in memory reference"), which wrongly identified
e87fb6a6d0cd ("x86/gas: support quoted address scale factor in AT&T
syntax") as the root cause of PR gas/30248. (The testcase is left in
place, no matter that it's at best marginally useful in that shape.)

The problem instead is that parse_register() alters the string handed to
it, thus breaking valid assumptions in subsequent parsing code. Since
the function's behavior is a result of get_symbol_name()'s, make a copy
of the incoming string before invoking that function.

Like for parse_real_register() follow the model of strtol() et al: input
string is const-qualified to signal that the string isn't altered, but
the returned "end" pointer is not const-qualified, requiring const to be
cast away (which generally is a bad idea, but the alternative would
again be more convoluted code).

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -159,7 +159,7 @@ static int i386_att_operand (char *);
 static int i386_intel_operand (char *, int);
 static int i386_intel_simplify (expressionS *);
 static int i386_intel_parse_name (const char *, expressionS *);
-static const reg_entry *parse_register (char *, char **);
+static const reg_entry *parse_register (const char *, char **);
 static const char *parse_insn (const char *, char *, bool);
 static char *parse_operands (char *, const char *);
 static void swap_operands (void);
@@ -12497,11 +12497,7 @@ i386_att_operand (char *operand_string)
 	  temp_string = base_string;
 
 	  /* Skip past '(' and whitespace.  */
-	  if (*base_string != '(')
-	    {
-	      as_bad (_("unbalanced braces"));
-	      return 0;
-	    }
+	  gas_assert (*base_string == '(');
 	  ++base_string;
 	  if (is_space_char (*base_string))
 	    ++base_string;
@@ -13818,7 +13814,7 @@ parse_real_register (const char *reg_str
 /* REG_STRING starts *before* REGISTER_PREFIX.  */
 
 static const reg_entry *
-parse_register (char *reg_string, char **end_op)
+parse_register (const char *reg_string, char **end_op)
 {
   const reg_entry *r;
 
@@ -13829,12 +13825,12 @@ parse_register (char *reg_string, char *
   if (!r)
     {
       char *save = input_line_pointer;
-      char c;
+      char *buf = xstrdup (reg_string), *name;
       symbolS *symbolP;
 
-      input_line_pointer = reg_string;
-      c = get_symbol_name (&reg_string);
-      symbolP = symbol_find (reg_string);
+      input_line_pointer = buf;
+      get_symbol_name (&name);
+      symbolP = symbol_find (name);
       while (symbolP && S_GET_SEGMENT (symbolP) != reg_section)
 	{
 	  const expressionS *e = symbol_get_value_expression(symbolP);
@@ -13852,7 +13848,7 @@ parse_register (char *reg_string, char *
 	      know (e->X_add_number >= 0
 		    && (valueT) e->X_add_number < i386_regtab_size);
 	      r = i386_regtab + e->X_add_number;
-	      *end_op = input_line_pointer;
+	      *end_op = (char *) reg_string + (input_line_pointer - buf);
 	    }
 	  if (r && !check_register (r))
 	    {
@@ -13861,8 +13857,8 @@ parse_register (char *reg_string, char *
 	      r = &bad_reg;
 	    }
 	}
-      *input_line_pointer = c;
       input_line_pointer = save;
+      free (buf);
     }
   return r;
 }


  parent reply	other threads:[~2023-03-31 10:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31 10:03 [PATCH 0/3] better fix for PR gas/30248 Jan Beulich
2023-03-31 10:04 ` [PATCH 1/3] x86: parse_real_register() does not alter the parsed string Jan Beulich
2023-03-31 10:05 ` Jan Beulich [this message]
2023-03-31 10:07 ` [PATCH 3/3] gas: document that get_symbol_name() can clobber the input buffer Jan Beulich

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=e0e3ab15-50da-a92a-bcec-e392f09a9c37@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=hjl.tools@gmail.com \
    /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).