make a debug check thread-safe

[llunak]

Summary

See https://reviews.llvm.org/D122975#3430613 .

[JDevlieghere]

LGTM

[urnathan]

you need to modify the original in libcxxabi/src/demangle and then run the cp script there. Thanks for spotting this!

ETA:but it doesn't matter whether this check runs multiple times, in racing threads? I guess one wants an atomic set though. The once-only bits you've added are probably not available in libcxxabi?

[llunak]

In D123158#3432365, @urnathan wrote:

ETA:but it doesn't matter whether this check runs multiple times, in racing threads? I guess one wants an atomic set though.

AFAICT the problem is only that it causes warnings from thread-sanitizer, there is no actual problem as such. I don't know if an atomic set would help (@JDevlieghere can you try please?). But even if, I presume it's not available in libcxxabi either if once isn't?

The once-only bits you've added are probably not available in libcxxabi?

I have no idea about libcxxabi. I'm relatively unfamiliar with LLVM, I've just created this PR because the warning got mentioned as triggered by code in my other PR and I thought this would be a simple fix. If it isn't, then I probably don't know how to do better here.

[urnathan]

In D123158#3437191, @llunak wrote:

In D123158#3432365, @urnathan wrote:

ETA:but it doesn't matter whether this check runs multiple times, in racing threads? I guess one wants an atomic set though.

AFAICT the problem is only that it causes warnings from thread-sanitizer, there is no actual problem as such. I don't know if an atomic set would help (@JDevlieghere can you try please?). But even if, I presume it's not available in libcxxabi either if once isn't?

ah, that explains it. I didn't want to expose the array outside of the lookup function, but that would be another solution. Then a unit test can test it.

The once-only bits you've added are probably not available in libcxxabi?

I have no idea about libcxxabi. I'm relatively unfamiliar with LLVM, I've just created this PR because the warning got mentioned as triggered by code in my other PR and I thought this would be a simple fix. If it isn't, then I probably don't know how to do better here.

understood. Mind if I grab the PR? (Is there an actual PR to grab?)

[urnathan]

D123390 is available for review.

[llunak]

In D123158#3438694, @urnathan wrote:

understood. Mind if I grab the PR? (Is there an actual PR to grab?)

No, sorry, I meant Phabricator D's. Thank you for taking care of this.

llvm/include/llvm/Demangle/ItaniumDemangle.h

 //===----------------------------------------------------------------------===//
 
 #ifndef DEMANGLE_ITANIUMDEMANGLE_H
 #define DEMANGLE_ITANIUMDEMANGLE_H
 
 #include "DemangleConfig.h"
 #include "StringView.h"
 #include "Utility.h"
+#include "llvm/Support/Threading.h"
 #include <algorithm>
 #include <cassert>
 #include <cctype>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <limits>
 #include <utility>
static const OperatorInfo Ops[] = {
       {"ti", OperatorInfo::OfIdOp, /*Type*/ true, Node::Prec::Postfix,
        "typeid "},
   };
   const auto NumOps = sizeof(Ops) / sizeof(Ops[0]);
 
 #ifndef NDEBUG
   {
     // Verify table order.
-    static bool Done;
-    if (!Done) {
-      Done = true;
+    static llvm::once_flag Done;
+    llvm::call_once(Done, []() {
       for (const auto *Op = &Ops[0]; Op != &Ops[NumOps - 1]; Op++)
         assert(Op[0] < Op[1] && "Operator table is not ordered");
-    }
+    });
   }
 #endif
 
   if (numLeft() < 2)
     return nullptr;
 
   auto Op = std::lower_bound(
       &Ops[0], &Ops[NumOps], First,