[libcxxabi] Prevent cmake from removing our explicit system C++ include paths

[sbc100]

Summary

We build with -nostdinc++ and add our own header path via
LIBCXXABI_LIBCXX_INCLUDES. However cmake tries to be clever and if
LIBCXXABI_LIBCXX_INCLUDES happens to match the compilers system path
it will remove the -I flag meaning we can't access any C++ headers.

This problem only happens when LIBCXXABI_LIBCXX_INCLUDES and the
compilers default C++ include path are the same thing. This happens on the
wasm waterfall where we build and install libcxx into the compiler default
location before we build libcxxabi.

Ideally cmake would be able see that we are using -nostdinc++ and
disable this behaviour.

See https://gitlab.kitware.com/cmake/cmake/issues/19227

[sbc100]

We upgraded cmake and our build started failing. This was the best fix I could come up with.

[phosek]

@beanz @abdulras @smeenai do you know if there's a better way to handle this?

[sbc100]

I'm not sure of the exact cmake version where this starting breaking but we saw it when upgrading from 3.7.2 to 3.15.3.

[smeenai]

You should report a bug to CMake (you're right that their implicit removal should account for the various -nostdinc-like options), but this seems like an okay workaround in the meantime.

Do you need something like this in libc++ as well?

[sbc100]

I don't think so no, its the dependency between libcxxabi and the libcxx that is the issue. I think libcxxabi assumes the libcxx is in non-default place when it builds. Perhaps out case that actually isn't true so another possible fix might be to add LIBCXXABI_USE_SYSTEM_LIBCXX which would avoid the need to -nostdinc (although maybe thats needed for other reasons too).

[beanz]

I suspect this behavior change was caused by: https://gitlab.kitware.com/cmake/cmake/commit/b0b820ea39fe4edaf0672ba899a7042ef93a20d2

[beanz]

Reviewing the CMake change more closely, that is a bug. They shouldn't have changed the default behavior in that way. At the very least that change should have been a policy change.

[sbc100]

Even before that change it looks like stripImplicitInclDirs = true.. looks like the behaviour might predate that change?

[beanz]

https://gitlab.kitware.com/cmake/cmake/issues/19227

[sbc100]

• Fix typos and add link to cmake bug

[sbc100]

Great. I added a reference to the upstream bug. This should be good to go no. Sadly we will need to keep this around even after the bug is fixes since the issue exists large version range of existing cmake releases.

[beanz]

This should only be broken for CMake 3.14 and later. We should include that in the comments around this change.

[sbc100]

• update comment

[smeenai]

In D69973#1738127, @sbc100 wrote:

I don't think so no, its the dependency between libcxxabi and the libcxx that is the issue. I think libcxxabi assumes the libcxx is in non-default place when it builds. Perhaps out case that actually isn't true so another possible fix might be to add LIBCXXABI_USE_SYSTEM_LIBCXX which would avoid the need to -nostdinc (although maybe thats needed for other reasons too).

Hmm, something like that might be cleaner, though the -nostdinc++ might still be useful for e.g. making sure you don't accidentally include the system's cxxabi.h (although it appears that Xcode puts cxxabi.h in the sysroot include directory instead of the C++-specific include directory, so -nostdinc++ isn't gonna do you any good there).

@ldionne, what do you think?

[sbc100]

Ping. I'd like to land this ASAP as its currently causing out waterfall fail: https://ci.chromium.org/p/wasm/g/toolchain/console

[smeenai]

Accepting to unblock, though I'd like to see the LIBCXXABI_USE_SYSTEM_LIBCXX solution explored as a follow-up.

[phosek]

LGTM

libcxxabi/CMakeLists.txt

if (LIBCXXABI_TARGET_TRIPLE)
   set(TARGET_TRIPLE "${LIBCXXABI_TARGET_TRIPLE}")
 endif()
 
 # Configure compiler. Must happen after setting the target flags.
 include(config-ix)
 
 if (LIBCXXABI_HAS_NOSTDINCXX_FLAG)
   list(APPEND LIBCXXABI_COMPILE_FLAGS -nostdinc++)
+  # cmake 3.14 and above remove system include paths that are explicitly
+  # passed on the command line.  We build with -nostdinc++ and explicitly add
+  # just the libcxx system include paths with -I on the command line.
+  # Setting CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES effectively prevents cmake
+  # from removing these.
+  # See: https://gitlab.kitware.com/cmake/cmake/issues/19227
+  set(CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "")
   # Remove -stdlib flags to prevent them from causing an unused flag warning.
   string(REPLACE "-stdlib=libc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   string(REPLACE "-stdlib=libstdc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 endif()
 
 # Let the library headers know they are currently being used to build the
 # library.
 add_definitions(-D_LIBCXXABI_BUILDING_LIBRARY)