* coffgrok access of u.auxent.x_sym.x_tagndx.p
@ 2023-03-27 11:27 Alan Modra
0 siblings, 0 replies; only message in thread
From: Alan Modra @ 2023-03-27 11:27 UTC (permalink / raw)
To: binutils
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 bf55d83530d..d4a2a5c3d62 100644
--- 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 b7a4f677411..b58ee8adedf 100644
--- 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 f2676e4c275..fa01203ab5e 100644
--- 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;
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-03-27 11:27 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 11:27 coffgrok access of u.auxent.x_sym.x_tagndx.p Alan Modra
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).