From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22884 invoked by alias); 10 Jan 2012 13:03:38 -0000 Received: (qmail 22856 invoked by uid 22791); 10 Jan 2012 13:03:34 -0000 X-SWARE-Spam-Status: No, hits=1.7 required=5.0 tests=AWL,BAYES_99,KAM_STOCKGEN X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 Jan 2012 13:03:19 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 76AFECB2FEB; Tue, 10 Jan 2012 14:03:19 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wsOVC7MLqAfn; Tue, 10 Jan 2012 14:03:19 +0100 (CET) Received: from ulanbator.act-europe.fr (ulanbator.act-europe.fr [10.10.1.67]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id 606C8CB2FEF; Tue, 10 Jan 2012 14:03:19 +0100 (CET) Subject: Re: [Patch macho/bfd/gas] .indirect_symbol, take 2. Mime-Version: 1.0 (Apple Message framework v1251.1) Content-Type: text/plain; charset=us-ascii From: Tristan Gingold In-Reply-To: <56BB999A-CDA7-4F2C-B58A-B4788E9B7034@sandoe-acoustics.co.uk> Date: Tue, 10 Jan 2012 13:03:00 -0000 Cc: binutils Development Content-Transfer-Encoding: quoted-printable Message-Id: References: <56BB999A-CDA7-4F2C-B58A-B4788E9B7034@sandoe-acoustics.co.uk> To: Iain Sandoe X-IsSubscribed: yes Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2012-01/txt/msg00138.txt.bz2 On Jan 9, 2012, at 2:00 PM, Iain Sandoe wrote: > Here is a different implementation of the .indirect_symbol patch. >=20 > In this we keep indirect_symbols in a linked list in the section to which= they belong. I don't think that linked list is the best choice. The overhead is large (= at least 2x), and the list structure is useful only for creating it (i.e. f= or gas). I would prefer to have an array in BFD, and the linked list in GA= S. > As of now, I can't see a reason to keep them in GAS as well. >=20 > If this is OK, then a TODO would be to populate the list when a mach-o fi= le is read in - > - this is nicely symmetrical - > - and means that sections can be reordered/removed without having to cros= s-check the dysymtab. >=20 > The only thing that is less satisfactory, is that there is no way (AFAICS= ) to mark a symbol as 'referenced by an indirect' which means if one delete= s (or wishes to delete) symbols - then one should cross-check the usage fro= m the indirect tables. Yes, this is an issue. I need to investigate how to address that. > tests follow as separate patch (they've all be posted before anyway). Thanks! > OK? See some additional comments. Tristan. > Iain >=20 > bfd: >=20 > * mach-o.c (bfd_mach_o_count_indirect_symbols): New. > (bfd_mach_o_build_dysymtab_command): Populate indirect table. > * mach-o.h (bfd_mach_o_section): Add fields for indirect symbol > lists. >=20 > gas: >=20 > * config/obj-macho.c (obj_mach_o_set_symbol_qualifier): Switch off > lazy for extern/private extern. > (obj_mach_o_indirect_symbol): New. > (obj_mach_o_placeholder): Remove. > (mach_o_pseudo_table): Use obj_mach_o_indirect_symbol. > (obj_macho_frob_label): Take care not to force local symbols into > the regular table. > (obj_macho_frob_symbol): Likewise. Ensure undefined and comm syms > have their fields set. > (obj_mach_o_frob_section): New. > * config/obj-macho.h (obj_frob_section): Define. > (obj_mach_o_frob_section): Declare. >=20 > bfd/mach-o.c | 63 +++++++++++++- > bfd/mach-o.h | 16 ++++ > gas/config/obj-macho.c | 214 +++++++++++++++++++++++++++++++++++++++++--= ----- > gas/config/obj-macho.h | 3 + > 4 files changed, 262 insertions(+), 34 deletions(-) >=20 > diff --git a/bfd/mach-o.c b/bfd/mach-o.c > index a1f6596..493116a 100644 > --- a/bfd/mach-o.c > +++ b/bfd/mach-o.c > @@ -2067,6 +2067,33 @@ bfd_mach_o_build_seg_command (const char *segment, > return TRUE; > } >=20 > +/* Count the number of indirect symbols in the image. > + Requires that the sections are in their final order. */ > + > +static unsigned int > +bfd_mach_o_count_indirect_symbols (bfd *abfd, bfd_mach_o_data_struct *md= ata) > +{ > + unsigned int i; > + unsigned int nisyms =3D 0; > + > + for (i =3D 0; i < mdata->nsects; ++i) > + { > + bfd_mach_o_section *sec =3D mdata->sections[i]; > + > + switch (sec->flags & BFD_MACH_O_SECTION_TYPE_MASK) > + { > + case BFD_MACH_O_S_NON_LAZY_SYMBOL_POINTERS: > + case BFD_MACH_O_S_LAZY_SYMBOL_POINTERS: > + case BFD_MACH_O_S_SYMBOL_STUBS: > + nisyms +=3D bfd_mach_o_section_get_nbr_indirect (abfd, sec); > + break; > + default: > + break; > + } > + } > + return nisyms; > +} > + ok. > static bfd_boolean > bfd_mach_o_build_dysymtab_command (bfd *abfd, > bfd_mach_o_data_struct *mdata, > @@ -2123,9 +2150,11 @@ bfd_mach_o_build_dysymtab_command (bfd *abfd, > dsym->nundefsym =3D 0; > } >=20 > + dsym->nindirectsyms =3D bfd_mach_o_count_indirect_symbols (abfd, mdata= ); > if (dsym->nindirectsyms > 0) > { > unsigned i; > + unsigned n; >=20 > mdata->filelen =3D FILE_ALIGN (mdata->filelen, 2); > dsym->indirectsymoff =3D mdata->filelen; > @@ -2134,11 +2163,37 @@ bfd_mach_o_build_dysymtab_command (bfd *abfd, > dsym->indirect_syms =3D bfd_zalloc (abfd, dsym->nindirectsyms * 4); > if (dsym->indirect_syms =3D=3D NULL) > return FALSE; > - > - /* So fill in the indices. */ > - for (i =3D 0; i < dsym->nindirectsyms; ++i) > +=09=09 > + n =3D 0; > + for (i =3D 0; i < mdata->nsects; ++i) > { > - /* TODO: fill in the table. */ > + bfd_mach_o_section *sec =3D mdata->sections[i]; > + > + switch (sec->flags & BFD_MACH_O_SECTION_TYPE_MASK) > + { > + case BFD_MACH_O_S_NON_LAZY_SYMBOL_POINTERS: > + case BFD_MACH_O_S_LAZY_SYMBOL_POINTERS: > + case BFD_MACH_O_S_SYMBOL_STUBS: > + { > + bfd_mach_o_indirect_sym *isym =3D sec->indirectsyms; > + if (isym =3D=3D NULL) > + break; > + /* Record the starting index in the reserved1 field. */ > + sec->reserved1 =3D n; > + do > + { > + dsym->indirect_syms[n] =3D isym->sym->symbol.udata.i; > + n++; > + /* Final safety net. */ > + if (n > dsym->nindirectsyms) > + abort (); Use bfd_assert instead. Note that we should take care of local and absolute symbols. > + } > + while ((isym =3D isym->next) !=3D NULL); > + } > + break; > + default: > + break; > + } > } > } >=20 > diff --git a/bfd/mach-o.h b/bfd/mach-o.h > index ca810a0..31a4095 100644 > --- a/bfd/mach-o.h > +++ b/bfd/mach-o.h > @@ -63,6 +63,12 @@ typedef struct bfd_mach_o_section >=20 > /* Corresponding bfd section. */ > asection *bfdsection; > + > + /* Linked list of indirect symbols for this section (only applies to > + stub and reference sections). */ > + struct bfd_mach_o_indirect_sym *indirectsyms; > + /* Pointer to the last indirect, to save reversing the list. */ > + struct bfd_mach_o_indirect_sym *lastindirectsym; >=20 > /* Simply linked list. */ > struct bfd_mach_o_section *next; > @@ -117,6 +123,16 @@ typedef struct bfd_mach_o_asymbol > } > bfd_mach_o_asymbol; >=20 > +/* An element in a list of indirect symbols the root of which > + is "indirectsyms" in the section to which they apply. */ > + > +typedef struct bfd_mach_o_indirect_sym > +{ > + struct bfd_mach_o_indirect_sym *next; > + struct bfd_mach_o_asymbol *sym; > +} > +bfd_mach_o_indirect_sym; > + > /* The symbol table is sorted like this: > (1) local. > (otherwise in order of generation) > diff --git a/gas/config/obj-macho.c b/gas/config/obj-macho.c > index 43f4fba..5d82977 100644 > --- a/gas/config/obj-macho.c > +++ b/gas/config/obj-macho.c > @@ -39,6 +39,7 @@ >=20 > #include "as.h" > #include "subsegs.h" > +#include "struc-symbol.h" /* For local symbol stuff in frob_label/symbol= . */ > #include "symbols.h" > #include "write.h" > #include "mach-o.h" > @@ -1032,6 +1033,7 @@ obj_mach_o_set_symbol_qualifier (symbolS *sym, int = type) >=20 > case OBJ_MACH_O_SYM_PRIV_EXT: > s->n_type |=3D BFD_MACH_O_N_PEXT ; > + s->n_desc &=3D ~LAZY; /* The native tool swithes this off too. */ > /* We follow the system tools in marking PEXT as also global. */ > /* Fall through. */ Unrelated chunk ? >=20 > @@ -1131,13 +1133,74 @@ obj_mach_o_sym_qual (int ntype) > demand_empty_rest_of_line (); > } >=20 > -/* Dummy function to allow test-code to work while we are working > - on things. */ > - > static void > -obj_mach_o_placeholder (int arg ATTRIBUTE_UNUSED) > +obj_mach_o_indirect_symbol (int arg ATTRIBUTE_UNUSED) > { > - ignore_rest_of_line (); > + bfd_mach_o_section *sec =3D bfd_mach_o_get_mach_o_section (now_seg); > + unsigned lazy =3D 0; > + > +#ifdef md_flush_pending_output > + md_flush_pending_output (); > +#endif > + > + switch (sec->flags & BFD_MACH_O_SECTION_TYPE_MASK) > + { > + case BFD_MACH_O_S_SYMBOL_STUBS: > + case BFD_MACH_O_S_LAZY_SYMBOL_POINTERS: > + lazy =3D LAZY; > + /* Fall through. */ > + case BFD_MACH_O_S_NON_LAZY_SYMBOL_POINTERS: > + { > + bfd_mach_o_indirect_sym *isym; > + char *name =3D input_line_pointer; > + char c =3D get_symbol_end (); > + symbolS *sym =3D symbol_find_or_make (name); > + unsigned int elsize =3D > + bfd_mach_o_section_get_entry_size (stdoutput, sec); > + > + if (elsize =3D=3D 0) > + { > + as_bad (_("attempt to add an indirect_symbol to a stub or" > + " reference section with a zero-sized element at %s"), > + name); > + *input_line_pointer =3D c; > + ignore_rest_of_line (); > + return; > + } > + > + *input_line_pointer =3D c; > + > + /* This should be allocated in bfd, since it is owned there. */ > + isym =3D (bfd_mach_o_indirect_sym *) > + bfd_zalloc (stdoutput, sizeof (bfd_mach_o_indirect_sym)); > + if (isym =3D=3D NULL) > + abort (); > + > + isym->sym =3D (bfd_mach_o_asymbol *) symbol_get_bfdsym (sym); > + if (sec->indirectsyms =3D=3D NULL) > + sec->indirectsyms =3D isym; > + else > + sec->lastindirectsym->next =3D isym; > + sec->lastindirectsym =3D isym; > +=09 > + /* We put in the lazy flag, it will get reset if the symbols is later > + defined, or if the symbol becomes private_extern. */ > + if (isym->sym->symbol.section =3D=3D bfd_und_section_ptr > + && ! (isym->sym->n_type & BFD_MACH_O_N_PEXT)) > + { > + isym->sym->n_desc =3D lazy; > + isym->sym->symbol.udata.i =3D SYM_MACHO_FIELDS_NOT_VALIDATED; > + } > + } > + break; > + > + default: > + as_bad (_("an .indirect_symbol must be in a symbol pointer" > + " or stub section.")); > + ignore_rest_of_line (); > + return; > + } > + demand_empty_rest_of_line (); > } >=20 > const pseudo_typeS mach_o_pseudo_table[] =3D > @@ -1231,7 +1294,7 @@ const pseudo_typeS mach_o_pseudo_table[] =3D > {"no_dead_strip", obj_mach_o_sym_qual, OBJ_MACH_O_SYM_NO_DEAD_STRIP}, > {"weak", obj_mach_o_sym_qual, OBJ_MACH_O_SYM_WEAK}, /* ext */ >=20 > - {"indirect_symbol", obj_mach_o_placeholder, 0}, > + {"indirect_symbol", obj_mach_o_indirect_symbol, 0}, >=20 > /* File flags. */ > { "subsections_via_symbols", obj_mach_o_fileprop, > @@ -1270,15 +1333,33 @@ obj_mach_o_type_for_symbol (bfd_mach_o_asymbol *s) >=20 > void obj_macho_frob_label (struct symbol *sp) > { > - bfd_mach_o_asymbol *s =3D (bfd_mach_o_asymbol *) symbol_get_bfdsym (sp= ); > - /* This is the base symbol type, that we mask in. */ > - unsigned base_type =3D obj_mach_o_type_for_symbol (s); > - bfd_mach_o_section *sec =3D bfd_mach_o_get_mach_o_section (s->symbol.s= ection); > + bfd_mach_o_asymbol *s; > + unsigned base_type; > + bfd_mach_o_section *sec; > int sectype =3D -1; >=20 > + /* Leave local symbols alone (unless they've already been made into a = real > + one). */ > + > + if (0 && sp->bsym =3D=3D NULL) > + { > + if (((struct local_symbol *) sp)->lsy_section !=3D reg_section) > + return; > + else > + /* We have a local which has been added to the symbol table, so carry > + on with the checking. */ > + sp =3D ((struct local_symbol *) sp)->u.lsy_sym; > + } Don't let 'if (0)' code. Please, use macro accessor instead of '((struct local_symbol *) sp)->lsy_se= ction !=3D reg_section'. > + > + s =3D (bfd_mach_o_asymbol *) symbol_get_bfdsym (sp); > + /* Leave debug symbols alone. */ > if ((s->n_type & BFD_MACH_O_N_STAB) !=3D 0) > - return; /* Leave alone. */ > - > + return; > + > + /* This is the base symbol type, that we mask in. */ > + base_type =3D obj_mach_o_type_for_symbol (s); > + > + sec =3D bfd_mach_o_get_mach_o_section (s->symbol.section); > if (sec !=3D NULL) > sectype =3D sec->flags & BFD_MACH_O_SECTION_TYPE_MASK; >=20 > @@ -1307,34 +1388,50 @@ void obj_macho_frob_label (struct symbol *sp) > int > obj_macho_frob_symbol (struct symbol *sp) > { > - bfd_mach_o_asymbol *s =3D (bfd_mach_o_asymbol *) symbol_get_bfdsym (sp= ); > - unsigned base_type =3D obj_mach_o_type_for_symbol (s); > - bfd_mach_o_section *sec =3D bfd_mach_o_get_mach_o_section (s->symbol.s= ection); > + bfd_mach_o_asymbol *s; > + unsigned base_type; > + bfd_mach_o_section *sec; > int sectype =3D -1; > - > + > + /* Leave local symbols alone (unless they've already been made into a = real > + one). */ > + > + if (0 && sp->bsym =3D=3D NULL) > + { > + if (((struct local_symbol *) sp)->lsy_section !=3D reg_section) > + return 0; > + else > + /* We have a local which has been added to the symbol table, so carry > + on with the checking. */ > + sp =3D ((struct local_symbol *) sp)->u.lsy_sym; > + } Likewise. > + > + s =3D (bfd_mach_o_asymbol *) symbol_get_bfdsym (sp); > + /* Leave debug symbols alone. */ > + if ((s->n_type & BFD_MACH_O_N_STAB) !=3D 0) > + return 0; > + > + base_type =3D obj_mach_o_type_for_symbol (s); > + sec =3D bfd_mach_o_get_mach_o_section (s->symbol.section); > if (sec !=3D NULL) > sectype =3D sec->flags & BFD_MACH_O_SECTION_TYPE_MASK; >=20 > - if ((s->n_type & BFD_MACH_O_N_STAB) !=3D 0) > - return 0; /* Leave alone. */ > - else if (s->symbol.section =3D=3D bfd_und_section_ptr) > + if (s->symbol.section =3D=3D bfd_und_section_ptr) > { > /* ??? Do we really gain much from implementing this as well as the > mach-o specific ones? */ > if (s->symbol.flags & BSF_WEAK) > s->n_desc |=3D BFD_MACH_O_N_WEAK_REF; >=20 > - /* Undefined references, become extern. */ > - if (s->n_desc & REFE) > - { > - s->n_desc &=3D ~REFE; > - s->n_type |=3D BFD_MACH_O_N_EXT; > - } > - > - /* So do undefined 'no_dead_strip's. */ > - if (s->n_desc & BFD_MACH_O_N_NO_DEAD_STRIP) > - s->n_type |=3D BFD_MACH_O_N_EXT; > - > + /* Undefined syms, become extern. */ > + s->n_type |=3D BFD_MACH_O_N_EXT; > + S_SET_EXTERNAL (sp); > + } > + else if (s->symbol.section =3D=3D bfd_com_section_ptr) > + { > + /* ... so to comm. */ > + s->n_type |=3D BFD_MACH_O_N_EXT; > + S_SET_EXTERNAL (sp); > } > else > { > @@ -1353,6 +1450,7 @@ obj_macho_frob_symbol (struct symbol *sp) > { > /* Anything here that should be added that is non-standard. */ > s->n_desc &=3D ~BFD_MACH_O_REFERENCE_MASK; > + s->symbol.udata.i =3D SYM_MACHO_FIELDS_NOT_VALIDATED; > } > else if (s->symbol.udata.i =3D=3D SYM_MACHO_FIELDS_NOT_VALIDATED) > { > @@ -1388,6 +1486,62 @@ obj_macho_frob_symbol (struct symbol *sp) > return 0; > } >=20 > +void > +obj_mach_o_frob_section (asection *sec) > +{ > + bfd_vma sect_size =3D bfd_section_size (stdoutput, sec); > + bfd_mach_o_section *ms =3D bfd_mach_o_get_mach_o_section (sec); > + > + /* Process indirect symbols to determine if we have errors there. */ > + > + switch (ms->flags & BFD_MACH_O_SECTION_TYPE_MASK) > + { > + case BFD_MACH_O_S_SYMBOL_STUBS: > + case BFD_MACH_O_S_LAZY_SYMBOL_POINTERS: > + case BFD_MACH_O_S_NON_LAZY_SYMBOL_POINTERS: > + { > + unsigned int nactual; > + unsigned int ncalc; > + bfd_mach_o_indirect_sym *isym =3D ms->indirectsyms; > + unsigned long eltsiz =3D > + bfd_mach_o_section_get_entry_size (stdoutput, ms); > + > + /* If we somehow added indirect symbols to a section with a zero > + entry size, we're dead ... */ > + if (eltsiz =3D=3D 0 && isym !=3D NULL) > + abort (); gas_assert. > + > + ncalc =3D (unsigned int) (sect_size / eltsiz); > + > + nactual =3D 0; > + if (isym !=3D NULL) > + do > + { > + /* Count it. */ > + nactual++; > + if (isym->sym->symbol.section =3D=3D bfd_und_section_ptr) > + { > + /* If the referenced symbol is undefined, make > + it extern. */ > + isym->sym->n_type |=3D BFD_MACH_O_N_EXT; > + isym->sym->symbol.flags |=3D BSF_GLOBAL; > + } > + } > + while ((isym =3D isym->next) !=3D NULL); > + > + if (nactual !=3D ncalc) > + as_bad (_("there %s %d indirect_symbol%sin section %s but" > + " %d %s expected"), (nactual =3D=3D 1)?"is":"are", nactual, "is":"are" don't make sense with other languages ! > + (nactual =3D=3D 1)?" ":"s ", sec->name, ncalc, > + (ncalc =3D=3D 1)?"is":"are"); > + } > + break; > + > + default: > + break; > + } > +} > + > /* Support stabs for mach-o. */ >=20 > void > diff --git a/gas/config/obj-macho.h b/gas/config/obj-macho.h > index cbc3a4f..ceb1097 100644 > --- a/gas/config/obj-macho.h > +++ b/gas/config/obj-macho.h > @@ -62,6 +62,9 @@ extern void obj_macho_frob_label (struct symbol *); > #define obj_frob_symbol(s, punt) punt =3D obj_macho_frob_symbol(s) > extern int obj_macho_frob_symbol (struct symbol *); >=20 > +#define obj_frob_section(s) obj_mach_o_frob_section(s) > +void obj_mach_o_frob_section (asection *); > + > #define EMIT_SECTION_SYMBOLS 0 >=20 > #define OBJ_PROCESS_STAB(SEG,W,S,T,O,D) obj_mach_o_process_stab(W,S,T,O,D) Tristan.