[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

arphaman added a comment. In D45639#2713196 , @phosek wrote: > FYI I had to revert this change again because it broke ubsan build. The > problem is that ubsan for Darwin is built as universal shared library and it > links against libc++abi, but we

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. FYI I had to revert this change again because it broke ubsan build. The problem is that ubsan for Darwin is built as universal shared library and it links against libc++abi, but we currently don't support building libc++ and libc++abi as universal binaries in the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706605 , @ldionne wrote: > In D45639#2706589 , @dexonsmith > wrote: > >> I'm not sure I'm totally following, but just want to double check that the >> tests won't somehow

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In D45639#2706589 , @dexonsmith wrote: > I'm not sure I'm totally following, but just want to double check that the > tests won't somehow use the libc++ from the SDK instead of ToT? I guess the > test uses `-nostdinc++` or

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

dexonsmith added a comment. > No, the bot is also meant to catch changes to libc++ breaking LLDB (or at > least making sure we update the corresponding data formatters). > Given that these tests are macOS specific and already require a specific SDK, > I'll just update them to use the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706364 , @JDevlieghere wrote: > Given that these tests are macOS specific and already require a specific SDK, > I'll just update them to use the compiler from the SDK instead of the > just-built one. Done in

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. Given that these tests are macOS specific and already require a specific SDK, I'll just update them to use the compiler from the SDK instead of the just-built one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2705919 , @phosek wrote: > In D45639#2705702 , @ldionne wrote: > >> In D45639#2703913 , @JDevlieghere >> wrote: >> >>> This breaks

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In D45639#2705702 , @ldionne wrote: > In D45639#2703913 , @JDevlieghere > wrote: > >> This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed >> build:

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In D45639#2703913 , @JDevlieghere wrote: > This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed > build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31346/ > [...] > > Based on your description

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. @phosek I've reverted this in 05eeed9691aeb3e0316712195b998e9078cdceb0 to turn the bot green again. Happy to help you look into this tomorrow. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31346/ /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang main.o -g -O0 -fno-builtin -isysroot

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. Another attempt is rGcaff17e503fe . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/new/ https://reviews.llvm.org/D45639

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In D45639#2702746 , @thakis wrote: > Looks like this breaks tests on windows: > http://45.33.8.238/win/37191/step_7.txt Should be addressed by rGf5efe0aa048b .

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/37191/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/new/ https://reviews.llvm.org/D45639 ___

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

This revision was automatically updated to reflect the committed changes. Closed by commit rGae8b2cab6740: [Driver] Support default libc++ library location on Darwin (authored by phosek). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. In D45639#2693706 , @phosek wrote: > It's depends on the order: whichever comes first wins. The default order of > paths that the driver uses is (1)

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In D45639#2692142 , @ldionne wrote: > In D45639#2383754 , @smeenai wrote: > >> Just following up on this, cos I'm curious :) I have 12.1 now, and I still >> only see the C++ headers in the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In D45639#2383754 , @smeenai wrote: > Just following up on this, cos I'm curious :) I have 12.1 now, and I still > only see the C++ headers in the toolchain and not in any of the SDKs. Look in Xcode 12.5 beta 3, you should see

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. I think this looks good. I must admit I'm a bit worried about any unintended consequences this might have or breakage this might cause in cases where we'd switch from linking against the SDK libc++.dylib to the toolchain one. This would only impact the toolchain

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. I agree with your analysis and I'd also prefer **Solution (1)**, I've rebased the change and included a simple test. Right now, there's nothing specific to libc++, we're simply relying on the linker. Alternative would be to have a more explicit logic in

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek updated this revision to Diff 304342. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/new/ https://reviews.llvm.org/D45639 Files: clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/darwin-ld.c Index: clang/test/Driver/darwin-ld.c

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In D45639#2360619 , @smeenai wrote: > In D45639#2360267 , @ldionne wrote: > >> - AppleClang prefers the headers in the SDK, and the library in the SDK. >> (The headers are currently still

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In D45639#2360267 , @ldionne wrote: > - AppleClang prefers the headers in the SDK, and the library in the SDK. (The > headers are currently still shipped in the toolchain but they should be > ignored if you have a sufficiently

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. Note: This patch basically implements **Solution (1)**. I would love to see it rebased onto `master` and for tests to be added if we're all comfortable going down that route. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/new/

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. Re-reading this. the whole discussion about `filesystem` is now irrelevant, since it's part of the dylib. The comment I have is that `libc++.dylib` is considered to be a system library on macOS, not a toolchain-provided library. This matters because we make sure that

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

dexonsmith resigned from this revision. dexonsmith added a reviewer: Bigcheese. dexonsmith added a subscriber: Bigcheese. dexonsmith added a comment. Herald added a subscriber: dexonsmith. If this is still relevant, I’m happy for @ldionne and @Bigcheese to make the call. Repository: rC Clang

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In https://reviews.llvm.org/D45639#1243010, @ldionne wrote: > Sorry, my comment was wrong. You're right, using new libc++ headers and > linking against an old `libc++.dylib` is a supported use case, and in fact > this is exactly what happens whenever you use new libc++

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In https://reviews.llvm.org/D45639#1243017, @kristina wrote: > I think on Darwin it would **not** make sense to have `libc++fs.a` ship in > `libc++.dylib` especially considering that it ends up in the dyld cache and > that has a lot of other implications. It would make

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

kristina added a comment. I think on Darwin it would **not** make sense to have `libc++fs.a` ship in `libc++.dylib` especially considering that it ends up in the dyld cache and that has a lot of other implications. It would make sense to ship it as a separate library, perhaps as part of the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In https://reviews.llvm.org/D45639#1242444, @phosek wrote: > In https://reviews.llvm.org/D45639#1193112, @ldionne wrote: > > > @phosek I don't understand how you can expect code compiled with new > > headers to link against an old dylib, unless you're setting the target

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In https://reviews.llvm.org/D45639#1193112, @ldionne wrote: > @phosek I don't understand how you can expect code compiled with new headers > to link against an old dylib, unless you're setting the target platform, in > which case anything that would fail to link will

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. I must say I therefore don't understand the purpose of this patch. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. @phosek I don't understand how you can expect code compiled with new headers to link against an old dylib, unless you're setting the target platform, in which case anything that would fail to link will instead be a compiler error. I mean, we're adding stuff to the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

jfb added a comment. In https://reviews.llvm.org/D45639#1192849, @beanz wrote: > Adding @jfb since this is his domain now too. @ldionne is the libc++ expert :-) Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

beanz added a subscriber: jfb. beanz added a comment. Adding @jfb since this is his domain now too. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. I think we just ran into one such issue. We're using our own Clang that's usually following tip-of-tree and we recently switched to C++17, but that started failing on our macOS 10.12 bots with: Undefined symbols for architecture x86_64: "operator delete(void*,

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

beanz added a reviewer: bruno. beanz added a subscriber: bruno. beanz added a comment. @dexonsmith is often super busy, so @bruno may be able to weigh in instead. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In https://reviews.llvm.org/D45639#1078494, @thakis wrote: > > because the headers that are part of Clang toolchain are incompatible with > > the system library > > Do you have details on this? This isn't supposed to be the case as far as I > know. We link chrome/mac

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In https://reviews.llvm.org/D45639#1078494, @thakis wrote: > > because the headers that are part of Clang toolchain are incompatible with > > the system library > > Do you have details on this? This isn't supposed to be the case as far as I > know. We link chrome/mac

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

thakis added a comment. > because the headers that are part of Clang toolchain are incompatible with > the system library Do you have details on this? This isn't supposed to be the case as far as I know. We link chrome/mac against system libc++ with the bundled headers, and it at least seems

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek added a comment. In https://reviews.llvm.org/D45639#1068086, @compnerd wrote: > I'm not sure I understand this. The proper location for libc++ on the darwin > layout is in the SDK, not relative to the driver. The default behaviour is > similar to cross-compiling, and with a (derived)

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

compnerd added a comment. I'm not sure I understand this. The proper location for libc++ on the darwin layout is in the SDK, not relative to the driver. The default behaviour is similar to cross-compiling, and with a (derived) SDK. This definitely needs to be reviewed by @dexonsmith

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

phosek created this revision. phosek added reviewers: dexonsmith, compnerd. Herald added a reviewer: EricWF. Herald added a subscriber: cfe-commits. Darwin driver currently uses libc++ headers that are part of Clang toolchain when available (by default ../include/c++/v1 relative to executable),