I uploaded D127465, weakening the test.

This change is needed to keep the clang/test/Preprocessor/cuda-types.cu test passing after D28213 changes the value of __CLANG_ATOMIC_LLONG_LOCK_FREE from 1 to 2 for i586 and up.

Ok, I can upload a patch omitting *_ATOMIC_LLONG_LOCK_FREE from the macro testing.

In D127267#3570269, @yaxunl wrote:

This patch is to fix an issue, right? At least we need a test to prevent that issue from happening again.

In D127267#3568388, @rprichard wrote:

Aside: There are only two calls to AllocateTarget: one in TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. This change removes NVPTXTargetInfo's call.

I don't expect this change to affect the compiler behavior by itself -- is there a particular test that should be written?

This change is needed for D28213, so that the __GCC_ATOMIC_LLONG_LOCK_FREE macro matches for -target i386-unknown-linux-gnu between --cuda-host-only and --cuda-device-only. This is tested in clang/test/Preprocessor/cuda-types.cu.

Rebase this revision.

@mgorny Do you mind if I commandeer this Phabricator revision (and also D29542, which I would abandon in favor of D59566 which is already merged)? I need this CL to get libc++ tests passing on i686-linux after D119931 was submitted.

The fix and the test both look good to me.

I'm seeing this test fail on 32-bit x86 Linux currently because __CLANG_ATOMIC_LLONG_LOCK_FREE is still 1.

Maybe this change is obsolete now that D59566 is merged?

In D123428#3476371, @rprichard wrote:

In D123428#3475346, @smeenai wrote:

In D123428#3444212, @MaskRay wrote:

https://github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/address_is_readable.cc has gone through several iterations. We can use rt_sigprocmask.

@rprichard, do you know if this would work for Android? It has the TOCTOU issue, but I imagine it's much simpler than having to manage and synchronize the pipe fd, and we could live with the TOCTOU in practice.

I'm glad @MaskRay found this -- I think it will probably work, and it seems better than assuming the PC is readable.

I see rt_procsigmask listed in bionic/libc/SYSCALLS.TXT, and I don't see it mentioned in any of the bionic/libc/SECCOMP*.txt files. I think that means seccomp is allowing the system call. I looked at kernel/signal.c, and AFAICT it's not doing any security checks that could be a problem. Bionic itself uses rt_sigprocmask for (at least) spawning new processes, creating/exiting threads, TLS-related locking, POSIX timers, and abort(). I think any seccomp-like blocking of rt_sigprocmask would have to be very targeted, so I think the syscall is probably allowed everywhere on Android.

It is assuming that the kernel will validate the address before validating the how. The kernel has a principle of not breaking userland -- is there a more specific guarantee we can rely on? e.g. The code has this comment:

// This strategy depends on Linux implementation details,
// so we rely on the test to alert us if it stops working.

The kernel source is structured as two wrappers around sigprocmask, SYSCALL_DEFINE4(rt_sigprocmask, ...) and COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, ...). The wrappers copy user memory to/from kernel memory before calling sigprocmask, so it makes sense that they would validate the address first.

In D123428#3475346, @smeenai wrote:

In D123428#3444212, @MaskRay wrote:

https://github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/address_is_readable.cc has gone through several iterations. We can use rt_sigprocmask.

@rprichard, do you know if this would work for Android? It has the TOCTOU issue, but I imagine it's much simpler than having to manage and synchronize the pipe fd, and we could live with the TOCTOU in practice.

In D122251#3446881, @ychen wrote:

In D122251#3446870, @rprichard wrote:

  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 80, in _executeScriptInternal
    litConfig = lit.LitConfig.LitConfig(
TypeError: __init__() missing 1 required positional argument: 'order'

Fixed in 81b51b61f849fa91bdf5d069591

I think this change broke libc++ tests. _executeScriptInternal in libcxx/utils/libcxx/test/dsl.py does not pass an argument for LitConfig's new order parameter:

In D123428#3444170, @smeenai wrote:

In D123428#3444134, @rprichard wrote:

Using /proc/self/maps would be subject to TOCTOU, but I think most methods wouldn't, e.g.:

• Open /proc/self/mem and pread() the address. This seems strictly better than /proc/self/maps?
• Create a pipe using pipe(), write() the bytes into the pipe buffer and read() them back out. I believe a Linux pipe buffer is guaranteed to be big enough (>= 8 bytes).
• process_vm_readv

I wonder if security configurations are a problem. Maybe I should experiment on an Android build.

With all those methods, there's still the chance (however unlikely) that the address is readable at the time of the check but somehow becomes unreadable by the time we perform the actual read, right? I doubt it matters much in practice though.

... that's still subject to time-of-check-to-time-of-use issues

Suggested issue title: [libcxx] locale_bionic.h: skip ndk-version.h on Android platform

Yeah, I'm OK with this change. (I'm not really familiar with the MTE stuff.)

In D110720#3109821, @probinson wrote:

The lines where CHECK is split into CHECK-LINUX and CHECK-NOLINUX are also enabling the test for nvptx.

It's true that the nvptx case is now checking more than it used to, but not everything; making it check everything could probably be done, but is it worthwhile? I experimented with passing CHECK to the nvptx case, and it doesn't pass; there would have to be a CHECK-NVPTX line paralleling each CHECK: line, pretty much. Seems like a lot of effort, but is nvptx really that special?
I'll do it if someone says Yes, it's worth the trouble; but unless someone can say that, I'd rather not.

... but it seems much easier to put the -fno-outline-atomics flag into the linker's .bp file instead of changing the behavior of __system_property_get.

In D114523#3165542, @bc-lee wrote:

From what I understand, for dynamic executables, si->call_constructors() [1] calls the constructor of each loaded shared object. Right?

I put the LD_DEBUG=2 environment variable and found that libc.so's constructor is called before the main executable or any other shared object's constructor. And libc.so calls __libc_preinit_impl -> __libc_init_common -> __system_properties_init.
So the point is that __system_properties_init is initialized before this compiler-rt code is called. But I don't know if this is guaranteed behavior.

Of course, I think this code should work fine even if __system_properties_init is not yet called.

[1] https://cs.android.com/android/platform/superproject/+/9aafec59ccad4b3851269e4119561efd8a2f5e65:bionic/linker/linker_main.cpp;l=508

Also: is it OK to use a mix of LSE and non-LSE atomic operations to access the same memory concurrently? I would think that should be fine, but it seems worth mentioning.

I noticed that using __system_property_get from a constructor function probably doesn't work within the Bionic loader (linker[64]), but maybe Bionic ought to be fixed (and it shouldn't matter much in practice):

• The Bionic dynamic loader calls the constructor functions before calling __system_properties_init. (see tmp_linker_so.call_constructors();)
• However, a static executable calls __system_properties_init before the constructors, so static executables should be OK.
• Maybe the dynamic loader ought to call __system_properties_init before constructors.
• Calling __system_property_get before __system_properties_init is probably OK? It looks like SystemProperties::Find would return nullptr, and __system_property_get would return 0.
• In any case, it wouldn't actually matter unless someone ran a new loader on an affected device+kernel.

The motivating issue happened with Android (D107509).

Adding some Android folks to the subscriber list.

The dereferenceable.ll and dereferenceable-inseltpoison.ll files are also confusing to me. It *sounds* like they're trying to test inferred attributes, but its CHECK* lines would allow any or no attributes on the functions, so it's not actually testing anything attribute-related.

Follow up from D107527 review.

I noticed this inconsistency while reviewing D107509.

Update test.

Yes, Android did add stpcpy and stpncpy in API 21:
https://android.googlesource.com/platform/bionic/+/refs/heads/android11-dev/libc/libc.map.txt

In D107127#2917667, @eugenis wrote:

It resolves everything to the first definition in the lookup order. The only special handling of weak symbols that I've found is that missing definition is not an error.

In D107127#2917555, @eugenis wrote:

Weak symbol is a purely link-time concept, AFAIK. It is ignored in the dynamic loader, so, unless we link multiple copies of this file into the same DSO, "weak" attribute here should not have any effect.

Yeah, this looks fine to me.

Update another place.

error: [FE_UPWARD] in __compiler_rt_scalbn(-0x1p+0 [BFF0000000000000], 10000) = -0x1.fffffffffffffp+1023 [FFEFFFFFFFFFFFFF] != -inf [FFF0000000000000]

In D96403#2639210, @srhines wrote:

It is actually using clang-r416183 from toolchains.py. From there, you can go to the manifest for building those prebuilts. This line shows you the SHA for the toolchain/llvm-project used to build the prebuilts. Finally, you can look here to see what was merged. Note that there's a typo for the revision number in the commit message, but the upstream LLVM SHA mentioned there is correct. Here is where this toolchain was built in case that is useful/interesting.

In D96403#2638883, @smeenai wrote:

With NDK r22, I only see libunwind.a for armv7. Will it be provided for other architectures in future NDK versions?

Rebase this revision.

I think this is OK for Android.

MSVC's scalbn appears to ignore the current rounding mode, so the value returned from __compiler_rt_scalbn[f] disagreed with the value returned from scalbn[f]. I think it's sufficient to disable the non-default-rounding-mode tests for MSVC. With this change, the __divdc3 and __divsc3 builtins, when compiled with MSVC, should produce the same results as on Linux systems (or as with MinGW and the other Unix systems I tested).

Disable non-default-rounding-mode scalbn[f] tests when using the MSVC libraries.

In D97356#2586381, @vitalybuka wrote:

This happens only once after clean build, so it did not annoy us, but I don't mind for even bigger value e.g. 10m

The new unit tests failed on sanitizer-windows. I reverted it for now. https://lab.llvm.org/buildbot/#builders/127/builds/6620