Basically this should be Ok. We set the LLVM_HOST_TRIPLE to match the GCC triple on almost all platforms now.

In D110900#4160169, @tstellar wrote:

We switched to using <arch>-redhat-linux-gnu as the default triple and we patch clang to translate the triple from <arch>-redhat-linux-gnu to <arch>-redhat-linux only when searching for the gcc installation, but not for anything else.

In D110900#3044159, @aaronpuchert wrote:

Can't speak for RedHat, but on SUSE we have

> gcc -dumpmachine
x86_64-suse-linux
> clang -dumpmachine
x86_64-unknown-linux-gnu

Should be obsolete after 016785d9316d8c5abc5fdf3cdb86479095bbb677.

@beanz, a configuration error caused by this in a third-party project has been reported to me:

This might have caused a documentation build error:

Warning, treated as error:
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/build/tools/clang/docs/index.rst:81:toctree contains reference to nonexisting document 'ClangNvlinkWrapper'

Perhaps we need to remove the entries from clang/docs/index.rst?

diff --git a/clang/docs/index.rst b/clang/docs/index.rst
index 4befe61ee5ca..e572f706c01f 100644
--- a/clang/docs/index.rst
+++ b/clang/docs/index.rst
@@ -87,9 +87,7 @@ Using Clang Tools
    ClangFormatStyleOptions
    ClangFormattedStatus
    ClangLinkerWrapper
-   ClangNvlinkWrapper
    ClangOffloadBundler
-   ClangOffloadWrapper
    ClangOffloadPackager
    ClangRepl

Yeah, I don't usually do this. But this has been sitting for a very long time with a couple of pings and no attention.

Doesn't seem like I can get this reviewed, so let me just land it and see if someone complains.

In D137043#3935526, @Origami404 wrote:

In D137043#3935129, @aaronpuchert wrote:

This include-if-exists mechanism seems brittle to me.

Do you mean the way that we used to test a file and include it (inserting #if __has_include) is brittle or compilation flags like --include-if-exists themselves are?

This include-if-exists mechanism seems brittle to me. Can we not make it dependent on the triple, i.e. include the file if we're using a libc implementation that's known to provide (and require) this file?

Nice to see that we have this now! (There was an earlier attempt in D18964 that somehow didn't make it.)

In D136447#3874695, @dmgreen wrote:

Do you mean converting ule 0 -> eq 0? I wanted the code here to be correct without needing it.

Thanks for the detailed write-up, very much appreciated.

In D129755#3865342, @rupprecht wrote:

I'm seeing a fair number of breakages from this patch (not really sure how many we truly have, I've hit ~5-10 so far in widely used libraries, but I suspect we have far more in the long tail).

@aaron.ballman, would like some feedback on the release notes. Should I additionally write something under "Potentially Breaking Changes", or is it enough to mention this under "Improvements to Clang's diagnostics"? Though I guess we could also add this later on if we get more complaints that this breaks things.

@hans, where you able to fix or work around the warnings? I'd like to land this again, but if you need more time it can also wait a bit.

Add sentence to release notes that we now look into more constructor calls and that this could lead to additional warnings being emitted.

In D129755#3843144, @gulfem wrote:

We also started seeing -Wthread-safety-precise error in our Fuchsia code.
https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release/b8800959115965408001/overview
I'm trying to verify with our team whether it is a false positive, but I just wanted to give you heads up!

• Pass til::LiteralPtr *Self for automatic object destructors into handling of require_capability and locks_excluded attributes.
• Add [[maybe_unused]] attribute to silence -Wunused-variable warinng in Release builds.
• A few more tests.

In D129755#3843206, @aaronpuchert wrote:

Presumably Cursor is some kind of alias to VmoCursor, as we don't look at base destructors yet. Since the code is not easily searchable for me, can you look up the annotations on DiscardableVmosLock::Get, the constructor of Cursor/VmoCursor being used here, Cursor::lock_ref, and AssertHeld?

In D129755#3843144, @gulfem wrote:

We also started seeing -Wthread-safety-precise error in our Fuchsia code.
https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release/b8800959115965408001/overview
I'm trying to verify with our team whether it is a false positive, but I just wanted to give you heads up!

In D129755#3842729, @hans wrote:

We're hitting a false positive in grpc after this:

> ../../third_party/grpc/src/src/core/lib/gprpp/ref_counted_ptr.h:335:31: error: calling function 'TlsSessionKeyLoggerCache' requires holding mutex 'g_tls_session_key_log_cache_mu' exclusively [-Werror,-Wthread-safety-analysis]
>   return RefCountedPtr<T>(new T(std::forward<Args>(args)...));
>                               ^
> ../../third_party/grpc/src/src/core/tsi/ssl/key_logging/ssl_key_logging.cc:121:26: note: in instantiation of function template specialization 'grpc_core::MakeRefCounted<tsi::TlsSessionKeyLoggerCache>' requested here
>      cache = grpc_core::MakeRefCounted<TlsSessionKeyLoggerCache>();
>                         ^


The code looks like this:

grpc_core::RefCountedPtr<TlsSessionKeyLogger> TlsSessionKeyLoggerCache::Get(
    std::string tls_session_key_log_file_path) {
  gpr_once_init(&g_cache_mutex_init, do_cache_mutex_init);
  GPR_DEBUG_ASSERT(g_tls_session_key_log_cache_mu != nullptr);
  if (tls_session_key_log_file_path.empty()) {
    return nullptr;
  }
  {
    grpc_core::MutexLock lock(g_tls_session_key_log_cache_mu);        <---------- holding the mutex
    grpc_core::RefCountedPtr<TlsSessionKeyLoggerCache> cache;
    if (g_cache_instance == nullptr) {
      // This will automatically set g_cache_instance.
      cache = grpc_core::MakeRefCounted<TlsSessionKeyLoggerCache>();      <------ line 121



lock is holding a MutexLock (I assume that's an exclusive thing) on g_tls_session_key_log_cache_mu.

See https://bugs.chromium.org/p/chromium/issues/detail?id=1372394#c4 for how to reproduce.

I've reverted this in https://github.com/llvm/llvm-project/commit/a4afa2bde6f4db215ddd3267a8d11c04367812e5 in the meantime.

No longer needed as we went with a different solution in D132799.

Add trimmed-down documentation back in and a release note.

Perhaps wait for @mgorny before you submit. Just a minor suggestion from me how you could simplify this.

In D129755#3779997, @aaron.ballman wrote:

Please be sure to add a release note for the changes!

Go back to only adding libatomic. The LLVM_SYSTEM_LIBS contain many libraries that seem to be private dependencies only, and for now we don't properly track private vs public dependencies on component libraries.

In D132799#3779971, @beanz wrote:

The Z3 Solver is an external dependency that gets put on any user of libLLVM if they are building with Z3, so that's correct too.

In D132799#3784103, @aaronpuchert wrote:

As an alternative, we strip when the path is part of the standard linker search paths.

Hmm, seems that since D79219 we use find_package, which has a habit of producing full paths instead of e.g. -lz for zlib, so for llvm-config we strip off the path and pretend we've been linking against the system library all along.

In D132799#3783603, @mstorsjo wrote:

CMake manages to find a custom-installed libzstd in a nondefault location (/home/linuxbrew/.linuxbrew/lib/libzstd.so.1.5.2), and for every other place that these dependencies are used, it's linked by just adding that absolute path to the linking command line. However after this commit, when linking, it links by passing both -lzstd and /home/linuxbrew/.linuxbrew/lib/libzstd.so.1.5.2, and linking fails since the -lzstd is unresolved.

Ping.

I was under the impression that we've already switched to C++17, but the Windows pre-submit build failed with:

C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2107): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'
C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2120): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'
C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2418): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'

Perhaps I should move the init statements out again?

Use SmallDenseMap plus some minor changes.

Thanks!

Add all dependencies from Support's LLVM_SYSTEM_LIBS as public link dependencies of libLLVM.

In D132799#3759418, @beanz wrote:

We probably should replace atomic here with the LLVM_SYSTEM_LIBS property from the LLVMSupport target.

In D132873#3756364, @mstorsjo wrote:

If you care to share, it'd be interesting to know under what circumstances you end up with no struct definition.

Landed in 0c5ce1d7fba38948c27ed6b875f962cd60895574.

The documentation build (docs-{llvm,clang}-{html,man}) is fine after this, in fact it fixes:

Warning, treated as error:
[...]/build/tools/clang/docs/ReleaseNotes.rst:632:Bullet list ends without a blank line; unexpected unindent.

by adding some indentation.

Use links instead :manpage:, which is meant for man pages cross-referencing each other.

This is for release/15.x.

Ping.

In D129160#3721943, @isuruf wrote:

Sure. If an application links to libclang.so when the application is being built, the application will hardcode libclang.so.13 in it and will look for it.
When the SONAME changes to libclang.so.15 in LLVM 15, the application will not be able to use the libclang from LLVM 15 unless the
application was rebuilt with libclang.so in LLVM 15.

Also, this change did not really acheive it's purpose of allowing apps to use newer versions of libclang.so without rebuilding, because a new version of libclang.so requires a new version of libLLVM.so, which does not have a stable ABI.

This has been accepted for quite some time, any reason why we're not landing it?

By the way, the original bug for this is #34191.

In D129752#3682977, @aaron.ballman wrote:

Do you think this warrants a release note (or did it close any open issues in the tracker)?