PR30308, infinite recursion in i386_intel_simplify

Message ID Zc1osM8Uw97ZVfni@squeak.grove.modra.org
State New
Headers
Series PR30308, infinite recursion in i386_intel_simplify |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Patch is already merged

Commit Message

Alan Modra Feb. 15, 2024, 1:28 a.m. UTC
  This patch exposes the symbol "resolving" flag for use in
i386_intel_simplify, not only preventing infinite recursion on the
testcase in the PR but also more complicated cases like:

 .intel_syntax
 b = a
 a = b
 mov eax, [a]

	PR 30308
	* symbols.c (symbol_mark_resolving, symbol_clear_resolving),
	(symbol_resolving_p): New functions.
	* symbols.h: Declare them.
	* config/tc-i386-intel.c (i386_intel_simplify): Delete forward
	declaration.  Formatting.
	(i386_intel_simplify_symbol): Use resolving flag to prevent
	infinite recursion.
  

Comments

Jan Beulich Feb. 15, 2024, 9:23 a.m. UTC | #1
On 15.02.2024 02:28, Alan Modra wrote:
> This patch exposes the symbol "resolving" flag for use in
> i386_intel_simplify, not only preventing infinite recursion on the
> testcase in the PR but also more complicated cases like:
> 
>  .intel_syntax
>  b = a
>  a = b
>  mov eax, [a]

Thanks for addressing this. I wonder though: Wouldn't such circular
equates better be rejected when they're created?

> +/* Return whether a symbol is being resolved.  */
> +
> +int
> +symbol_resolving_p (symbolS *s)
> +{
> +  return s->flags.resolving;
> +}

Mind me asking that such predicate (and alike) functions, when introduced
anew, take pointer-to-const right away?

Jan
  
Alan Modra Feb. 15, 2024, 10:50 a.m. UTC | #2
On Thu, Feb 15, 2024 at 10:23:16AM +0100, Jan Beulich wrote:
> On 15.02.2024 02:28, Alan Modra wrote:
> > This patch exposes the symbol "resolving" flag for use in
> > i386_intel_simplify, not only preventing infinite recursion on the
> > testcase in the PR but also more complicated cases like:
> > 
> >  .intel_syntax
> >  b = a
> >  a = b
> >  mov eax, [a]
> 
> Thanks for addressing this. I wonder though: Wouldn't such circular
> equates better be rejected when they're created?

I wouldn't be inclined to do that.  If the symbols are unused then
they don't cause any problem.

> > +/* Return whether a symbol is being resolved.  */
> > +
> > +int
> > +symbol_resolving_p (symbolS *s)
> > +{
> > +  return s->flags.resolving;
> > +}
> 
> Mind me asking that such predicate (and alike) functions, when introduced
> anew, take pointer-to-const right away?

Yes, that would have been a good idea.
  
Alan Modra Feb. 15, 2024, 11:31 a.m. UTC | #3
On Thu, Feb 15, 2024 at 09:20:43PM +1030, Alan Modra wrote:
> On Thu, Feb 15, 2024 at 10:23:16AM +0100, Jan Beulich wrote:
> > Mind me asking that such predicate (and alike) functions, when introduced
> > anew, take pointer-to-const right away?
> 
> Yes, that would have been a good idea.

I'll commit this after regression testing.

	* symbols.c (S_IS_FUNCTION, S_IS_EXTERNAL, S_IS_WEAK),
	(S_IS_WEAKREFR, S_IS_WEAKREFD, S_IS_COMMON, S_IS_DEFINED),
	(S_FORCE_RELOC, S_IS_DEBUG, S_IS_LOCAL, S_IS_STABD),
	(symbol_previous, symbol_next, symbol_X_add_number),
	(symbol_get_frag, symbol_used_p, symbol_used_in_reloc_p),
	(symbol_mri_common_p, symbol_written_p, symbol_removed_p),
	(symbol_resolved_p, symbol_resolving_p, symbol_section_p),
	(symbol_equated_p, symbol_equated_reloc_p, symbol_constant_p),
	(symbol_shadow_p, symbol_same_p): Constify sym args.
	* symbols.h: Update prototypes.

diff --git a/gas/symbols.c b/gas/symbols.c
index 4df83bab0e9..b57dbfc64f4 100644
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -2302,7 +2302,7 @@ copy_symbol_attributes (symbolS *dest, symbolS *src)
 }
 
 int
-S_IS_FUNCTION (symbolS *s)
+S_IS_FUNCTION (const symbolS *s)
 {
   flagword flags;
 
@@ -2315,7 +2315,7 @@ S_IS_FUNCTION (symbolS *s)
 }
 
 int
-S_IS_EXTERNAL (symbolS *s)
+S_IS_EXTERNAL (const symbolS *s)
 {
   flagword flags;
 
@@ -2332,7 +2332,7 @@ S_IS_EXTERNAL (symbolS *s)
 }
 
 int
-S_IS_WEAK (symbolS *s)
+S_IS_WEAK (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2346,7 +2346,7 @@ S_IS_WEAK (symbolS *s)
 }
 
 int
-S_IS_WEAKREFR (symbolS *s)
+S_IS_WEAKREFR (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2354,7 +2354,7 @@ S_IS_WEAKREFR (symbolS *s)
 }
 
 int
-S_IS_WEAKREFD (symbolS *s)
+S_IS_WEAKREFD (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2362,7 +2362,7 @@ S_IS_WEAKREFD (symbolS *s)
 }
 
 int
-S_IS_COMMON (symbolS *s)
+S_IS_COMMON (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2370,7 +2370,7 @@ S_IS_COMMON (symbolS *s)
 }
 
 int
-S_IS_DEFINED (symbolS *s)
+S_IS_DEFINED (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return ((struct local_symbol *) s)->section != undefined_section;
@@ -2386,7 +2386,7 @@ S_IS_DEFINED (symbolS *s)
    symbols or eliminated from expressions, because they may be
    overridden by the linker.  */
 int
-S_FORCE_RELOC (symbolS *s, int strict)
+S_FORCE_RELOC (const symbolS *s, int strict)
 {
   segT sec;
   if (s->flags.local_symbol)
@@ -2405,7 +2405,7 @@ S_FORCE_RELOC (symbolS *s, int strict)
 }
 
 int
-S_IS_DEBUG (symbolS *s)
+S_IS_DEBUG (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2415,7 +2415,7 @@ S_IS_DEBUG (symbolS *s)
 }
 
 int
-S_IS_LOCAL (symbolS *s)
+S_IS_LOCAL (const symbolS *s)
 {
   flagword flags;
   const char *name;
@@ -2455,7 +2455,7 @@ S_IS_LOCAL (symbolS *s)
 }
 
 int
-S_IS_STABD (symbolS *s)
+S_IS_STABD (const symbolS *s)
 {
   return S_GET_NAME (s) == 0;
 }
@@ -2703,7 +2703,7 @@ S_SET_FORWARD_REF (symbolS *s)
 /* Return the previous symbol in a chain.  */
 
 symbolS *
-symbol_previous (symbolS *s)
+symbol_previous (const symbolS *s)
 {
   if (s->flags.local_symbol)
     abort ();
@@ -2713,7 +2713,7 @@ symbol_previous (symbolS *s)
 /* Return the next symbol in a chain.  */
 
 symbolS *
-symbol_next (symbolS *s)
+symbol_next (const symbolS *s)
 {
   if (s->flags.local_symbol)
     abort ();
@@ -2744,7 +2744,7 @@ symbol_set_value_expression (symbolS *s, const expressionS *exp)
 /* Return whether 2 symbols are the same.  */
 
 int
-symbol_same_p (symbolS *s1, symbolS *s2)
+symbol_same_p (const symbolS *s1, const symbolS *s2)
 {
   return s1 == s2;
 }
@@ -2752,7 +2752,7 @@ symbol_same_p (symbolS *s1, symbolS *s2)
 /* Return a pointer to the X_add_number component of a symbol.  */
 
 offsetT *
-symbol_X_add_number (symbolS *s)
+symbol_X_add_number (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return (offsetT *) &((struct local_symbol *) s)->value;
@@ -2787,7 +2787,7 @@ symbol_set_frag (symbolS *s, fragS *f)
 /* Return the frag of a symbol.  */
 
 fragS *
-symbol_get_frag (symbolS *s)
+symbol_get_frag (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return ((struct local_symbol *) s)->frag;
@@ -2819,7 +2819,7 @@ symbol_clear_used (symbolS *s)
 /* Return whether a symbol has been used.  */
 
 int
-symbol_used_p (symbolS *s)
+symbol_used_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 1;
@@ -2849,7 +2849,7 @@ symbol_clear_used_in_reloc (symbolS *s)
 /* Return whether a symbol has been used in a reloc.  */
 
 int
-symbol_used_in_reloc_p (symbolS *s)
+symbol_used_in_reloc_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2879,7 +2879,7 @@ symbol_clear_mri_common (symbolS *s)
 /* Return whether a symbol is an MRI common symbol.  */
 
 int
-symbol_mri_common_p (symbolS *s)
+symbol_mri_common_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2909,7 +2909,7 @@ symbol_clear_written (symbolS *s)
 /* Return whether a symbol has been written.  */
 
 int
-symbol_written_p (symbolS *s)
+symbol_written_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2929,7 +2929,7 @@ symbol_mark_removed (symbolS *s)
 /* Return whether a symbol has been marked to be removed.  */
 
 int
-symbol_removed_p (symbolS *s)
+symbol_removed_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2947,7 +2947,7 @@ symbol_mark_resolved (symbolS *s)
 /* Return whether a symbol has been resolved.  */
 
 int
-symbol_resolved_p (symbolS *s)
+symbol_resolved_p (const symbolS *s)
 {
   return s->flags.resolved;
 }
@@ -2969,7 +2969,7 @@ symbol_clear_resolving (symbolS *s)
 /* Return whether a symbol is being resolved.  */
 
 int
-symbol_resolving_p (symbolS *s)
+symbol_resolving_p (const symbolS *s)
 {
   return s->flags.resolving;
 }
@@ -2977,7 +2977,7 @@ symbol_resolving_p (symbolS *s)
 /* Return whether a symbol is a section symbol.  */
 
 int
-symbol_section_p (symbolS *s)
+symbol_section_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2987,7 +2987,7 @@ symbol_section_p (symbolS *s)
 /* Return whether a symbol is equated to another symbol.  */
 
 int
-symbol_equated_p (symbolS *s)
+symbol_equated_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -2998,7 +2998,7 @@ symbol_equated_p (symbolS *s)
    treated specially when writing out relocs.  */
 
 int
-symbol_equated_reloc_p (symbolS *s)
+symbol_equated_reloc_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
@@ -3017,7 +3017,7 @@ symbol_equated_reloc_p (symbolS *s)
 /* Return whether a symbol has a constant value.  */
 
 int
-symbol_constant_p (symbolS *s)
+symbol_constant_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 1;
@@ -3028,7 +3028,7 @@ symbol_constant_p (symbolS *s)
    symbol list.  */
 
 int
-symbol_shadow_p (symbolS *s)
+symbol_shadow_p (const symbolS *s)
 {
   if (s->flags.local_symbol)
     return 0;
diff --git a/gas/symbols.h b/gas/symbols.h
index c61fabcbe0a..ee7ca67ac4c 100644
--- a/gas/symbols.h
+++ b/gas/symbols.h
@@ -95,17 +95,17 @@ extern valueT S_GET_VALUE (symbolS *);
 extern valueT S_GET_VALUE_WHERE (symbolS *, const char *, unsigned int);
 extern void S_SET_VALUE (symbolS *, valueT);
 
-extern int S_IS_FUNCTION (symbolS *);
-extern int S_IS_EXTERNAL (symbolS *);
-extern int S_IS_WEAK (symbolS *);
-extern int S_IS_WEAKREFR (symbolS *);
-extern int S_IS_WEAKREFD (symbolS *);
-extern int S_IS_COMMON (symbolS *);
-extern int S_IS_DEFINED (symbolS *);
-extern int S_FORCE_RELOC (symbolS *, int);
-extern int S_IS_DEBUG (symbolS *);
-extern int S_IS_LOCAL (symbolS *);
-extern int S_IS_STABD (symbolS *);
+extern int S_IS_FUNCTION (const symbolS *);
+extern int S_IS_EXTERNAL (const symbolS *);
+extern int S_IS_WEAK (const symbolS *);
+extern int S_IS_WEAKREFR (const symbolS *);
+extern int S_IS_WEAKREFD (const symbolS *);
+extern int S_IS_COMMON (const symbolS *);
+extern int S_IS_DEFINED (const symbolS *);
+extern int S_FORCE_RELOC (const symbolS *, int);
+extern int S_IS_DEBUG (const symbolS *);
+extern int S_IS_LOCAL (const symbolS *);
+extern int S_IS_STABD (const symbolS *);
 extern int S_CAN_BE_REDEFINED (const symbolS *);
 extern int S_IS_VOLATILE (const symbolS *);
 extern int S_IS_FORWARD_REF (const symbolS *);
@@ -173,8 +173,6 @@ void symbol_insert (symbolS * addme, symbolS * target,
 void symbol_remove (symbolS * symbolP, symbolS ** rootP,
 		    symbolS ** lastP);
 
-extern symbolS *symbol_previous (symbolS *);
-
 extern int symbol_on_chain (symbolS *s, symbolS *rootPP, symbolS *lastPP);
 
 void verify_symbol_chain (symbolS * rootP, symbolS * lastP);
@@ -182,42 +180,43 @@ void verify_symbol_chain (symbolS * rootP, symbolS * lastP);
 void symbol_append (symbolS * addme, symbolS * target,
 		    symbolS ** rootP, symbolS ** lastP);
 
-extern symbolS *symbol_next (symbolS *);
+extern symbolS *symbol_previous (const symbolS *);
+extern symbolS *symbol_next (const symbolS *);
 
 extern expressionS *symbol_get_value_expression (symbolS *);
 extern void symbol_set_value_expression (symbolS *, const expressionS *);
-extern offsetT *symbol_X_add_number (symbolS *);
+extern offsetT *symbol_X_add_number (const symbolS *);
 extern void symbol_set_value_now (symbolS *);
 extern void symbol_set_frag (symbolS *, fragS *);
-extern fragS *symbol_get_frag (symbolS *);
+extern fragS *symbol_get_frag (const symbolS *);
 extern void symbol_mark_used (symbolS *);
 extern void symbol_clear_used (symbolS *);
-extern int symbol_used_p (symbolS *);
+extern int symbol_used_p (const symbolS *);
 extern void symbol_mark_used_in_reloc (symbolS *);
 extern void symbol_clear_used_in_reloc (symbolS *);
-extern int symbol_used_in_reloc_p (symbolS *);
+extern int symbol_used_in_reloc_p (const symbolS *);
 extern void symbol_mark_mri_common (symbolS *);
 extern void symbol_clear_mri_common (symbolS *);
-extern int symbol_mri_common_p (symbolS *);
+extern int symbol_mri_common_p (const symbolS *);
 extern void symbol_mark_written (symbolS *);
 extern void symbol_clear_written (symbolS *);
-extern int symbol_written_p (symbolS *);
+extern int symbol_written_p (const symbolS *);
 extern void symbol_mark_removed (symbolS *);
-extern int symbol_removed_p (symbolS *);
+extern int symbol_removed_p (const symbolS *);
 extern void symbol_mark_resolved (symbolS *);
-extern int symbol_resolved_p (symbolS *);
+extern int symbol_resolved_p (const symbolS *);
 extern void symbol_mark_resolving (symbolS *);
 extern void symbol_clear_resolving (symbolS *);
-extern int symbol_resolving_p (symbolS *);
-extern int symbol_section_p (symbolS *);
-extern int symbol_equated_p (symbolS *);
-extern int symbol_equated_reloc_p (symbolS *);
-extern int symbol_constant_p (symbolS *);
-extern int symbol_shadow_p (symbolS *);
+extern int symbol_resolving_p (const symbolS *);
+extern int symbol_section_p (const symbolS *);
+extern int symbol_equated_p (const symbolS *);
+extern int symbol_equated_reloc_p (const symbolS *);
+extern int symbol_constant_p (const symbolS *);
+extern int symbol_shadow_p (const symbolS *);
 extern symbolS *symbol_symbolS (symbolS *);
 extern asymbol *symbol_get_bfdsym (symbolS *);
 extern void symbol_set_bfdsym (symbolS *, asymbol *);
-extern int symbol_same_p (symbolS *, symbolS *);
+extern int symbol_same_p (const symbolS *, const symbolS *);
 
 #ifdef OBJ_SYMFIELD_TYPE
 OBJ_SYMFIELD_TYPE *symbol_get_obj (symbolS *);
  

Patch

diff --git a/gas/config/tc-i386-intel.c b/gas/config/tc-i386-intel.c
index c95af419cf7..3011606d574 100644
--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -369,21 +369,25 @@  i386_intel_simplify_register (expressionS *e)
   return 2;
 }
 
-static int i386_intel_simplify (expressionS *);
-
-static INLINE int i386_intel_simplify_symbol(symbolS *sym)
+static int
+i386_intel_simplify_symbol (symbolS *sym)
 {
-  int ret = i386_intel_simplify (symbol_get_value_expression (sym));
+  if (symbol_resolving_p (sym))
+    return 1;
 
+  symbol_mark_resolving (sym);
+  int ret = i386_intel_simplify (symbol_get_value_expression (sym));
   if (ret == 2)
-  {
-    S_SET_SEGMENT(sym, absolute_section);
-    ret = 1;
-  }
+    {
+      S_SET_SEGMENT (sym, absolute_section);
+      ret = 1;
+    }
+  symbol_clear_resolving (sym);
   return ret;
 }
 
-static int i386_intel_simplify (expressionS *e)
+static int
+i386_intel_simplify (expressionS *e)
 {
   const reg_entry *the_reg = (this_operand >= 0
 			      ? i.op[this_operand].regs : NULL);
diff --git a/gas/symbols.c b/gas/symbols.c
index 41f273c85ff..4df83bab0e9 100644
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -2936,7 +2936,7 @@  symbol_removed_p (symbolS *s)
   return s->flags.removed;
 }
 
-/* Mark a symbol has having been resolved.  */
+/* Mark a symbol as having been resolved.  */
 
 void
 symbol_mark_resolved (symbolS *s)
@@ -2952,6 +2952,28 @@  symbol_resolved_p (symbolS *s)
   return s->flags.resolved;
 }
 
+/* Mark a symbol as being resolved.  */
+
+void
+symbol_mark_resolving (symbolS *s)
+{
+  s->flags.resolving = 1;
+}
+
+void
+symbol_clear_resolving (symbolS *s)
+{
+  s->flags.resolving = 0;
+}
+
+/* Return whether a symbol is being resolved.  */
+
+int
+symbol_resolving_p (symbolS *s)
+{
+  return s->flags.resolving;
+}
+
 /* Return whether a symbol is a section symbol.  */
 
 int
diff --git a/gas/symbols.h b/gas/symbols.h
index 3232f1b2a33..c61fabcbe0a 100644
--- a/gas/symbols.h
+++ b/gas/symbols.h
@@ -206,6 +206,9 @@  extern void symbol_mark_removed (symbolS *);
 extern int symbol_removed_p (symbolS *);
 extern void symbol_mark_resolved (symbolS *);
 extern int symbol_resolved_p (symbolS *);
+extern void symbol_mark_resolving (symbolS *);
+extern void symbol_clear_resolving (symbolS *);
+extern int symbol_resolving_p (symbolS *);
 extern int symbol_section_p (symbolS *);
 extern int symbol_equated_p (symbolS *);
 extern int symbol_equated_reloc_p (symbolS *);