C++11 lacks Errors when thread safety attribute is applied to static members

[llvmbot]

Bugzilla Link	32066
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@AaronBallman,@DougGregor,@pogo59

Extended Description

When compiling at C++11, no error is issued when thread safety attributes applied to static members.

I discovered this while updating the following Lit test to be C++11 compatible.
test/SemaCXX/warn-thread-safety-parsing.cpp

Here is a reduced test case b.cpp.

#define LOCKABLE            __attribute__ ((lockable))
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
#define GUARDED_BY(x)       __attribute__ ((guarded_by(x)))

class LOCKABLE Mutex { };

class FooStream;

class Foo {
public:
  Mutex mu;

  static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
  static int si GUARDED_BY(mu);
  friend FooStream& operator<<(FooStream& s, const Foo& f) EXCLUSIVE_LOCKS_REQUIRED(mu);
};

Compiling the above test at C++98 gives three errors

$ clang -c -Wthread-safety b.cpp -std=c++98
b.cpp:13:46: error: invalid use of member 'mu' in static member function
  static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
                                             ^~
b.cpp:2:82: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
                                                                                 ^~~~~~~~~~~
b.cpp:14:28: error: invalid use of non-static data member 'mu'
  static int si GUARDED_BY(mu);
                           ^~
b.cpp:3:56: note: expanded from macro 'GUARDED_BY'
#define GUARDED_BY(x)       __attribute__ ((guarded_by(x)))
                                                       ^
b.cpp:15:85: error: invalid use of non-static data member 'mu'
  friend FooStream& operator<<(FooStream& s, const Foo& f) EXCLUSIVE_LOCKS_REQUIRED(mu);
                                                                                    ^~
b.cpp:2:82: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
                                                                                 ^~~~~~~~~~~
3 errors generated.

Compiling the same test at C++11 gives no error.

$ /home/chli/Source/uclsvn/build/bin/clang -c -Wthread-safety b.cpp -std=c++11

[AaronBallman]

The code in C++11 mode will give you a warning about the use of a non-static data member in an unevaluated context being incompatible with C++98, so in some sense this incompatibility is expected. However, I agree that it should still be diagnosed because the non-static member should not be usable from a static context.

I kind of wonder if we need a new "unevaluated" context for the attribute parsing case, which is effectively the same as unevaluated but without the relaxation? I tried using DiscardedStatement because it comes close, but that also impacted things like capturing this in a lambda marked as exclusive locks required (see SemaCXX/warn-thread-safety-analysis.cpp:4454 as an example).

[aaronpuchert]

What's strange is that these member references are not parsed as MemberExpr but as DeclRefExpr as it were an ordinary variable. I wonder if that's intended behavior. (AST simplified.)

CXXMethodDecl foo 'void ()' static
`-RequiresCapabilityAttr exclusive_locks_required
  `-DeclRefExpr 'Mutex' lvalue Field 'mu' 'Mutex' non_odr_use_unevaluated
VarDecl si 'int' static
`-GuardedByAttr
  `-DeclRefExpr 'Mutex' lvalue Field 'mu' 'Mutex' non_odr_use_unevaluated
FriendDecl
`-FunctionDecl friend_undeclared operator<< 'FooStream &(FooStream &, const Foo &)'
  |-ParmVarDecl s 'FooStream &'
  |-ParmVarDecl f 'const Foo &'
  `-RequiresCapabilityAttr exclusive_locks_required
    `-DeclRefExpr 'Mutex' lvalue Field 'mu' 'Mutex' non_odr_use_unevaluated

It even says that this is a Field(Decl).

[AaronBallman]

I wonder if that's intended behavior.

Semi intended behavior, perhaps. I suspect we don't have a member expression because a MemberExpr has a base expression and when we parse, we only see the identifier, hence we get a DeclRefExpr that we resolve later contextually. We may want to introduce a base expression so we get a MemberExpr, but this also touches on bounds safety attribute behavior that's been in a bit of a state of flux lately. CC @bwendling @isanbard @rapidsna

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [32066](https://llvm.org/bz32066) | | Version | trunk | | OS | Linux | | Reporter | LLVM Bugzilla Contributor | | CC | @AaronBallman,@DougGregor,@pogo59 |

Extended Description

When compiling at C++11, no error is issued when thread safety attributes applied to static members.

I discovered this while updating the following Lit test to be C++11 compatible.
test/SemaCXX/warn-thread-safety-parsing.cpp

Here is a reduced test case b.cpp.

#define LOCKABLE            __attribute__ ((lockable))
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
#define GUARDED_BY(x)       __attribute__ ((guarded_by(x)))

class LOCKABLE Mutex { };

class FooStream;

class Foo {
public:
  Mutex mu;

  static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
  static int si GUARDED_BY(mu);
  friend FooStream& operator<<(FooStream& s, const Foo& f) EXCLUSIVE_LOCKS_REQUIRED(mu);
};

Compiling the above test at C++98 gives three errors

$ clang -c -Wthread-safety b.cpp -std=c++98
b.cpp:13:46: error: invalid use of member 'mu' in static member function
  static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
                                             ^~
b.cpp:2:82: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
                                                                                 ^~~~~~~~~~~
b.cpp:14:28: error: invalid use of non-static data member 'mu'
  static int si GUARDED_BY(mu);
                           ^~
b.cpp:3:56: note: expanded from macro 'GUARDED_BY'
#define GUARDED_BY(x)       __attribute__ ((guarded_by(x)))
                                                       ^
b.cpp:15:85: error: invalid use of non-static data member 'mu'
  friend FooStream& operator<<(FooStream& s, const Foo& f) EXCLUSIVE_LOCKS_REQUIRED(mu);
                                                                                    ^~
b.cpp:2:82: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__ ((exclusive_locks_required(__VA_ARGS__)))
                                                                                 ^~~~~~~~~~~
3 errors generated.

Compiling the same test at C++11 gives no error.

$ /home/chli/Source/uclsvn/build/bin/clang -c -Wthread-safety b.cpp -std=c++11