Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Oct 14, 2022

This fixes unused option warnings on -pie for macOS. (On macOS cmake produces "-fPIE -Xlinker -pie")

Bump required cmake version to 3.14 for CheckPIESupported.

References:
https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html https://cmake.org/cmake/help/latest/module/CheckPIESupported.html#module:CheckPIESupported


include (${WAMR_ROOT_DIR}/build-scripts/runtime_lib.cmake)

set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--gc-sections -pie -fPIE")
Copy link
Contributor Author

@yamt yamt Oct 14, 2022

Choose a reason for hiding this comment

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

i removed this because there is no executable target in this file.

# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

cmake_minimum_required (VERSION 2.9)
cmake_minimum_required (VERSION 3.14)
Copy link
Collaborator

@wenyongh wenyongh Oct 18, 2022

Choose a reason for hiding this comment

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

A little afraid whether it is friendly to upgrade the cmake version, maybe we can resolve the pie -fPIE issue by checking whether the compiler is clang, for example for the CMakeLists.txt of wamrc:

--- a/wamr-compiler/CMakeLists.txt
+++ b/wamr-compiler/CMakeLists.txt
@@ -237,7 +237,7 @@ endif()
 if (NOT MSVC)
   add_definitions(-D_FORTIFY_SOURCE=2)
   set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -ftrapv")
-  if (NOT WIN32)
+  if (NOT CMAKE_C_COMPILER MATCHES ".*clang.*" AND NOT CMAKE_C_COMPILER_ID MATCHES ".*Clang")
     set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pie -fPIE")
   endif()
 endif()

@loganek, @lum1n0us, @no1wudi What is your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with the change of cmake requirement.

But how about other CMakeLists.txt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with 3.14 as well, it's already 3 years old and it comes with lots of good features (compared to 2.9) so I think it's worth taking that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's ok.
BTW, is there anyway to merge these dupped code into one ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, thanks. @yamt, could you resolve the conflict so that I can merge the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased

This fixes unused option warnings on -pie for macOS.
(On macOS cmake produces "-fPIE -Xlinker -pie")

Bump required cmake version to 3.14 for CheckPIESupported.

References:
https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html
https://cmake.org/cmake/help/latest/module/CheckPIESupported.html#module:CheckPIESupported
@wenyongh wenyongh merged commit 654ac5f into bytecodealliance:main Oct 18, 2022
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…ytecodealliance#1598)

This fixes unused option warnings on -pie for macOS.
(On macOS cmake produces "-fPIE -Xlinker -pie")

Bump required cmake version to 3.14 for CheckPIESupported.

References:
https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html
https://cmake.org/cmake/help/latest/module/CheckPIESupported.html#module:CheckPIESupported
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants