Opened 2 years ago

Closed 2 years ago

#10974 closed bug (fixed)

Fix SysTools.readElfSection on platforms where "readelf" have different name

Reported by: ony Owned by:
Priority: normal Milestone: 8.0.1
Component: Compiler Version: 7.10.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case: driver/recomp011
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1326 Phab:D1335 Phab:D1381
Wiki Page:

Description

Some linux systems may use target-prefixed names for binutils. For example on my Exherbo Linux readelf is named as x86_64-pc-linux-gnu-readelf. This leads to errors like:

readelf: createProcess: runInteractiveProcess: exec: does not exist (No such file or directory)

To fix this GHC needs to give ability to configure name of this tool (like SysTools.runSplit and SysTools.runCc) or use libelf-like library.

Change History (13)

comment:1 Changed 2 years ago by ony

Status: newpatch

comment:2 Changed 2 years ago by thomie

Differential Rev(s): Phab:D1326
Milestone: 8.0.1

comment:3 in reply to:  2 Changed 2 years ago by ony

Replying to thomie: As I understand I'll cherry-pick this change to branch ghc-7.10 it will go into GHC 7.10.3 (or whatever will come out after change will be accepted).

comment:4 Changed 2 years ago by thomie

Milestone: 8.0.17.10.3

comment:5 Changed 2 years ago by ony

Differential Rev(s): Phab:D1326Phab:D1326 Phab:D1335

Adapted change https://phabricator.haskell.org/D1335 for ghc-7.10 branch.

comment:6 Changed 2 years ago by bgamari

Milestone: 7.10.38.0.1

Merged Phab:D1335 (updated for consistency with existing tools autoconf usage) to 7.10.3. Still need to merge to master.

comment:7 Changed 2 years ago by hsyl20

Is there a way to use Data.Binary.Get in Systools? We could totally avoid using readelf and it should be faster too (no fork, no parsing, no need to use String).

comment:8 Changed 2 years ago by thomie

Add binary to Build-Depends in compiler/ghc.cabal.in (and compiler/ghc.cabal, so you don't have to rerun configure).

comment:9 in reply to:  8 ; Changed 2 years ago by ony

Replying to hsyl20:

Is there a way to use Data.Binary.Get in Systools? We could totally avoid using readelf and it should be faster too (no fork, no parsing, no need to use String).

Data.Binary.Get is also monad parser library but for bytestrings. Writing yet another ELF-file parser but pure sounds interesting, but ineffective (better to spend that effort on something else). I do agree that using libelf might be faster. But I also like idea that ghc requires only binutils. Adding more dependency on something while we can get the same functionality with existing dependency looks bad for me.

Replying to thomie:

Add binary to Build-Depends in compiler/ghc.cabal.in (and compiler/ghc.cabal, so you don't have to rerun configure).

This might be true for release tar-balls, but not for source code repository which should always go through autoreconf (perl boot) and configure to generate from files from their .in. And that's described in both README.md and HACKING.md. Note that compiler/ghc.cabal is in ignores where it should be. Though file is committed into repo - which is wrong. Probably some-one did that by mistake and/or when .gitignore entry were added no-one removed it.

comment:10 in reply to:  9 Changed 2 years ago by hsyl20

I have recently written my own ELF parser (https://github.com/hsyl20/ViperVM/tree/master/src/lib/ViperVM/Format/Elf) so it is still fresh in my head. I have adapted it to GHC by removing intermediate data structures and everything that is not strictly required to extract a section.

https://phabricator.haskell.org/D1381

I'm not an expert of lazy IO, so reviews are welcome.

comment:11 Changed 2 years ago by hsyl20

Differential Rev(s): Phab:D1326 Phab:D1335Phab:D1326 Phab:D1335 Phab:D1381

comment:12 Changed 2 years ago by Ben Gamari <ben@…>

In 109d7ce/ghc:

Systools: read ELF section without calling readelf

This patch tackles two issues:

1) GHC stores a "link info" string into a ELF section. Initially a
section with type "note" was used but GHC didn't follow the ELF
specification which specifies a record-based format for these sections.
With D1375 we switched to a "progbits" section type for which there
isn't any format constraint. This is an issue for D1242 which use GCC's
--gc-sections which collects "unused" sections, such as our section
containing link info... In this patch, we fall back to a section with
type "note" but we respect the specified format.

2) Reading back the ELF section was done by parsing the result of a
call to "readelf". Calling readelf is problematic because the program
may not be available or it may be renamed on some platforms (see
D1326). Moreover we have no garanty that its output layout will stay
the same in future releases of readelf. Finally we would need to fix
the parsing to support  "note" sections because of 1. Instead, this
patch proposes to use Data.Binary.Get to directly read the "link info"
note into its section. ELF has a specification, hence it should work on
every conforming platform.

This patch "reverts" D1375, hence it supersedes D1432. It makes D1326
not necessary anymore.

Test Plan:
- recomp011 should pass (test that relinking is avoided when both "link
info" match)
- we should add a test for ELF objects with more than 0xff00 sections
=> added test "recomp015"
- we should check that GAS generates 32-bit words with .int on every
supported platform using ELF (or find a place where this is
documented). harbomaster and I (@hsyl20) only tested on x86-64. On
platforms where it is not true, it should make recomp011 fail. =>
tested to work on Linux/amd64, Solaris/i386 and OpenBSD/amd64

Reviewers: olsner, ony, thomie, kgardas, austin, bgamari

Reviewed By: thomie, bgamari

Subscribers: kgardas, rwbarton, thomie

Differential Revision: https://phabricator.haskell.org/D1381

GHC Trac Issues: #10974, #11022

comment:13 Changed 2 years ago by bgamari

Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.