[clang] Missed optimization: dead store to thread-local not optimized out

[tavianator]

For this code

void foo(void **ptr) {
    static _Thread_local void *tmp;
    tmp = *ptr;
    *ptr = 0;
}

Clang generates

foo:
        movq    (%rdi), %rax
        movq    %rax, %fs:foo.tmp@TPOFF
        movq    $0, (%rdi)
        retq

I think it's an alias analysis issue, because a _Thread_local int is optimized out. But even without alias analysis, we know that tmp hasn't had its address taken.

[dtcxzyw]

Related patch: https://reviews.llvm.org/D69428

[dtcxzyw]

@eleviant @nunoplopes @MaskRay @Sterling-Augustine @vitalybuka @nikic @efriedma-quic Do you guys have plan to reland https://reviews.llvm.org/D69428? What is the blocker of this patch?

IIRC if we don't want to remove a root which may be used by a leak detector, we should mark it with __attribute__((__used__)) and volatile. BTW the problem in BuryPointer has been fixed by https://reviews.llvm.org/D101217.

[efriedma-quic]

There was a long thread https://lists.llvm.org/pipermail/llvm-dev/2021-April/149828.html ... basically, it interferes with certain kinds of run-time instrumentation, and nobody really had a solution. Maybe it makes sense to put it under a flag. (Certainly not the first time LLVM optimizations interfere with Valgrind, and probably not the last...)

Maybe the current code could also be cleaned up; see also #84694.

[nikic]

As far as I can tell, GCC doesn't try to do similar pointer root special casing. If that is the case, this is clearly not a reliable pattern anyway. So I think we should do the change. We can add a temporary cl::opt option to keep the pointer root behavior to give Google (and other people relying on current behavior) time to migrate to the recommended __used__ pattern.

GCC and Clang for trivial cases: https://gcc.godbolt.org/z/Yq5bn1TTb Worth noting that Clang actually optimizes the store away in the simplest case, because the global -> alloca transform doesn't try to protect against pointer roots.

[vitalybuka]

In additional to the copt<>, may I ask also to disable the feature for asan/hwasan?
Unfortunately we can't do that for lsan, as lsan is link time feature.