Avoid a spurious "*ABS*" symbol (was: Re: stage one of gas reloc rewrite)
Hans-Peter Nilsson
hp@bitrange.com
Sun Sep 1 15:22:00 GMT 2002
On Sun, 1 Sep 2002, Alan Modra wrote:
> The symbol being added is the absolute section symbol. You'll find
> that in these cases it is in fact being used in relocs, so it's the
> generally correct thing to do. Note the comment at the end of
> adjust_reloc_syms about BFD not handling relocs without symbols very
> well. The comment is right; bfd_install_relocation will segfault
> without a symbol.
I assume you mean the comment in this code:
if (fixp->fx_addsy)
...
else
{
/* There was no symbol required by this relocation. However,
BFD doesn't really handle relocations without symbols well.
So fake up a local symbol in the absolute section. */
fixp->fx_addsy = section_symbol (absolute_section);
}
I'm confused; it looks like the code and comment has things
mixed up. Yeah, the BFD comment is right. But what does that
have to do with setting fixp->fx_addsy here? If a tc_gen_reloc
sees fixp->fx_addsy == NULL, it should itself put the absolute
section symbol in the created reloc; no need to set fx_addsy
here in adjust_reloc_syms.
I did some experimental code changes on top of your gas reloc
rewrite patch (leaving out the test-suite changes) to allow for
fx_addsy == NULL. This got rid of that emitted "*ABS*" symbol.
(BTW it wasn't even marked STT_SECTION in the ELF object, hah! ;-)
With this, no MMIX test-suite tweaks were needed after the
cleanup I committed yesterday which parts of your patch
addressed (which confused me until I noticed the results weren't
clean before your patch!).
These are just minimal changes. I'm interested in cleaning up
those changes and patch ports as necessary (and adjust the
test-suite results if needed of course). Note that the
section_symbol change is a true bug-fix; symbol_section_p would
not identify the abs section symbol. To proceed I need
feedback:
- Do you (Alan and other people) agree this (handling fx_addsy
NULL for fixups for constant expressions -- usually fx_pcrel)
is the right route?
- What amount of testing would be needed? Would it suffice that
none of the targets you (Alan) mentioned have test-suite
regressions? Or what else would be needed?
Some alternatives I have *not* tried:
- Don't call symbol_mark_used_in_reloc if fx_addsy is the abs
section symbol in fixup_segment. IMHO this'd be just as
hackish as setting fx_addsy to the abs section symbol in the
first place.
- Add hook to call symbol_clear_used_in_reloc for the abs
section symbol for ELF (or just MMIX).
Bad for two reasons: port- or object-format-local and adding
a new hook: there was frob_file_before_adjust and now there'd
be a frob_file_after_adjust!
- Redefine the object-format hook EMIT_SECTION_SYMBOLS to be
EMIT_SECTION_SYMBOLS(seg), meaning don't emit a section symbol
for SEG and set it to "seg != absolute_section" for ELF. With
this route there'd still be a question why gas thought there
was a need for the abs section symbol in the first place.
gas:
* write.c (adjust_reloc_syms): Don't set fixp->fx_addsy to
section_symbol (absolute_section) if NULL.
(write_relocs): Handle fixups with NULL fx_addsy.
(fixup_segment) [BFD_ASSEMBLER]: Don't set fixP->fx_addsy to
section_symbol (absolute_section) if NULL for fx_pcrel fixups.
* subsegs.c (section_symbol): Mark BFD symbol with
BSF_SECTION_SYM.
* config/tc-mmix.c (md_apply_fix3): A fixP->fx_addsy == NULL
with fixP->fx_pcrel cannot be processed at assembly time.
(tc_gen_reloc): When fixP->fx_addsy == NULL, use the absolute
section symbol for the reloc symbol pointer.
diff -cpr ./gas/config/tc-mmix.c ../rewsrc/gas/config/tc-mmix.c
*** ./gas/config/tc-mmix.c Sun Sep 1 23:07:15 2002
--- ../rewsrc/gas/config/tc-mmix.c Sun Sep 1 20:30:29 2002
*************** md_apply_fix3 (fixP, valP, segment)
*** 2415,2420 ****
--- 2415,2425 ----
fixP->fx_done = 0;
return;
}
+ else if (fixP->fx_addsy == NULL && fixP->fx_pcrel)
+ {
+ fixP->fx_done = 0;
+ return;
+ }
else if (fixP->fx_r_type == BFD_RELOC_MMIX_LOCAL
|| fixP->fx_r_type == BFD_RELOC_VTABLE_INHERIT
|| fixP->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
*************** tc_gen_reloc (section, fixP)
*** 2849,2855 ****
relP = (arelent *) xmalloc (sizeof (arelent));
assert (relP != 0);
relP->sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
! *relP->sym_ptr_ptr = baddsy;
relP->address = fixP->fx_frag->fr_address + fixP->fx_where;
relP->addend = addend;
--- 2854,2863 ----
relP = (arelent *) xmalloc (sizeof (arelent));
assert (relP != 0);
relP->sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
! *relP->sym_ptr_ptr
! = (baddsy == NULL
! ? symbol_get_bfdsym (section_symbol (absolute_section))
! : baddsy);
relP->address = fixP->fx_frag->fr_address + fixP->fx_where;
relP->addend = addend;
diff -cpr ./gas/subsegs.c ../rewsrc/gas/subsegs.c
*** ./gas/subsegs.c Sat Jul 6 21:53:41 2002
--- ../rewsrc/gas/subsegs.c Sun Sep 1 23:02:19 2002
*************** section_symbol (sec)
*** 556,561 ****
--- 556,564 ----
if (obj_sec_sym_ok_for_reloc (sec))
symbol_set_bfdsym (s, sec->symbol);
+ /* Mark it properly as a BFD section symbol. See also symbol_section_p. */
+ symbol_get_bfdsym(s)->flags |= BSF_SECTION_SYM;
+
seginfo->sym = s;
return s;
}
diff -cpr ./gas/write.c ../rewsrc/gas/write.c
*** ./gas/write.c Sun Sep 1 23:07:14 2002
--- ../rewsrc/gas/write.c Sun Sep 1 19:56:53 2002
*************** adjust_reloc_syms (abfd, sec, xxx)
*** 883,895 ****
print_fixup (fixp);
#endif
}
- else
- {
- /* There was no symbol required by this relocation. However,
- BFD doesn't really handle relocations without symbols well.
- So fake up a local symbol in the absolute section. */
- fixp->fx_addsy = section_symbol (absolute_section);
- }
dump_section_relocs (abfd, sec, stderr);
}
--- 883,888 ----
*************** write_relocs (abfd, sec, xxx)
*** 949,955 ****
symbol, then generate the reloc against the latter symbol
rather than the former. */
sym = fixp->fx_addsy;
! while (symbol_equated_reloc_p (sym))
{
symbolS *n;
--- 942,948 ----
symbol, then generate the reloc against the latter symbol
rather than the former. */
sym = fixp->fx_addsy;
! while (sym != NULL && symbol_equated_reloc_p (sym))
{
symbolS *n;
*************** fixup_segment (fixP, this_segment)
*** 2709,2716 ****
{
#ifndef BFD_ASSEMBLER
fixP->fx_addsy = &abs_symbol;
- #else
- fixP->fx_addsy = section_symbol (absolute_section);
#endif
}
}
--- 2702,2707 ----
brgds, H-P
More information about the Binutils
mailing list