- fix compilation of RTS Linker on platforms where SHN_XINDEX symbol is not defined and/or supported (like OpenBSD).
ClosedPublic

Authored by kgardas on Nov 7 2015, 8:14 AM.

Details

Summary

This patch fixes RTS Linker compilation issues on platforms
where SHN_XINDEX is not defined. Tested on OpenBSD. When SHN_XINDEX
is not defined, the code reverts to the old behavior, that means
behavior of the Linker before D1357 which added SHN_XINDEX based
functionality.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kgardas updated this revision to Diff 4965.Nov 7 2015, 8:14 AM
kgardas retitled this revision from to - fix compilation of RTS Linker on platforms where SHN_XINDEX symbol is not defined and/or supported (like OpenBSD)..
kgardas updated this object.
kgardas edited the test plan for this revision. (Show Details)
kgardas added reviewers: austin, bgamari.
thomie accepted this revision.Nov 10 2015, 5:40 AM
thomie added a reviewer: thomie.

LGTM

bgamari accepted this revision.Nov 11 2015, 4:00 AM
bgamari edited edge metadata.

I do wish it were possible to package this change up a bit more compactly. Recently @erikd has been doing great work paring down the amount of CPP we have in the linker. It would be nice to continue that trend.

Unfortunately I don't see any way to do this without either losing sharing of the section index table lookup or adding quite a bit of verbosity.

Ben, I truly admire Erik's work on Linker. I also feel kind of guilty to kind of reverse its direction with this patch. On the other hand I tried to have that as simple as possible for Erik to exactly see where the culprit is. BTW: the issue is not only in SHN_INDEX being undefined but also in SHT_SYMTAB_SHNDX which is neither defined. In the current patch version this is hidden by SHN_INDEX #ifdef so fixed. Anyway, I still think there is a possibility to refactor this cleanly, but IMHO it's better when one man does this. There is too much code going into Linker.c these days and too much breakage slipping from it so no need to make things even more complex by stepping on Erik's foots with any attempt to cleverly refactor this into a clean code...
Anyway, if I understand this right, Erik does have final word here and even if you accept this code he still need to either ok this or require change(s). Erik, this is up to you now. Thanks for the review!

Ben, I truly admire Erik's work on Linker. I also feel kind of guilty to kind of reverse its direction with this patch. On the other hand I tried to have that as simple as possible for Erik to exactly see where the culprit is. BTW: the issue is not only in SHN_INDEX being undefined but also in SHT_SYMTAB_SHNDX which is neither defined. In the current patch version this is hidden by SHN_INDEX #ifdef so fixed. Anyway, I still think there is a possibility to refactor this cleanly, but IMHO it's better when one man does this. There is too much code going into Linker.c these days and too much breakage slipping from it so no need to make things even more complex by stepping on Erik's foots with any attempt to cleverly refactor this into a clean code...

Sure, sounds good to me. I was really just lamenting the lack of clean solutions here. @erikd, if there's no objection from you I can merge this as-is.

erikd accepted this revision.Nov 11 2015, 1:16 PM
erikd edited edge metadata.

I can live with this. Maybe we can find a way to clean it up later.

This revision is now accepted and ready to land.Nov 11 2015, 1:16 PM
This revision was automatically updated to reflect the committed changes.