llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.02k stars 11.96k forks source link

-Wthread-safety: unlock_function and default destructor #101199

Open plmonette opened 3 months ago

plmonette commented 3 months ago

I'm writing macro that check if the code is executed on the right virtual thread. I want to use -Wthread-safety to enforce that this check is done everywhere a member variable is accessed by using the "guarded_by" attribute.

This works by "locking" a virtual thread checker and "unlocking" it at the end of the scope. The constructor of the scoped object does the actual check (by looking at the ID of the virtual thread), but the destructor does nothing.

With a real lock, the destructor would call unlock on the lock object, but in my use case there is nothing to unlock. We've already verified the function runs in the right virtual thread at the beginning of its execution.

Now, because the destructor does nothing, I want to define it using "= default". But this will result in an error: warning: virtual thread 'vtc_' is still held at the end of the function [-Wthread_safety-analysis]

Defining the destructor using {} works and I get no errors.

My guess is that this is either an error, or intentional because with a real scoped object, you'd want to do something in your destructor to unlock a lock?

Minimal repro (compile on godbolt using "x86-64 clang (trunk)" and with "-Wthread-safety".

#define CHECK(condition)  \
  if (condition) __builtin_unreachable()

#define CHECK_ON_VIRTUAL_THREAD(virtual_thread_checker) \
  ScopedVirtualThreadChecker scoped_virtual_thread_chcker(virtual_thread_checker)

class __attribute__((capability("virtual thread"))) VirtualThreadChecker {
  public:
   bool IsOnCorrectVirtualThread() const {
     /* The actual virtual thread verification would be done here. */
     return true;
   }    
};

class __attribute__((scoped_lockable)) ScopedVirtualThreadChecker {
 public:
  ScopedVirtualThreadChecker(const VirtualThreadChecker& vtc) __attribute((exclusive_lock_function(vtc))) {
    CHECK(vtc.IsOnCorrectVirtualThread());    
  }
  // Change = default to {} to fix errors.
  ~ScopedVirtualThreadChecker() __attribute((unlock_function())) = default;
};

class MyClass {
 public:
  MyClass() = default;
  ~MyClass() = default;

  void Increment() {
    CHECK_ON_VIRTUAL_THREAD(vtc_);

    ++count_;
  }

 private:
  int count_ __attribute__((guarded_by(vtc_))) = 0;

  VirtualThreadChecker vtc_;
};

int main() {
  MyClass my_class;
  my_class.Increment();
}
aaronpuchert commented 2 months ago

The problem is likely that trivial destructors are omitted from the CFG. Not sure what we can do about that.