New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove LD hacks #123

Merged
merged 1 commit into from Nov 22, 2016

Conversation

Projects
None yet
2 participants
@hannesm
Copy link
Contributor

hannesm commented Nov 18, 2016

Makefile.common included first Makeconf, and set AS, CC, LD unconditionally
afterwards. This lead to the LD in Makeconf never having any effect.

It turns out (I didn't yet dig which commit) that FreeBSD's ld (version 2.17.50)
works fine with Solo5 now. I tested this with a) solo5 examples,
b) mirage-skeleton examples up to stackv4 and static_website_tls.

This commit a) removes the LD detection (executed on FreeBSD only), b) moves the .include in Makefile.common to a later point so that we can actually override these variables in Makeconf. It worked before #106, but not after (before ?= was used, after =).

@mato a review would be appreciated. This complements mirage/mirage#692 pretty well.

Here is readelf -S mir-stackv4.virtio compiled with ld 2.17.50:

  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0] (null)            NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .bootstrap        PROGBITS         0000000000100000  00001000
       0000000000000020  0000000000000000  AX       0     0     4
  [ 2] .text             PROGBITS         0000000000101000  00002000
       00000000001116cf  0000000000000000  AX       0     0     4096
  [ 3] .rodata           PROGBITS         0000000000213000  00114000
       00000000000076a8  0000000000000000   A       0     0     16
  [ 4] .eh_frame         X86_64_UNWIND    000000000021a6a8  0011b6a8
       00000000000336b0  0000000000000000   A       0     0     8
  [ 5] .got              PROGBITS         000000000024e000  0014f000
       000000000000e718  0000000000000008  WA       0     0     8
  [ 6] .data             PROGBITS         000000000025c740  0015d740
       00000000000ba440  0000000000000000  WA       0     0     64
  [ 7] .bss              NOBITS           0000000000317000  00217b80
       0000000000019694  0000000000000000  WA       0     0     16
  [ 8] .comment          PROGBITS         0000000000000000  00217b80
       0000000000000052  0000000000000001  MS       0     0     1
  [ 9] .debug_aranges    PROGBITS         0000000000000000  00217be0
       0000000000001890  0000000000000000           0     0     16
  [10] .debug_pubnames   PROGBITS         0000000000000000  00219470
       0000000000007501  0000000000000000           0     0     1
  [11] .debug_info       PROGBITS         0000000000000000  00220971
       000000000002a8b3  0000000000000000           0     0     1
  [12] .debug_abbrev     PROGBITS         0000000000000000  0024b224
       0000000000007479  0000000000000000           0     0     1
  [13] .debug_line       PROGBITS         0000000000000000  0025269d
       000000000002ce28  0000000000000000           0     0     1
  [14] .debug_str        PROGBITS         0000000000000000  0027f4c5
       0000000000007bf0  0000000000000001  MS       0     0     1
  [15] .debug_loc        PROGBITS         0000000000000000  002870b5
       0000000000026a04  0000000000000000           0     0     1
  [16] .debug_macinfo    PROGBITS         0000000000000000  002adab9
       0000000000000041  0000000000000000           0     0     1
  [17] .debug_pubtypes   PROGBITS         0000000000000000  002adafa
       0000000000003a90  0000000000000000           0     0     1
  [18] .debug_ranges     PROGBITS         0000000000000000  002b158a
       00000000000070b0  0000000000000000           0     0     1
  [19] .shstrtab         STRTAB           0000000000000000  002b863a
       00000000000000df  0000000000000000           0     0     1
  [20] .symtab           SYMTAB           0000000000000000  002b8ca0
       00000000000671e8  0000000000000018          21   9900     8
  [21] .strtab           STRTAB           0000000000000000  0031fe88
       000000000005d1b4  0000000000000000           0     0     1

And here with 2.27:

  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0] (null)            NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .bootstrap        PROGBITS         0000000000100000  00001000
       0000000000000020  0000000000000000  AX       0     0     4
  [ 2] .text             PROGBITS         0000000000101000  00002000
       00000000001116cf  0000000000000000  AX       0     0     4096
  [ 3] .rodata           PROGBITS         0000000000213000  00114000
       00000000000076a8  0000000000000000   A       0     0     16
  [ 4] .eh_frame         X86_64_UNWIND    000000000021a6a8  0011b6a8
       00000000000336b0  0000000000000000   A       0     0     8
  [ 5] .got              PROGBITS         000000000024e000  0014f000
       0000000000000018  0000000000000008  WA       0     0     8
  [ 6] .data             PROGBITS         000000000024e040  0014f040
       00000000000ba440  0000000000000000  WA       0     0     64
  [ 7] .bss              NOBITS           0000000000309000  00209480
       0000000000019694  0000000000000000  WA       0     0     16
  [ 8] .comment          PROGBITS         0000000000000000  00209480
       0000000000000052  0000000000000001  MS       0     0     1
  [ 9] .debug_aranges    PROGBITS         0000000000000000  002094e0
       0000000000001890  0000000000000000           0     0     16
  [10] .debug_pubnames   PROGBITS         0000000000000000  0020ad70
       0000000000007501  0000000000000000           0     0     1
  [11] .debug_info       PROGBITS         0000000000000000  00212271
       000000000002a8b3  0000000000000000           0     0     1
  [12] .debug_abbrev     PROGBITS         0000000000000000  0023cb24
       0000000000007479  0000000000000000           0     0     1
  [13] .debug_line       PROGBITS         0000000000000000  00243f9d
       000000000002ce28  0000000000000000           0     0     1
  [14] .debug_str        PROGBITS         0000000000000000  00270dc5
       0000000000007bf0  0000000000000001  MS       0     0     1
  [15] .debug_loc        PROGBITS         0000000000000000  002789b5
       0000000000026a04  0000000000000000           0     0     1
  [16] .debug_macinfo    PROGBITS         0000000000000000  0029f3b9
       0000000000000041  0000000000000000           0     0     1
  [17] .debug_pubtypes   PROGBITS         0000000000000000  0029f3fa
       0000000000003a90  0000000000000000           0     0     1
  [18] .debug_ranges     PROGBITS         0000000000000000  002a2e8a
       00000000000070b0  0000000000000000           0     0     1
  [19] .shstrtab         STRTAB           0000000000000000  0036e1ae
       00000000000000df  0000000000000000           0     0     1
  [20] .symtab           SYMTAB           0000000000000000  002a9f40
       0000000000067218  0000000000000018          21   9902     8
  [21] .strtab           STRTAB           0000000000000000  00311158
       000000000005d056  0000000000000000           0     0     1

I tested this with a FreeBSD-CURRENT (the ld in RELENG_11 is the same, as it is in FreeBSD-11 RELEASE). Would be great if someone else could test this PR and verify that the linker issue is really gone.

remove LD hacks
Makefile.common included first Makeconf, and set AS, CC, LD unconditionally
afterwards.  This lead to the LD in Makeconf never having any effect.

It turns out (I didn't yet dig which commit) that FreeBSD's ld (version 2.17.50)
works fine with Solo5 now.  I tested this with a) solo5 examples,
b) mirage-skeleton examples up to stackv4 and static_website_tls.
@mato

This comment has been minimized.

Copy link
Member

mato commented Nov 20, 2016

At first glance this seems OK. I also noticed that at some point the standard FreeBSD ld started working. Howver, I'd still like to verify which exact commit fixed this. I'll also test it on my FreeBSD machine tomorrow.

Just to confirm, the two readelf dumps above are from the exact same code and build process, just linked with ld vs. /usr/local/bin/ld?

@hannesm

This comment has been minimized.

Copy link
Contributor

hannesm commented Nov 20, 2016

sure. yes, the two readelf are same ocaml source code and binaries, only once using system ld and once the one from ports

@mato

This comment has been minimized.

Copy link
Member

mato commented Nov 21, 2016

As far as I can tell there have been no changes to the FreeBSD system ld. This was probably "fixed" as a side-effect of #76 (fix for #73). However, I still want to investigate this more before I actually merge it, so it'll have to wait a bit longer. Notably, I'd like to understand why .got size is different in the two outputs you posted.

@hannesm

This comment has been minimized.

Copy link
Contributor

hannesm commented Nov 21, 2016

sure. I was surprised by this as well. FreeBSD has not changed its ld since Feb 11 2015 (see https://github.com/freebsd/freebsd/commits/master/contrib/binutils/ld)

@mato

This comment has been minimized.

Copy link
Member

mato commented Nov 22, 2016

I've done some more tests with the Solo5 standalone tests and stackv4. The only material difference in readelf -S output is that the GNU ld output has a much smaller or non-existent .got, consistent with behaviour on Linux.

My theory is this is due to the newer (GNU) ld being smarter about removing unneeded output sections. So, this change should not break anything. However: binaries linked with the system ld will be slightly larger on disk and in memory (the difference with stackv4 is about 57kB).

@hannesm If you're OK with the above analysis and caveat then I'm happy to merge this.

@mato

This comment has been minimized.

Copy link
Member

mato commented Nov 22, 2016

Also just verified against static_website_tls, identical behaviour, size difference is 128kB.

@hannesm

This comment has been minimized.

Copy link
Contributor

hannesm commented Nov 22, 2016

I'm ok with merging, and thanks for the detailed analysis. I'm willing to accept the extension of 57kB/128kB.

@mato mato merged commit 7f78e69 into Solo5:master Nov 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hannesm hannesm deleted the hannesm:no-ld branch Nov 22, 2016

hannesm added a commit to hannesm/solo5 that referenced this pull request Dec 14, 2016

mato added a commit that referenced this pull request Dec 19, 2016

Merge pull request #147 from hannesm/no-ld
remove unnecessary depext to binutils on FreeBSD since #123 is merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment