Re: PATCH: Fix SHF_LINK_ORDER and SHT_IA_64_UNWIND handling

["H. J. Lu" <hjl at lucon dot org>]

On Fri, Jul 23, 2004 at 06:00:46PM -0700, Jim Wilson wrote:
> On Thu, 2004-07-22 at 10:12, H. J. Lu wrote:
> > The current linker/assembler don't handle SHF_LINK_ORDER right. Linker
> > never tries to preserve it at all. IA64 uses section name for
> > SHT_IA_64_UNWIND, which has SHF_LINK_ORDER. It doesn't work with
> > multiple text sections with the same same.
> 
> In general, I think this is an improvement.  Saving a pointer to the
> text section in the assembler is much better than trying to deduce it in
> the linker from section names.
> 
> However, I have a number of questions that came up while looking at this
> patch.
> 1) The gABI says that SHF_LINK_ORDER causes sh_info to be set.  But you
> are setting sh_link.  This seems wrong.

Yes, it is confusing. The gABI says different things in different
places. I am trying to get a clarification on that.

> 2) Despite the comments in the code, I can't find anyplace in the IA-64
> psABI or the IA-64 SCRA that clearly says we have to set SHF_LINK_ORDER
> and/or sh_link.  The psABI just refers to the SCRA, and the SCRA says
> nothing about section header contents.  My copies are up to date
> according to the Intel developer web site.  Can you point to anywhere
> where this is documented?

The psABI only says .IA64.unwind section has SHF_LINK_ORDER and nothing
more. I will ask. I think it makes senses to have it linked to the
corresponding text section.

> 3) The comments imply that HPUX got it wrong by setting sh_info, but
> HPUX seems to be the right one, as it agrees with the gABI.  It looks
> like we got it wrong when we thought sh_link had to be set at all.  We
> can probably drop all this stuff about sh_link and HPUX, and just set
> sh_info as the gABI says to.

And fix the the gABI.

> 4) The ia64_elf_section_change_hook patch looks wrong.  You are
> unconditionally setting elf_linked_to_section, which might overwrite a
> previous value set by start_unwind_section.  Why do you need this?  If
> it is needed, then it should be somehow conditional on it not already
> being set, such as the byteorder setting is already done.

For SHT_IA_64_UNWIND section, ia64_elf_section_change_hook is always
called before link-to section is set in start_unwind_section. It is
needed by the testcase:

        .section .IA_64.unwind, "ao", "unwind"
	data8 1234

I added this testcase. Now I am not sure if we should support it at
all.

> 5) For the elf.c change, I haven't tried to figure out if it is right
> except as mentioned above that setting sh_link is wrong, as it should be
> setting sh_info.  If "if" part looks fine.  The "else" part is using bfd
> stuff that I am unfamiliar with.  I would need some kind of testcase to
> be able to figure out what it is doing.

It is for "ld -r". Try

# ld -r -o foo.o x.o x.o
# readelf -gS foo.o

Watch for sh_link of SHT_IA_64_UNWIND sections. It isn't set without
my patch.


H.J.

Attachment: x.o
Description: Binary data

Attachment: y.o
Description: Binary data