MIPS multigot fixes for Linux
Daniel Jacobowitz
drow@mvista.com
Wed Nov 12 22:02:00 GMT 2003
More information about the Binutils mailing list
Wed Nov 12 22:02:00 GMT 2003
- Previous message (by thread): MIPS multigot fixes for Linux
- Next message (by thread): MIPS multigot fixes for Linux
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On Wed, Nov 12, 2003 at 09:47:55PM +0000, Richard Sandiford wrote: > Daniel Jacobowitz <drow@mvista.com> writes: > > On Tue, Nov 11, 2003 at 11:43:39PM +0000, Richard Sandiford wrote: > > > It looks like the cases are: > > > > > > A B C D E > > > ------------------------------------------------------ > > > symbol defined locally? Y Y N N N > > > symbol defined externally? - - N Y - > > > creating a DSO? N Y N N Y > > > ------------------------------------------------------ > > > relocation needed? N Y N Y Y > > > ------------------------------------------------------ > > > glibc relocation subtrahend: - 0 - 0 0 > > > initial glibc entry: S 0 S=0 S=0 S=0 > > > ------------------------------------------------------ > > > irix relocation subtrahend: - S - S=0 S=0 > > > initial irix entry: S S S=0 S=0 S=0 > > > ------------------------------------------------------ > > > > > > where S = st_value. Like you say, none of the symbols can be lazily > > > bound, so S should be 0 for all undefined symbols. And it's really > > > just (B) that needs special handling. > > > > > > Does that match your thinking? If so, then your change to > > > _bfd_mips_elf_finish_dynamic_symbol looks good to me: > > > > I'm not entirely sure what you mean for case C. > > Undefined weak symbols. > > > > Looking at the code above, I see VALUE is set by: > > > > > > if (info->shared > > > || h->root.type == bfd_link_hash_undefined > > > || h->root.type == bfd_link_hash_undefweak) > > > value = 0; > > > else if (sym->st_value) > > > value = sym->st_value; > > > else > > > value = h->root.u.def.value; > > > > > > If the table above is right, then I think your patch will make this > > > equivalent to: > > > > > > value = st_value; > > > > > > which IMO makes things easier to follow. > > > > And later: > > > Uh. Equivalent for _glibc_. For irix, it fixes a bug, since the > > > relocation field should contain the symbol value even if info->shared. > > > > I'm not sure about this. I'd rather not make that change myself, since > > I don't understand it - for instance, I don't understand why > > h->root.u.def.value was there in the first place. Did it have > > something to do with vestigial quickstart code? > > Yeah, I think so. h->root.u.def.value would be the value of the symbol > found in an external object. And if we supported quickstart, that would > indeed be the right value to use, since it's also the value we'd put in > the primary GOT. > > If we apply your patch, then VALUE is only inserted into the GOT if > SGI_COMPAT or if no relocation is needed. And going by the table above, > I think the value we want is always st_value in this case. > > > Besides, I don't see what you mean. If the symbol has a value but we > > are emitting a relocation (for data objects, for instance) we must > > still put 0 in the GOT for glibc. > > But doesn't your patch take care of that? My point is that (from the > irix line above) the "right" thing to do is insert st_value for all > cases. And your patch successfully works around the cases in glibc > where that isn't true. Both changes together should do the right thing. OK, I see where you're going now. And if > > > || h->root.type == bfd_link_hash_undefined > > > || h->root.type == bfd_link_hash_undefweak) then sym->st_value is presumably 0. "value" is also passed to create_dynamic_relocation but will not be used, so this should be safe. Revised attached. I haven't got time to retest it right now, though. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2003-11-12 Daniel Jacobowitz <drow@mvista.com> * elfxx-mips.c (mips_elf_set_global_got_offset): Don't set no_fn_stub. (mips_elf_set_no_stub): New function. (mips_elf_multi_got): Call it. (_bfd_mips_elf_finish_dynamic_symbol): Fill relocated GOT entries with zero for ! SGI_COMPAT. Simplify calculation of value. Index: elfxx-mips.c =================================================================== RCS file: /big/fsf/rsync/src-cvs/src/bfd/elfxx-mips.c,v retrieving revision 1.78 diff -u -p -r1.78 elfxx-mips.c --- elfxx-mips.c 13 Oct 2003 19:51:09 -0000 1.78 +++ elfxx-mips.c 12 Nov 2003 22:01:31 -0000 @@ -2338,10 +2338,6 @@ mips_elf_set_global_got_offset (entryp, BFD_ASSERT (g->global_gotsym == NULL); entry->gotidx = arg->value * (long) g->assigned_gotno++; - /* We can't do lazy update of GOT entries for - non-primary GOTs since the PLT entries don't use the - right offsets, so punt at it for now. */ - entry->d.h->no_fn_stub = TRUE; if (arg->info->shared || (elf_hash_table (arg->info)->dynamic_sections_created && ((entry->d.h->root.elf_link_hash_flags @@ -2357,6 +2353,30 @@ mips_elf_set_global_got_offset (entryp, return 1; } +/* Mark any global symbols referenced in the GOT we are iterating over + as inelligible for lazy resolution stubs. */ +static int +mips_elf_set_no_stub (entryp, p) + void **entryp; + void *p ATTRIBUTE_UNUSED; +{ + struct mips_got_entry *entry = (struct mips_got_entry *)*entryp; + + if (entry->abfd != NULL && entry->symndx == -1 + && entry->d.h->root.dynindx != -1) + { + /* We can't do lazy update of GOT entries for non-primary GOTs since + the PLT entries don't use the right offsets, so punt at it for now. + We set this here because we are called via mips_elf_multi_got + before _bfd_mips_elf_adjust_dynamic_symbol reads the no_fn_stub + flag; this only matters for the global case, but + _bfd_mips_elf_size_dynamic_sections is too late. */ + entry->d.h->no_fn_stub = TRUE; + } + + return 1; +} + /* Follow indirect and warning hash entries so that each got entry points to the final symbol definition. P must point to a pointer to the hash table we're traversing. Since this traversal may @@ -2624,6 +2644,11 @@ mips_elf_multi_got (abfd, info, g, got, g->next = gg->next; gg->next = g; g = gn; + + /* Mark global symbols in every non-primary GOT as ineligible for + stubs. */ + if (g) + htab_traverse (g->got_entries, mips_elf_set_no_stub, NULL); } while (g); @@ -6662,7 +6687,6 @@ _bfd_mips_elf_finish_dynamic_symbol (out bfd_vma offset; bfd_vma value; Elf_Internal_Rela rel[3]; - bfd_vma addend = 0; gg = g; @@ -6670,14 +6694,10 @@ _bfd_mips_elf_finish_dynamic_symbol (out e.symndx = -1; e.d.h = (struct mips_elf_link_hash_entry *)h; - if (info->shared - || h->root.type == bfd_link_hash_undefined - || h->root.type == bfd_link_hash_undefweak) - value = 0; - else if (sym->st_value) - value = sym->st_value; - else - value = h->root.u.def.value; + /* Since we don't support Quickstart, the initial value of the GOT + entry should be simply the symbol's value. With the caveat about + glibc, below. */ + value = sym->st_value; memset (rel, 0, sizeof (rel)); rel[0].r_info = ELF_R_INFO (output_bfd, 0, R_MIPS_REL32); @@ -6693,18 +6713,30 @@ _bfd_mips_elf_finish_dynamic_symbol (out MIPS_ELF_PUT_WORD (output_bfd, value, sgot->contents + offset); - if ((info->shared - || (elf_hash_table (info)->dynamic_sections_created - && p->d.h != NULL - && ((p->d.h->root.elf_link_hash_flags - & ELF_LINK_HASH_DEF_DYNAMIC) != 0) - && ((p->d.h->root.elf_link_hash_flags - & ELF_LINK_HASH_DEF_REGULAR) == 0))) - && ! (mips_elf_create_dynamic_relocation - (output_bfd, info, rel, - e.d.h, NULL, value, &addend, sgot))) - return FALSE; - BFD_ASSERT (addend == 0); + if (info->shared + || (elf_hash_table (info)->dynamic_sections_created + && p->d.h != NULL + && ((p->d.h->root.elf_link_hash_flags + & ELF_LINK_HASH_DEF_DYNAMIC) != 0) + && ((p->d.h->root.elf_link_hash_flags + & ELF_LINK_HASH_DEF_REGULAR) == 0))) + { + bfd_vma addend = 0; + + /* ??? The dynamic linker should subtract the original + value of the symbol's GOT entry (which is always + st_value) and add in the final value. However, glibc's + ld.so just adds the final value, so the in-place addend + must be zero. */ + if (!SGI_COMPAT (output_bfd)) + MIPS_ELF_PUT_WORD (output_bfd, 0, sgot->contents + offset); + + if (! (mips_elf_create_dynamic_relocation + (output_bfd, info, rel, + e.d.h, NULL, value, &addend, sgot))) + return FALSE; + BFD_ASSERT (addend == 0); + } } } }
- Previous message (by thread): MIPS multigot fixes for Linux
- Next message (by thread): MIPS multigot fixes for Linux
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the Binutils mailing list