code review 3491042: 6l, 8l: support for linking ELF and Mach-O .o files

6l, 8l: support for linking ELF and Mach-O .o files

More support for references to dynamic symbols,
including full GOT and PLT for ELF objects.

For Mach-O everything ends up in the GOT: dealing
with the real lazy PLT is too hard for now so we punt.

[rsc, 2010-12-07 22:18:22]

Hello iant (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[iant, 2010-12-07 23:24:06]

LGTM

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c
File src/cmd/6l/asm.c (right):

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode214
src/cmd/6l/asm.c:214: diag("unexpected R_X86_64_64 relocation for dynamic symbol
%s", targ->name);
Note that if I'm following this code correctly, this case can occur if the C
code does something like
    char *p = &array_defined_in_dynamic_object;

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode222
src/cmd/6l/asm.c:222: // TODO: What is the difference between all these?
The difference is in how an out of range reference is detected.

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode269
src/cmd/6l/asm.c:269: if(s->type != SDATA)
Why not SBSS here?

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode341
src/cmd/6l/asm.c:341: addaddrplus(got, lookup(".dynamic", 0), 0);
I think you can just write a zero here, no need to look up the .dynamic section.

http://codereview.appspot.com/3491042/diff/9001/src/cmd/8l/asm.c
File src/cmd/8l/asm.c (right):

http://codereview.appspot.com/3491042/diff/9001/src/cmd/8l/asm.c#newcode319
src/cmd/8l/asm.c:319: addaddrplus(got, lookup(".dynamic", 0), 0);
I think this can just be adduint32(got, 0);

[rsc1, 2010-12-07 23:52:55]

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c
File src/cmd/6l/asm.c (right):

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode214
src/cmd/6l/asm.c:214: diag("unexpected R_X86_64_64 relocation for dynamic symbol
%s", targ->name);
On 2010/12/07 23:24:07, iant wrote:
> Note that if I'm following this code correctly, this case can occur if the C
> code does something like
>     char *p = &array_defined_in_dynamic_object;

I am compiling with -fPIC.  Can that still happen?

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode222
src/cmd/6l/asm.c:222: // TODO: What is the difference between all these?
On 2010/12/07 23:24:07, iant wrote:
> The difference is in how an out of range reference is detected.

I thought that might be it.  Thanks.

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode269
src/cmd/6l/asm.c:269: if(s->type != SDATA)
On 2010/12/07 23:24:07, iant wrote:
> Why not SBSS here?

s is the symbol in which the relocation is found.
A BSS symbol would be uninitialized and not 
contain relocated data references.  No?

http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode341
src/cmd/6l/asm.c:341: addaddrplus(got, lookup(".dynamic", 0), 0);
On 2010/12/07 23:24:07, iant wrote:
> I think you can just write a zero here, no need to look up the .dynamic
section.

Interesting.  I think I got that idea by looking 
at the GOT of a gcc-produced binary.  But maybe
I looked at a running image?

[iant2, 2010-12-08 00:01:12]

rsc@google.com writes:

> http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode214
> src/cmd/6l/asm.c:214: diag("unexpected R_X86_64_64 relocation for
> dynamic symbol %s", targ->name);
>> Note that if I'm following this code correctly, this case can occur if
> the C
>> code does something like
>>      char *p = &array_defined_in_dynamic_object;
>
> I am compiling with -fPIC.  Can that still happen?

I think so.  Initialize a global variable.


> http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode269
> src/cmd/6l/asm.c:269: if(s->type != SDATA)
> On 2010/12/07 23:24:07, iant wrote:
>> Why not SBSS here?
>
> s is the symbol in which the relocation is found.
> A BSS symbol would be uninitialized and not
> contain relocated data references.  No?

Ah, OK.


> http://codereview.appspot.com/3491042/diff/9001/src/cmd/6l/asm.c#newcode341
> src/cmd/6l/asm.c:341: addaddrplus(got, lookup(".dynamic", 0), 0);
> On 2010/12/07 23:24:07, iant wrote:
>> I think you can just write a zero here, no need to look up the
> .dynamic section.
>
> Interesting.  I think I got that idea by looking
> at the GOT of a gcc-produced binary.  But maybe
> I looked at a running image?

Perhaps.  That value is initialized by the dynamic linker.

Ian

[rsc, 2010-12-08 18:56:45]

*** Submitted as 5849cd9393cd ***

6l, 8l: support for linking ELF and Mach-O .o files

More support for references to dynamic symbols,
including full GOT and PLT for ELF objects.

For Mach-O everything ends up in the GOT: dealing
with the real lazy PLT is too hard for now so we punt.

R=iant, iant2
CC=golang-dev
http://codereview.appspot.com/3491042