From: Alan Modra Date: Thu, 4 May 2023 09:19:04 +0000 (+0930) Subject: coffcode.h handle_COMDAT tidy X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=cb3f0ff4795381fb19e128a85e258149ebed9cd6;p=binutils-gdb.git coffcode.h handle_COMDAT tidy I started down the path of attempting to fix https://sourceware.org/pipermail/binutils/2023-April/127263.html but decided after a while that I didn't want to mess with this code.. This patch is a just a few things that I thought worth doing, the main one being reporting of errors up the call chain. The while loop to for loop change is shamelessly stolen from Oleg. * coffcode.h (handle_COMDAT): Return bool. Make sec_flags a flagword*, and adjust to suit. Replace while loop with for loop. Check isym.n_numaux before reading aux entries. Alloc coff_comdat_info and name in one call to bfd_alloc. Remove goto breakloop. (styp_to_sec_flags): Adjust handle_COMDAT call. --- diff --git a/bfd/coffcode.h b/bfd/coffcode.h index ab22ed092d6..e52d652616e 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -852,9 +852,9 @@ styp_to_sec_flags (bfd *abfd, #else /* COFF_WITH_PE */ -static flagword +static bool handle_COMDAT (bfd * abfd, - flagword sec_flags, + flagword *sec_flags, void * hdr, const char *name, asection *section) @@ -864,7 +864,7 @@ handle_COMDAT (bfd * abfd, int seen_state = 0; char *target_name = NULL; - sec_flags |= SEC_LINK_ONCE; + *sec_flags |= SEC_LINK_ONCE; /* Unfortunately, the PE format stores essential information in the symbol table, of all places. We need to extract that @@ -884,18 +884,19 @@ handle_COMDAT (bfd * abfd, rather messy. */ if (! _bfd_coff_get_external_symbols (abfd)) - return sec_flags; + return true; esymstart = esym = (bfd_byte *) obj_coff_external_syms (abfd); esymend = esym + obj_raw_syment_count (abfd) * bfd_coff_symesz (abfd); - while (esym < esymend) + for (struct internal_syment isym; + esym < esymend; + esym += (isym.n_numaux + 1) * bfd_coff_symesz (abfd)) { - struct internal_syment isym; char buf[SYMNMLEN + 1]; const char *symname; - bfd_coff_swap_sym_in (abfd, esym, & isym); + bfd_coff_swap_sym_in (abfd, esym, &isym); BFD_ASSERT (sizeof (internal_s->s_name) <= SYMNMLEN); @@ -933,7 +934,7 @@ handle_COMDAT (bfd * abfd, { _bfd_error_handler (_("%pB: unable to load COMDAT section name"), abfd); - break; + return false; } switch (seen_state) @@ -968,7 +969,7 @@ handle_COMDAT (bfd * abfd, cf PR 21781. */ _bfd_error_handler (_("%pB: error: unexpected symbol '%s' in COMDAT section"), abfd, symname); - goto breakloop; + return false; } /* FIXME LATER: MSVC generates section names @@ -982,22 +983,8 @@ handle_COMDAT (bfd * abfd, " does not match section name '%s'"), abfd, symname, name); - seen_state = 1; - - /* PR 17512: file: e2cfe54f. */ - if (esym + bfd_coff_symesz (abfd) >= esymend) - { - /* xgettext:c-format */ - _bfd_error_handler (_("%pB: warning: no symbol for" - " section '%s' found"), - abfd, symname); - break; - } /* This is the section symbol. */ - bfd_coff_swap_aux_in (abfd, (esym + bfd_coff_symesz (abfd)), - isym.n_type, isym.n_sclass, - 0, isym.n_numaux, & aux); - + seen_state = 1; target_name = strchr (name, '$'); if (target_name != NULL) { @@ -1007,6 +994,24 @@ handle_COMDAT (bfd * abfd, target_name += 1; } + if (isym.n_numaux == 0) + aux.x_scn.x_comdat = 0; + else + { + /* PR 17512: file: e2cfe54f. */ + if (esym + bfd_coff_symesz (abfd) >= esymend) + { + /* xgettext:c-format */ + _bfd_error_handler (_("%pB: warning: no symbol for" + " section '%s' found"), + abfd, symname); + break; + } + bfd_coff_swap_aux_in (abfd, esym + bfd_coff_symesz (abfd), + isym.n_type, isym.n_sclass, + 0, isym.n_numaux, &aux); + } + /* FIXME: Microsoft uses NODUPLICATES and ASSOCIATIVE, but gnu uses ANY and SAME_SIZE. Unfortunately, gnu doesn't do @@ -1027,23 +1032,23 @@ handle_COMDAT (bfd * abfd, { case IMAGE_COMDAT_SELECT_NODUPLICATES: #ifdef STRICT_PE_FORMAT - sec_flags |= SEC_LINK_DUPLICATES_ONE_ONLY; + *sec_flags |= SEC_LINK_DUPLICATES_ONE_ONLY; #else - sec_flags &= ~SEC_LINK_ONCE; + *sec_flags &= ~SEC_LINK_ONCE; #endif break; case IMAGE_COMDAT_SELECT_ANY: - sec_flags |= SEC_LINK_DUPLICATES_DISCARD; + *sec_flags |= SEC_LINK_DUPLICATES_DISCARD; break; case IMAGE_COMDAT_SELECT_SAME_SIZE: - sec_flags |= SEC_LINK_DUPLICATES_SAME_SIZE; + *sec_flags |= SEC_LINK_DUPLICATES_SAME_SIZE; break; case IMAGE_COMDAT_SELECT_EXACT_MATCH: /* Not yet fully implemented ??? */ - sec_flags |= SEC_LINK_DUPLICATES_SAME_CONTENTS; + *sec_flags |= SEC_LINK_DUPLICATES_SAME_CONTENTS; break; /* debug$S gets this case; other @@ -1056,16 +1061,16 @@ handle_COMDAT (bfd * abfd, case IMAGE_COMDAT_SELECT_ASSOCIATIVE: #ifdef STRICT_PE_FORMAT /* FIXME: This is not currently implemented. */ - sec_flags |= SEC_LINK_DUPLICATES_DISCARD; + *sec_flags |= SEC_LINK_DUPLICATES_DISCARD; #else - sec_flags &= ~SEC_LINK_ONCE; + *sec_flags &= ~SEC_LINK_ONCE; #endif break; default: /* 0 means "no symbol" */ /* debug$F gets this case; other implications ??? */ - sec_flags |= SEC_LINK_DUPLICATES_DISCARD; + *sec_flags |= SEC_LINK_DUPLICATES_DISCARD; break; } } @@ -1082,7 +1087,6 @@ handle_COMDAT (bfd * abfd, symname + (TARGET_UNDERSCORE ? 1 : 0)) != 0) { /* Not the name we're looking for */ - esym += (isym.n_numaux + 1) * bfd_coff_symesz (abfd); continue; } /* Fall through. */ @@ -1090,42 +1094,30 @@ handle_COMDAT (bfd * abfd, /* MSVC mode: the lexically second symbol (or drop through from the above). */ { - char *newname; - size_t amt; - /* This must the second symbol with the section #. It is the actual symbol name. Intel puts the two adjacent, but Alpha (at least) spreads them out. */ - amt = sizeof (struct coff_comdat_info); - coff_section_data (abfd, section)->comdat - = (struct coff_comdat_info *) bfd_alloc (abfd, amt); - if (coff_section_data (abfd, section)->comdat == NULL) - abort (); - - coff_section_data (abfd, section)->comdat->symbol = - (esym - esymstart) / bfd_coff_symesz (abfd); + struct coff_comdat_info *comdat; + size_t len = strlen (symname) + 1; - amt = strlen (symname) + 1; - newname = (char *) bfd_alloc (abfd, amt); - if (newname == NULL) - abort (); + comdat = bfd_alloc (abfd, sizeof (*comdat) + len); + if (comdat == NULL) + return false; - strcpy (newname, symname); - coff_section_data (abfd, section)->comdat->name - = newname; + coff_section_data (abfd, section)->comdat = comdat; + comdat->symbol = (esym - esymstart) / bfd_coff_symesz (abfd); + char *newname = (char *) (comdat + 1); + comdat->name = newname; + memcpy (newname, symname, len); + return true; } - - goto breakloop; } } - - esym += (isym.n_numaux + 1) * bfd_coff_symesz (abfd); } - breakloop: - return sec_flags; + return true; } @@ -1276,7 +1268,8 @@ styp_to_sec_flags (bfd *abfd, break; case IMAGE_SCN_LNK_COMDAT: /* COMDAT gets very special treatment. */ - sec_flags = handle_COMDAT (abfd, sec_flags, hdr, name, section); + if (!handle_COMDAT (abfd, &sec_flags, hdr, name, section)) + result = false; break; default: /* Silently ignore for now. */