This repository has been archived by the owner on Nov 1, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 509
Linux improvements (fixing linker args & Documentation updates) #6843
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
d275e74
Added explicit -nopie to default linker options for Linux platforms w…
hasuo 82a757c
Updated documentation with Linux issues
hasuo 56306ca
Fixed Documentation typo's.
hasuo 01863ad
Revert "Added explicit -nopie to default linker options for Linux pla…
hasuo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the error you are working around with
-nopie
?If I understand it correctly,
-nopie
disables ASLR and thus makes the binaries less secure. We do not really want the CoreRT binaries to be less secure...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I totally agree, and I hope you guys will be able to fix it "propper".
Here is what I pieced together so far:
The current binary distributed by the nuget package segfaults for me. To investigate, I checked out the repo and tried to build it. However, while compilation succeeded, all JIT-Tests fail with:
R_X86_64_PC32 against symbol `memcpy@@GLIBC_2.14' can not be used
at the Linking stage.
The reason behind this seems to be this, but I am unable to tell you if that is true.
https://stackoverflow.com/questions/43367427/32-bit-absolute-addresses-no-longer-allowed-in-x86-64-linux
Arch Linux (the distribution I use) ships with compilers that assume -use-pie by default, for security reasons. I assume that the build machine does not, (hence tests working)
Mainly because the distributed ILC itself has PIE switched off:
So I decided to make a pull request, with this work around. AFAIK it does not make things works.
But I'd like a different fix much more, PIE adds a lot of benefits for a tiny performance hit.
I use this script: https://github.com/slimm609/checksec.sh to check if files are made with PIE.
I hope this information helps.
Edit: Adding a gist to the linker output here https://gist.github.com/sebastianulm/f1bba12ec15bd5cb707f04388b171edd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had several reports of this https://github.com/dotnet/corert/search?q=R_X86_64_PC32&type=Issues . It is a bug in the objectwriter component. #538 has best hints about what the fix may be.
How this does not work? Is the binary crashing when you try to run it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, object writer emits only 32 bit Symbols, which can not be relocated in 64 bit memory, hence it cannot link with -fPIE. I don't consider this a bug. They are not broken, there are valid considerations to disable this for some builds. For example if you build render software or a bitcoin miner or anything very CPU intensive, you want to use 32 bit symbols.
They are a bit faster, even on 64 bit platforms.
Aside from these special cases virtually everybody wants to build full 64 bits, because its required to randomize the address space.
So, in a perfect world, libObjWriter would have a flag that switches between the two. I consider it more a missing feature the a Bug. There are genuine reasons for making this a parameter (which is why all linux compilers offer this flag)
Sorry, this is a typo. I meant to write, "AFAIK it does not make things worse".
Currently, if I check out clang-3.9, build it and use it as a linker, it will not produce PIC-Binaries by default.
The Microsoft.NETCore.Native.Unix.props file does not state -fPIC nor -Fno-PIC. It simply uses what ever has been set as the default when building clang.
Most modern distribution ship binaries of clang with –enable-default-pie, which changes the default from -Fno-PIC to -fPIC. Ubuntu started doing this in with 16.10 https://wiki.ubuntu.com/SecurityTeam/PIE
I think the build machine either has an older version, or does not use the binary from the distribution. Hence all the tests are passing on the build machine, but fail out in the wild.
I may be wrong here, so this is something you should check. Grabbing an artifact from the build machine and checking if it was build with PIC enabled should be enough to find out.
The people from the issues you linked all have the same issue as me, and I know not nearly enough to add that missing feature to objWriter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please exclude this change from this PR?
Hopefully, #6847 will land soon and we will have proper fix for this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
I'll try to help with testing on those new changes asap.
Unfortunately I know very little about clang and assembler. I know a bit about the elf file format, and linux tools, so testing things is probably something I can do.