To close the loop here, we were able to fix the build rules in Android without needing this flag.

In D119358#3371019, @pirama wrote:

My local ninja check-sanitizer passed but the automated build has failed: https://reviews.llvm.org/B153417. Let me do a ninja check-all and fix.

Looks like this assert is broken? https://reviews.llvm.org/B153417 and https://reviews.llvm.org/B153416 are some early indications.

My local ninja check-sanitizer passed but the automated build has failed: https://reviews.llvm.org/B153417. Let me do a ninja check-all and fix.

Looks like all comments are addressed. I'll merge after a quick ninja check.

Unrelated to missing resolver definition, this change doesn't accommodate resolvers that take parameters. (Curiously, this verification only fails with ThinLTO).

In D102097#3306103, @MaskRay wrote:

In D102097#3306040, @pirama wrote:

In D102097#3306014, @MaskRay wrote:

Hi, nice to see that musl gets more use. Can you clarify the Android use case? SANITIZER_ANDROID is defined while musl is used?

You had suggested in a comment to use SANITIZER_LINUX && !SANITIZER_GLIBC to detect MUSL but bionic would pass that check as well. If bionic and musl have different types/definitions, we'd then need SANITIZER_LINUX && !SANITIZER_GLIBC && !SANITIZER_ANDROID, as used in the diff I posted.

Not defining a musl macro is to follow its spirit https://wiki.musl-libc.org/faq.html "Why is there no MUSL macro?"

From my experience, most issues are related to glibc specific extensions or behaviors, less related to musl behaviors.
It seems that Android bionic may port quite a few glibc specific extensions as well.

So in many places SANITIZER_GLIBC || SANITIZER_ANDROID may be more appropriate than !SANITIZER_MUSL.

In D102097#3306014, @MaskRay wrote:

Hi, nice to see that musl gets more use. Can you clarify the Android use case? SANITIZER_ANDROID is defined while musl is used?

@ccross is building musl runtimes for Android host tools and we seem to need the following diff. The SANITIZER_LINUX && !SANITIZER_GLIBC idiom may not apply everywhere since it'd include Android as well. @vchuravy @MaskRay any thoughts? We could also consider an explicit CMake variable similar to _LIBCPP_HAS_MUSL_LIBC.

@alxu Just realized you don't have commit access. Do you want one of us to merge this?

In D117728#3256550, @arichardson wrote:

Do they still fail after rG89103bd2f06d? I was able to run ninja check-all successfully with a gdb linked against 2.7.

Thanks for the cleanup here.

In D114473#3162613, @danalbert wrote:

Doesn't seem like we use this for Android, but @pirama just in case.

reword commit message.

Will defer to Dan's review. I think these changes are safe for the Android platform (where we only use lld). The NDK only supports lld but that does not preclude external users from using other linkers.

arc diff --verbatim to update phabricator message

Do not hide __llvm_profile_raw_version on Apple platforms.

In D108006#3091859, @MaskRay wrote:

(I assume that Android will fix the issues so I can request changes to remove this from my very long review queue :)

In D111759#3079067, @MaskRay wrote:

According to https://reviews.llvm.org/rGf5380185623be243ba0f1b18d4bd594ac5cc7163 Apple's API verification tool (tapi) checks the exported __llvm_profile_raw_version.
However, ld64 cannot export a private-extern (hidden) symbol.

I wonder whether Apple's tool should lift the restriction. If not, we may need to special case __APPLE__.

In D111759#3078775, @zequanwu wrote:

In D111759#3078677, @pirama wrote:

In D111759#3078579, @zequanwu wrote:

In D111759#3076827, @pirama wrote:

In D111759#3076819, @zequanwu wrote:

This causes instrprof-darwin-exports.c test failure on MacOS:
ld: warning: cannot export hidden symbol ___llvm_profile_raw_version from /Users/zequanwu/llvm-project/build/release/lib/clang/14.0.0/lib/darwin/libclang_rt.profile_osx.a(InstrProfilingVersionVar.c.o)

I think we should stop exporting ___llvm_profile_raw_version from Darwin::addProfileRTLibs() in clang/lib/Driver/ToolChains/Darwin.cpp. @vsk Can you confirm?

Can you revert it for now to unblock the test failure on MacOS bots?

Done. Zequan, can you help test if removing https://github.com/llvm/llvm-project/blob/ab3d5d0533678ee93c93bd62db79c091741096f0/clang/lib/Driver/ToolChains/Darwin.cpp#L1292 fixes all failures? I'll submit this again once @vsk confirms if that is acceptable.

That doesn't help. I got different error:

Command Output (stdout):
--
2d1
< ___llvm_profile_raw_version

--

In D111759#3078579, @zequanwu wrote:

In D111759#3076827, @pirama wrote:

In D111759#3076819, @zequanwu wrote:

This causes instrprof-darwin-exports.c test failure on MacOS:
ld: warning: cannot export hidden symbol ___llvm_profile_raw_version from /Users/zequanwu/llvm-project/build/release/lib/clang/14.0.0/lib/darwin/libclang_rt.profile_osx.a(InstrProfilingVersionVar.c.o)

I think we should stop exporting ___llvm_profile_raw_version from Darwin::addProfileRTLibs() in clang/lib/Driver/ToolChains/Darwin.cpp. @vsk Can you confirm?

Can you revert it for now to unblock the test failure on MacOS bots?

In D111759#3076819, @zequanwu wrote:

This causes instrprof-darwin-exports.c test failure on MacOS:
ld: warning: cannot export hidden symbol ___llvm_profile_raw_version from /Users/zequanwu/llvm-project/build/release/lib/clang/14.0.0/lib/darwin/libclang_rt.profile_osx.a(InstrProfilingVersionVar.c.o)

Describe Android use case.

This is not related to weak attribute to __llvm_profile_raw_version

In D111759#3062678, @MaskRay wrote:

This leads to a runtime link failure if the libB.so available at runtime does not provide this symbol (but provides the other dependent symbols). Such a scenario can occur in Android's mainline modules.

The symbol is defined at link time but not available at run time? That's a pilot error.

Apply clang-format linter suggestion.

In D111759#3062703, @davidxl wrote:

In the summary, do you mean 'not excluded' instead of 'not included' ?

Remove attribute from reference.

Fix typos

@haowei Can you share instructions to run this exact build? I briefly tried https://fuchsia.googlesource.com/fuchsia/+/refs/heads/main/docs/development/build/toolchain.md but it is different from the failure at https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8834613633802961617/overview, which also seems to set CMAKE_SYSROOT.

@nickdesaulniers I presume this is still necessary? See https://reviews.llvm.org/D53463 for why we added this. (The Android kernel build does not use the Android triple.)

• rename file
• append '.s' to split-file output names

In D108328#2957896, @MaskRay wrote:

Worth checking whether archive-fetch.s can be folded into the test.

Update with review comments.

For the Android problem, it looks like a workaround as well. Do you know why gold linked executables were fine when Android was still using gold?

In D108006#2944140, @MaskRay wrote:

Ideally test coverage improvement should be a separate patch.

In D108006#2943030, @MaskRay wrote:

It's important to have symmetry. a) ld.lld def.a ref.so b) ld.lld ref.so def.a.
The patch addresses a) but not b).

I still need to add tests but wanted to upload it to get early thoughts on this approach.

Refactor preprocessor check to kmp_config.h.

Thanks for the reviews @mstorsjo @jhenderson.

Address review comments

Update test with revew comments.

Rename test, expand comments and remove '-S -x' from RUN line.

Simplify check and update expected error for one test.

Skip IMAGE_DEBUG_TYPE_UNKNOWN debug entries as well.

In D107324#2921410, @pirama wrote:

In D107324#2921407, @mstorsjo wrote:

Do you happen to know if the same is the case for any other debug directory types, or is this the only one?

I'm not sure if this is also the case for other debug directory types. Maybe someone familiar with Windows debug info can help? (+thakis, rnk)

In D107324#2921407, @mstorsjo wrote:

Do you happen to know if the same is the case for any other debug directory types, or is this the only one?

I uploaded this as a starter but we should abstract the check to avoid repetition. Is kmp_config.h.cmake or kmp_platform.h a good place for defining a 'KMP_USE_SHMEM' and using it here?

Remove note in comment.

s/CHECK-LTO/CHECK-LTO-NEXT in new-pm-O0-defaults.ll

Looks like quick_exit + dlclose interaction is not well-defined and this change may cause calls into dlclose-d libraries.

Thanks for the review @vsk and @richkadel for the details about Rust coverage instrumentation.