Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framework: Stop forcing CMake to pass to -fPIE on all compilations, as this leads to link failures with Intel 19 #10984

Closed
wants to merge 1 commit into from
Closed

Framework: Stop forcing CMake to pass to -fPIE on all compilations, as this leads to link failures with Intel 19 #10984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions cmake/std/atdm/ATDMDevEnvSettings.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,6 @@ IF ((ATDM_COMPILER MATCHES ".*CLANG.*") AND ATDM_ADDRESS_SANITIZER)
SET(EXTRA_EXTRA_LINK_FLAGS
"${EXTRA_EXTRA_LINK_FLAGS} -ldl -fsanitize=address")
ENDIF()
# FPIC
IF (ATDM_FPIC)
SET(CMAKE_POSITION_INDEPENDENT_CODE ON CACHE BOOL "Build targets with position independent code")
ENDIF()
Comment on lines -220 to -223
Copy link
Member

@bartlettroscoe bartlettroscoe Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@PhilMiller, I just remembered that some SPARC builds expect fpic to be on for its shared builds. Therefore, to remove this may break SPARC's usage.

How about we change the default for fpic from on to off and require those shared builds that want to set fpic to have to on have to include the keyword fpic the build name? Then the SPARC shared builds that need fpic could just add the keyword fpic to their build names.

So we would update:

Then we could leave this logic here?

I think to change the default we just need to edit the following files:

Copy link
Contributor Author

@PhilMiller PhilMiller Sep 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Which does SPARC actually want, -fPIC so it can link objects into shared libraries, or -fPIE to link objects into Position-Independent Executables? PIE is generally only desirable for relocatable loading to support ASLR in a server process, or for very niche use cases in which other processes will load an executable image as a dynamic library.

The ATDM_FPIC setting is currently asking CMake to provide PIE, according to https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html#prop_tgt:POSITION_INDEPENDENT_CODE and its observed behavior, which besides breaking builds, also hurts performance if it's used in circumstances where it's not actually necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rppawlo asked me to weigh in. SPARC wants Trilinos static libraries so that we can link them into either shared or static builds of our application executables.


SET(CMAKE_C_FLAGS
"$ENV{ATDM_CONFIG_C_FLAGS} ${CMAKE_C_FLAGS}")
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/ini-files/config-specs.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ opt-set-cmake-var Trilinos_ENABLE_COMPLEX BOOL : OFF
#

[USE-FPIC|YES]
opt-set-cmake-var CMAKE_POSITION_INDEPENDENT_CODE BOOL : ON
# Nothing to do here.

[USE-FPIC|NO]
# Nothing to do here.
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/pr_tools/trilinosprhelpers/unittests/config-specs.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ opt-set-cmake-var Trilinos_ENABLE_COMPLEX BOOL : OFF
#

[USE-FPIC|YES]
opt-set-cmake-var CMAKE_POSITION_INDEPENDENT_CODE BOOL : ON
# Nothing to do here.

[USE-FPIC|NO]
# Nothing to do here.
Expand Down