r346652 - Make clang-based tools find libc++ on MacOS

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 29 07:50:07 PST 2019


So this happens because c-index-test would silently drop argv[0] and not
pass it to the driver (this happens in write_pch_file from
'c-index-test.'c).
The fix that comes to mind is starting to pas argv[0] there (it's simple
apart from argv manipulation in C, which might require some boilerplate).

I've filed https://bugs.llvm.org/show_bug.cgi?id=40515 to see if someone
proposes an alternative fix.
If no one responds or picks it up until tomorrow, will take a look at
fixing this myself.

On Tue, Jan 29, 2019 at 1:43 PM Ilya Biryukov <ibiryukov at google.com> wrote:

> This change got reverted, but the follow-up r348365 might be causing the
> issues.
> I wonder why the Driver can't detect install dir on its own, will take a
> look, thanks.
>
> On Mon, Jan 28, 2019 at 7:20 PM Nico Weber <thakis at chromium.org> wrote:
>
>> This seems to fix it, but I don't know if it's the right fix:
>>
>> diff --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
>> b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
>> index b62416ffd9e..d933f8c1b70 100644
>> --- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
>> +++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
>> @@ -47,6 +47,10 @@ std::unique_ptr<CompilerInvocation>
>> clang::createInvocationFromCommandLine(
>>    driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
>>                             *Diags, VFS);
>>
>> +  StringRef InstalledPathParent(llvm::sys::path::parent_path(Args[0]));
>> +  if (llvm::sys::fs::exists(InstalledPathParent))
>> +    TheDriver.setInstalledDir(InstalledPathParent);
>> +
>>    // Don't check that inputs exist, they may have been remapped.
>>    TheDriver.setCheckInputsExist(false);
>>
>>
>>
>> On Mon, Jan 28, 2019 at 10:08 AM Nico Weber <thakis at chromium.org> wrote:
>>
>>> I think c-index-test has the same problem:
>>>
>>> $ cat test.cc
>>> #include <iostream>
>>> $ out/gn/bin/c-index-test -write-pch foo.pch test.cc
>>> test.cc:1:10: fatal error: 'iostream' file not found
>>>
>>> Since https://reviews.llvm.org/rL350714 was a driver-side fix and
>>> c-index-test doesn't use lib/Driver, it didn't do anything here. Can you
>>> take a look at c-index-test too?
>>>
>>> On Wed, Jan 9, 2019 at 8:15 AM Ilya Biryukov <ibiryukov at google.com>
>>> wrote:
>>>
>>>> Glad to help. The fix has landed. Let me know if the problem persists
>>>> after it's integrated.
>>>>
>>>> On Tue, Jan 8, 2019 at 7:36 PM Nico Weber <thakis at chromium.org> wrote:
>>>>
>>>>> That looks like it should help. Thanks for the quick fix!
>>>>>
>>>>> On Tue, Jan 8, 2019 at 1:11 PM Ilya Biryukov <ibiryukov at google.com>
>>>>> wrote:
>>>>>
>>>>>> Hi Nico,
>>>>>>
>>>>>> This is clearly a bug, it's supposed to search in a sibling directory.
>>>>>> Are you running clang as './clang' in the scripts?  The code seems to
>>>>>> break in that case, https://reviews.llvm.org/D56446 should fix this.
>>>>>>
>>>>>> On Tue, Jan 8, 2019 at 5:12 PM Nico Weber <thakis at chromium.org>
>>>>>> wrote:
>>>>>>
>>>>>>> It looks like clang now looks for libc++ headers
>>>>>>> in -internal-isystem Release+Asserts/bin/include/c++/v1 , compared
>>>>>>> to -internal-isystem Release+Asserts/include/c++/v1. `make install` puts
>>>>>>> the libc++ headers in Release+Asserts/include, the old location. Was this
>>>>>>> an intentional change?
>>>>>>>
>>>>>>> As-is, this seems to break chromium's clang ability to find libc++
>>>>>>> headers (https://crbug.com/919761) because we bundle libc++ headers
>>>>>>> in an "include" directory that's a sibling to the "bin" directory (and have
>>>>>>> been doing so for 4.5 years, since
>>>>>>> https://codereview.chromium.org/281753002).
>>>>>>>
>>>>>>> On Mon, Nov 12, 2018 at 8:58 AM Ilya Biryukov via cfe-commits <
>>>>>>> cfe-commits at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>> Author: ibiryukov
>>>>>>>> Date: Mon Nov 12 05:55:55 2018
>>>>>>>> New Revision: 346652
>>>>>>>>
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=346652&view=rev
>>>>>>>> Log:
>>>>>>>> Make clang-based tools find libc++ on MacOS
>>>>>>>>
>>>>>>>> Summary:
>>>>>>>> When they read compiler args from compile_commands.json.
>>>>>>>> This change allows to run clang-based tools, like clang-tidy or
>>>>>>>> clangd,
>>>>>>>> built from head using the compile_commands.json file produced for
>>>>>>>> XCode
>>>>>>>> toolchains.
>>>>>>>>
>>>>>>>> On MacOS clang can find the C++ standard library relative to the
>>>>>>>> compiler installation dir.
>>>>>>>>
>>>>>>>> The logic to do this was based on resource dir as an approximation
>>>>>>>> of
>>>>>>>> where the compiler is installed. This broke the tools that read
>>>>>>>> 'compile_commands.json' and don't ship with the compiler, as they
>>>>>>>> typically change resource dir.
>>>>>>>>
>>>>>>>> To workaround this, we now use compiler install dir detected by the
>>>>>>>> driver
>>>>>>>> to better mimic the behavior of the original compiler when
>>>>>>>> replaying the
>>>>>>>> compilations using other tools.
>>>>>>>>
>>>>>>>> Reviewers: sammccall, arphaman, EricWF
>>>>>>>>
>>>>>>>> Reviewed By: sammccall
>>>>>>>>
>>>>>>>> Subscribers: ioeric, christof, kadircet, cfe-commits
>>>>>>>>
>>>>>>>> Differential Revision: https://reviews.llvm.org/D54310
>>>>>>>>
>>>>>>>> Added:
>>>>>>>>     cfe/trunk/test/Tooling/Inputs/mock-libcxx/
>>>>>>>>     cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/
>>>>>>>>     cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/
>>>>>>>>     cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/
>>>>>>>>
>>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
>>>>>>>>     cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp
>>>>>>>>     cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp
>>>>>>>> Modified:
>>>>>>>>     cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>>>>>>>>     cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>>>     cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>>>>>>>>     cfe/trunk/lib/Tooling/Tooling.cpp
>>>>>>>>
>>>>>>>> Modified: cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearchOptions.h?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/include/clang/Lex/HeaderSearchOptions.h (original)
>>>>>>>> +++ cfe/trunk/include/clang/Lex/HeaderSearchOptions.h Mon Nov 12
>>>>>>>> 05:55:55 2018
>>>>>>>> @@ -108,6 +108,13 @@ public:
>>>>>>>>    /// etc.).
>>>>>>>>    std::string ResourceDir;
>>>>>>>>
>>>>>>>> +  /// Compiler install dir as detected by the Driver.
>>>>>>>> +  /// This is typically the directory that contains the clang
>>>>>>>> executable, i.e.
>>>>>>>> +  /// the 'bin/' subdir of a clang distribution.
>>>>>>>> +  /// Only used to add include dirs for libc++ on Darwin. Please
>>>>>>>> avoid relying
>>>>>>>> +  /// on this field for other purposes.
>>>>>>>> +  std::string InstallDir;
>>>>>>>> +
>>>>>>>>    /// The directory used for the module cache.
>>>>>>>>    std::string ModuleCachePath;
>>>>>>>>
>>>>>>>>
>>>>>>>> Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>>> (original)
>>>>>>>> +++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Mon
>>>>>>>> Nov 12 05:55:55 2018
>>>>>>>> @@ -11,17 +11,18 @@
>>>>>>>>  //
>>>>>>>>
>>>>>>>>  //===----------------------------------------------------------------------===//
>>>>>>>>
>>>>>>>> -#include "clang/Frontend/Utils.h"
>>>>>>>>  #include "clang/Basic/DiagnosticOptions.h"
>>>>>>>> +#include "clang/Driver/Action.h"
>>>>>>>>  #include "clang/Driver/Compilation.h"
>>>>>>>>  #include "clang/Driver/Driver.h"
>>>>>>>> -#include "clang/Driver/Action.h"
>>>>>>>>  #include "clang/Driver/Options.h"
>>>>>>>>  #include "clang/Driver/Tool.h"
>>>>>>>>  #include "clang/Frontend/CompilerInstance.h"
>>>>>>>>  #include "clang/Frontend/FrontendDiagnostic.h"
>>>>>>>> +#include "clang/Frontend/Utils.h"
>>>>>>>>  #include "llvm/Option/ArgList.h"
>>>>>>>>  #include "llvm/Support/Host.h"
>>>>>>>> +#include "llvm/Support/Path.h"
>>>>>>>>  using namespace clang;
>>>>>>>>  using namespace llvm::opt;
>>>>>>>>
>>>>>>>> @@ -102,5 +103,8 @@ std::unique_ptr<CompilerInvocation> clan
>>>>>>>>                                       CCArgs.size(),
>>>>>>>>                                       *Diags))
>>>>>>>>      return nullptr;
>>>>>>>> +  // Patch up the install dir, so we find the same standard
>>>>>>>> library as the
>>>>>>>> +  // original compiler on MacOS.
>>>>>>>> +  CI->getHeaderSearchOpts().InstallDir =
>>>>>>>> TheDriver.getInstalledDir();
>>>>>>>>    return CI;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
>>>>>>>> +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Mon Nov 12 05:55:55
>>>>>>>> 2018
>>>>>>>> @@ -476,14 +476,9 @@ void InitHeaderSearch::AddDefaultInclude
>>>>>>>>        if (triple.isOSDarwin()) {
>>>>>>>>          // On Darwin, libc++ may be installed alongside the
>>>>>>>> compiler in
>>>>>>>>          // include/c++/v1.
>>>>>>>> -        if (!HSOpts.ResourceDir.empty()) {
>>>>>>>> -          // Remove version from foo/lib/clang/version
>>>>>>>> -          StringRef NoVer =
>>>>>>>> llvm::sys::path::parent_path(HSOpts.ResourceDir);
>>>>>>>> -          // Remove clang from foo/lib/clang
>>>>>>>> -          StringRef Lib = llvm::sys::path::parent_path(NoVer);
>>>>>>>> -          // Remove lib from foo/lib
>>>>>>>> -          SmallString<128> P = llvm::sys::path::parent_path(Lib);
>>>>>>>> -
>>>>>>>> +        if (!HSOpts.InstallDir.empty()) {
>>>>>>>> +          // Get from foo/bin to foo.
>>>>>>>> +          SmallString<128>
>>>>>>>> P(llvm::sys::path::parent_path(HSOpts.InstallDir));
>>>>>>>>            // Get foo/include/c++/v1
>>>>>>>>            llvm::sys::path::append(P, "include", "c++", "v1");
>>>>>>>>            AddUnmappedPath(P, CXXSystem, false);
>>>>>>>>
>>>>>>>> Modified: cfe/trunk/lib/Tooling/Tooling.cpp
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/lib/Tooling/Tooling.cpp (original)
>>>>>>>> +++ cfe/trunk/lib/Tooling/Tooling.cpp Mon Nov 12 05:55:55 2018
>>>>>>>> @@ -327,6 +327,9 @@ bool ToolInvocation::run() {
>>>>>>>>      Invocation->getPreprocessorOpts().addRemappedFile(It.getKey(),
>>>>>>>>
>>>>>>>>  Input.release());
>>>>>>>>    }
>>>>>>>> +  // Patch up the install dir, so we find the same standard
>>>>>>>> library as the
>>>>>>>> +  // original compiler on MacOS.
>>>>>>>> +  Invocation->getHeaderSearchOpts().InstallDir =
>>>>>>>> Driver->getInstalledDir();
>>>>>>>>    return runInvocation(BinaryName, Compilation.get(),
>>>>>>>> std::move(Invocation),
>>>>>>>>                         std::move(PCHContainerOps));
>>>>>>>>  }
>>>>>>>>
>>>>>>>> Added:
>>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c%2B%2B/v1/mock_vector?rev=346652&view=auto
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> ---
>>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector (added)
>>>>>>>> +++
>>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector Mon
>>>>>>>> Nov 12 05:55:55 2018
>>>>>>>> @@ -0,0 +1 @@
>>>>>>>> +class vector {};
>>>>>>>>
>>>>>>>> Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp?rev=346652&view=auto
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp
>>>>>>>> (added)
>>>>>>>> +++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp Mon
>>>>>>>> Nov 12 05:55:55 2018
>>>>>>>> @@ -0,0 +1,17 @@
>>>>>>>> +// Clang on MacOS can find libc++ living beside the installed
>>>>>>>> compiler.
>>>>>>>> +// This test makes sure our libTooling-based tools emulate this
>>>>>>>> properly.
>>>>>>>> +//
>>>>>>>> +// RUN: rm -rf %t
>>>>>>>> +// RUN: mkdir %t
>>>>>>>> +//
>>>>>>>> +// Install the mock libc++ (simulates the libc++ directory
>>>>>>>> structure).
>>>>>>>> +// RUN: cp -r %S/Inputs/mock-libcxx %t/
>>>>>>>> +//
>>>>>>>> +// Pretend clang is installed beside the mock library that we
>>>>>>>> provided.
>>>>>>>> +// RUN: echo
>>>>>>>> '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++
>>>>>>>> -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e
>>>>>>>> 's/\\/\//g' > %t/compile_commands.json
>>>>>>>> +// RUN: cp "%s" "%t/test.cpp"
>>>>>>>> +// clang-check will produce an error code if the mock library is
>>>>>>>> not found.
>>>>>>>> +// RUN: clang-check -p "%t" "%t/test.cpp"
>>>>>>>> +
>>>>>>>> +#include <mock_vector>
>>>>>>>> +vector v;
>>>>>>>>
>>>>>>>> Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp?rev=346652&view=auto
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp
>>>>>>>> (added)
>>>>>>>> +++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp Mon
>>>>>>>> Nov 12 05:55:55 2018
>>>>>>>> @@ -0,0 +1,17 @@
>>>>>>>> +// Clang on MacOS can find libc++ living beside the installed
>>>>>>>> compiler.
>>>>>>>> +// This test makes sure our libTooling-based tools emulate this
>>>>>>>> properly.
>>>>>>>> +//
>>>>>>>> +// RUN: rm -rf %t
>>>>>>>> +// RUN: mkdir %t
>>>>>>>> +//
>>>>>>>> +// Install the mock libc++ (simulates the libc++ directory
>>>>>>>> structure).
>>>>>>>> +// RUN: cp -r %S/Inputs/mock-libcxx %t/
>>>>>>>> +//
>>>>>>>> +// Pretend clang is installed beside the mock library that we
>>>>>>>> provided.
>>>>>>>> +// RUN: echo
>>>>>>>> '[{"directory":"%t","command":"mock-libcxx/bin/clang++ -stdlib=libc++
>>>>>>>> -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e
>>>>>>>> 's/\\/\//g' > %t/compile_commands.json
>>>>>>>> +// RUN: cp "%s" "%t/test.cpp"
>>>>>>>> +// clang-check will produce an error code if the mock library is
>>>>>>>> not found.
>>>>>>>> +// RUN: clang-check -p "%t" "%t/test.cpp"
>>>>>>>> +
>>>>>>>> +#include <mock_vector>
>>>>>>>> +vector v;
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> cfe-commits mailing list
>>>>>>>> cfe-commits at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Ilya Biryukov
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Ilya Biryukov
>>>>
>>>
>
> --
> Regards,
> Ilya Biryukov
>


-- 
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190129/20524058/attachment-0001.html>


More information about the cfe-commits mailing list