Perhaps we should move this to another source file? SysTools is already a bit of a grab-bag.

Haven't read over the code carefully or compared it to the ELF spec but I am generally in favor of this approach. If it is so easy to do, then as you say, parsing the ELF format, which has a spec, must be more sensible than parsing the output of readelf, which does not. (Think of the trouble with parsing the output of nm in deriveConstants...)

The situation is a bit delicate though with the existence of D1326 and the timing of 7.10.3. It would be a little unfortunate to add a -pgmreadelf option and then immediately deprecate it in the next version of GHC, but there's also not lots of time to test this patch on exotic systems. Should we just assume that it will work on all ELF platforms? Maybe it's prudent to print a warning and then return Nothing in the event of parse failure, then nothing can really go wrong since this is currently only used to detect whether to avoid relinking the final executable.

In D1381#39667, @rwbarton wrote:

Maybe it's prudent to print a warning and then return Nothing in the event of parse failure, then nothing can really go wrong since this is currently only used to detect whether to avoid relinking the final executable.

Oh, you already implemented this while I was writing my comment. Great.

In D1381#39667, @rwbarton wrote:

(Think of the trouble with parsing the output of `nm` in `deriveConstants`...)

I have code to read ELF symbol information too ;-)

In D1381#39698, @hsyl20 wrote:

In D1381#39667, @rwbarton wrote:

(Think of the trouble with parsing the output of `nm` in `deriveConstants`...)

I have code to read ELF symbol information too ;-)

Unfortunately that part of the code definitely needs to support all systems, and not just those which use the ELF object format. Probably once you add Mach-O and the Windows object format(s) it's not worth it to do the work ourselves.

It would be nice to have some external library which can be used by user programs as well. Thus I'd prefer to see it a separate project pulled in as a git submodule similar to other libraries user in GHC.

There is already http://hackage.haskell.org/package/elf but extracting a section and a note is relatively trivial so I don't think it is worth adding another submodule. However, if we have to manage symbol tables and the like in the future, it could be a solution.

Looks good in general. A few remarks:

• Can you put a link to the ELF specification you based this on in compiler/main/Elf.hs? rts/Linker.c refers to a draft for a "next" version:

These extensions seem to be undocumented in version 4.1 of the ABI and only
appear in the drafts for the "next" version:
   https://refspecs.linuxfoundation.org/elf/gabi4+/contents.html

I also found this draft, dated 10 June 2013: http://www.sco.com/developers/gabi/latest/contents.html
Does the code work with either specification?

• What if the binary contains a huge number of sections, as can happen after @olsner's patch. From https://refspecs.linuxfoundation.org/elf/gabi4+/ch4.sheader.html:

" If the number of sections is greater than or equal to SHN_LORESERVE (0xff00), e_shnum has the value SHN_UNDEF (0) and the actual number of section header table entries is contained in the sh_size field of the section header at index 0 (otherwise, the sh_size member of the initial entry contains 0)."

"If the section name string table section index is greater than or equal to SHN_LORESERVE (0xff00), this member has the value SHN_XINDEX (0xffff) and the actual index of the section name string table section is contained in the sh_link field of the section header at index 0. (Otherwise, the sh_link member of the initial entry contains 0.) "

If it isn't necessary to look at those fields, please explain so in the code.

• You mentioned on irc: "It works on my x86-64 arch but I don't know for other arch". Is more testing needed? Harbormaster only tests x86-64 as well.

• What happens when readElfNote is called with a sectionName that doesn't contain Notes but something else instead. I suppose bad things will happen? If there is no way to prevent this, maybe expand the docstring on readElfNote a little, saying you shouldn't do that.

Nice improvements. I like the refactorings you did, and the Note [ELF specification] is excellent. All my questions were indeed answered.

I double checked the code against the spec (including all calculations, Elf64_Word representing 4 bytes is tricky), and I vouch for its correctness.

I'm not an expert of lazy IO

I'm not either, so a review by someone else might be a good idea. @bgamari or @austin or @rwbarton? (This patch unblocks D1242, which is slated for ghc-8)?

@kgardas: maybe you could check that recomp011 still passes on your platforms using this patch. See testplan. Thanks.

@thomie: Solaris/i386 runs fine. Solaris/amd64 fails with:

=====> recomp011(normal) 1 of 1 [0, 0, 0] 
cd . && $MAKE -s --no-print-directory recomp011    </dev/null > recomp011.run.stdout 2> recomp011.run.stderr
Actual stdout output differs from expected:
Warning: missing newline at end of file ./recomp011.stdout.normalised
Warning: missing newline at end of file ./recomp011.run.stdout.normalised
--- ./recomp011.stdout.normalised       Tue Nov 10 10:25:19 2015
+++ ./recomp011.run.stdout.normalised   Tue Nov 10 10:25:19 2015
@@ -1,10 +1,6 @@
 [1 of 1] Compiling Main             ( Main.hs, Main.o )
 Linking Main ...
 42
-[1 of 1] Compiling Main             ( Main.hs, Main.o ) [B.hsinc changed]
-Linking Main ...
-43
-[1 of 1] Compiling Main             ( Main.hs, Main.o ) [A.hsinc changed]
-Linking Main ...
-4343
-4343
+42
+42
+42
*** unexpected failure for recomp011(normal)

Unexpected results from:
TEST="recomp011"

OpenBSD/amd64 would run fine if there would not be OpenBSD specific warnings:

+++ ./recomp011.run.stderr.normalised   Tue Nov 10 10:30:20 2015
@@ -0,0 +1,27 @@
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsFlags.o): In function `copyArg':
+
+rts/RtsFlags.c:1773:0:
+     warning: warning: strcpy() is almost always misused, please use strlcpy()
+/usr/local/lib/libgmp.so.9.0: warning: warning: vsprintf() is often misused, please use vsnprintf()
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsUtils.o): In function `showStgWord64':
+
+rts/RtsUtils.c:205:0:
+     warning: warning: sprintf() is often misused, please use snprintf()
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsFlags.o): In function `copyArg':
+
+rts/RtsFlags.c:1773:0:
+     warning: warning: strcpy() is almost always misused, please use strlcpy()
+/usr/local/lib/libgmp.so.9.0: warning: warning: vsprintf() is often misused, please use vsnprintf()
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsUtils.o): In function `showStgWord64':
+
+rts/RtsUtils.c:205:0:
+     warning: warning: sprintf() is often misused, please use snprintf()
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsFlags.o): In function `copyArg':
+
+rts/RtsFlags.c:1773:0:
+     warning: warning: strcpy() is almost always misused, please use strlcpy()
+/usr/local/lib/libgmp.so.9.0: warning: warning: vsprintf() is often misused, please use vsnprintf()
+/home/karel/vcs/ghc-src/d1381-test/rts/dist/build/libHSrts.a(RtsUtils.o): In function `showStgWord64':
+
+rts/RtsUtils.c:205:0:
+     warning: warning: sprintf() is often misused, please use snprintf()
\ No newline at end of file
*** unexpected failure for recomp011(normal)

Unexpected results from:
TEST="recomp011"

is there anything I shall test on Solaris/amd64 in order to fix that?

@kgardas: recomp011 actually already fails on Solaris/amd64 with HEAD, see http://haskell.inf.elte.hu/builders/solaris-amd64-head/391/22.html. It has been failing for at least a year: http://haskell.inf.elte.hu/builders/solaris-amd64-head/10/22.html, so we don't really know if this patch makes the problem worse or not.

It is a test for Trac #3589: "Recompilation checker doesn't take into account CPP headers."

The test itself looks ok to me. I don't why it doesn't pass on Solaris/amd64. The patch attached to the ticket doesn't seem to have any platform specific bits: https://git.haskell.org/ghc.git/commitdiff/3f34e0913efcc82dd90be56d04c5e57ec60d3677
I'm afraid this'll be a difficult one to crack.

@thomie: silly me! I've kept in head a need to verify that this test passes on builder, but then submitted too quickly. :-) Of course since it already fails this is no regression so for my platforms this D looks fine.