[PATCH v2] x86/Intel: avoid infinite recursion in i386_intel_simplify_symbol() (again)

Alan Modra amodra@gmail.com
Wed May 20 09:22:48 GMT 2026


On Wed, May 20, 2026 at 08:50:39AM +0200, Jan Beulich wrote:
> On 20.05.2026 01:44, Alan Modra wrote:
> > I poked at this problem a little yesterday, and came up with the
> > following patch that passes for all the testcases I threw at it.
> > 
> > ===
> > Commit 22f8905d9f38 reintroduced pr30308.  This fix resolves equates
> > safely in the presence of symbol loops by using symbol_equated_to.
> > Symbols aren't prematurely modified by copying their value
> > expressions before i386_intel_simplify does its work.  I figure that
> > it is safer to do this for all symbols rather than just the particular
> > case of equates, so the in_equate parameter can disappear.
> 
> This of course is a very nice (side-)effect of the change. I was worried
> of copying too many symbols, but ...
> 
> > --- a/gas/config/tc-i386-intel.c
> > +++ b/gas/config/tc-i386-intel.c
> > @@ -381,49 +381,38 @@ i386_intel_simplify_register (expressionS *e)
> >  }
> >  
> >  static symbolS *
> > -i386_intel_simplify_symbol (symbolS *sym, bool in_equate)
> > +i386_intel_simplify_symbol (symbolS *sym)
> >  {
> > -  if (symbol_resolving_p (sym))
> > -    return sym;
> > +  symbolS *orig = sym;
> > +  offsetT off;
> > +  sym = symbol_equated_to (sym, &off);
> > +  if (sym == NULL || off != 0)
> > +    return orig;
> >  
> >    segT seg = S_GET_SEGMENT (sym);
> > -  if (seg != expr_section && seg != reg_section && !symbol_equated_p(sym))
> > +  if (seg == undefined_section
> > +      || (seg != expr_section && seg != reg_section && sym == orig))
> >      return sym;
> >  
> > -  for (;;)
> > -    {
> > -      /* While we're after equates, symbol_equated_p() isn't suitable here.  */
> > -      if (symbol_on_chain(sym, symbol_rootP, symbol_lastP))
> > -	{
> > -	  in_equate = true;
> > -	  sym = symbol_clone (sym, 0);
> > -	}
> > -      else if (in_equate)
> > -	{
> > -	  expressionS *e = symbol_get_value_expression (sym);
> > -
> > -	  if (e->X_op == O_symbol && !e->X_add_number)
> > -	    {
> > -	      sym = e->X_add_symbol;
> > -	      continue;
> > -	    }
> > -	  sym = make_expr_symbol (e);
> > -	}
> > -
> > -      break;
> > -    }
> > -
> > -  symbol_mark_resolving (sym);
> > -  int ret = i386_intel_simplify (symbol_get_value_expression (sym), in_equate);
> > -  if (ret == 2)
> > -    S_SET_SEGMENT (sym, absolute_section);
> > -  symbol_clear_resolving (sym);
> > +  /* i386_intel_simplify modifies its arg.  We don't want to make
> > +     premature changes to symbols here, particularly for a symbol
> > +     equate.  Changing a symbol may affect future uses of that
> > +     symbol.  So copy the symbol value and make a new symbol after
> > +     i386_intel_simplify has done its work.  */
> > +  expressionS *e = symbol_get_value_expression (sym);
> > +  expressionS exp;
> > +  memcpy (&exp, e, sizeof exp);
> > +  int ret = i386_intel_simplify (&exp);
> > +  if (ret == 0)
> > +    return NULL;
> >  
> > -  return ret ? sym : NULL;
> > +  if (memcmp (&exp, e, sizeof exp))
> > +    sym = make_expr_symbol (&exp);
> 
> ... this way you limit things as well. Let's go with your patch, and thank
> you very much to looking into this.
> 
> One further remark lightly related to H-P's: I was first tempted to suggest
> to replace the memcpy() by structure assignment.

I wrote it that way at first..

> Yet then I realized that
> this would be at risk of breaking the use of memcmp(). Perhaps the comment
> could be extended to briefly mention this, to reduce the risk of someone
> trying to replace the memcpy()?

Done.

-- 
Alan Modra


More information about the Binutils mailing list