Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix setting PAX options for crossgen #13950

Merged
merged 1 commit into from
Sep 13, 2017
Merged

Conversation

janvorli
Copy link
Member

In my previous PR that added marking executables with paxctl, I've made
a mistake in the order of calling _add_library and add_dependencies.
But that was hidden due to the fact that we have yet another copy of the
add_library_clr and add_executable_clr functions in src/CMakeLists.txt.
I have no idea how that happened.
This one has overriden the other, which has hidden the problem, but also
caused the crossgen to not to be modified by the paxctl.
So I am fixing the order and removing the extra definitions of those
functions.

In my previous PR that added marking executables with paxctl, I've made
a mistake in the order of calling _add_library and add_dependencies.
But that was hidden due to the fact that we have yet another copy of the
add_library_clr and add_executable_clr functions in src/CMakeLists.txt.
I have no idea how that happened.
This one has overriden the other, which has hidden the problem, but also
caused the crossgen to not to be modified by the paxctl.
So I am fixing the order and removing the extra definitions of those
functions.
@janvorli janvorli added area-Build os-linux Linux OS (any supported distro) labels Sep 13, 2017
@janvorli janvorli added this to the Future milestone Sep 13, 2017
@janvorli janvorli self-assigned this Sep 13, 2017
@jkotas
Copy link
Member

jkotas commented Sep 13, 2017

crossgen should not allocate RWX memory (if it does - it should be trivial to fix). I do not think we need to set this bit for crossgen.

@janvorli
Copy link
Member Author

@jkotas it does do that, since it fails without the PAX flags on Alpine. This change is needed anyways since the cmake script was incorrect. I'll create a separate issue for figuring out and removing the RWX allocation from crossgen and removing crossgen from the set of files processed by paxctl.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@janvorli
Copy link
Member Author

I've created issue #13951 to track it.

@janvorli janvorli merged commit 9fc84cf into dotnet:master Sep 13, 2017
@janvorli janvorli deleted the add-paxctl-2 branch September 14, 2017 16:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-coreclr os-linux Linux OS (any supported distro)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants