Incompatibility of PIC and non-PIC

Hi,

Shared libraries work well, but we've got a bigger problem now. Our PIC and non-PIC code models are mutually incompatible in a big way (due to the use of %r19 And, it seems, while it's fine if non-PIC can't be used in shared libraries, it's not all right if PIC can't be linked statically.

The reason this comes up is that libgcc.a has to be built with -fPIC since it gets linked into libc.so. However libgcc.a is also getting statically linked with the binary itself (the symbols are not exported by libc.so, though I'm not sure if this is the correct behaviour or not), and of course it has to get linked in for statically linked things anyway.

The result is of course that anything which uses 'long long', like, say, GCC, will crash.

How do other architectures solve this issue? (or do they just have ABIs that let you mix PIC and non-PIC...)

For that matter how does 32-bit HP/UX solve this issue?

One potential solution is to use %dp as the data pointer across the board (which would solve the problem of mixing PIC and non-PIC in static binaries), and generate export stubs for shared library calls which restore the old %dp value (which would solve the problem of maintaining the non-PIC %dp, useless though it may be).

<thinking wishful="wishful">
Another one is to ditch our ... ahem ... unique ABI, use function prologues instead, have a single global pointer and a single link register, and have PIC, function pointers and dynamic linking work the way $DEITY intended :-)
</thinking>

Of course that's a lot more work and would probably involve breaking the published ELF standard for HPPA, such as it is, as well as foregoing a lot of code sharing with the rest of the PA-RISC stuff in gcc and binutils :-(

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

The libgcc problem can be solved by generating a PIC libgcc.so and non-PIC libgcc.a

__builtin_return_address. See the hoops you'll be jumping through in gcc/config/pa/pa.c:return_addr_rtx.

One way to keep out current ABI is to generate a .plt and import stubs when statically linking PIC code. That should be relatively easy to do in the linker.

Alan

Linuxcare.  Support for the Revolution.

A bit of head-scratching, a couple of added functions, and the linker now detects PIC functions and handles them appropriately.

bfd/ChangeLog
* elf32-hppa.c (elf32_hppa_link_hash_entry): Add pic_call. (hppa_link_hash_newfunc): Init it. (elf32_hppa_link_hash_table): Add pic_call_detected. (elf32_hppa_link_hash_table_create): Init it. multi_subspace too. (hppa_type_of_stub): Use an import stub if pic_call. (elf32_hppa_check_relocs): Set SEC_HAS_GOT_REF appropriately. Don't just create the .got on GOT references, create a .plt too. (hppa_handle_PIC_calls): New function. (elf32_hppa_size_dynamic_sections): Call it. (hppa_setup_plt_PIC_call): New function. (elf32_hppa_build_stubs): Call it.
* section.c (SEC_HAS_GOT_REF): Define new flag for asection.
* bfd_in2.: Regenerate.

The linker detects PIC functions by noting DLTIND relocations within a section. If there aren't any, it's either not PIC, or it's PIC that doesn't use the .got Without .got references, we won't need to set up r19. Ta DA!

Alan Modra

Linuxcare.  Support for the Revolution.

Sorry to burst your bubble but this doesn't work in the real world, where "real world" is defined as 'hppa-linux-gcc -o hello hello.c'

In fact this new linker ends up marking basically everything as potentially PIC, creating unnecessary import stubs for most functions in libc.a, and *not* actually switching the relocations to point to import stubs in the case where I am actually calling PIC in libgcc.a.

Oh and it segfaults on undefined weak symbols though that's easily fixed.

I'm not even sure where to start fixing this. I guess I'll try to find something else to do today :(

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

Actually it wasn't so bad after all. Here's a patch that fixes the case of statically linked binaries, though I'm reluctant to check it in because I'm not quite sure the logic is 100% correct; without removing the check for defined weak symbols, we end up generating a *lot* of useless import stubs when linking with a static libc, but this might not be the right way to avoid this.

It's not fixed in the case of dynamically linked binaries that are statically linked with PIC (which is basically all dynamically linked binaries since libgcc.a is linked statically). It might make sense to unify the generation of import stubs and PLT slots for all PIC symbols whether they are linked statically or dynamically, but I don't know if I want to attempt that myself :-)

Oh, also, I've been bad about ChangeLog entries lately, sorry - should I retroactively create some for the recent checkins I've made?

2000-08-18 David Huggins-Daines <[email protected]>

* elf32-hppa.c (final_link_relocate): Call an import stub in the case of statically linked PIC symbols. (hppa_handle_PIC_calls): Don't try to handle undefined weak symbols at all, and don't create import stubs for locally defined weak symbols unless they are PIC.

Index: bfd/elf32-hppa.c
===================================================================
RCS file: /home/cvs/parisc/binutils-2.10/bfd/elf32-hppa.c,v retrieving revision 1.22

diff -u -r1.22 elf32-hppa.c
@@ -1961,8 +1961,8 @@
       || (h->elf_link_hash_flags & ELF_LINK_HASH_NEEDS_PLT) != 0)
     {
       if (h->plt.refcount <= 0
+	  || h->root.type == bfd_link_hash_undefweak
 	  || ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) != 0
-	      && h->root.type != bfd_link_hash_defweak
 	      && (!info->shared || info->symbolic)
 	      && !(!info->shared
 		   && (h->root.type == bfd_link_hash_defined

@@ -1971,13 +1971,13 @@
 	{
 	  /* The .plt entry is not needed when:
 	     a) Garbage collection has removed all references to the
-	     symbol, or
-	     b) We know for certain the symbol is defined in this
-	     object, and it's not a weak definition.  Either this
-	     object is the application or we are doing a shared
-	     symbolic link.  As a special sop to the hppa ABI, we
-	     keep a .plt entry for functions in sections containing
-	     PIC code.  */
+	     symbol.
+	     b) It's an undefined weak symbol.
+	     c) We know for certain the symbol is defined in this
+	     object.  Either this object is the application or we are
+	     doing a shared symbolic link.  As a special sop to the
+	     hppa ABI, we keep a .plt entry for functions in sections
+	     containing PIC code.  */
 	  h->plt.offset = (bfd_vma) -1;
 	  h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
 	  return true;

@@ -2851,8 +2851,11 @@
     case R_PARISC_PCREL17F:
     case R_PARISC_PCREL22F:
       /* If this is a call to a function defined in another dynamic
-	 library, then find the import stub in the stub hash.  */
-      if (sym_sec == NULL || sym_sec->output_section == NULL)
+	 library, or if it is a call to a PIC function in the same
+	 object, then find the import stub in the stub hash.  */
+      if (sym_sec == NULL
+	  || sym_sec->output_section == NULL
+	  || (h != NULL && h->pic_call))
 	{
 	  stub_entry = hppa_get_stub_entry (input_section, sym_sec,
 					    h, rel, info);

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

I was thingk about it in the shower this morning, and _knew_ I'd be seeing some mail from you. You see, I really missed the whole point: The special PIC stubs are really only needed when intermixing calls to dynamic libraries and statically linked PIC routines - which case we currently ignore. The fully statically linked case be solved by changing gcc to reserve r19, although I think I'll leave the code there to add the stubs and .plt entries. With your fixes, of course!

Alan Modra

Linuxcare.  Support for the Revolution.

In some ways not having any hardware to run hppa-linux binaries on is good, as it gives me some excuse. :-)

Let me know how this one goes.

bfd/ChangeLog
2000-08-19 Alan Modra <[email protected]>

* elf32-hppa.c (hppa_setup_plt_PIC_call): Move function into.. (hppa_build_one_stub): ..here.
(elf32_hppa_build_stubs): Don't call it. (elf32_hppa_create_dynamic_sections): Don't try to create the .plt and .got twice.
(elf32_hppa_adjust_dynamic_symbol): Handle pic_call here. Don't make symbol dynamic, and don't emit a reloc if pic_call. (hppa_handle_PIC_calls): Simplify as we.. (elf32_hppa_size_dynamic_sections): ..only call hppa_handle_PIC_calls when not shared. Don't create DT_PLTREL etc. entries unless we have a .rela.plt section. (elf32_hppa_finish_dynamic_symbol): Set up .plt entries with final relocated values when it's for a pic_call function we include in the link. Don't create relocs for pic_call either.

2000-08-18 David Huggins-Daines <[email protected]>

* elf32-hppa.c (final_link_relocate): Look for an import stub in the case of statically linked PIC symbols.

Linuxcare.  Support for the Revolution.

Well I'm enjoying tracking these problems down, even if the solutions I propose are usually wrong :-)

Quite well, once I did this:

Index: bfd/elf32-hppa.c
===================================================================
RCS file: /home/cvs/parisc/binutils-2.10/bfd/elf32-hppa.c,v retrieving revision 1.24

diff -u -p -r1.24 elf32-hppa.c
@@ -957,7 +957,8 @@ hppa_build_one_stub (gen_entry, in_arg)
 
       if (!info->shared
 	  && stub_entry->h != NULL
-	  && stub_entry->h->pic_call)
+	  && stub_entry->h->pic_call
+	  && stub_entry->h->elf.plt.offset != (bfd_vma) -1)
 	{
 	  /* Build the .plt entry needed to call a PIC function from
 	     statically linked code.  We don't need any relocs.  */

I'm not sure why it's generating multiple stubs for the same symbol, though.

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

Oh, that patch was obviously bogus. Sorry. The real bogosity is setting elf.plt.offset to -1 in order to prevent finish_dynamic_symbol from filling in the PLT entry, which, aside from overwriting (part of) the first PLT entry with the target for the last import stub, also causes these stubs themselves to contain references to PLT - 1 (these immediately cause SIGBUS at the moment, which is probably a good thing).

This is doubly strange because finish_dynamic_symbol has a check for h->pic_call in it already :-)

I've hacked around it for my own purposes but I imagine you know best what to do.

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

I think just deleting these lines (which predated the check in finish_dynamic_symbol) in build_one_stub

	  /* Flag this .plt entry as having been filled in so that
	     finish_dynamic_symbol doesn't repeat our work here.  */
	  eh->elf.plt.offset = (bfd_vma) -1;

will do the trick. I simply forgot to remove it after fixing finish_dynamic_symbol to genereate correct .plt entries for the cases where we don't have a dynamic relocation.

The problem is that you get one stub per input section to ensure that the stub can be reached. Messing with plt.offset on the first stub has dire consequences for subsequent ones.

Alan

Linuxcare.  Support for the Revolution.

cvs update. I think I've done things properly this time, although you'll no doubt find some silly bugs left when you actually run code on real hardware.

BTW, Adding "-p" to your diff options would be a good idea.

Regards, Alan

Linuxcare.  Support for the Revolution.

so here's a .cvsrc file which should help people:

cvs -z3 -q
diff -uNp
update -Pd
rm -f

btw, is there any option to cvs diff which produces files which can be applied by any known version of patch? in my experience, patch can't find files specified by Index: lines _unless_ both the +++ and the --- files can't be found -- which is rarely the case with common names such as Makefile or ChangeLog. I hacked a copy of patch myself to apply one of the cvs diff patches you posted here, but there surely has to be a better way (complaining to Paul Eggert that while patch may or may not be POSIX or GNU compliant it isn't _USEFUL_ may be the right answer here :-)

Revolutions do not require corporate support.

I haven't found one, and it's a real pain. I tend to edit patches covering multiple directories before passing them to patch.

"sed -e '/^[+-][+-]/d' < cvs_diff_file | patch -p0 --posix" ought to work.

Alan

Linuxcare.  Support for the Revolution.

Here's a tiny Perl script I wrote for this purpose once in a fit of pique. Use the -i option to specify the 'input' directory prefix (i.e. in the --- line) and -o to specify the 'output' directory prefix.

#!/usr/bin/perl -p
use strict;
use File::Basename;
use vars qw($in $out $path $name $dir);

BEGIN {
    my @args;
    while (defined($_ = shift)) {
	if (/-i/) {
	    $in = shift;
	    $in =~ s,$,/,;
	    $in =~ tr,/,,s;
	} elsif (/-o/) {
	    $out = shift;
	    $out =~ s,$,/,;
	    $out =~ tr,/,,s;
	} else {
	    push @args, $_;
	}
    }
    $out ||= $in;
    $in  ||= $out;
    @ARGV=@args;
}

/^Index: (.*)/ and do {
    ($name, $dir) = fileparse($1);
};
/^(---|\+\+\+) ([^\t]+)/ and do {
    die "Your CVS isn't broken, $2 doesn't match $name\n" unless $2 eq $name;
    my $prefix = ($1 eq '---') ? $in : $out;
    s,$2,${prefix}${dir}${name},;
};

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.

Yeah, that message got retired out of order, so to speak. Trying your new stuff now.

[email protected], http://www.linuxcare.com/
Linuxcare. Support for the revolution.