[PATCH] D70972: [AIX] Make sure we use export lists for plugins

Steven Wan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 11 08:48:12 PST 2020


stevewan added inline comments.


================
Comment at: llvm/CMakeLists.txt:840
+  # well as setting -G when building libraries, so clear them out.
+  string(REPLACE "-Wl,-brtl" "" CMAKE_SHARED_LINKER_FLAGS  "${CMAKE_SHARED_LINKER_FLAGS}")
+  string(REPLACE "-Wl,-brtl" "" CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS}")
----------------
daltenty wrote:
> hubert.reinterpretcast wrote:
> > Is this safe? The string might require replacing `-Wl,-brtl,` with `-Wl,` because of having additional options passed through to the linker via the same `-Wl`. This also does not handle `-brtl` in other positions in such a list. Please consider for all lines.
> I've updated this to use REGEX to make it position independent.Replacing with `-Wl,` however is also unsafe because this will generate warnings which will fail a Werror build. We know that CMake will not pass multiple combined flags with -brtl itself. 
> 
> I guess in theory a user could force this somehow, but that would mess up their build in other ways anyhow so I'm inclined to say that's user error.
Is it better to make the pattern `(-Wl,)?-brtl` here? So that it handles the case where `-brtl` doesn't come right after `-Wl`. Or if having multiple combined flags with `-brtl` is strictly prohibited, then we should make this more explicit by adding a comment here that says so.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70972/new/

https://reviews.llvm.org/D70972





More information about the llvm-commits mailing list