coffgrok access of u.auxent.x_sym.x_tagndx.p

u.auxent.x_sym.x_tagndx is a union.  The p field is only valid when
fix_tag is set.  This patch fixes code in coffgrok.c that accessed the
field without first checking fix_tag, and removes a whole lot of code
validating bogus pointers to prevent segfaults (which no longer
happen, I checked the referenced PR 17512 testcases).  The patch also
documents this in the fix_tag comment, makes is_sym a bitfield, and
sorts the selecter fields a little.

bfd/
	* coffcode.h (combined_entry_type): Make is_sym a bitfield.
	Sort and comment on union selectors.
	* libcoff.h: Regenerate.
binutils/
	* coffgrok.c (do_type): Make aux a combined_entry_type.  Test
	fix_tag before accessing u.auxent.x_sym.x_tagndx.p.  Remove
	now unnecessary pointer bounds checking.

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index bf55d83530d8fa0867265e11472f7c97226ca7ea..d4a2a5c3d6209785b322882d47b29f6951aa4479 100644 (file)
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -294,27 +294,30 @@ CODE_FRAGMENT
 .typedef struct coff_ptr_struct
 .{
 .  {* Remembers the offset from the first symbol in the file for
-.     this symbol. Generated by coff_renumber_symbols.  *}
+.     this symbol.  Generated by coff_renumber_symbols.  *}
 .  unsigned int offset;
 .
-.  {* Should the value of this symbol be renumbered.  Used for
-.     XCOFF C_BSTAT symbols.  Set by coff_slurp_symbol_table.  *}
-.  unsigned int fix_value : 1;
+.  {* Selects between the elements of the union below.  *}
+.  unsigned int is_sym : 1;
 .
-.  {* Should the tag field of this symbol be renumbered.
-.     Created by coff_pointerize_aux.  *}
+.  {* Selects between the elements of the x_sym.x_tagndx union.  If set,
+.     p is valid and the field will be renumbered.  *}
 .  unsigned int fix_tag : 1;
 .
-.  {* Should the endidx field of this symbol be renumbered.
-.     Created by coff_pointerize_aux.  *}
+.  {* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx
+.     union.  If set, p is valid and the field will be renumbered.  *}
 .  unsigned int fix_end : 1;
 .
-.  {* Should the x_csect.x_scnlen field be renumbered.
-.     Created by coff_pointerize_aux.  *}
+.  {* Selects between the elements of the x_csect.x_scnlen union.  If set,
+.     p is valid and the field will be renumbered.  *}
 .  unsigned int fix_scnlen : 1;
 .
-.  {* Fix up an XCOFF C_BINCL/C_EINCL symbol.  The value is the
-.     index into the line number entries.  Set by coff_slurp_symbol_table.  *}
+.  {* If set, u.syment.n_value contains a pointer to a symbol.  The final
+.     value will be the offset field.  Used for XCOFF C_BSTAT symbols.  *}
+.  unsigned int fix_value : 1;
+.
+.  {* If set, u.syment.n_value is an index into the line number entries.
+.     Used for XCOFF C_BINCL/C_EINCL symbols.  *}
 .  unsigned int fix_line : 1;
 .
 .  {* The container for the symbol structure as read and translated
@@ -325,9 +328,6 @@ CODE_FRAGMENT
 .    struct internal_syment syment;
 .  } u;
 .
-. {* Selector for the union above.  *}
-. bool is_sym;
-.
 . {* An extra pointer which can used by format based on COFF (like XCOFF)
 .    to provide extra information to their backend.  *}
 . void *extrap;

diff --git a/bfd/libcoff.h b/bfd/libcoff.h
index b7a4f677411b67cf7907b3b0b9e0725a1ef100f1..b58ee8adedfa7a520ac15535fc55d2528554084c 100644 (file)
--- a/bfd/libcoff.h
+++ b/bfd/libcoff.h
@@ -633,27 +633,30 @@ extern bool _bfd_ppc_xcoff_relocate_section
 typedef struct coff_ptr_struct
 {
   /* Remembers the offset from the first symbol in the file for
-     this symbol. Generated by coff_renumber_symbols.  */
+     this symbol.  Generated by coff_renumber_symbols.  */
   unsigned int offset;
 
-  /* Should the value of this symbol be renumbered.  Used for
-     XCOFF C_BSTAT symbols.  Set by coff_slurp_symbol_table.  */
-  unsigned int fix_value : 1;
+  /* Selects between the elements of the union below.  */
+  unsigned int is_sym : 1;
 
-  /* Should the tag field of this symbol be renumbered.
-     Created by coff_pointerize_aux.  */
+  /* Selects between the elements of the x_sym.x_tagndx union.  If set,
+     p is valid and the field will be renumbered.  */
   unsigned int fix_tag : 1;
 
-  /* Should the endidx field of this symbol be renumbered.
-     Created by coff_pointerize_aux.  */
+  /* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx
+     union.  If set, p is valid and the field will be renumbered.  */
   unsigned int fix_end : 1;
 
-  /* Should the x_csect.x_scnlen field be renumbered.
-     Created by coff_pointerize_aux.  */
+  /* Selects between the elements of the x_csect.x_scnlen union.  If set,
+     p is valid and the field will be renumbered.  */
   unsigned int fix_scnlen : 1;
 
-  /* Fix up an XCOFF C_BINCL/C_EINCL symbol.  The value is the
-     index into the line number entries.  Set by coff_slurp_symbol_table.  */
+  /* If set, u.syment.n_value contains a pointer to a symbol.  The final
+     value will be the offset field.  Used for XCOFF C_BSTAT symbols.  */
+  unsigned int fix_value : 1;
+
+  /* If set, u.syment.n_value is an index into the line number entries.
+     Used for XCOFF C_BINCL/C_EINCL symbols.  */
   unsigned int fix_line : 1;
 
   /* The container for the symbol structure as read and translated
@@ -664,9 +667,6 @@ typedef struct coff_ptr_struct
     struct internal_syment syment;
   } u;
 
- /* Selector for the union above.  */
- bool is_sym;
-
  /* An extra pointer which can used by format based on COFF (like XCOFF)
     to provide extra information to their backend.  */
  void *extrap;

diff --git a/binutils/coffgrok.c b/binutils/coffgrok.c
index f2676e4c275434d8b761f337df7a4434f5387393..fa01203ab5e80b3cd0441b0b755ed3c8573d493d 100644 (file)
--- a/binutils/coffgrok.c
+++ b/binutils/coffgrok.c
@@ -341,7 +341,7 @@ static struct coff_type *
 do_type (unsigned int i)
 {
   struct internal_syment *sym;
-  union internal_auxent *aux;
+  combined_entry_type *aux;
   struct coff_type *res = (struct coff_type *) xmalloc (sizeof (struct coff_type));
   int type;
   int which_dt = 0;
@@ -357,7 +357,7 @@ do_type (unsigned int i)
   if (sym->n_numaux == 0 || i >= rawcount -1 || rawsyms[i + 1].is_sym)
     aux = NULL;
   else
-    aux = &rawsyms[i + 1].u.auxent;
+    aux = &rawsyms[i + 1];
 
   type = sym->n_type;
 
@@ -374,7 +374,7 @@ do_type (unsigned int i)
          res->type = coff_secdef_type;
          if (aux == NULL)
            fatal (_("Section definition needs a section length"));
-         res->size = aux->x_scn.x_scnlen;
+         res->size = aux->u.auxent.x_scn.x_scnlen;
 
          /* PR 17512: file: 081c955d.
             Fill in the asecdef structure as well.  */
@@ -426,26 +426,9 @@ do_type (unsigned int i)
          if (aux == NULL)
            fatal (_("Aggregate definition needs auxiliary information"));
 
-         if (aux->x_sym.x_tagndx.p)
+         if (aux->fix_tag)
            {
-             unsigned int idx;
-
-             /* PR 17512: file: e72f3988.  */
-             if (aux->x_sym.x_tagndx.l < 0 || aux->x_sym.x_tagndx.p < rawsyms)
-               {
-                 non_fatal (_("Invalid tag index %#lx encountered"), aux->x_sym.x_tagndx.l);
-                 idx = 0;
-               }
-             else
-               idx = INDEXOF (aux->x_sym.x_tagndx.p);
-
-             if (idx >= rawcount)
-               {
-                 if (rawcount == 0)
-                   fatal (_("Symbol index %u encountered when there are no symbols"), idx);
-                 non_fatal (_("Invalid symbol index %u encountered"), idx);
-                 idx = 0;
-               }
+             unsigned int idx = INDEXOF (aux->u.auxent.x_sym.x_tagndx.p);
 
              /* Referring to a struct defined elsewhere.  */
              res->type = coff_structref_type;
@@ -461,7 +444,7 @@ do_type (unsigned int i)
              res->u.astructdef.elements = empty_scope ();
              res->u.astructdef.idx = 0;
              res->u.astructdef.isstruct = (type & 0xf) == T_STRUCT;
-             res->size = aux->x_sym.x_misc.x_lnsz.x_size;
+             res->size = aux->u.auxent.x_sym.x_misc.x_lnsz.x_size;
            }
        }
       else
@@ -475,13 +458,10 @@ do_type (unsigned int i)
     case T_ENUM:
       if (aux == NULL)
        fatal (_("Enum definition needs auxiliary information"));
-      if (aux->x_sym.x_tagndx.p)
+      if (aux->fix_tag)
        {
-         unsigned int idx = INDEXOF (aux->x_sym.x_tagndx.p);
+         unsigned int idx = INDEXOF (aux->u.auxent.x_sym.x_tagndx.p);
 
-         /* PR 17512: file: 1ef037c7.  */
-         if (idx >= rawcount)
-           fatal (_("Invalid enum symbol index %u encountered"), idx);
          /* Referring to a enum defined elsewhere.  */
          res->type = coff_enumref_type;
          res->u.aenumref.ref = tindex[idx];
@@ -497,7 +477,7 @@ do_type (unsigned int i)
          last_enum = res;
          res->type = coff_enumdef_type;
          res->u.aenumdef.elements = empty_scope ();
-         res->size = aux->x_sym.x_misc.x_lnsz.x_size;
+         res->size = aux->u.auxent.x_sym.x_misc.x_lnsz.x_size;
        }
       break;
     case T_MOE:
@@ -519,7 +499,7 @@ do_type (unsigned int i)
            if (aux == NULL)
              fatal (_("Array definition needs auxiliary information"));
            els = (dimind < DIMNUM
-                  ? aux->x_sym.x_fcnary.x_ary.x_dimen[dimind]
+                  ? aux->u.auxent.x_sym.x_fcnary.x_ary.x_dimen[dimind]
                   : 0);
 
            ++dimind;