[clang] Thread safety analysis: provide printSCFG definition. (PR #80277)
https://github.com/aaronpuchert approved this pull request. Thanks, looks good to me! https://github.com/llvm/llvm-project/pull/80277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: provide printSCFG definition. (PR #80277)
aaronpuchert wrote: This sounds like a good idea! https://github.com/llvm/llvm-project/pull/80277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: provide printSCFG definition. (PR #80277)
aaronpuchert wrote: It might have been commented out so that it doesn't take up space in the compiled binary. I'd like seeing it compiled, just to make sure it doesn't break. But I'd also like if it doesn't appear in the final binary. Perhaps we can change visibility so that `--gc-sections` or LTO can optimize it out? But I'm not sure if that's enough. https://github.com/llvm/llvm-project/pull/80277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)
https://github.com/aaronpuchert approved this pull request. https://github.com/llvm/llvm-project/pull/76007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)
https://github.com/aaronpuchert approved this pull request. For now I guess this is Ok, although I think the better fix would be to diagnose missing or duplicate `default` labels even in the dependent case. Because `default` labels themselves are never dependent. https://github.com/llvm/llvm-project/pull/76007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote:
@gendalph, this warning is meant to always warn if a {{default}} label is
missing, even if all enumeration values are covered. If you don't want a
warning on enumerations, use the previously mentioned clang-tidy check
[bugprone-switch-missing-default-case](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/switch-missing-default-case.html).
We can probably discuss adding this as warning, though I doubt we'll get
consensus on that. This warning was added despite never going to be always-on
because GCC has it (under the same name) and some code styles ask for it.
https://github.com/llvm/llvm-project/pull/73077
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema , Decl *D, const
ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
aaronpuchert wrote:
Hmm, it seems that GCC is also warning in cases where we're not forwarding the
format string, for example here:
```c
void forward_tgt(char* tgt) {
va_list va;
const char* fmt;
vsprintf(tgt, fmt, va);
}
```
produces "warning: function 'void forward_tgt(char*)' might be a candidate for
'gnu_printf' format attribute [-Wsuggest-attribute=format]". But I think that's
a false positive, and we can easily check for proper forwarding instead.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema , Decl *D, const
ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
+
+ // Check if there is parent function format attribute which type matches
+ // calling function format attribute type.
+ if (llvm::any_of(
+ ParentFuncDecl->specific_attrs(),
+ [&](const FormatAttr *Attr) { return Attr->getType() == AttrType; }))
+return;
+
+ unsigned int FirstToCheck =
+ ParentFuncDecl->isVariadic() ? (ParentFuncDecl->getNumParams() + 1) : 0;
+
+ // Diagnose missing format attributes
+ std::string InsertionText;
+ llvm::raw_string_ostream OS(InsertionText);
+ OS << "__attribute__((format(" << AttrType->getName() << ", "
+ << std::to_string(StringIndex) << ", " << std::to_string(FirstToCheck)
aaronpuchert wrote:
I'm surprised that we need `std::to_string` here, can
`llvm::raw_string_ostream` not print integers directly?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema , Decl *D, const
ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
aaronpuchert wrote:
I don't think we can decide this based on the type. We should look at the
argument and check if it's a `DeclRefExpr` referring to a parameter of the
parent function. There can be other (unrelated) string arguments. (See also my
comments in the tests.)
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+#include
+#include
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */)
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 1, 4)))
+}
+
+void f2(char *out, va_list args)
+{
+vprintf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'printf' format attribute to the declaration of 'f2'}}
+// CHECK-FIXES: __attribute__((format(printf, 1, 0)))
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f2'}}
+ // CHECK-FIXES: __attribute__((format(scanf, 1, 0)))
+}
+
+void f3(char* out, ... /* args */)
+{
+va_list args;
+vprintf("test", args); // no warning
+}
+
+void f4(char *out, ... /* args */)
+{
+const char *ch;
+va_list args;
+vscanf(ch, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f4'}}
+ // CHECK-FIXES: __attribute__((format(scanf, 1, 2)))
aaronpuchert wrote:
I don't think we can propagate the attribute here, since the format string is a
local variable. It doesn't come from the caller.
The `out` parameter seems unused here, and has no relation to the format string
`ch`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,132 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+
+void f1(const std::string , ... /* args */)
+{
+va_list args;
+vscanf(str.c_str(), args); // no warning
+vprintf(str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error: {{format argument not
a string type}}
+void f2(const std::string , ... /* args */);
+
+void f3(std::string_view str, ... /* args */)
+{
+va_list args;
+vscanf(std::string(str).c_str(), args); // no warning
+vprintf(std::string(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f4(std::string_view str, ... /* args */);
+
+void f5(const std::wstring , ... /* args */)
+{
+va_list args;
+vscanf((const char *)str.c_str(), args); // no warning
+vprintf((const char *)str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f6(const std::wstring , ... /* args */);
+
+void f7(std::wstring_view str, ... /* args */)
+{
+va_list args;
+vscanf((const char *) std::wstring(str).c_str(), args); // no warning
+vprintf((const char *) std::wstring(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f8(std::wstring_view str, ... /* args */);
+
+void f9(const wchar_t *out, ... /* args */)
+{
+va_list args;
+vprintf(out, args); // expected-error {{no matching function for call to
'vprintf'}}
+vscanf((const char *) out, args); // no warning
+vscanf((char *) out, args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f10(const wchar_t *out, ... /* args */);
+
+void f11(const char16_t *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f12(const char16_t *out, ... /* args */);
+
+void f13(const char32_t *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2))) // expected-error {{format argument not a
string type}}
+void f14(const char32_t *out, ... /* args */);
+
+void f15(const char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f15'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f16(const char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // no warning
+}
+
+void f17(const unsigned char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f18(const unsigned char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+void f19(const signed char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f20(const signed char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+void f21(const char out[], ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f21'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f22(const char out[], ... /* args */)
+{
+va_list args;
+vscanf(out, args); // no warning
+}
+
+template
+void func(char ()[N], ...)
+{
+va_list args;
+vprintf(str, args); // no warning
+}
aaronpuchert wrote:
What makes C++ interesting is the implicit `this` parameter in non-static
member functions, maybe you can add tests for that? The existing tests here
seem to be mostly about cases where the attribute does not apply due to some
technicality.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema , Decl *D, const
ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
aaronpuchert wrote:
Do we need to do a bounds check before we access into the array or has this
already been checked?
Also, the attribute arguments start counting at 1, but array access starts at
0, so I would assume we're off by one here?
And shouldn't we look up `Attr->getFormatIdx()` instead? The "first argument"
can be 0, if the caller takes a `va_list` for example.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert commented: Two additional checks that might be interesting: * Look at the `FormatIdx` argument. Is it a `DeclRefExpr` referring to a `ParmVarDecl`, perhaps with some conversions? (There is probably going to be an `LValueToRValue` conversion at least. There are special `Ignore*` functions on `Expr` to unwrap that.) If it is, we should propagate the attribute, if not then not. * For `FirstArg` it seems that GCC only warns when we call a `v*` function, or the called function has `FirstArg == 0`, probably because forwarding to a variadic function isn't really possible in C. It doesn't seem to check whether we're actually forwarding. What you've implemented here (deciding based on `ParentFuncDecl->isVariadic()`) seems like a reasonable heuristic. But it probably makes sense to additionally restrict based on the archetype or `FirstArg`. Or is that what you're doing with `Args[Attr->getFirstArg()]`? https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema , Decl *D, const
ParsedAttr ) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
+
+ // Check if there is parent function format attribute which type matches
+ // calling function format attribute type.
+ if (llvm::any_of(
+ ParentFuncDecl->specific_attrs(),
+ [&](const FormatAttr *Attr) { return Attr->getType() == AttrType; }))
+return;
aaronpuchert wrote:
The index arguments could still be off, should we warn about that?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+#include
+#include
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */)
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 1, 4)))
aaronpuchert wrote:
Shouldn't it be `__attribute__((format(printf, 3, 4)))`, since the format
string is the third argument? The first argument is just the target buffer.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert approved this pull request. Thanks, looks good to me! https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -2487,15 +2486,15 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
// Clean up constructed object even if there are no attributes to
// keep the number of objects in limbo as small as possible.
- if (auto Object = LocksetBuilder.ConstructedObjects.find(
+ if (auto Object = LocksetBuilder.Analyzer->ConstructedObjects.find(
TD.getBindTemporaryExpr()->getSubExpr());
- Object != LocksetBuilder.ConstructedObjects.end()) {
+ Object != LocksetBuilder.Analyzer->ConstructedObjects.end()) {
const auto *DD = TD.getDestructorDecl(AC.getASTContext());
if (DD->hasAttrs())
// TODO: the location here isn't quite correct.
LocksetBuilder.handleCall(nullptr, DD, Object->second,
TD.getBindTemporaryExpr()->getEndLoc());
-LocksetBuilder.ConstructedObjects.erase(Object);
+LocksetBuilder.Analyzer->ConstructedObjects.erase(Object);
aaronpuchert wrote:
We're in `ThreadSafetyAnalyzer` here, so you should be able to drop
`LocksetBuilder.Analyzer->` and refer to the member directly.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1530,7 +1532,6 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
}
namespace {
-
aaronpuchert wrote:
Nitpick: can you undo the whitespace change?
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: > Aaron is the real decision maker here Specifically @AaronBallman, not me. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: In the end it probably boils down to how you view an enumeration type. Is it a type with the declared enumerators as inhabitants, or is it an integer type of some length with associated symbolic constants of that type? In other words, is an enumeration a "strong" type or a "weak" type? The standard itself seems more in the "weak" camp. Yes, it does not allow integer values larger than what the largest enumeration value requires (if the underlying integer type isn't declared), but otherwise integer values that don't correspond to enumeration values can be cast to the enumeration type just fine. In [[dcl.enum]p8](https://eel.is/c++draft/enum#dcl.enum-8): > [For an enumeration whose underlying type is not fixed,] the values of the > enumeration are the values representable by a hypothetical integer type with > minimal width _M_ such that all enumerators can be represented. However, nothing prevents a developer from assuming a stricter model, where only the declared enumeration values are allowed and everything else is undefined behavior. (If not in terms of the language, then in terms of the program.) That is what we have done in the LLVM code base, and it's also the model that I personally prefer. But it is probably legitimate to follow the weaker model implied by the standard and assume that enumerations can have non-enumerator values, which requires a default label, enforced by this warning. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: > There is one clang-tidy check (bugprone-switch-missing-default-case) also for > this feature. This excludes enumeration types though, while GCC `-Wswitch-default` warns on them as well. (Even if all enumeration values are covered.) > whether switch statements should have a default is not something the industry > seem to agree on, so it would be a very opinionated warning. Yes. Though I have to say, even though I'm squarely in the `-Wcovered-switch-default` camp (i.e. no `default` label in switches that cover all enumeration values), I think it's still valuable to have this warning because some style guides/policies demand a `default`, and it is very little effort on our side to maintain this. I understand the policy against off-by-default warnings to avoid crowding the core compiler, but this is essentially just two lines of code, and if GCC has it we're not going out on a limb here. And since we already seem to support the flag for GCC compatibility, we might as well make it work if it's that easy. > LLVM development heavily relies on `-Wswitch-enum`, for example. I think we're using `-Wswitch`, as `-Wswitch-enum` requires to cover all enumeration values even if there is a default label. Our policy is: cover all enumeration values or have a `default`. This is "enforced" by `-Wswitch` in combination with `-Wcovered-switch-default`. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{}, a = 5;
}
+ void temporary2(int x) {
+if (check(MutexLock{}) || x) {
+
+}
aaronpuchert wrote:
The `if` is probably not needed here, right? We could just write
```suggestion
check(MutexLock{}) || x;
```
and should have the same CFG artifact.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -2392,6 +2397,8 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
for (const auto : LocksReleased)
ExpectedFunctionExitSet.removeLock(FactMan, Lock);
+ ConstructedObjectMapTy ConstructedObjects;
aaronpuchert wrote:
I wonder if we should put it into the `ThreadSafetyAnalyzer`, where we already
have the similar `LocalVariableMap`. Then we don't need to pass it into
`BuildLockset`, because that already has a pointer to the
`ThreadSafetyAnalyzer`.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert commented: I have some suggestions, but in principle this is absolutely right. Thanks for finding this and providing a fix! > The issue is that the map lives within a CFG block. It didn't cross my mind to check how long `BuildLockset` lived, I always assumed it lived for the entire function. But you're right, it does not, and is therefore an unsuitable home for the map. https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{}, a = 5;
}
+ void temporary2(int x) {
aaronpuchert wrote:
I would suggest a speaking name for the test, like `temporary_cfg`.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{}, a = 5;
}
+ void temporary2(int x) {
+if (check(MutexLock{}) || x) {
+
+}
+check(MutexLock{});
aaronpuchert wrote:
The `check` here doesn't do anything, right?
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1544,7 +1544,10 @@ class BuildLockset : public
ConstStmtVisitor {
// The fact set for the function on exit.
const FactSet
/// Maps constructed objects to `this` placeholder prior to initialization.
- llvm::SmallDenseMap ConstructedObjects;
+ /// The map lives longer than this builder so this is a reference to an
+ /// existing one. The map lives so long as the CFG while this builder is for
+ /// one CFG block.
aaronpuchert wrote:
It's probably sufficient to explain this in the commit message.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
aaronpuchert wrote:
We probably also want to check the other attribute arguments.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
aaronpuchert wrote:
Why don't we move them into the switch above? Presumably they can simply not be
used prior to C99? If we need this at all.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
aaronpuchert wrote:
This seems to be missing an important ingredient, unless I'm overlooking it: we
only want to diagnose if the format string and the var args are forwarded.
Simply calling one of these functions does not mean that we need to propagate
the attribute, e.g. if we're calling it with a string literal.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
+hasFormatAttr = true;
+break;
+ }
+}
+if (!hasFormatAttr) {
+ Diag(Loc, diag::warn_missing_format_attribute)
+ << ParentFuncDecl << Attr->getType();
aaronpuchert wrote:
Would be cool if we could add a fix-it. There are even ways to find macros that
wrap the attribute, see the implementation for `-Wimplicit-fallthrough`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
+hasFormatAttr = true;
+break;
+ }
+}
aaronpuchert wrote:
Seems like you could use `llvm::any_of`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
aaronpuchert wrote:
What about calling a regular function that also has a `format` attribute? The
GCC warning catches that as well.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s + +#include +#include + +va_list args; aaronpuchert wrote: This should be in the function body together with `va_start`. https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
aaronpuchert wrote:
Couldn't we use `getCurFunctionDecl`?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s
+
+#include
+#include
+
+va_list args;
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void foo(char *out, const size_t len, const char *format, ... /* args */)
+{
+vsnprintf(out, len, format, args); // expected-warning {{Function 'foo'
might be candidate for 'printf' format attribute}}
aaronpuchert wrote:
Add a test where we're calling the function with non-argument format string and
variadic arguments. Then the warning should not be emitted. Not sure about
mixing argument format string and non-argument variadic arguments and the other
way around, but these should probably not warn either, since we can't propagate
the attribute.
Also where we're taking a `va_list` as argument and forwarding that. This
should also warn.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert commented: I assume this is meant to imitate the GCC warning of the same name, which I found pretty useful. Would be nice to have the same in Clang! https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
aaronpuchert wrote:
Unless I'm missing something, the only caller is already passing a non-null
`FunctionDecl`.
```suggestion
void Sema::DiagnoseMissingFormatAttributes(FunctionDecl *FD,
SourceLocation Loc) {
```
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (PR #68572)
@@ -1061,11 +1061,13 @@ def Most : DiagGroup<"most", [ ]>; // Thread Safety warnings -def ThreadSafetyAttributes : DiagGroup<"thread-safety-attributes">; -def ThreadSafetyAnalysis : DiagGroup<"thread-safety-analysis">; -def ThreadSafetyPrecise: DiagGroup<"thread-safety-precise">; -def ThreadSafetyReference : DiagGroup<"thread-safety-reference">; -def ThreadSafetyNegative : DiagGroup<"thread-safety-negative">; +def ThreadSafetyAttributes : DiagGroup<"thread-safety-attributes">; +def ThreadSafetyAnalysis : DiagGroup<"thread-safety-analysis">; +def ThreadSafetyPrecise : DiagGroup<"thread-safety-precise">; aaronpuchert wrote: I would suggest to undo the indentation changes here, as the flag is intended to be temporary. https://github.com/llvm/llvm-project/pull/68572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (PR #68572)
@@ -2278,7 +2303,7 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
PostOrderCFGView::CFGBlockSet VisitedBlocks(CFGraph);
CFGBlockInfo = BlockInfo[CFGraph->getEntry().getBlockID()];
- CFGBlockInfo= BlockInfo[CFGraph->getExit().getBlockID()];
+ CFGBlockInfo = BlockInfo[CFGraph->getExit().getBlockID()];
aaronpuchert wrote:
This change seems unnecessary, but I don't mind either way.
https://github.com/llvm/llvm-project/pull/68572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (PR #68572)
https://github.com/aaronpuchert approved this pull request. Assuming that this is otherwise identical to the Phab review. I would suggest to add some release notes (in a separate change). https://github.com/llvm/llvm-project/pull/68572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (PR #68572)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/68572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)
Timm =?utf-8?q?Bäder?= Message-ID: In-Reply-To: https://github.com/aaronpuchert commented: > This was much easier than expected actually. Making it a `DeclOrType` attribute is indeed a nice idea, this would allow existing attributes to stay where they are. Is it still inheritable, i.e. does it also apply to later redeclarations? Of course I'm also wondering why we don't have to change anything in the analysis, aren't we currently only looking at declaration attributes, and wouldn't consider attributes on the type? Or is there some mechanism that gives us the combined set of declaration and type attributes? > I'm starting a new review here instead of continuing the one in Phab since > this is a completely different attempt. I hope that's ok. Yeah, since it's a completely different approach than https://reviews.llvm.org/D152246 we're not really losing any history. https://github.com/llvm/llvm-project/pull/67095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)
Timm =?utf-8?q?Bäder?=
Message-ID:
In-Reply-To:
@@ -136,6 +136,17 @@ int main(void) {
// Cleanup happens automatically -> no warning.
}
+ /// Function pointers
+ {
+int __attribute__((requires_capability())) (*function_ptr)(int) =
Foo_fun1;
aaronpuchert wrote:
Right, we would likely want to catch function pointer conversions that drop the
attribute. But this should be a separate change.
https://github.com/llvm/llvm-project/pull/67095
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)
Timm =?utf-8?q?Bäder?=
Message-ID:
In-Reply-To:
@@ -8141,6 +8141,16 @@ static void handleRequiresCapabilityAttr(Sema , Decl
*D,
if (!AL.checkAtLeastNumArgs(S, 1))
return;
+ // We allow this on function declaration as well as
+ // variable declarations of function pointer type.
+ if (!D->isFunctionPointerType() && !isa(D)) {
aaronpuchert wrote:
This looks like the attribute sits on the `Decl` and not its type, or am I
misunderstanding this?
https://github.com/llvm/llvm-project/pull/67095
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)
Timm =?utf-8?q?Bäder?= Message-ID: In-Reply-To: https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/67095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] WIP: Warn on mismatched RequiresCapability attributes (PR #67520)
aaronpuchert wrote: Yeah, it's a tricky question. On one hand there have been previous issues like #42194 (which this would basically address), and it would definitely improve consistency and prevent beginner's mistakes. On the other hand it seems useful to allow adding attributes e.g. to third-party libraries where you don't control the headers. Perhaps we should introduce such checks, but put them under a subflag of `-Wthread-safety-attributes` to allow opting out of them (even locally via pragmas). And of course it should apply more or less to every thread safety annotation, not just `requires_capability`. https://github.com/llvm/llvm-project/pull/67520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][TSA] Consider cleanup functions for thread safety analysis (PR #65462)
Timm =?utf-8?q?Bäder?= aaronpuchert wrote: We still have until the end of September to finish the review on Phabricator. I would suggest that we keep the review there until we have to move. Here I can't see the difference to the last version that I reviewed. https://github.com/llvm/llvm-project/pull/65462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 54bfd04 - Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls
Author: Aaron Puchert
Date: 2022-10-13T19:36:15+02:00
New Revision: 54bfd04846156dbd5e0a6b88f539c3d4569a455f
URL:
https://github.com/llvm/llvm-project/commit/54bfd04846156dbd5e0a6b88f539c3d4569a455f
DIFF:
https://github.com/llvm/llvm-project/commit/54bfd04846156dbd5e0a6b88f539c3d4569a455f.diff
LOG: Thread safety analysis: Support copy-elided production of scoped
capabilities through arbitrary calls
When support for copy elision was initially added in e97654b2f2807, it
was taking attributes from a constructor call, although that constructor
call is actually not involved. It seems more natural to use attributes
on the function returning the scoped capability, which is where it's
actually coming from. This would also support a number of interesting
use cases, like producing different scope kinds without the need for tag
types, or producing scopes from a private mutex.
Changing the behavior was surprisingly difficult: we were not handling
CXXConstructorExpr calls like regular calls but instead handled them
through the DeclStmt they're contained in. This was based on the
assumption that constructors are basically only called in variable
declarations (not true because of temporaries), and that variable
declarations necessitate constructors (not true with C++17 anymore).
Untangling this required separating construction from assigning a
variable name. When a call produces an object, we use a placeholder
til::LiteralPtr for `this`, and we collect the call expression and
placeholder in a map. Later when going through a DeclStmt, we look up
the call expression and set the placeholder to the new VarDecl.
The change has a couple of nice side effects:
* We don't miss constructor calls not contained in DeclStmts anymore,
allowing patterns like
MutexLock{}, requiresMutex();
The scoped lock temporary will be destructed at the end of the full
statement, so it protects the following call without the need for a
scope, but with the ability to unlock in case of an exception.
* We support lifetime extension of temporaries. While unusual, one can
now write
const MutexLock = MutexLock();
and have it behave as expected.
* Destructors used to be handled in a weird way: since there is no
expression in the AST for implicit destructor calls, we instead
provided a made-up DeclRefExpr to the variable being destructed, and
passed that instead of a CallExpr. Then later in translateAttrExpr
there was special code that knew that destructor expressions worked a
bit different.
* We were producing dummy DeclRefExprs in a number of places, this has
been eliminated. We now use til::SExprs instead.
Technically this could break existing code, but the current handling
seems unexpected enough to justify this change.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D129755
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/docs/ThreadSafetyAnalysis.rst
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1fe4be39e7aba..b036764803007 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -307,6 +307,13 @@ Improvements to Clang's diagnostics
function definition without a prototype which is preceded by a static
declaration of the function with a prototype. Fixes
`Issue 58181 `_.
+- Copy-elided initialization of lock scopes is now handled
diff erently in
+ ``-Wthread-safety-analysis``: annotations on the move constructor are no
+ longer taken into account, in favor of annotations on the function returning
+ the lock scope by value. This could result in new warnings if code depended
+ on the previous undocumented behavior. As a side effect of this change,
+ constructor calls outside of initializer expressions are no longer ignored,
+ which can result in new warnings (or make existing warnings disappear).
Non-comprehensive list of changes in this release
-
diff --git a/clang/docs/ThreadSafetyAnalysis.rst
b/clang/docs/ThreadSafetyAnalysis.rst
index 23f460b248e11..dcde0c706c704 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -408,7 +408,8 @@ and destructor refer to the capability via
diff erent names; see the
Scoped capabilities are treated as capabilities that are implicitly acquired
on construction and released on destruction. They are associated with
the set of (regular) capabilities named in thread safety attributes on the
[clang] 0041a69 - Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls
Author: Aaron Puchert
Date: 2022-10-06T22:19:09+02:00
New Revision: 0041a69495f828f6732803cfb0f1e3fddd7fbf2a
URL:
https://github.com/llvm/llvm-project/commit/0041a69495f828f6732803cfb0f1e3fddd7fbf2a
DIFF:
https://github.com/llvm/llvm-project/commit/0041a69495f828f6732803cfb0f1e3fddd7fbf2a.diff
LOG: Thread safety analysis: Support copy-elided production of scoped
capabilities through arbitrary calls
When support for copy elision was initially added in e97654b2f2807, it
was taking attributes from a constructor call, although that constructor
call is actually not involved. It seems more natural to use attributes
on the function returning the scoped capability, which is where it's
actually coming from. This would also support a number of interesting
use cases, like producing different scope kinds without the need for tag
types, or producing scopes from a private mutex.
Changing the behavior was surprisingly difficult: we were not handling
CXXConstructorExpr calls like regular calls but instead handled them
through the DeclStmt they're contained in. This was based on the
assumption that constructors are basically only called in variable
declarations (not true because of temporaries), and that variable
declarations necessitate constructors (not true with C++17 anymore).
Untangling this required separating construction from assigning a
variable name. When a call produces an object, we use a placeholder
til::LiteralPtr for `this`, and we collect the call expression and
placeholder in a map. Later when going through a DeclStmt, we look up
the call expression and set the placeholder to the new VarDecl.
The change has a couple of nice side effects:
* We don't miss constructor calls not contained in DeclStmts anymore,
allowing patterns like
MutexLock{}, requiresMutex();
The scoped lock temporary will be destructed at the end of the full
statement, so it protects the following call without the need for a
scope, but with the ability to unlock in case of an exception.
* We support lifetime extension of temporaries. While unusual, one can
now write
const MutexLock = MutexLock();
and have it behave as expected.
* Destructors used to be handled in a weird way: since there is no
expression in the AST for implicit destructor calls, we instead
provided a made-up DeclRefExpr to the variable being destructed, and
passed that instead of a CallExpr. Then later in translateAttrExpr
there was special code that knew that destructor expressions worked a
bit different.
* We were producing dummy DeclRefExprs in a number of places, this has
been eliminated. We now use til::SExprs instead.
Technically this could break existing code, but the current handling
seems unexpected enough to justify this change.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D129755
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/docs/ThreadSafetyAnalysis.rst
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1aea7e2753d..b9a5e0507e35 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -235,6 +235,11 @@ Improvements to Clang's diagnostics
function definition without a prototype which is preceded by a static
declaration of the function with a prototype. Fixes
`Issue 58181 `_.
+- Copy-elided initialization of lock scopes is now handled
diff erently in
+ ``-Wthread-safety-analysis``: annotations on the move constructor are no
+ longer taken into account, in favor of annotations on the function returning
+ the lock scope by value. This could result in new warnings if code depended
+ on the previous undocumented behavior.
Non-comprehensive list of changes in this release
-
diff --git a/clang/docs/ThreadSafetyAnalysis.rst
b/clang/docs/ThreadSafetyAnalysis.rst
index 23f460b248e1..dcde0c706c70 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -408,7 +408,8 @@ and destructor refer to the capability via
diff erent names; see the
Scoped capabilities are treated as capabilities that are implicitly acquired
on construction and released on destruction. They are associated with
the set of (regular) capabilities named in thread safety attributes on the
-constructor. Acquire-type attributes on other member functions are treated as
+constructor or function returning them by value (using C++17 guaranteed copy
+elision). Acquire-type attributes on
[clang] d8fa40d - Thread safety analysis: Handle additional cast in scoped capability construction
Author: Aaron Puchert
Date: 2022-10-06T22:18:26+02:00
New Revision: d8fa40dfa7adb062c969ce7ea6ce505e10626838
URL:
https://github.com/llvm/llvm-project/commit/d8fa40dfa7adb062c969ce7ea6ce505e10626838
DIFF:
https://github.com/llvm/llvm-project/commit/d8fa40dfa7adb062c969ce7ea6ce505e10626838.diff
LOG: Thread safety analysis: Handle additional cast in scoped capability
construction
We might have a CK_NoOp cast and a further CK_ConstructorConversion.
As an optimization, drop some IgnoreParens calls: inside of the
CK_{Constructor,UserDefined}Conversion should be no more parentheses,
and inside the CXXBindTemporaryExpr should also be none.
Lastly, we factor out the unpacking so that we can reuse it for
MaterializeTemporaryExprs later on.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D129752
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 3291d7f8e3b3..a29134c6a5e7 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2087,6 +2087,19 @@ static Expr *buildFakeCtorCall(CXXConstructorDecl *CD,
ArrayRef Args,
SourceRange(Loc, Loc));
}
+static Expr *UnpackConstruction(Expr *E) {
+ if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp)
+ E = CE->getSubExpr()->IgnoreParens();
+ if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+ E = CE->getSubExpr();
+ if (auto *BTE = dyn_cast(E))
+E = BTE->getSubExpr();
+ return E;
+}
+
void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
// adjust the context
LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -2101,13 +2114,7 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
// handle constructors that involve temporaries
if (auto *EWC = dyn_cast(E))
E = EWC->getSubExpr()->IgnoreParens();
- if (auto *CE = dyn_cast(E))
-if (CE->getCastKind() == CK_NoOp ||
-CE->getCastKind() == CK_ConstructorConversion ||
-CE->getCastKind() == CK_UserDefinedConversion)
- E = CE->getSubExpr()->IgnoreParens();
- if (auto *BTE = dyn_cast(E))
-E = BTE->getSubExpr()->IgnoreParens();
+ E = UnpackConstruction(E);
if (const auto *CE = dyn_cast(E)) {
const auto *CtorD = dyn_cast_or_null(CE->getConstructor());
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index ac854dce0f7b..e1cfa1f3fd17 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1683,6 +1683,13 @@ struct TestScopedLockable {
a = 5;
}
+#ifdef __cpp_guaranteed_copy_elision
+ void const_lock() {
+const MutexLock mulock = MutexLock();
+a = 5;
+ }
+#endif
+
void foo2() {
ReaderMutexLock mulock1();
if (getBool()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e0c66c6 - Thread safety analysis: Don't erase TIL_Opcode type (NFC)
Author: Aaron Puchert
Date: 2022-07-14T13:36:35+02:00
New Revision: e0c66c699eb000f604e24b1c4e73b899b4d942d3
URL:
https://github.com/llvm/llvm-project/commit/e0c66c699eb000f604e24b1c4e73b899b4d942d3
DIFF:
https://github.com/llvm/llvm-project/commit/e0c66c699eb000f604e24b1c4e73b899b4d942d3.diff
LOG: Thread safety analysis: Don't erase TIL_Opcode type (NFC)
This is mainly for debugging, but it also eliminates some casts.
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
index 77a800c28754c..65556c8d584c9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
@@ -75,7 +75,7 @@ namespace til {
class BasicBlock;
/// Enum for the
diff erent distinct classes of SExpr
-enum TIL_Opcode {
+enum TIL_Opcode : unsigned char {
#define TIL_OPCODE_DEF(X) COP_##X,
#include "ThreadSafetyOps.def"
#undef TIL_OPCODE_DEF
@@ -278,7 +278,7 @@ class SExpr {
public:
SExpr() = delete;
- TIL_Opcode opcode() const { return static_cast(Opcode); }
+ TIL_Opcode opcode() const { return Opcode; }
// Subclasses of SExpr must define the following:
//
@@ -321,7 +321,7 @@ class SExpr {
SExpr(TIL_Opcode Op) : Opcode(Op) {}
SExpr(const SExpr ) : Opcode(E.Opcode), Flags(E.Flags) {}
- const unsigned char Opcode;
+ const TIL_Opcode Opcode;
unsigned char Reserved = 0;
unsigned short Flags = 0;
unsigned SExprID = 0;
@@ -332,7 +332,7 @@ class SExpr {
namespace ThreadSafetyTIL {
inline bool isTrivial(const SExpr *E) {
- unsigned Op = E->opcode();
+ TIL_Opcode Op = E->opcode();
return Op == COP_Variable || Op == COP_Literal || Op == COP_LiteralPtr;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] bfe63ab - Thread safety analysis: Support builtin pointer-to-member operators
Author: Aaron Puchert
Date: 2022-07-14T13:36:14+02:00
New Revision: bfe63ab63e22b61bd5898c65425e8ebe43189913
URL:
https://github.com/llvm/llvm-project/commit/bfe63ab63e22b61bd5898c65425e8ebe43189913
DIFF:
https://github.com/llvm/llvm-project/commit/bfe63ab63e22b61bd5898c65425e8ebe43189913.diff
LOG: Thread safety analysis: Support builtin pointer-to-member operators
We consider an access to x.*pm as access of the same kind into x, and
an access to px->*pm as access of the same kind into *px. Previously we
missed reads and writes in the .* case, and operations to the pointed-to
data for ->* (we didn't miss accesses to the pointer itself, because
that requires an LValueToRValue cast that we treat independently).
We added support for overloaded operator->* in D124966.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D129514
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 03bbf078d7e89..32d950864ce78 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1679,6 +1679,17 @@ void BuildLockset::checkAccess(const Expr *Exp,
AccessKind AK,
return;
}
+ if (const auto *BO = dyn_cast(Exp)) {
+switch (BO->getOpcode()) {
+case BO_PtrMemD: // .*
+ return checkAccess(BO->getLHS(), AK, POK);
+case BO_PtrMemI: // ->*
+ return checkPtAccess(BO->getLHS(), AK, POK);
+default:
+ return;
+}
+ }
+
if (const auto *AE = dyn_cast(Exp)) {
checkPtAccess(AE->getLHS(), AK, POK);
return;
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index ea229fef649b9..ac854dce0f7b0 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4870,6 +4870,8 @@ class PtGuardedByCorrectnessTest {
intsa[10] GUARDED_BY(mu1);
Cell sc[10] GUARDED_BY(mu1);
+ static constexpr int Cell::*pa = ::a;
+
void test1() {
mu1.Lock();
if (a == 0) doSomething(); // OK, we don't dereference.
@@ -4889,9 +4891,11 @@ class PtGuardedByCorrectnessTest {
if (c->a == 0) doSomething();// expected-warning {{reading the value
pointed to by 'c' requires holding mutex 'mu2'}}
c->a = 0;// expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+c->*pa = 0; // expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
if ((*c).a == 0) doSomething(); // expected-warning {{reading the value
pointed to by 'c' requires holding mutex 'mu2'}}
(*c).a = 0; // expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+(*c).*pa = 0;// expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
if (a[0] == 42) doSomething(); // expected-warning {{reading the value
pointed to by 'a' requires holding mutex 'mu2'}}
a[0] = 57; // expected-warning {{writing the value
pointed to by 'a' requires holding mutex 'mu2' exclusively}}
@@ -4923,6 +4927,7 @@ class PtGuardedByCorrectnessTest {
sa[0] = 57; // expected-warning {{writing variable
'sa' requires holding mutex 'mu1' exclusively}}
if (sc[0].a == 42) doSomething(); // expected-warning {{reading variable
'sc' requires holding mutex 'mu1'}}
sc[0].a = 57; // expected-warning {{writing variable
'sc' requires holding mutex 'mu1' exclusively}}
+sc[0].*pa = 57; // expected-warning {{writing variable
'sc' requires holding mutex 'mu1' exclusively}}
if (*sa == 42) doSomething(); // expected-warning {{reading variable
'sa' requires holding mutex 'mu1'}}
*sa = 57; // expected-warning {{writing variable
'sa' requires holding mutex 'mu1' exclusively}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ac7a9ef - Resolve overload ambiguity on Mac OS when printing size_t in diagnostics
Author: Aaron Puchert
Date: 2022-05-14T12:37:36+02:00
New Revision: ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720
URL:
https://github.com/llvm/llvm-project/commit/ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720
DIFF:
https://github.com/llvm/llvm-project/commit/ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720.diff
LOG: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics
Precommit builds cover Linux and Windows, but this ambiguity would only
show up on Mac OS: there we have int32_t = int, int64_t = long long and
size_t = unsigned long. So printing a size_t, while successful on the
other two architectures, cannot be unambiguously resolved on Mac OS.
This is not really meant to support printing arguments of type long or
size_t, but more as a way to prevent build breakage that would not be
detected in precommit builds, as happened in D125429.
Technically we have no guarantee that one of these types has the 64 bits
that afdac5fbcb6a3 wanted to provide, so proposals are welcome. We do
have a guarantee though that these three types are different, so we
should be fine with overload resolution.
Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D125580
Added:
Modified:
clang/include/clang/Basic/Diagnostic.h
clang/lib/AST/CommentParser.cpp
Removed:
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index dc1a0efe1c47..33ad0827c0ca 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1404,7 +1404,13 @@ inline const StreamingDiagnostic <<(const
StreamingDiagnostic ,
}
inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- int64_t I) {
+ long I) {
+ DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
+ return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ long long I) {
DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
return DB;
}
@@ -1426,7 +1432,13 @@ inline const StreamingDiagnostic <<(const
StreamingDiagnostic ,
}
inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- uint64_t I) {
+ unsigned long I) {
+ DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
+ return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ unsigned long long I) {
DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
return DB;
}
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index d78b3ace2bb8..7bac1fb99b88 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@ InlineCommandComment *Parser::parseInlineCommand() {
if (Args.size() < Info->NumArgs) {
Diag(CommandTok.getEndLocation().getLocWithOffset(1),
diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name <<
(uint64_t)Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
<< Info->NumArgs
<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 25862f5 - Try to disambiguate between overloads on Mac
Author: Aaron Puchert
Date: 2022-05-13T16:29:02+02:00
New Revision: 25862f53cce966cef2957825095861dec631d4f1
URL:
https://github.com/llvm/llvm-project/commit/25862f53cce966cef2957825095861dec631d4f1
DIFF:
https://github.com/llvm/llvm-project/commit/25862f53cce966cef2957825095861dec631d4f1.diff
LOG: Try to disambiguate between overloads on Mac
Presumably Mac has a different understanding of how long `long` is.
Should fix a build error introduced by D125429 that's not visible on
other architectures.
Added:
Modified:
clang/lib/AST/CommentParser.cpp
Removed:
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index 7bac1fb99b88..d78b3ace2bb8 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@ InlineCommandComment *Parser::parseInlineCommand() {
if (Args.size() < Info->NumArgs) {
Diag(CommandTok.getEndLocation().getLocWithOffset(1),
diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name <<
(uint64_t)Args.size()
<< Info->NumArgs
<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d2396d8 - Comment parsing: Treat properties as zero-argument inline commands
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: d2396d896ee12ad20bc740174edfce2120d742b2
URL:
https://github.com/llvm/llvm-project/commit/d2396d896ee12ad20bc740174edfce2120d742b2
DIFF:
https://github.com/llvm/llvm-project/commit/d2396d896ee12ad20bc740174edfce2120d742b2.diff
LOG: Comment parsing: Treat properties as zero-argument inline commands
That is more accurate, and using a separate class in TableGen seems
appropriate since these are not parts of the text but properties of the
declaration itself.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125473
Added:
Modified:
clang/include/clang/AST/CommentCommands.td
Removed:
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index a3b9eb313fcf..e839031752cd 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -63,6 +63,11 @@ class VerbatimLineCommand : Command {
let IsVerbatimLineCommand = 1;
}
+class PropertyCommand : Command {
+ let NumArgs = 0;
+ let IsInlineCommand = 1;
+}
+
class DeclarationVerbatimLineCommand :
VerbatimLineCommand {
let IsDeclarationCommand = 1;
@@ -275,31 +280,6 @@ def Until: VerbatimLineCommand<"until">;
def NoOp : VerbatimLineCommand<"noop">;
-// These have actually no arguments, but we can treat them as line commands.
-def CallGraph : VerbatimLineCommand<"callgraph">;
-def HideCallGraph : VerbatimLineCommand<"hidecallgraph">;
-def CallerGraph : VerbatimLineCommand<"callergraph">;
-def HideCallerGraph : VerbatimLineCommand<"hidecallergraph">;
-def ShowInitializer : VerbatimLineCommand<"showinitializer">;
-def HideInitializer : VerbatimLineCommand<"hideinitializer">;
-def ShowRefBy : VerbatimLineCommand<"showrefby">;
-def HideRefBy : VerbatimLineCommand<"hiderefby">;
-def ShowRefs: VerbatimLineCommand<"showrefs">;
-def HideRefs: VerbatimLineCommand<"hiderefs">;
-
-// These also have no argument.
-def Private : VerbatimLineCommand<"private">;
-def Protected : VerbatimLineCommand<"protected">;
-def Public: VerbatimLineCommand<"public">;
-def Pure : VerbatimLineCommand<"pure">;
-def Static: VerbatimLineCommand<"static">;
-
-// These also have no argument.
-def NoSubgrouping: VerbatimLineCommand<"nosubgrouping">;
-def PrivateSection : VerbatimLineCommand<"privatesection">;
-def ProtectedSection : VerbatimLineCommand<"protectedsection">;
-def PublicSection: VerbatimLineCommand<"publicsection">;
-
// We might also build proper support for if/ifnot/else/elseif/endif.
def If : VerbatimLineCommand<"if">;
def IfNot : VerbatimLineCommand<"ifnot">;
@@ -311,6 +291,32 @@ def Endif : VerbatimLineCommand<"endif">;
def Cond: VerbatimLineCommand<"cond">;
def EndCond : VerbatimLineCommand<"endcond">;
+//===--===//
+// PropertyCommand
+//===--===//
+
+def CallGraph : PropertyCommand<"callgraph">;
+def HideCallGraph : PropertyCommand<"hidecallgraph">;
+def CallerGraph : PropertyCommand<"callergraph">;
+def HideCallerGraph : PropertyCommand<"hidecallergraph">;
+def ShowInitializer : PropertyCommand<"showinitializer">;
+def HideInitializer : PropertyCommand<"hideinitializer">;
+def ShowRefBy : PropertyCommand<"showrefby">;
+def HideRefBy : PropertyCommand<"hiderefby">;
+def ShowRefs: PropertyCommand<"showrefs">;
+def HideRefs: PropertyCommand<"hiderefs">;
+
+def Private : PropertyCommand<"private">;
+def Protected : PropertyCommand<"protected">;
+def Public: PropertyCommand<"public">;
+def Pure : PropertyCommand<"pure">;
+def Static: PropertyCommand<"static">;
+
+def NoSubgrouping: PropertyCommand<"nosubgrouping">;
+def PrivateSection : PropertyCommand<"privatesection">;
+def ProtectedSection : PropertyCommand<"protectedsection">;
+def PublicSection: PropertyCommand<"publicsection">;
+
//===--===//
// DeclarationVerbatimLineCommand
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d3a4033 - Comment parsing: Allow inline commands to have 0 or more than 1 argument
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783
URL:
https://github.com/llvm/llvm-project/commit/d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783
DIFF:
https://github.com/llvm/llvm-project/commit/d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783.diff
LOG: Comment parsing: Allow inline commands to have 0 or more than 1 argument
That's required to support `\n`, but can also be used for other commands.
We already had the infrastructure in place to parse a varying number of
arguments, we simply needed to generalize it so that it would work not
only for block commands.
This should fix #55319.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125429
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/include/clang/AST/CommentCommands.td
clang/include/clang/AST/CommentParser.h
clang/include/clang/AST/CommentSema.h
clang/include/clang/Basic/DiagnosticCommentKinds.td
clang/lib/AST/CommentParser.cpp
clang/lib/AST/CommentSema.cpp
clang/test/AST/ast-dump-comment.cpp
clang/test/Headers/x86-intrinsics-headers-clean.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index 5ecc35791b7b0..0b68c11316649 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -194,6 +194,11 @@ class Comment {
#include "clang/AST/CommentNodes.inc"
};
+ struct Argument {
+SourceRange Range;
+StringRef Text;
+ };
+
Comment(CommentKind K,
SourceLocation LocBegin,
SourceLocation LocEnd) :
@@ -296,13 +301,6 @@ class TextComment : public InlineContentComment {
/// A command with word-like arguments that is considered inline content.
class InlineCommandComment : public InlineContentComment {
public:
- struct Argument {
-SourceRange Range;
-StringRef Text;
-
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
- };
-
/// The most appropriate rendering mode for this command, chosen on command
/// semantics in Doxygen.
enum RenderKind {
@@ -588,15 +586,6 @@ class ParagraphComment : public BlockContentComment {
/// arguments depends on command name) and a paragraph as an argument
/// (e. g., \\brief).
class BlockCommandComment : public BlockContentComment {
-public:
- struct Argument {
-SourceRange Range;
-StringRef Text;
-
-Argument() { }
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
- };
-
protected:
/// Word-like arguments.
ArrayRef Args;
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index d357ec1cf8b99..a3b9eb313fcf5 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -31,6 +31,7 @@ class Command {
}
class InlineCommand : Command {
+ let NumArgs = 1;
let IsInlineCommand = 1;
}
@@ -86,6 +87,7 @@ def C : InlineCommand<"c">;
def P : InlineCommand<"p">;
def A : InlineCommand<"a">;
def E : InlineCommand<"e">;
+def N : InlineCommand<"n"> { let NumArgs = 0; }
def Em : InlineCommand<"em">;
def Emoji : InlineCommand<"emoji">;
diff --git a/clang/include/clang/AST/CommentParser.h
b/clang/include/clang/AST/CommentParser.h
index 1a0cfb06e52b7..e11e818b1af0a 100644
--- a/clang/include/clang/AST/CommentParser.h
+++ b/clang/include/clang/AST/CommentParser.h
@@ -97,9 +97,8 @@ class Parser {
void parseTParamCommandArgs(TParamCommandComment *TPC,
TextTokenRetokenizer );
- void parseBlockCommandArgs(BlockCommandComment *BC,
- TextTokenRetokenizer ,
- unsigned NumArgs);
+ ArrayRef
+ parseCommandArgs(TextTokenRetokenizer , unsigned NumArgs);
BlockCommandComment *parseBlockCommand();
InlineCommandComment *parseInlineCommand();
diff --git a/clang/include/clang/AST/CommentSema.h
b/clang/include/clang/AST/CommentSema.h
index 015ce8f8652ac..5e30ff8adb915 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -128,16 +128,10 @@ class Sema {
void actOnTParamCommandFinish(TParamCommandComment *Command,
ParagraphComment *Paragraph);
- InlineCommandComment *actOnInlineCommand(SourceLocation CommandLocBegin,
- SourceLocation CommandLocEnd,
- unsigned CommandID);
-
InlineCommandComment *actOnInlineCommand(SourceLocation CommandLocBegin,
SourceLocation CommandLocEnd,
unsigned CommandID,
- SourceLocation ArgLocBegin,
-
[clang] 99d3582 - Comment parsing: Specify argument numbers for some block commands
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: 99d35826a043916b259a0e440a2aa5cabbad2773
URL:
https://github.com/llvm/llvm-project/commit/99d35826a043916b259a0e440a2aa5cabbad2773
DIFF:
https://github.com/llvm/llvm-project/commit/99d35826a043916b259a0e440a2aa5cabbad2773.diff
LOG: Comment parsing: Specify argument numbers for some block commands
The command traits have a member NumArgs for which all the parsing
infrastructure is in place, but no command was setting it to a value
other than 0. By doing so we get warnings when passing an empty
paragraph to \retval (the first argument is the return value, then comes
the description). We also take \xrefitem along for the ride, although as
the documentation states it's unlikely to be used directly.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125422
Added:
Modified:
clang/include/clang/AST/CommentCommands.td
clang/test/AST/ast-dump-comment.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index 7e962a4b4171b..d357ec1cf8b99 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@ def Post : BlockCommand<"post">;
def Pre: BlockCommand<"pre">;
def Remark : BlockCommand<"remark">;
def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
def Sa : BlockCommand<"sa">;
def See: BlockCommand<"see">;
def Since : BlockCommand<"since">;
@@ -162,7 +162,7 @@ def Test : BlockCommand<"test">;
def Todo : BlockCommand<"todo">;
def Version: BlockCommand<"version">;
def Warning: BlockCommand<"warning">;
-def XRefItem : BlockCommand<"xrefitem">;
+def XRefItem : BlockCommand<"xrefitem"> { let NumArgs = 3; }
// HeaderDoc commands
def Abstract : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
def ClassDesign : RecordLikeDetailCommand<"classdesign">;
diff --git a/clang/test/AST/ast-dump-comment.cpp
b/clang/test/AST/ast-dump-comment.cpp
index 11c96024546e0..1936c732cb989 100644
--- a/clang/test/AST/ast-dump-comment.cpp
+++ b/clang/test/AST/ast-dump-comment.cpp
@@ -32,6 +32,13 @@ int Test_BlockCommandComment;
// CHECK-NEXT: ParagraphComment
// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+/// \retval 42 Aaa
+int Test_BlockCommandComment_WithArgs();
+// CHECK: FunctionDecl{{.*}}Test_BlockCommandComment_WithArgs
+// CHECK:BlockCommandComment{{.*}} Name="retval" Arg[0]="42"
+// CHECK-NEXT: ParagraphComment
+// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+
/// \param Aaa xxx
/// \param [in,out] Bbb yyy
void Test_ParamCommandComment(int Aaa, int Bbb);
diff --git a/clang/test/Sema/warn-documentation.cpp
b/clang/test/Sema/warn-documentation.cpp
index 353c94a47eb6f..570b5baf54029 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@ int test_multiple_returns3(int);
int test_multiple_returns4(int);
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
// expected-warning@+1 {{'\param' command used in a comment that is not
attached to a function declaration}}
/// \param a Blah blah.
int test_param1_backslash;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 44ae49e - Thread safety analysis: Handle compound assignment and ->* overloads
Author: Aaron Puchert
Date: 2022-05-09T15:35:43+02:00
New Revision: 44ae49e1a72576ca6aa8835b3f72df9605516403
URL:
https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403
DIFF:
https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403.diff
LOG: Thread safety analysis: Handle compound assignment and ->* overloads
Like regular assignment, compound assignment operators can be assumed to
write to their left-hand side operand. So we strengthen the requirements
there. (Previously only the default read access had been required.)
Just like operator->, operator->* can also be assumed to dereference the
left-hand side argument, so we require read access to the pointee. This
will generate new warnings if the left-hand side has a pt_guarded_by
attribute. This overload is rarely used, but it was trivial to add, so
why not. (Supporting the builtin operator requires changes to the TIL.)
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124966
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 73fb5a4f3c1f5..a82ae8845e65c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -204,6 +204,9 @@ Improvements to Clang's diagnostics
``-Wimplicit-int``.
- ``-Wmisexpect`` warns when the branch weights collected during profiling
conflict with those added by ``llvm.expect``.
+- ``-Wthread-safety-analysis`` now considers overloaded compound assignment and
+ increment/decrement operators as writing to their first argument, thus
+ requiring an exclusive lock if the argument is guarded.
Non-comprehensive list of changes in this release
-
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index b8fe6000d9716..03bbf078d7e89 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1988,16 +1988,27 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
examineArguments(CE->getDirectCallee(), CE->arg_begin(), CE->arg_end());
} else if (const auto *OE = dyn_cast(Exp)) {
-auto OEop = OE->getOperator();
+OverloadedOperatorKind OEop = OE->getOperator();
switch (OEop) {
- case OO_Equal: {
-const Expr *Target = OE->getArg(0);
-const Expr *Source = OE->getArg(1);
-checkAccess(Target, AK_Written);
-checkAccess(Source, AK_Read);
+ case OO_Equal:
+ case OO_PlusEqual:
+ case OO_MinusEqual:
+ case OO_StarEqual:
+ case OO_SlashEqual:
+ case OO_PercentEqual:
+ case OO_CaretEqual:
+ case OO_AmpEqual:
+ case OO_PipeEqual:
+ case OO_LessLessEqual:
+ case OO_GreaterGreaterEqual:
+checkAccess(OE->getArg(1), AK_Read);
+LLVM_FALLTHROUGH;
+ case OO_PlusPlus:
+ case OO_MinusMinus:
+checkAccess(OE->getArg(0), AK_Written);
break;
- }
case OO_Star:
+ case OO_ArrowStar:
case OO_Arrow:
case OO_Subscript:
if (!(OEop == OO_Star && OE->getNumArgs() > 1)) {
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 9cd44fb07230d..ea229fef649b9 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -82,6 +82,9 @@ class SmartPtr {
T* ptr_;
};
+template
+U& operator->*(const SmartPtr& ptr, U T::*p) { return ptr->*p; }
+
// For testing destructor calls and cleanup.
class MyString {
@@ -4338,6 +4341,21 @@ class Data {
void operator()() { }
+ Data& operator+=(int);
+ Data& operator-=(int);
+ Data& operator*=(int);
+ Data& operator/=(int);
+ Data& operator%=(int);
+ Data& operator^=(int);
+ Data& operator&=(int);
+ Data& operator|=(int);
+ Data& operator<<=(int);
+ Data& operator>>=(int);
+ Data& operator++();
+ Data& operator++(int);
+ Data& operator--();
+ Data& operator--(int);
+
private:
int dat;
};
@@ -4404,6 +4422,20 @@ class Foo {
// expected-warning {{reading variable 'datap1_'
requires holding mutex 'mu_'}}
data_ = *datap2_; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}} \
// expected-warning {{reading the value pointed to
by 'datap2_' requires holding mutex 'mu_'}}
+data_ += 1; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}}
+data_ -= 1; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}}
+data_ *= 1; // expected-warning {{writing variable 'data_'
[clang] 0314dba - Thread safety analysis: Don't pass capability kind where not needed (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: 0314dbac026f58aaaf0a9ee4515f401f0d43ee76
URL:
https://github.com/llvm/llvm-project/commit/0314dbac026f58aaaf0a9ee4515f401f0d43ee76
DIFF:
https://github.com/llvm/llvm-project/commit/0314dbac026f58aaaf0a9ee4515f401f0d43ee76.diff
LOG: Thread safety analysis: Don't pass capability kind where not needed (NFC)
If no capability is held, or the capability expression is invalid, there
is obviously no capability kind and so none would be reported.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124132
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafety.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index bfa9870a1e1f..1808d1d71e05 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -98,9 +98,8 @@ class ThreadSafetyHandler {
virtual ~ThreadSafetyHandler();
/// Warn about lock expressions which fail to resolve to lockable objects.
- /// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param Loc -- the SourceLocation of the unresolved expression.
- virtual void handleInvalidLockExp(StringRef Kind, SourceLocation Loc) {}
+ virtual void handleInvalidLockExp(SourceLocation Loc) {}
/// Warn about unlock function calls that do not have a prior matching lock
/// expression.
@@ -169,14 +168,12 @@ class ThreadSafetyHandler {
SourceLocation Loc2) {}
/// Warn when a protected operation occurs while no locks are held.
- /// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param D -- The decl for the protected variable or function
/// \param POK -- The kind of protected operation (e.g. variable access)
/// \param AK -- The kind of access (i.e. read or write) that occurred
/// \param Loc -- The location of the protected operation.
- virtual void handleNoMutexHeld(StringRef Kind, const NamedDecl *D,
- ProtectedOperationKind POK, AccessKind AK,
- SourceLocation Loc) {}
+ virtual void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind
POK,
+ AccessKind AK, SourceLocation Loc) {}
/// Warn when a protected operation occurs while the specific mutex
protecting
/// the operation is not locked.
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 63cc61b07c14..b8fe6000d971 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -74,7 +74,7 @@ static void warnInvalidLock(ThreadSafetyHandler ,
// FIXME: add a note about the attribute location in MutexExp or D
if (Loc.isValid())
-Handler.handleInvalidLockExp(Kind, Loc);
+Handler.handleInvalidLockExp(Loc);
}
namespace {
@@ -1696,7 +1696,7 @@ void BuildLockset::checkAccess(const Expr *Exp,
AccessKind AK,
return;
if (D->hasAttr() && FSet.isEmpty(Analyzer->FactMan)) {
-Analyzer->Handler.handleNoMutexHeld("mutex", D, POK, AK, Loc);
+Analyzer->Handler.handleNoMutexHeld(D, POK, AK, Loc);
}
for (const auto *I : D->specific_attrs())
@@ -1734,8 +1734,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp,
AccessKind AK,
return;
if (D->hasAttr() && FSet.isEmpty(Analyzer->FactMan))
-Analyzer->Handler.handleNoMutexHeld("mutex", D, PtPOK, AK,
-Exp->getExprLoc());
+Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc());
for (auto const *I : D->specific_attrs())
warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, Exp->getExprLoc());
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index ac5ad52c0b1d..bf282bbb400d 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1844,7 +1844,7 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
}
}
- void handleInvalidLockExp(StringRef Kind, SourceLocation Loc) override {
+ void handleInvalidLockExp(SourceLocation Loc) override {
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_cannot_resolve_lock)
<< Loc);
Warnings.emplace_back(std::move(Warning), getNotes());
@@ -1922,9 +1922,8 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
Warnings.emplace_back(std::move(Warning), getNotes(Note));
}
- void handleNoMutexHeld(StringRef Kind, const NamedDecl *D,
- ProtectedOperationKind POK, AccessKind AK,
-
[clang] f8afb8f - Thread safety analysis: Store capability kind in CapabilityExpr
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: f8afb8fdedae04ad2670857c97925c439d47d862
URL:
https://github.com/llvm/llvm-project/commit/f8afb8fdedae04ad2670857c97925c439d47d862
DIFF:
https://github.com/llvm/llvm-project/commit/f8afb8fdedae04ad2670857c97925c439d47d862.diff
LOG: Thread safety analysis: Store capability kind in CapabilityExpr
This should make us print the right capability kind in many more cases,
especially when attributes name multiple capabilities of different kinds.
Previously we were trying to deduce the capability kind from the
original attribute, but most attributes can name multiple capabilities,
which could be of different kinds. So instead we derive the kind when
translating the attribute expression, and then store it in the returned
CapabilityExpr. Then we can extract the corresponding capability name
when we need it, which saves us lots of plumbing and almost guarantees
that the name is right.
I didn't bother adding any tests for this because it's just a usability
improvement and it's pretty much evident from the code that we don't
fall back to "mutex" anymore (save for a few cases that I'll address in
a separate change).
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124131
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index 392eecdb1897e..da69348ea9388 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -273,14 +273,23 @@ class CapabilityExpr {
/// The capability expression and whether it's negated.
llvm::PointerIntPair CapExpr;
+ /// The kind of capability as specified by @ref CapabilityAttr::getName.
+ StringRef CapKind;
+
public:
- CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E, Neg) {}
+ CapabilityExpr() : CapExpr(nullptr, false) {}
+ CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
+ : CapExpr(E, Neg), CapKind(Kind) {}
+
+ // Don't allow implicitly-constructed StringRefs since we'll capture them.
+ template CapabilityExpr(const til::SExpr *, T, bool) = delete;
const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
+ StringRef getKind() const { return CapKind; }
bool negative() const { return CapExpr.getInt(); }
CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
}
bool equals(const CapabilityExpr ) const {
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 2b461a72dc2eb..63cc61b07c14a 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -139,12 +139,12 @@ class FactEntry : public CapabilityExpr {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler ) const = 0;
virtual void handleLock(FactSet , FactManager ,
- const FactEntry , ThreadSafetyHandler ,
- StringRef DiagKind) const = 0;
+ const FactEntry ,
+ ThreadSafetyHandler ) const = 0;
virtual void handleUnlock(FactSet , FactManager ,
const CapabilityExpr , SourceLocation UnlockLoc,
-bool FullyRemove, ThreadSafetyHandler ,
-StringRef DiagKind) const = 0;
+bool FullyRemove,
+ThreadSafetyHandler ) const = 0;
// Return true if LKind >= LK, where exclusive > shared
bool isAtLeast(LockKind LK) const {
@@ -865,21 +865,21 @@ class LockableFactEntry : public FactEntry {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler ) const override {
if (!asserted() && !negative() && !isUniversal()) {
- Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
+ Handler.handleMutexHeldEndOfScope(getKind(), toString(), loc(), JoinLoc,
LEK);
}
}
void handleLock(FactSet , FactManager , const FactEntry ,
- ThreadSafetyHandler ,
- StringRef DiagKind) const override {
-Handler.handleDoubleLock(DiagKind, entry.toString(), loc(), entry.loc());
+ ThreadSafetyHandler ) const override {
+Handler.handleDoubleLock(entry.getKind(), entry.toString(), loc(),
+
[clang] d65c922 - Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5
URL:
https://github.com/llvm/llvm-project/commit/d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5
DIFF:
https://github.com/llvm/llvm-project/commit/d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5.diff
LOG: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry
(NFC)
For now this doesn't make a whole lot of sense, but it will allow us to
store the capability kind in a CapabilityExpr and make sure it doesn't
get lost. The capabilities managed by a scoped lockable can of course be
of different kind, so we'll need to store that per entry.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124128
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 9cc990bd35a3..2b461a72dc2e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -41,7 +41,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/Optional.h"
-#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -897,25 +896,27 @@ class ScopedLockableFactEntry : public FactEntry {
UCK_ReleasedExclusive, ///< Exclusive capability that was released.
};
- using UnderlyingCapability =
- llvm::PointerIntPair;
+ struct UnderlyingCapability {
+CapabilityExpr Cap;
+UnderlyingCapabilityKind Kind;
+ };
- SmallVector UnderlyingMutexes;
+ SmallVector UnderlyingMutexes;
public:
ScopedLockableFactEntry(const CapabilityExpr , SourceLocation Loc)
: FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
void addLock(const CapabilityExpr ) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
}
void addExclusiveUnlock(const CapabilityExpr ) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+UnderlyingMutexes.push_back(UnderlyingCapability{M,
UCK_ReleasedExclusive});
}
void addSharedUnlock(const CapabilityExpr ) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
+UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedShared});
}
void
@@ -923,15 +924,13 @@ class ScopedLockableFactEntry : public FactEntry {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler ) const override {
for (const auto : UnderlyingMutexes) {
- const auto *Entry = FSet.findLock(
- FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
- if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
- (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
+ const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
+ if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
+ (UnderlyingMutex.Kind != UCK_Acquired && !Entry)) {
// If this scoped lock manages another mutex, and if the underlying
// mutex is still/not held, then warn about the underlying mutex.
Handler.handleMutexHeldEndOfScope(
-"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(),
JoinLoc,
-LEK);
+"mutex", UnderlyingMutex.Cap.toString(), loc(), JoinLoc, LEK);
}
}
}
@@ -940,13 +939,12 @@ class ScopedLockableFactEntry : public FactEntry {
ThreadSafetyHandler ,
StringRef DiagKind) const override {
for (const auto : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
- if (UnderlyingMutex.getInt() == UCK_Acquired)
-lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), ,
- DiagKind);
+ if (UnderlyingMutex.Kind == UCK_Acquired)
+lock(FSet, FactMan, UnderlyingMutex.Cap, entry.kind(), entry.loc(),
+ , DiagKind);
else
-unlock(FSet, FactMan, UnderCp, entry.loc(), , DiagKind);
+unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), ,
+ DiagKind);
}
}
@@ -956,18 +954,18 @@ class ScopedLockableFactEntry : public FactEntry {
StringRef DiagKind) const override {
assert(!Cp.negative() && "Managing object cannot be negative.");
for (const auto : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
// Remove/lock the underlying mutex if it exists/is still unlocked; warn
// on double unlocking/locking if we're not destroying the scoped object.
ThreadSafetyHandler *TSHandler = FullyRemove ? nullptr :
- if (UnderlyingMutex.getInt() == UCK_Acquired) {
-
[clang] dd1790c - Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: dd1790cd05ae124e9e5d57dfe9279ff54f34b488
URL:
https://github.com/llvm/llvm-project/commit/dd1790cd05ae124e9e5d57dfe9279ff54f34b488
DIFF:
https://github.com/llvm/llvm-project/commit/dd1790cd05ae124e9e5d57dfe9279ff54f34b488.diff
LOG: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)
We're storing these quite frequently: FactEntry inherits from
CapabilityExpr, and the FactManager has a vector of such entries.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124127
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index 2f6a78126a1d..392eecdb1897 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -30,6 +30,7 @@
#include "clang/Analysis/CFG.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
#include
@@ -269,28 +270,27 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression.
- const til::SExpr* CapExpr;
-
- /// True if this is a negative capability.
- bool Negated;
+ /// The capability expression and whether it's negated.
+ llvm::PointerIntPair CapExpr;
public:
- CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E), Negated(Neg) {}
+ CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E, Neg) {}
- const til::SExpr* sexpr() const { return CapExpr; }
- bool negative() const { return Negated; }
+ const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
+ bool negative() const { return CapExpr.getInt(); }
CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr, !Negated);
+return CapabilityExpr(CapExpr.getPointer(), !CapExpr.getInt());
}
bool equals(const CapabilityExpr ) const {
-return (Negated == other.Negated) && sx::equals(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::equals(sexpr(), other.sexpr());
}
bool matches(const CapabilityExpr ) const {
-return (Negated == other.Negated) && sx::matches(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::matches(sexpr(), other.sexpr());
}
bool matchesUniv(const CapabilityExpr ) const {
@@ -298,27 +298,27 @@ class CapabilityExpr {
}
bool partiallyMatches(const CapabilityExpr ) const {
-return (Negated == other.Negated) &&
-sx::partiallyMatches(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::partiallyMatches(sexpr(), other.sexpr());
}
const ValueDecl* valueDecl() const {
-if (Negated || CapExpr == nullptr)
+if (negative() || sexpr() == nullptr)
return nullptr;
-if (const auto *P = dyn_cast(CapExpr))
+if (const auto *P = dyn_cast(sexpr()))
return P->clangDecl();
-if (const auto *P = dyn_cast(CapExpr))
+if (const auto *P = dyn_cast(sexpr()))
return P->clangDecl();
return nullptr;
}
std::string toString() const {
-if (Negated)
- return "!" + sx::toString(CapExpr);
-return sx::toString(CapExpr);
+if (negative())
+ return "!" + sx::toString(sexpr());
+return sx::toString(sexpr());
}
- bool shouldIgnore() const { return CapExpr == nullptr; }
+ bool shouldIgnore() const { return sexpr() == nullptr; }
bool isInvalid() const { return sexpr() && isa(sexpr()); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] c9a16e8 - Drop '* text=auto' from .gitattributes and normalize
Author: Aaron Puchert
Date: 2022-04-28T03:05:10+02:00
New Revision: c9a16e8c3d99173c7525e576d78eed57110d2b08
URL:
https://github.com/llvm/llvm-project/commit/c9a16e8c3d99173c7525e576d78eed57110d2b08
DIFF:
https://github.com/llvm/llvm-project/commit/c9a16e8c3d99173c7525e576d78eed57110d2b08.diff
LOG: Drop '* text=auto' from .gitattributes and normalize
Git wants to check in 'text' files with LF endings, so this changes them
in the repository but not in the checkout, where they keep CRLF endings.
Differential Revision: https://reviews.llvm.org/D124563
Added:
Modified:
clang-tools-extra/test/.gitattributes
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
Removed:
diff --git a/clang-tools-extra/test/.gitattributes
b/clang-tools-extra/test/.gitattributes
index 9d17a32c7b261..42567e3196d61 100644
--- a/clang-tools-extra/test/.gitattributes
+++ b/clang-tools-extra/test/.gitattributes
@@ -2,10 +2,6 @@
# rely on or check fixed character -offset, Offset: or FileOffset: locations
# will fail when run on input files checked out with
diff erent line endings.
-# Most test input files should use native line endings, to ensure that we run
-# tests against both line ending types.
-* text=auto
-
# These test input files rely on one-byte Unix (LF) line-endings, as they use
# fixed -offset, FileOffset:, or Offset: numbers in their tests.
clang-apply-replacements/ClangRenameClassReplacements.cpp text eol=lf
diff --git
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
index 26f79968f556d..09e50c292cc91 100644
--- a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
@@ -1,6 +1,6 @@
-
-// This file intentionally uses a CRLF newlines!
-
-void foo() {
- int *x = 0;
-}
+
+// This file intentionally uses a CRLF newlines!
+
+void foo() {
+ int *x = 0;
+}
diff --git
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
index ad8e907856283..7a5e11354748d 100644
---
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
+++
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
@@ -1,6 +1,6 @@
-
-// This file intentionally uses a CRLF newlines!
-
-void foo() {
- int *x = nullptr;
-}
+
+// This file intentionally uses a CRLF newlines!
+
+void foo() {
+ int *x = nullptr;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1721d52 - Let clang-repl link privately against Clang components
Author: Aaron Puchert Date: 2022-03-28T23:53:53+02:00 New Revision: 1721d52a62067b8a5ceec58b417b2c73ad870b13 URL: https://github.com/llvm/llvm-project/commit/1721d52a62067b8a5ceec58b417b2c73ad870b13 DIFF: https://github.com/llvm/llvm-project/commit/1721d52a62067b8a5ceec58b417b2c73ad870b13.diff LOG: Let clang-repl link privately against Clang components First of all, this is the convention: all other tools have their dependencies private. While it does not have an effect on linking (there is no linking against executables), it does have an effect on exporting: having the targets private allows installing the tools without the libraries in a statically linked build, or a build against libclang-cpp.so. Reviewed By: v.g.vassilev Differential Revision: https://reviews.llvm.org/D122546 Added: Modified: clang/tools/clang-repl/CMakeLists.txt Removed: diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 30e3b2be9ed37..b51a18c10cdca 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -11,7 +11,7 @@ add_clang_tool(clang-repl ClangRepl.cpp ) -clang_target_link_libraries(clang-repl PUBLIC +clang_target_link_libraries(clang-repl PRIVATE clangBasic clangFrontend clangInterpreter ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 9f0fa65 - Comment parsing: Don't recognize commands in single-line double quotation
Author: Aaron Puchert
Date: 2022-01-14T22:46:07+01:00
New Revision: 9f0fa6544012ed8f7b6b3d72fce6535bf4430e40
URL:
https://github.com/llvm/llvm-project/commit/9f0fa6544012ed8f7b6b3d72fce6535bf4430e40
DIFF:
https://github.com/llvm/llvm-project/commit/9f0fa6544012ed8f7b6b3d72fce6535bf4430e40.diff
LOG: Comment parsing: Don't recognize commands in single-line double quotation
This is consistent with the behavior of Doxygen, and allows users to
write strings with C escapes or document input/output formats containing
special characters (@ or \) without escaping them, which might be
confusing. For example, if a function wants to document its expected
input format as "user@host" it doesn't have to write user\@host instead,
which would look right in the documentation but confusing in the code.
Now users can just use double quotes (which they might do anyway).
This fixes a lot of false positives of -Wdocumentation-unknown-command,
but it could also fix issues with -Wdocumentation if the text triggers
an actual command.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D116190
Added:
Modified:
clang/include/clang/AST/CommentLexer.h
clang/lib/AST/CommentLexer.cpp
clang/test/Sema/warn-documentation-unknown-command.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/CommentLexer.h
b/clang/include/clang/AST/CommentLexer.h
index 94f778501e758..9aa1681cb2c5c 100644
--- a/clang/include/clang/AST/CommentLexer.h
+++ b/clang/include/clang/AST/CommentLexer.h
@@ -320,6 +320,9 @@ class Lexer {
/// Eat string matching regexp \code \s*\* \endcode.
void skipLineStartingDecorations();
+ /// Skip over pure text.
+ const char *skipTextToken();
+
/// Lex comment text, including commands if ParseCommands is set to true.
void lexCommentText(Token );
diff --git a/clang/lib/AST/CommentLexer.cpp b/clang/lib/AST/CommentLexer.cpp
index 6e00c2aa7c280..61ce8979f13f5 100644
--- a/clang/lib/AST/CommentLexer.cpp
+++ b/clang/lib/AST/CommentLexer.cpp
@@ -270,6 +270,29 @@ void Lexer::formTokenWithChars(Token , const char
*TokEnd,
BufferPtr = TokEnd;
}
+const char *Lexer::skipTextToken() {
+ const char *TokenPtr = BufferPtr;
+ assert(TokenPtr < CommentEnd);
+ StringRef TokStartSymbols = ParseCommands ? "\n\r\\@\"&<" : "\n\r";
+
+again:
+ size_t End =
+ StringRef(TokenPtr, CommentEnd -
TokenPtr).find_first_of(TokStartSymbols);
+ if (End == StringRef::npos)
+return CommentEnd;
+
+ // Doxygen doesn't recognize any commands in a one-line double quotation.
+ // If we don't find an ending quotation mark, we pretend it never began.
+ if (*(TokenPtr + End) == '\"') {
+TokenPtr += End + 1;
+End = StringRef(TokenPtr, CommentEnd - TokenPtr).find_first_of("\n\r\"");
+if (End != StringRef::npos && *(TokenPtr + End) == '\"')
+ TokenPtr += End + 1;
+goto again;
+ }
+ return TokenPtr + End;
+}
+
void Lexer::lexCommentText(Token ) {
assert(CommentState == LCS_InsideBCPLComment ||
CommentState == LCS_InsideCComment);
@@ -290,17 +313,8 @@ void Lexer::lexCommentText(Token ) {
skipLineStartingDecorations();
return;
- default: {
- StringRef TokStartSymbols = ParseCommands ? "\n\r\\@&<" : "\n\r";
- size_t End = StringRef(TokenPtr, CommentEnd - TokenPtr)
- .find_first_of(TokStartSymbols);
- if (End != StringRef::npos)
-TokenPtr += End;
- else
-TokenPtr = CommentEnd;
- formTextToken(T, TokenPtr);
- return;
- }
+ default:
+return formTextToken(T, skipTextToken());
}
};
diff --git a/clang/test/Sema/warn-documentation-unknown-command.cpp
b/clang/test/Sema/warn-documentation-unknown-command.cpp
index 4328c9682f212..2cb261d627c56 100644
--- a/clang/test/Sema/warn-documentation-unknown-command.cpp
+++ b/clang/test/Sema/warn-documentation-unknown-command.cpp
@@ -9,6 +9,15 @@ int test_unknown_comand_1;
/// \retur aaa
int test_unknown_comand_2();
+/// We don't recognize commands in double quotes: "\n\t @unknown2".
+int test_unknown_comand_3();
+
+// expected-warning@+2 {{unknown command tag name}}
+// expected-warning@+2 {{unknown command tag name}}
+/// But it has to be a single line: "\unknown3
+/// @unknown4" (Doxygen treats multi-line quotes inconsistently.)
+int test_unknown_comand_4();
+
// RUN: c-index-test -test-load-source all -Wdocumentation-unknown-command %s
> /dev/null 2> %t.err
// RUN: FileCheck < %t.err -check-prefix=CHECK-RANGE %s
// CHECK-RANGE: warn-documentation-unknown-command.cpp:5:9:{5:9-5:17}:
warning: unknown command tag name
diff --git a/clang/test/Sema/warn-documentation.cpp
b/clang/test/Sema/warn-documentation.cpp
index 7243e791bba60..353c94a47eb6f 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++
[clang] bd0a970 - Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)
Author: Aaron Puchert
Date: 2022-01-14T22:45:10+01:00
New Revision: bd0a970f5341f9981a9ad33fdfda99f8ff7348b3
URL:
https://github.com/llvm/llvm-project/commit/bd0a970f5341f9981a9ad33fdfda99f8ff7348b3
DIFF:
https://github.com/llvm/llvm-project/commit/bd0a970f5341f9981a9ad33fdfda99f8ff7348b3.diff
LOG: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)
Inspection of the first character can just be handled by the loop as
well, it does exactly the same thing. Dereferencing the pointer a second
time shouldn't be an issue: the middle end can eliminate that second
read as it's separated from the first only by a pure function call.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D116186
Added:
Modified:
clang/lib/AST/CommentLexer.cpp
Removed:
diff --git a/clang/lib/AST/CommentLexer.cpp b/clang/lib/AST/CommentLexer.cpp
index 93531c06192d4..6e00c2aa7c280 100644
--- a/clang/lib/AST/CommentLexer.cpp
+++ b/clang/lib/AST/CommentLexer.cpp
@@ -94,31 +94,12 @@ void Lexer::skipLineStartingDecorations() {
if (BufferPtr == CommentEnd)
return;
- switch (*BufferPtr) {
- case ' ':
- case '\t':
- case '\f':
- case '\v': {
-const char *NewBufferPtr = BufferPtr;
-NewBufferPtr++;
-if (NewBufferPtr == CommentEnd)
+ const char *NewBufferPtr = BufferPtr;
+ while (isHorizontalWhitespace(*NewBufferPtr))
+if (++NewBufferPtr == CommentEnd)
return;
-
-char C = *NewBufferPtr;
-while (isHorizontalWhitespace(C)) {
- NewBufferPtr++;
- if (NewBufferPtr == CommentEnd)
-return;
- C = *NewBufferPtr;
-}
-if (C == '*')
- BufferPtr = NewBufferPtr + 1;
-break;
- }
- case '*':
-BufferPtr++;
-break;
- }
+ if (*NewBufferPtr == '*')
+BufferPtr = NewBufferPtr + 1;
}
namespace {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 3010883 - Comment AST: Recognize function-like objects via return type (NFC)
Author: Aaron Puchert
Date: 2021-11-12T21:11:11+01:00
New Revision: 3010883fc296619def051e0a2f97d40fb15960d7
URL:
https://github.com/llvm/llvm-project/commit/3010883fc296619def051e0a2f97d40fb15960d7
DIFF:
https://github.com/llvm/llvm-project/commit/3010883fc296619def051e0a2f97d40fb15960d7.diff
LOG: Comment AST: Recognize function-like objects via return type (NFC)
Instead of pretending that function pointer type aliases or variables
are functions, and thereby losing the information that they are type
aliases or variables, respectively, we use the existence of a return
type in the DeclInfo to signify a "function-like" object.
That seems pretty natural, since it's also the return type (or parameter
list) from the DeclInfo that we compare the documentation with.
Addresses a concern voiced in D111264#3115104.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D113691
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/include/clang/AST/CommentSema.h
clang/lib/AST/Comment.cpp
clang/lib/AST/CommentSema.cpp
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index fff0a985028a..4184e103206d 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1019,9 +1019,6 @@ struct DeclInfo {
/// \li member function template,
/// \li member function template specialization,
/// \li ObjC method,
-/// \li variable of function pointer, member function pointer or block
type,
-/// \li a typedef for a function pointer, member function pointer,
-/// ObjC block.
FunctionKind,
/// Something that we consider a "class":
@@ -1089,6 +1086,8 @@ struct DeclInfo {
TemplateDeclKind getTemplateKind() const LLVM_READONLY {
return static_cast(TemplateKind);
}
+
+ bool involvesFunctionType() const { return !ReturnType.isNull(); }
};
/// A full comment attached to a declaration, contains block content.
diff --git a/clang/include/clang/AST/CommentSema.h
b/clang/include/clang/AST/CommentSema.h
index 4a5174e08bff..b4e564e81185 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -201,6 +201,10 @@ class Sema {
/// Emit diagnostics about unknown parametrs.
void resolveParamCommandIndexes(const FullComment *FC);
+ /// \returns \c true if the declaration that this comment is attached to
+ /// is a pointer to function/method/block type or has such a type.
+ bool involvesFunctionType();
+
bool isFunctionDecl();
bool isAnyFunctionDecl();
diff --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index aac0591b057e..fae3640d5ff7 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -250,6 +250,7 @@ void DeclInfo::fill() {
IsClassMethod = !IsInstanceMethod;
}
IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
break;
}
case Decl::ObjCMethod: {
@@ -261,6 +262,7 @@ void DeclInfo::fill() {
IsInstanceMethod = MD->isInstanceMethod();
IsClassMethod = !IsInstanceMethod;
IsVariadic = MD->isVariadic();
+assert(involvesFunctionType());
break;
}
case Decl::FunctionTemplate: {
@@ -272,6 +274,7 @@ void DeclInfo::fill() {
ReturnType = FD->getReturnType();
TemplateParameters = FTD->getTemplateParameters();
IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
break;
}
case Decl::ClassTemplate: {
@@ -352,11 +355,11 @@ void DeclInfo::fill() {
TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
FunctionTypeLoc FTL;
if (getFunctionTypeLoc(TL, FTL)) {
- Kind = FunctionKind;
ParamVars = FTL.getParams();
ReturnType = FTL.getReturnLoc().getType();
if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
IsVariadic = FPT->isVariadic();
+ assert(involvesFunctionType());
}
}
diff --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index 8251802482a0..087f103e4931 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -86,7 +86,7 @@ ParamCommandComment *Sema::actOnParamCommandStart(
new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
CommandMarker);
- if (!isFunctionDecl())
+ if (!involvesFunctionType())
Diag(Command->getLocation(),
diag::warn_doc_param_not_attached_to_a_function_decl)
<< CommandMarker
@@ -588,7 +588,7 @@ void Sema::checkReturnsCommand(const BlockCommandComment
*Command) {
// to document the value that the property getter returns.
if (isObjCPropertyDecl())
return;
- if (isFunctionDecl()) {
+ if (involvesFunctionType()) {
assert(!ThisDeclInfo->ReturnType.isNull() &&
"should have a valid return type");
if (ThisDeclInfo->ReturnType->isVoidType()) {
@@
[clang] 59b1e98 - Comment Sema: Make most of CommentSema private (NFC)
Author: Aaron Puchert
Date: 2021-11-12T21:11:52+01:00
New Revision: 59b1e98137e961a61ffaeb609b6790999c461bec
URL:
https://github.com/llvm/llvm-project/commit/59b1e98137e961a61ffaeb609b6790999c461bec
DIFF:
https://github.com/llvm/llvm-project/commit/59b1e98137e961a61ffaeb609b6790999c461bec.diff
LOG: Comment Sema: Make most of CommentSema private (NFC)
We only need to expose setDecl, copyArray and the actOn* methods.
Added:
Modified:
clang/include/clang/AST/CommentSema.h
Removed:
diff --git a/clang/include/clang/AST/CommentSema.h
b/clang/include/clang/AST/CommentSema.h
index b4e564e81185..015ce8f8652a 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -181,6 +181,7 @@ class Sema {
FullComment *actOnFullComment(ArrayRef Blocks);
+private:
void checkBlockCommandEmptyParagraph(BlockCommandComment *Command);
void checkReturnsCommand(const BlockCommandComment *Command);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 4e7df1e - Comment AST: Find out if function is variadic in DeclInfo::fill
Author: Aaron Puchert
Date: 2021-11-12T21:10:56+01:00
New Revision: 4e7df1ef7b679953c1501177539166876c4cbda4
URL:
https://github.com/llvm/llvm-project/commit/4e7df1ef7b679953c1501177539166876c4cbda4
DIFF:
https://github.com/llvm/llvm-project/commit/4e7df1ef7b679953c1501177539166876c4cbda4.diff
LOG: Comment AST: Find out if function is variadic in DeclInfo::fill
Then we don't have to look into the declaration again. Also it's only
natural to collect this information alongside parameters and return
type, as it's also just a parameter in some sense.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D113690
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/lib/AST/Comment.cpp
clang/lib/AST/CommentSema.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index 50ed7eec82080..fff0a985028a8 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1077,6 +1077,9 @@ struct DeclInfo {
/// Can be true only if \c IsFunctionDecl is true.
unsigned IsClassMethod : 1;
+ /// Is \c CommentDecl something we consider a "function" that's variadic.
+ unsigned IsVariadic : 1;
+
void fill();
DeclKind getKind() const LLVM_READONLY {
diff --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index 5e6a7de5b5638..aac0591b057ec 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -210,6 +210,7 @@ void DeclInfo::fill() {
IsObjCMethod = false;
IsInstanceMethod = false;
IsClassMethod = false;
+ IsVariadic = false;
ParamVars = None;
TemplateParameters = nullptr;
@@ -248,6 +249,7 @@ void DeclInfo::fill() {
IsInstanceMethod = MD->isInstance();
IsClassMethod = !IsInstanceMethod;
}
+IsVariadic = FD->isVariadic();
break;
}
case Decl::ObjCMethod: {
@@ -258,6 +260,7 @@ void DeclInfo::fill() {
IsObjCMethod = true;
IsInstanceMethod = MD->isInstanceMethod();
IsClassMethod = !IsInstanceMethod;
+IsVariadic = MD->isVariadic();
break;
}
case Decl::FunctionTemplate: {
@@ -268,6 +271,7 @@ void DeclInfo::fill() {
ParamVars = FD->parameters();
ReturnType = FD->getReturnType();
TemplateParameters = FTD->getTemplateParameters();
+IsVariadic = FD->isVariadic();
break;
}
case Decl::ClassTemplate: {
@@ -351,6 +355,8 @@ void DeclInfo::fill() {
Kind = FunctionKind;
ParamVars = FTL.getParams();
ReturnType = FTL.getReturnLoc().getType();
+ if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
+IsVariadic = FPT->isVariadic();
}
}
diff --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index 3977e6c20a88a..8251802482a0c 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -830,26 +830,11 @@ bool Sema::isAnyFunctionDecl() {
}
bool Sema::isFunctionOrMethodVariadic() {
- if (!isFunctionDecl() || !ThisDeclInfo->CurrentDecl)
+ if (!ThisDeclInfo)
return false;
- if (const FunctionDecl *FD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FD->isVariadic();
- if (const FunctionTemplateDecl *FTD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FTD->getTemplatedDecl()->isVariadic();
- if (const ObjCMethodDecl *MD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return MD->isVariadic();
- if (const TypedefNameDecl *TD =
- dyn_cast(ThisDeclInfo->CurrentDecl)) {
-QualType Type = TD->getUnderlyingType();
-if (Type->isFunctionPointerType() || Type->isBlockPointerType())
- Type = Type->getPointeeType();
-if (const auto *FT = Type->getAs())
- return FT->isVariadic();
- }
- return false;
+ if (!ThisDeclInfo->IsFilled)
+inspectThisDecl();
+ return ThisDeclInfo->IsVariadic;
}
bool Sema::isObjCMethodDecl() {
diff --git a/clang/test/Sema/warn-documentation.cpp
b/clang/test/Sema/warn-documentation.cpp
index 3a25c31f76f59..7243e791bba60 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -1448,6 +1448,17 @@ typedef void (*VariadicFnType)(int a, ...);
*/
using VariadicFnType2 = void (*)(int a, ...);
+/*!
+ * Function pointer type variable.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+void (*variadicFnVar)(int a, ...);
+
// expected-warning@+2 {{empty paragraph passed to '@note' command}}
/**
@note
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 63ef0e1 - Comment AST: Add support for variable templates
Author: Aaron Puchert
Date: 2021-11-09T22:30:09+01:00
New Revision: 63ef0e17e28827eae53133b3467bdac7d9729318
URL:
https://github.com/llvm/llvm-project/commit/63ef0e17e28827eae53133b3467bdac7d9729318
DIFF:
https://github.com/llvm/llvm-project/commit/63ef0e17e28827eae53133b3467bdac7d9729318.diff
LOG: Comment AST: Add support for variable templates
We treat them as variables of course, though if they have function
pointer type we treat them as functions, i.e. allow parameter and return
value specifications. Just like VarDecls.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D111266
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/lib/AST/Comment.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index e394e9608426..50ed7eec8208 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1031,8 +1031,8 @@ struct DeclInfo {
ClassKind,
/// Something that we consider a "variable":
-/// \li namespace scope variables;
-/// \li static and non-static class data members;
+/// \li namespace scope variables and variable templates;
+/// \li static and non-static class data members and member templates;
/// \li enumerators.
VariableKind,
diff --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index ce90abf55294..5e6a7de5b563 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -294,6 +294,12 @@ void DeclInfo::fill() {
Kind = ClassKind;
break;
case Decl::Var:
+if (const VarTemplateDecl *VTD =
+cast(CommentDecl)->getDescribedVarTemplate()) {
+ TemplateKind = TemplateSpecialization;
+ TemplateParameters = VTD->getTemplateParameters();
+}
+LLVM_FALLTHROUGH;
case Decl::Field:
case Decl::EnumConstant:
case Decl::ObjCIvar:
@@ -305,6 +311,15 @@ void DeclInfo::fill() {
TSI = PD->getTypeSourceInfo();
Kind = VariableKind;
break;
+ case Decl::VarTemplate: {
+const VarTemplateDecl *VTD = cast(CommentDecl);
+Kind = VariableKind;
+TemplateKind = Template;
+TemplateParameters = VTD->getTemplateParameters();
+if (const VarDecl *VD = VTD->getTemplatedDecl())
+ TSI = VD->getTypeSourceInfo();
+break;
+ }
case Decl::Namespace:
Kind = NamespaceKind;
break;
diff --git a/clang/test/Sema/warn-documentation.cpp
b/clang/test/Sema/warn-documentation.cpp
index f30f05fccd71..3a25c31f76f5 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -1323,6 +1323,17 @@ namespace AllowParamAndReturnsOnFunctionPointerVars {
*/
int (*functionPointerVariable)(int i);
+#if __cplusplus >= 201402L
+/**
+ * functionPointerVariableTemplate
+ *
+ * @param i is something.
+ * @returns integer.
+ */
+template
+int (*functionPointerVariableTemplate)(T i);
+#endif
+
struct HasFields {
/**
* functionPointerField
@@ -1331,6 +1342,18 @@ struct HasFields {
* @returns integer.
*/
int (*functionPointerField)(int i);
+
+#if __cplusplus >= 201402L
+ /**
+ * functionPointerTemplateMember
+ *
+ * @tparam T some type.
+ * @param i is integer.
+ * @returns integer.
+ */
+ template
+ static int (*functionPointerTemplateMember)(int i);
+#endif
};
// expected-warning@+5 {{parameter 'p' not found in the function declaration}}
@@ -1343,6 +1366,23 @@ struct HasFields {
*/
void (*functionPointerVariableThatLeadsNowhere)();
+#if __cplusplus >= 201402L
+// expected-warning@+8 {{template parameter 'X' not found in the template
declaration}}
+// expected-note@+7 {{did you mean 'T'?}}
+// expected-warning@+7 {{parameter 'p' not found in the function declaration}}
+// expected-note@+6 {{did you mean 'x'?}}
+// expected-warning@+6 {{'\returns' command used in a comment that is attached
to a function returning void}}
+/**
+ * functionPointerVariable
+ *
+ * \tparam X typo
+ * \param p not here.
+ * \returns integer.
+ */
+template
+void (*functionPointerVariableTemplateThatLeadsNowhere)(T x);
+#endif
+
// Still warn about param/returns commands for variables that don't specify
// the type directly:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 4d63824 - Comment AST: Declare function pointer variables as functions
Author: Aaron Puchert
Date: 2021-11-09T22:30:08+01:00
New Revision: 4d6382430066465774f6f696ea3f4c402da1d705
URL:
https://github.com/llvm/llvm-project/commit/4d6382430066465774f6f696ea3f4c402da1d705
DIFF:
https://github.com/llvm/llvm-project/commit/4d6382430066465774f6f696ea3f4c402da1d705.diff
LOG: Comment AST: Declare function pointer variables as functions
We were doing this already for type aliases, and it deduplicates the
code looking through aliases and pointers to find a function type. As
a side effect, this finds two warnings that we apparently missed before.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D111264
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/include/clang/AST/CommentSema.h
clang/lib/AST/Comment.cpp
clang/lib/AST/CommentSema.cpp
clang/test/Sema/warn-documentation.cpp
clang/test/Sema/warn-documentation.m
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index 54a4b0a9cfe66..e394e96084264 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1019,6 +1019,7 @@ struct DeclInfo {
/// \li member function template,
/// \li member function template specialization,
/// \li ObjC method,
+/// \li variable of function pointer, member function pointer or block
type,
/// \li a typedef for a function pointer, member function pointer,
/// ObjC block.
FunctionKind,
diff --git a/clang/include/clang/AST/CommentSema.h
b/clang/include/clang/AST/CommentSema.h
index 6dfe0f4920d06..4a5174e08bff6 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -207,10 +207,6 @@ class Sema {
/// \returns \c true if declaration that this comment is attached to declares
/// a function pointer.
bool isFunctionPointerVarDecl();
- /// \returns \c true if the declaration that this comment is attached to
- /// declares a variable or a field whose type is a function or a block
- /// pointer.
- bool isFunctionOrBlockPointerVarLikeDecl();
bool isFunctionOrMethodVariadic();
bool isObjCMethodDecl();
bool isObjCPropertyDecl();
diff --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index 94f65466ad102..ce90abf552947 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -333,8 +333,7 @@ void DeclInfo::fill() {
TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
FunctionTypeLoc FTL;
if (getFunctionTypeLoc(TL, FTL)) {
- if (Kind == TypedefKind)
-Kind = FunctionKind;
+ Kind = FunctionKind;
ParamVars = FTL.getParams();
ReturnType = FTL.getReturnLoc().getType();
}
diff --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index e385c5817ef23..3977e6c20a88a 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -86,7 +86,7 @@ ParamCommandComment *Sema::actOnParamCommandStart(
new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
CommandMarker);
- if (!isFunctionDecl() && !isFunctionOrBlockPointerVarLikeDecl())
+ if (!isFunctionDecl())
Diag(Command->getLocation(),
diag::warn_doc_param_not_attached_to_a_function_decl)
<< CommandMarker
@@ -588,7 +588,7 @@ void Sema::checkReturnsCommand(const BlockCommandComment
*Command) {
// to document the value that the property getter returns.
if (isObjCPropertyDecl())
return;
- if (isFunctionDecl() || isFunctionOrBlockPointerVarLikeDecl()) {
+ if (isFunctionDecl()) {
assert(!ThisDeclInfo->ReturnType.isNull() &&
"should have a valid return type");
if (ThisDeclInfo->ReturnType->isVoidType()) {
@@ -871,36 +871,6 @@ bool Sema::isFunctionPointerVarDecl() {
return false;
}
-bool Sema::isFunctionOrBlockPointerVarLikeDecl() {
- if (!ThisDeclInfo)
-return false;
- if (!ThisDeclInfo->IsFilled)
-inspectThisDecl();
- if (ThisDeclInfo->getKind() != DeclInfo::VariableKind ||
- !ThisDeclInfo->CurrentDecl)
-return false;
- QualType QT;
- if (const auto *VD = dyn_cast(ThisDeclInfo->CurrentDecl))
-QT = VD->getType();
- else if (const auto *PD =
- dyn_cast(ThisDeclInfo->CurrentDecl))
-QT = PD->getType();
- else
-return false;
- // We would like to warn about the 'returns'/'param' commands for
- // variables that don't directly specify the function type, so type aliases
- // can be ignored.
- if (QT->getAs())
-return false;
- if (const auto *P = QT->getAs())
-if (P->getPointeeType()->getAs())
- return false;
- if (const auto *P = QT->getAs())
-if (P->getPointeeType()->getAs())
- return false;
- return QT->isFunctionPointerType() || QT->isBlockPointerType();
-}
-
bool Sema::isObjCPropertyDecl() {
if (!ThisDeclInfo)
return false;
diff
[clang] 3506e42 - Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)
Author: Aaron Puchert
Date: 2021-11-09T22:30:08+01:00
New Revision: 3506e42ab67eef41a1f27e180e7c552a27bb
URL:
https://github.com/llvm/llvm-project/commit/3506e42ab67eef41a1f27e180e7c552a27bb
DIFF:
https://github.com/llvm/llvm-project/commit/3506e42ab67eef41a1f27e180e7c552a27bb.diff
LOG: Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D111262
Added:
Modified:
clang/lib/AST/Comment.cpp
Removed:
diff --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index a02cc9d119fe..94f65466ad10 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -221,6 +221,7 @@ void DeclInfo::fill() {
CurrentDecl = CommentDecl;
Decl::Kind K = CommentDecl->getKind();
+ const TypeSourceInfo *TSI = nullptr;
switch (K) {
default:
// Defaults are should be good for declarations we don't handle explicitly.
@@ -297,72 +298,46 @@ void DeclInfo::fill() {
case Decl::EnumConstant:
case Decl::ObjCIvar:
case Decl::ObjCAtDefsField:
- case Decl::ObjCProperty: {
-const TypeSourceInfo *TSI;
+ case Decl::ObjCProperty:
if (const auto *VD = dyn_cast(CommentDecl))
TSI = VD->getTypeSourceInfo();
else if (const auto *PD = dyn_cast(CommentDecl))
TSI = PD->getTypeSourceInfo();
-else
- TSI = nullptr;
-if (TSI) {
- TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
- FunctionTypeLoc FTL;
- if (getFunctionTypeLoc(TL, FTL)) {
-ParamVars = FTL.getParams();
-ReturnType = FTL.getReturnLoc().getType();
- }
-}
Kind = VariableKind;
break;
- }
case Decl::Namespace:
Kind = NamespaceKind;
break;
case Decl::TypeAlias:
- case Decl::Typedef: {
+ case Decl::Typedef:
Kind = TypedefKind;
-// If this is a typedef / using to something we consider a function,
extract
-// arguments and return type.
-const TypeSourceInfo *TSI =
-K == Decl::Typedef
-? cast(CommentDecl)->getTypeSourceInfo()
-: cast(CommentDecl)->getTypeSourceInfo();
-if (!TSI)
- break;
-TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
-FunctionTypeLoc FTL;
-if (getFunctionTypeLoc(TL, FTL)) {
- Kind = FunctionKind;
- ParamVars = FTL.getParams();
- ReturnType = FTL.getReturnLoc().getType();
-}
+TSI = cast(CommentDecl)->getTypeSourceInfo();
break;
- }
case Decl::TypeAliasTemplate: {
const TypeAliasTemplateDecl *TAT =
cast(CommentDecl);
Kind = TypedefKind;
TemplateKind = Template;
TemplateParameters = TAT->getTemplateParameters();
-TypeAliasDecl *TAD = TAT->getTemplatedDecl();
-if (!TAD)
- break;
+if (TypeAliasDecl *TAD = TAT->getTemplatedDecl())
+ TSI = TAD->getTypeSourceInfo();
+break;
+ }
+ case Decl::Enum:
+Kind = EnumKind;
+break;
+ }
-const TypeSourceInfo *TSI = TAD->getTypeSourceInfo();
-if (!TSI)
- break;
+ // If the type is a typedef / using to something we consider a function,
+ // extract arguments and return type.
+ if (TSI) {
TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
FunctionTypeLoc FTL;
if (getFunctionTypeLoc(TL, FTL)) {
- Kind = FunctionKind;
+ if (Kind == TypedefKind)
+Kind = FunctionKind;
ParamVars = FTL.getParams();
ReturnType = FTL.getReturnLoc().getType();
}
-break;
- }
- case Decl::Enum:
-Kind = EnumKind;
-break;
}
IsFilled = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 196554d - Comment parsing: Complete list of Doxygen commands
Author: Aaron Puchert
Date: 2021-11-09T18:35:26+01:00
New Revision: 196554d42d329e45363afe2293d1fc19de75673d
URL:
https://github.com/llvm/llvm-project/commit/196554d42d329e45363afe2293d1fc19de75673d
DIFF:
https://github.com/llvm/llvm-project/commit/196554d42d329e45363afe2293d1fc19de75673d.diff
LOG: Comment parsing: Complete list of Doxygen commands
These should be all the commands from [1] except those that are marked
obsolete, and "link" / "endlink", as that conflicts with the existing
HeaderDoc pair "link / "/link". For some commands we don't have the
ideal category, but it should work good enough for most cases.
There seems to be no existing test for most commands (except the ones
interpreted by -Wdocumentation), and to some extent such a test wouldn't
look very interesting. But I added a test for the correct parsing of
formulas, as they're a bit special. And I had to adapt
comment-lots-of-unknown-commands.c because typo correction was kicking
in and recognizing some of the commands.
This should fix a couple of reported bugs: PR17437, PR19581, PR24062
(partially, no diagnostic for matching cond/endcond), PR32909, PR37813,
PR44243 (partially, em...@domain.com must be addressed separately).
[1] https://www.doxygen.nl/manual/commands.html
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D90
Added:
Modified:
clang/include/clang/AST/CommentCommands.td
clang/lib/AST/CommentLexer.cpp
clang/test/AST/ast-dump-comment.cpp
clang/test/Index/comment-lots-of-unknown-commands.c
clang/unittests/AST/CommentLexer.cpp
clang/utils/TableGen/ClangCommentCommandInfoEmitter.cpp
Removed:
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index fbbfc9f7e0b77..7e962a4b4171b 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -87,8 +87,21 @@ def P : InlineCommand<"p">;
def A : InlineCommand<"a">;
def E : InlineCommand<"e">;
def Em : InlineCommand<"em">;
-def Ref: InlineCommand<"ref">;
-def Anchor : InlineCommand<"anchor">;
+def Emoji : InlineCommand<"emoji">;
+
+def Anchor : InlineCommand<"anchor">;
+def Ref : InlineCommand<"ref">;
+def RefItem : InlineCommand<"refitem">;
+def Cite: InlineCommand<"cite">;
+
+def CopyBrief : InlineCommand<"copybrief">;
+def CopyDetails : InlineCommand<"copydetails">;
+def CopyDoc : InlineCommand<"copydoc">;
+
+// Typically not used inline, but they take a single word.
+def Extends: InlineCommand<"extends">;
+def Implements : InlineCommand<"implements">;
+def MemberOf : InlineCommand<"memberof">;
//===--===//
// BlockCommand
@@ -145,9 +158,11 @@ def Retval : BlockCommand<"retval">;
def Sa : BlockCommand<"sa">;
def See: BlockCommand<"see">;
def Since : BlockCommand<"since">;
+def Test : BlockCommand<"test">;
def Todo : BlockCommand<"todo">;
def Version: BlockCommand<"version">;
def Warning: BlockCommand<"warning">;
+def XRefItem : BlockCommand<"xrefitem">;
// HeaderDoc commands
def Abstract : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
def ClassDesign : RecordLikeDetailCommand<"classdesign">;
@@ -170,6 +185,8 @@ def SuperClass: RecordLikeDetailCommand<"superclass">;
defm Code : VerbatimBlockCommand<"code", "endcode">;
defm Verbatim : VerbatimBlockCommand<"verbatim", "endverbatim">;
+
+defm DocbookOnly : VerbatimBlockCommand<"docbookonly", "enddocbookonly">;
defm Htmlonly : VerbatimBlockCommand<"htmlonly", "endhtmlonly">;
defm Latexonly : VerbatimBlockCommand<"latexonly", "endlatexonly">;
defm Xmlonly : VerbatimBlockCommand<"xmlonly", "endxmlonly">;
@@ -178,10 +195,19 @@ defm Rtfonly : VerbatimBlockCommand<"rtfonly",
"endrtfonly">;
defm Dot : VerbatimBlockCommand<"dot", "enddot">;
defm Msc : VerbatimBlockCommand<"msc", "endmsc">;
+defm Uml : VerbatimBlockCommand<"startuml", "enduml">;
+
+// Actually not verbatim blocks, we should also parse commands within them.
+defm Internal : VerbatimBlockCommand<"internal", "endinternal">;
+// TODO: conflicts with HeaderDoc link, /link.
+//defm Link : VerbatimBlockCommand<"link", "endlink">;
+defm ParBlock : VerbatimBlockCommand<"parblock", "endparblock">;
+defm SecRefList : VerbatimBlockCommand<"secreflist", "endsecreflist">;
// These three commands have special support in CommentLexer to recognize their
// names.
def FDollar : VerbatimBlockCommand<"f$">; // Inline LaTeX formula
+defm FParen : VerbatimBlockCommand<"f(", "f)">; // Inline LaTeX text
defm FBracket : VerbatimBlockCommand<"f[", "f]">; // Displayed LaTeX formula
defm FBrace : VerbatimBlockCommand<"f{", "f}">; // LaTeX environment
@@ -199,11 +225,18 @@ def Addtogroup :
[clang] 6de19ea - Thread safety analysis: Drop special block handling
Author: Aaron Puchert
Date: 2021-09-20T15:20:15+02:00
New Revision: 6de19ea4b6264e64cea145e00ab66fe1530fc0a0
URL:
https://github.com/llvm/llvm-project/commit/6de19ea4b6264e64cea145e00ab66fe1530fc0a0
DIFF:
https://github.com/llvm/llvm-project/commit/6de19ea4b6264e64cea145e00ab66fe1530fc0a0.diff
LOG: Thread safety analysis: Drop special block handling
Previous changes like D101202 and D104261 have eliminated the special
status that break and continue once had, since now we're making
decisions purely based on the structure of the CFG without regard for
the underlying source code constructs.
This means we don't gain anything from defering handling for these
blocks. Dropping it moves some diagnostics, though arguably into a
better place. We're working around a "quirk" in the CFG that perhaps
wasn't visible before: while loops have an empty "transition block"
where continue statements and the regular loop exit meet, before
continuing to the loop entry. To get a source location for that, we
slightly extend our handling for empty blocks. The source location for
the transition ends up to be the loop entry then, but formally this
isn't a back edge. We pretend it is anyway. (This is safe: we can always
treat edges as back edges, it just means we allow less and don't modify
the lock set. The other way around it wouldn't be safe.)
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D106715
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/PCH/thread-safety-attrs.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 41a55f9579bd..d6bb8cf673f7 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -849,6 +849,11 @@ static void findBlockLocations(CFG *CFGraph,
// location.
CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc =
BlockInfo[(*CurrBlock->pred_begin())->getBlockID()].ExitLoc;
+} else if (CurrBlock->succ_size() == 1 && *CurrBlock->succ_begin()) {
+ // The block is empty, and has a single successor. Use its entry
+ // location.
+ CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc =
+ BlockInfo[(*CurrBlock->succ_begin())->getBlockID()].EntryLoc;
}
}
}
@@ -2415,7 +2420,6 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
// union because the real error is probably that we forgot to unlock M on
// all code paths.
bool LocksetInitialized = false;
-SmallVector SpecialBlocks;
for (CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(),
PE = CurrBlock->pred_end(); PI != PE; ++PI) {
// if *PI -> CurrBlock is a back edge
@@ -2432,17 +2436,6 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
// Okay, we can reach this block from the entry.
CurrBlockInfo->Reachable = true;
- // If the previous block ended in a 'continue' or 'break' statement, then
- // a
diff erence in locksets is probably due to a bug in that block, rather
- // than in some other predecessor. In that case, keep the other
- // predecessor's lockset.
- if (const Stmt *Terminator = (*PI)->getTerminatorStmt()) {
-if (isa(Terminator) || isa(Terminator)) {
- SpecialBlocks.push_back(*PI);
- continue;
-}
- }
-
FactSet PrevLockset;
getEdgeLockset(PrevLockset, PrevBlockInfo->ExitSet, *PI, CurrBlock);
@@ -2450,9 +2443,14 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
CurrBlockInfo->EntrySet = PrevLockset;
LocksetInitialized = true;
} else {
-intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset,
- CurrBlockInfo->EntryLoc,
- LEK_LockedSomePredecessors);
+// Surprisingly 'continue' doesn't always produce back edges, because
+// the CFG has empty "transition" blocks where they meet with the end
+// of the regular loop body. We still want to diagnose them as loop.
+intersectAndWarn(
+CurrBlockInfo->EntrySet, PrevLockset, CurrBlockInfo->EntryLoc,
+isa_and_nonnull((*PI)->getTerminatorStmt())
+? LEK_LockedSomeLoopIterations
+: LEK_LockedSomePredecessors);
}
}
@@ -2460,35 +2458,6 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
if (!CurrBlockInfo->Reachable)
continue;
-// Process continue and break blocks. Assume that the lockset for the
-// resulting block is unaffected by any discrepancies in them.
-for (const auto *PrevBlock : SpecialBlocks) {
- unsigned PrevBlockID = PrevBlock->getBlockID();
- CFGBlockInfo *PrevBlockInfo = [PrevBlockID];
-
- if (!LocksetInitialized) {
-
[clang] 9b889f8 - Thread safety analysis: Warn when demoting locks on back edges
Author: Aaron Puchert
Date: 2021-09-18T13:46:55+02:00
New Revision: 9b889f826ff587e9758c80823419512d502e457d
URL:
https://github.com/llvm/llvm-project/commit/9b889f826ff587e9758c80823419512d502e457d
DIFF:
https://github.com/llvm/llvm-project/commit/9b889f826ff587e9758c80823419512d502e457d.diff
LOG: Thread safety analysis: Warn when demoting locks on back edges
Previously in D104261 we warned about dropping locks from back edges,
this is the corresponding change for exclusive/shared joins. If we're
entering the loop with an exclusive change, which is then relaxed to a
shared lock before we loop back, we have already analyzed the loop body
with the stronger exclusive lock and thus might have false positives.
There is a minor non-observable change: we modify the exit lock set of a
function, but since that isn't used further it doesn't change anything.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D106713
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 5b2c882c4235..41a55f9579bd 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1050,7 +1050,7 @@ class ThreadSafetyAnalyzer {
const CFGBlock* PredBlock,
const CFGBlock *CurrBlock);
- bool join(const FactEntry , const FactEntry );
+ bool join(const FactEntry , const FactEntry , bool CanModify);
void intersectAndWarn(FactSet , const FactSet ,
SourceLocation JoinLoc, LockErrorKind EntryLEK,
@@ -2188,25 +2188,28 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
}
}
-/// Given two facts merging on a join point, decide whether to warn and which
-/// one to keep.
+/// Given two facts merging on a join point, possibly warn and decide whether
to
+/// keep or replace.
///
-/// \return false if we should keep \p A, true if we should keep \p B.
-bool ThreadSafetyAnalyzer::join(const FactEntry , const FactEntry ) {
+/// \param CanModify Whether we can replace \p A by \p B.
+/// \return false if we should keep \p A, true if we should take \p B.
+bool ThreadSafetyAnalyzer::join(const FactEntry , const FactEntry ,
+bool CanModify) {
if (A.kind() != B.kind()) {
// For managed capabilities, the destructor should unlock in the right mode
// anyway. For asserted capabilities no unlocking is needed.
if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) {
- // The shared capability subsumes the exclusive capability.
- return B.kind() == LK_Shared;
-} else {
- Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(),
A.loc());
- // Take the exclusive capability to reduce further warnings.
- return B.kind() == LK_Exclusive;
+ // The shared capability subsumes the exclusive capability, if possible.
+ bool ShouldTakeB = B.kind() == LK_Shared;
+ if (CanModify || !ShouldTakeB)
+return ShouldTakeB;
}
+Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
+// Take the exclusive capability to reduce further warnings.
+return CanModify && B.kind() == LK_Exclusive;
} else {
// The non-asserted capability is the one we want to track.
-return A.asserted() && !B.asserted();
+return CanModify && A.asserted() && !B.asserted();
}
}
@@ -2237,8 +2240,8 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
FactSet::iterator EntryIt = EntrySet.findLockIter(FactMan, ExitFact);
if (EntryIt != EntrySet.end()) {
- if (join(FactMan[*EntryIt], ExitFact) &&
- EntryLEK == LEK_LockedSomePredecessors)
+ if (join(FactMan[*EntryIt], ExitFact,
+ EntryLEK != LEK_LockedSomeLoopIterations))
*EntryIt = Fact;
} else if (!ExitFact.managed()) {
ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index e9d41da80517..125a1958c8ba 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2789,6 +2789,25 @@ void loopRelease() {
}
}
+void loopPromote() {
+ RelockableMutexLock scope(, SharedTraits{});
+ for (unsigned i = 1; i < 10; ++i) {
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+if (i == 5)
+ scope.PromoteShared();
+ }
+}
+
+void loopDemote() {
+ RelockableMutexLock scope(, ExclusiveTraits{}); // expected-note {{the
other acquisition of mutex 'mu' is here}}
+ // We have to warn on this join point despite the lock being managed ...
+ for (unsigned i = 1; i < 10; ++i) {
[clang] 0e64a52 - Thread safety analysis: Mock getter for private mutexes can be undefined
Author: Aaron Puchert
Date: 2021-07-23T14:46:02+02:00
New Revision: 0e64a525c12a0822683d3bdc51b6294b5265f860
URL:
https://github.com/llvm/llvm-project/commit/0e64a525c12a0822683d3bdc51b6294b5265f860
DIFF:
https://github.com/llvm/llvm-project/commit/0e64a525c12a0822683d3bdc51b6294b5265f860.diff
LOG: Thread safety analysis: Mock getter for private mutexes can be undefined
Usage in an annotation is no odr-use, so I think there needs to be no
definition. Upside is that in practice one will get linker errors if it
is actually odr-used instead of calling a function that returns 0.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D106375
Added:
Modified:
clang/docs/ThreadSafetyAnalysis.rst
Removed:
diff --git a/clang/docs/ThreadSafetyAnalysis.rst
b/clang/docs/ThreadSafetyAnalysis.rst
index 651229f01d03..69046ba18b8c 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -640,8 +640,8 @@ mutex. For example:
Mutex mu;
public:
-// For thread safety analysis only. Does not actually return mu.
-Mutex* getMu() RETURN_CAPABILITY(mu) { return 0; }
+// For thread safety analysis only. Does not need to be defined.
+Mutex* getMu() RETURN_CAPABILITY(mu);
void doSomething() REQUIRES(mu);
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e0b9077 - Thread safety analysis: Rename parameters of ThreadSafetyAnalyzer::intersectAndWarn (NFC)
Author: Aaron Puchert
Date: 2021-06-29T23:56:52+02:00
New Revision: e0b90771c318625e51c34c67db3f3dfbbb686df8
URL:
https://github.com/llvm/llvm-project/commit/e0b90771c318625e51c34c67db3f3dfbbb686df8
DIFF:
https://github.com/llvm/llvm-project/commit/e0b90771c318625e51c34c67db3f3dfbbb686df8.diff
LOG: Thread safety analysis: Rename parameters of
ThreadSafetyAnalyzer::intersectAndWarn (NFC)
In D104261 we made the parameters' meaning slightly more specific, this
changes their names accordingly. In all uses we're building a new lock
set by intersecting existing locksets. The first (modifiable) argument
is the new lock set being built, the second (non-modifiable) argument is
the exit set of a preceding block.
Reviewed By: aaron.ballman, delesley
Differential Revision: https://reviews.llvm.org/D104649
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index b09de2bd71f2..5b2c882c4235 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1052,13 +1052,13 @@ class ThreadSafetyAnalyzer {
bool join(const FactEntry , const FactEntry );
- void intersectAndWarn(FactSet , const FactSet ,
-SourceLocation JoinLoc, LockErrorKind LEK1,
-LockErrorKind LEK2);
+ void intersectAndWarn(FactSet , const FactSet ,
+SourceLocation JoinLoc, LockErrorKind EntryLEK,
+LockErrorKind ExitLEK);
- void intersectAndWarn(FactSet , const FactSet ,
-SourceLocation JoinLoc, LockErrorKind LEK1) {
-intersectAndWarn(FSet1, FSet2, JoinLoc, LEK1, LEK1);
+ void intersectAndWarn(FactSet , const FactSet ,
+SourceLocation JoinLoc, LockErrorKind LEK) {
+intersectAndWarn(EntrySet, ExitSet, JoinLoc, LEK, LEK);
}
void runAnalysis(AnalysisDeclContext );
@@ -2219,43 +2219,44 @@ bool ThreadSafetyAnalyzer::join(const FactEntry ,
const FactEntry ) {
/// are the same. In the event of a
diff erence, we use the intersection of these
/// two locksets at the start of D.
///
-/// \param FSet1 The first lockset.
-/// \param FSet2 The second lockset.
+/// \param EntrySet A lockset for entry into a (possibly new) block.
+/// \param ExitSet The lockset on exiting a preceding block.
/// \param JoinLoc The location of the join point for error reporting
-/// \param LEK1 The error message to report if a mutex is missing from LSet1
-/// \param LEK2 The error message to report if a mutex is missing from Lset2
-void ThreadSafetyAnalyzer::intersectAndWarn(FactSet ,
-const FactSet ,
+/// \param EntryLEK The warning if a mutex is missing from \p EntrySet.
+/// \param ExitLEK The warning if a mutex is missing from \p ExitSet.
+void ThreadSafetyAnalyzer::intersectAndWarn(FactSet ,
+const FactSet ,
SourceLocation JoinLoc,
-LockErrorKind LEK1,
-LockErrorKind LEK2) {
- FactSet FSet1Orig = FSet1;
-
- // Find locks in FSet2 that conflict or are not in FSet1, and warn.
- for (const auto : FSet2) {
-const FactEntry = FactMan[Fact];
-
-FactSet::iterator Iter1 = FSet1.findLockIter(FactMan, LDat2);
-if (Iter1 != FSet1.end()) {
- if (join(FactMan[*Iter1], LDat2) && LEK1 == LEK_LockedSomePredecessors)
-*Iter1 = Fact;
-} else if (!LDat2.managed()) {
- LDat2.handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
- Handler);
+LockErrorKind EntryLEK,
+LockErrorKind ExitLEK) {
+ FactSet EntrySetOrig = EntrySet;
+
+ // Find locks in ExitSet that conflict or are not in EntrySet, and warn.
+ for (const auto : ExitSet) {
+const FactEntry = FactMan[Fact];
+
+FactSet::iterator EntryIt = EntrySet.findLockIter(FactMan, ExitFact);
+if (EntryIt != EntrySet.end()) {
+ if (join(FactMan[*EntryIt], ExitFact) &&
+ EntryLEK == LEK_LockedSomePredecessors)
+*EntryIt = Fact;
+} else if (!ExitFact.managed()) {
+ ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,
+ EntryLEK, Handler);
}
}
- // Find locks in FSet1 that are not in FSet2, and remove them.
- for (const auto : FSet1Orig) {
-const FactEntry *LDat1 = [Fact];
-const FactEntry *LDat2 = FSet2.findLock(FactMan, *LDat1);
+ // Find locks in EntrySet that are not in ExitSet, and remove them.
+ for (const auto : EntrySetOrig) {
+const FactEntry *EntryFact = [Fact];
+const FactEntry *ExitFact = ExitSet.findLock(FactMan,
[clang] f664e2e - Thread safety analysis: Always warn when dropping locks on back edges
Author: Aaron Puchert
Date: 2021-06-29T23:56:52+02:00
New Revision: f664e2ec371f61b69e11147d7f9e045083335917
URL:
https://github.com/llvm/llvm-project/commit/f664e2ec371f61b69e11147d7f9e045083335917
DIFF:
https://github.com/llvm/llvm-project/commit/f664e2ec371f61b69e11147d7f9e045083335917.diff
LOG: Thread safety analysis: Always warn when dropping locks on back edges
We allow branches to join where one holds a managed lock but the other
doesn't, but we can't do so for back edges: because there we can't drop
them from the lockset, as we have already analyzed the loop with the
larger lockset. So we can't allow dropping managed locks on back edges.
We move the managed() check from handleRemovalFromIntersection up to
intersectAndWarn, where we additionally check if we're on a back edge if
we're removing from the first lock set (the entry set of the next block)
but not if we're removing from the second lock set (the exit set of the
previous block). Now that the order of arguments matters, I had to swap
them in one invocation, which also causes some minor differences in the
tests.
Reviewed By: delesley
Differential Revision: https://reviews.llvm.org/D104261
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 3eb1b640e7290..b09de2bd71f24 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -865,7 +865,7 @@ class LockableFactEntry : public FactEntry {
handleRemovalFromIntersection(const FactSet , FactManager ,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler ) const override {
-if (!managed() && !asserted() && !negative() && !isUniversal()) {
+if (!asserted() && !negative() && !isUniversal()) {
Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
LEK);
}
@@ -2239,7 +2239,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
if (Iter1 != FSet1.end()) {
if (join(FactMan[*Iter1], LDat2) && LEK1 == LEK_LockedSomePredecessors)
*Iter1 = Fact;
-} else {
+} else if (!LDat2.managed()) {
LDat2.handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
Handler);
}
@@ -2251,8 +2251,9 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
const FactEntry *LDat2 = FSet2.findLock(FactMan, *LDat1);
if (!LDat2) {
- LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
- Handler);
+ if (!LDat1->managed() || LEK2 == LEK_LockedSomeLoopIterations)
+LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
+ Handler);
if (LEK2 == LEK_LockedSomePredecessors)
FSet1.removeLock(FactMan, *LDat1);
}
@@ -2528,7 +2529,7 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
CFGBlock *FirstLoopBlock = *SI;
CFGBlockInfo *PreLoop = [FirstLoopBlock->getBlockID()];
CFGBlockInfo *LoopEnd = [CurrBlockID];
- intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet, PreLoop->EntryLoc,
+ intersectAndWarn(PreLoop->EntrySet, LoopEnd->ExitSet, PreLoop->EntryLoc,
LEK_LockedSomeLoopIterations);
}
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 8e8bb6f45dde4..e9d41da80517c 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -636,11 +636,11 @@ void shared_fun_0() {
void shared_fun_1() {
sls_mu.ReaderLock(); // \
-// expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in
the same scope}}
+// expected-note {{the other acquisition of mutex 'sls_mu' is here}}
do {
sls_mu.Unlock();
sls_mu.Lock(); // \
- // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
+ // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared
in the same scope}}
} while (getBool());
sls_mu.Unlock();
}
@@ -695,11 +695,11 @@ void shared_fun_11() {
void shared_bad_0() {
sls_mu.Lock(); // \
-// expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in
the same scope}}
+// expected-note {{the other acquisition of mutex 'sls_mu' is here}}
do {
sls_mu.Unlock();
sls_mu.ReaderLock(); // \
- // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
+ // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared
in the same scope}}
} while (getBool());
sls_mu.Unlock();
}
@@ -2773,6
[clang] cf0b337 - Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities
Author: Aaron Puchert
Date: 2021-05-27T17:46:04+02:00
New Revision: cf0b337c1b1f064c81fe40124ddba178572778d6
URL:
https://github.com/llvm/llvm-project/commit/cf0b337c1b1f064c81fe40124ddba178572778d6
DIFF:
https://github.com/llvm/llvm-project/commit/cf0b337c1b1f064c81fe40124ddba178572778d6.diff
LOG: Thread safety analysis: Allow exlusive/shared joins for managed and
asserted capabilities
Similar to how we allow managed and asserted locks to be held and not
held in joining branches, we also allow them to be held shared and
exclusive. The scoped lock should restore the original state at the end
of the scope in any event, and asserted locks need not be released.
We should probably only allow asserted locks to be subsumed by managed,
not by (directly) acquired locks, but that's for another change.
Reviewed By: delesley
Differential Revision: https://reviews.llvm.org/D102026
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index c65e9f307f114..6727e1315b5f5 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2194,9 +2194,16 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
/// \return false if we should keep \p A, true if we should keep \p B.
bool ThreadSafetyAnalyzer::join(const FactEntry , const FactEntry ) {
if (A.kind() != B.kind()) {
-Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
-// Take the exclusive capability to reduce further warnings.
-return B.kind() == LK_Exclusive;
+// For managed capabilities, the destructor should unlock in the right mode
+// anyway. For asserted capabilities no unlocking is needed.
+if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) {
+ // The shared capability subsumes the exclusive capability.
+ return B.kind() == LK_Shared;
+} else {
+ Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(),
A.loc());
+ // Take the exclusive capability to reduce further warnings.
+ return B.kind() == LK_Exclusive;
+}
} else {
// The non-asserted capability is the one we want to track.
return A.asserted() && !B.asserted();
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 369952eb397a2..8e8bb6f45dde4 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2773,6 +2773,67 @@ void unlockJoin() {
x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
}
+void exclusiveSharedJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.Lock();
+ else
+scope.ReaderLock();
+ // No warning on join point because the lock will be released by the scope
object anyway.
+ print(x);
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
+void sharedExclusiveJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.ReaderLock();
+ else
+scope.Lock();
+ // No warning on join point because the lock will be released by the scope
object anyway.
+ print(x);
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
+void assertJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.Lock();
+ else
+mu.AssertHeld();
+ x = 2;
+}
+
+void assertSharedJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.ReaderLock();
+ else
+mu.AssertReaderHeld();
+ print(x);
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
+void assertStrongerJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.ReaderLock();
+ else
+mu.AssertHeld();
+ print(x);
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
+void assertWeakerJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.Lock();
+ else
+mu.AssertReaderHeld();
+ print(x);
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
void directUnlock() {
RelockableExclusiveMutexLock scope();
mu.Unlock();
@@ -4506,8 +4567,8 @@ class Foo {
void test6() {
mu_.AssertHeld();
-mu_.Unlock();
- } // should this be a warning?
+mu_.Unlock(); // should this be a warning?
+ }
void test7() {
if (c) {
@@ -4528,9 +4589,10 @@ class Foo {
else {
mu_.AssertHeld();
}
+// FIXME: should warn, because it's unclear whether we need to release or
not.
int b = a;
a = 0;
-mu_.Unlock();
+mu_.Unlock(); // should this be a
[clang] 3d64677 - Thread safety analysis: Factor out function for merging locks (NFC)
Author: Aaron Puchert
Date: 2021-05-27T17:44:48+02:00
New Revision: 3d64677c28072867ea6025a22805977386b767f8
URL:
https://github.com/llvm/llvm-project/commit/3d64677c28072867ea6025a22805977386b767f8
DIFF:
https://github.com/llvm/llvm-project/commit/3d64677c28072867ea6025a22805977386b767f8.diff
LOG: Thread safety analysis: Factor out function for merging locks (NFC)
It's going to become a bit more complicated, so let's have it separate.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D102025
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 83410ebc2cae3..c65e9f307f114 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1050,6 +1050,8 @@ class ThreadSafetyAnalyzer {
const CFGBlock* PredBlock,
const CFGBlock *CurrBlock);
+ bool join(const FactEntry , const FactEntry );
+
void intersectAndWarn(FactSet , const FactSet ,
SourceLocation JoinLoc, LockErrorKind LEK1,
LockErrorKind LEK2);
@@ -2186,6 +2188,21 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
}
}
+/// Given two facts merging on a join point, decide whether to warn and which
+/// one to keep.
+///
+/// \return false if we should keep \p A, true if we should keep \p B.
+bool ThreadSafetyAnalyzer::join(const FactEntry , const FactEntry ) {
+ if (A.kind() != B.kind()) {
+Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
+// Take the exclusive capability to reduce further warnings.
+return B.kind() == LK_Exclusive;
+ } else {
+// The non-asserted capability is the one we want to track.
+return A.asserted() && !B.asserted();
+ }
+}
+
/// Compute the intersection of two locksets and issue warnings for any
/// locks in the symmetric
diff erence.
///
@@ -2213,20 +2230,8 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
FactSet::iterator Iter1 = FSet1.findLockIter(FactMan, LDat2);
if (Iter1 != FSet1.end()) {
- const FactEntry = FactMan[*Iter1];
- if (LDat1.kind() != LDat2.kind()) {
-Handler.handleExclusiveAndShared("mutex", LDat2.toString(),
LDat2.loc(),
- LDat1.loc());
-if (LEK1 == LEK_LockedSomePredecessors &&
-LDat1.kind() != LK_Exclusive) {
- // Take the exclusive lock, which is the one in FSet2.
- *Iter1 = Fact;
-}
- } else if (LEK1 == LEK_LockedSomePredecessors && LDat1.asserted() &&
- !LDat2.asserted()) {
-// The non-asserted lock in FSet2 is the one we want to track.
+ if (join(FactMan[*Iter1], LDat2) && LEK1 == LEK_LockedSomePredecessors)
*Iter1 = Fact;
- }
} else {
LDat2.handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
Handler);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a5c2ec9 - [AST] Store regular ValueDecl* in BindingDecl (NFC)
Author: Aaron Puchert
Date: 2021-05-20T16:28:58+02:00
New Revision: a5c2ec96e5f9f14b31b705e40bcb267257612316
URL:
https://github.com/llvm/llvm-project/commit/a5c2ec96e5f9f14b31b705e40bcb267257612316
DIFF:
https://github.com/llvm/llvm-project/commit/a5c2ec96e5f9f14b31b705e40bcb267257612316.diff
LOG: [AST] Store regular ValueDecl* in BindingDecl (NFC)
We were always storing a regular ValueDecl* as decomposition declaration
and haven't been using the opportunity to initialize it lazily.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D99455
Added:
Modified:
clang/include/clang/AST/DeclCXX.h
clang/lib/AST/DeclCXX.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclCXX.h
b/clang/include/clang/AST/DeclCXX.h
index c3c326b50397..07c4eb261aac 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -3827,7 +3827,7 @@ class StaticAssertDecl : public Decl {
/// DecompositionDecl of type 'int (&)[3]'.
class BindingDecl : public ValueDecl {
/// The declaration that this binding binds to part of.
- LazyDeclPtr Decomp;
+ ValueDecl *Decomp;
/// The binding represented by this declaration. References to this
/// declaration are effectively equivalent to this expression (except
/// that it is only evaluated once at the point of declaration of the
@@ -3853,7 +3853,7 @@ class BindingDecl : public ValueDecl {
/// Get the decomposition declaration that this binding represents a
/// decomposition of.
- ValueDecl *getDecomposedDecl() const;
+ ValueDecl *getDecomposedDecl() const { return Decomp; }
/// Get the variable (if any) that holds the value of evaluating the binding.
/// Only present for user-defined bindings for tuple-like types.
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 3d1faee0ab11..2bb7acc21a93 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3179,12 +3179,6 @@ BindingDecl *BindingDecl::CreateDeserialized(ASTContext
, unsigned ID) {
return new (C, ID) BindingDecl(nullptr, SourceLocation(), nullptr);
}
-ValueDecl *BindingDecl::getDecomposedDecl() const {
- ExternalASTSource *Source =
- Decomp.isOffset() ? getASTContext().getExternalSource() : nullptr;
- return cast_or_null(Decomp.get(Source));
-}
-
VarDecl *BindingDecl::getHoldingVar() const {
Expr *B = getBinding();
if (!B)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d21e1b7 - Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)
Author: Aaron Puchert
Date: 2021-05-06T23:07:42+02:00
New Revision: d21e1b79ff7d40bca537c30da706e31e48483f21
URL:
https://github.com/llvm/llvm-project/commit/d21e1b79ff7d40bca537c30da706e31e48483f21
DIFF:
https://github.com/llvm/llvm-project/commit/d21e1b79ff7d40bca537c30da706e31e48483f21.diff
LOG: Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)
We were modifying precisely when intersecting the lock sets of multiple
predecessors without back edge. That's no coincidence: we can't modify
on back edges, it doesn't make sense to modify at the end of a function,
and otherwise we always want to intersect on forward edges, because we
can build a new lock set for those.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101755
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 4377fc58a1d55..83410ebc2cae3 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1051,14 +1051,12 @@ class ThreadSafetyAnalyzer {
const CFGBlock *CurrBlock);
void intersectAndWarn(FactSet , const FactSet ,
-SourceLocation JoinLoc,
-LockErrorKind LEK1, LockErrorKind LEK2,
-bool Modify=true);
+SourceLocation JoinLoc, LockErrorKind LEK1,
+LockErrorKind LEK2);
void intersectAndWarn(FactSet , const FactSet ,
-SourceLocation JoinLoc, LockErrorKind LEK1,
-bool Modify=true) {
-intersectAndWarn(FSet1, FSet2, JoinLoc, LEK1, LEK1, Modify);
+SourceLocation JoinLoc, LockErrorKind LEK1) {
+intersectAndWarn(FSet1, FSet2, JoinLoc, LEK1, LEK1);
}
void runAnalysis(AnalysisDeclContext );
@@ -2206,8 +2204,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
const FactSet ,
SourceLocation JoinLoc,
LockErrorKind LEK1,
-LockErrorKind LEK2,
-bool Modify) {
+LockErrorKind LEK2) {
FactSet FSet1Orig = FSet1;
// Find locks in FSet2 that conflict or are not in FSet1, and warn.
@@ -2220,11 +2217,13 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
if (LDat1.kind() != LDat2.kind()) {
Handler.handleExclusiveAndShared("mutex", LDat2.toString(),
LDat2.loc(),
LDat1.loc());
-if (Modify && LDat1.kind() != LK_Exclusive) {
+if (LEK1 == LEK_LockedSomePredecessors &&
+LDat1.kind() != LK_Exclusive) {
// Take the exclusive lock, which is the one in FSet2.
*Iter1 = Fact;
}
- } else if (Modify && LDat1.asserted() && !LDat2.asserted()) {
+ } else if (LEK1 == LEK_LockedSomePredecessors && LDat1.asserted() &&
+ !LDat2.asserted()) {
// The non-asserted lock in FSet2 is the one we want to track.
*Iter1 = Fact;
}
@@ -2242,7 +2241,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
if (!LDat2) {
LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
Handler);
- if (Modify)
+ if (LEK2 == LEK_LockedSomePredecessors)
FSet1.removeLock(FactMan, *LDat1);
}
}
@@ -2469,8 +2468,7 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
// Do not update EntrySet.
intersectAndWarn(
CurrBlockInfo->EntrySet, PrevLockset, PrevBlockInfo->ExitLoc,
-IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors,
-!IsLoop);
+IsLoop ? LEK_LockedSomeLoopIterations :
LEK_LockedSomePredecessors);
}
}
@@ -2518,10 +2516,8 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
CFGBlock *FirstLoopBlock = *SI;
CFGBlockInfo *PreLoop = [FirstLoopBlock->getBlockID()];
CFGBlockInfo *LoopEnd = [CurrBlockID];
- intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet,
- PreLoop->EntryLoc,
- LEK_LockedSomeLoopIterations,
- false);
+ intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet, PreLoop->EntryLoc,
+ LEK_LockedSomeLoopIterations);
}
}
@@ -2549,11 +2545,8 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
ExpectedExitSet.removeLock(FactMan, Lock);
// FIXME: Should we call this function for all blocks which exit the
function?
- intersectAndWarn(ExpectedExitSet,
[clang] daca6ed - Thread safety analysis: Fix false negative on break
Author: Aaron Puchert
Date: 2021-05-03T14:03:17+02:00
New Revision: daca6edb31efae048a420595fae9750aaa91c733
URL:
https://github.com/llvm/llvm-project/commit/daca6edb31efae048a420595fae9750aaa91c733
DIFF:
https://github.com/llvm/llvm-project/commit/daca6edb31efae048a420595fae9750aaa91c733.diff
LOG: Thread safety analysis: Fix false negative on break
We weren't modifying the lock set when intersecting with one coming
from a break-terminated block. This is inconsistent, since break isn't a
back edge, and it leads to false negatives with scoped locks. We usually
don't warn for those when joining locksets aren't the same, we just
silently remove locks that are not in the intersection. But not warning
and not removing them isn't right.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101202
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/PCH/thread-safety-attrs.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index bb3a4f1616fca..4377fc58a1d55 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2467,11 +2467,10 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext ) {
PrevBlock, CurrBlock);
// Do not update EntrySet.
-intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset,
- PrevBlockInfo->ExitLoc,
- IsLoop ? LEK_LockedSomeLoopIterations
-: LEK_LockedSomePredecessors,
- false);
+intersectAndWarn(
+CurrBlockInfo->EntrySet, PrevLockset, PrevBlockInfo->ExitLoc,
+IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors,
+!IsLoop);
}
}
diff --git a/clang/test/PCH/thread-safety-attrs.cpp
b/clang/test/PCH/thread-safety-attrs.cpp
index ae2a413af31c8..3e0c081f134f9 100644
--- a/clang/test/PCH/thread-safety-attrs.cpp
+++ b/clang/test/PCH/thread-safety-attrs.cpp
@@ -311,7 +311,8 @@ void sls_fun_bad_12() {
}
sls_mu.Lock();
}
- sls_mu.Unlock();
+ sls_mu.Unlock(); // \
+// expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
#endif
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index b837206138a67..369952eb397a2 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -337,7 +337,8 @@ void sls_fun_bad_12() {
}
sls_mu.Lock();
}
- sls_mu.Unlock();
+ sls_mu.Unlock(); // \
+// expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
//-//
@@ -2582,6 +2583,7 @@ class Foo {
void test3();
void test4();
void test5();
+ void test6();
};
@@ -2620,6 +2622,18 @@ void Foo::test5() {
rlock.Release(); // expected-warning {{releasing mutex 'mu_' that was not
held}}
}
+void Foo::test6() {
+ ReleasableMutexLock rlock(_);
+ do {
+if (c) {
+ rlock.Release();
+ break;
+}
+ } while (c);
+ // No warning on join point because the lock will be released by the scope
object anyway
+ a = 1; // expected-warning {{writing variable 'a' requires holding mutex
'mu_' exclusively}}
+}
+
} // end namespace ReleasableScopedLock
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 530e074 - Thread safety analysis: Replace flags in FactEntry by SourceKind (NFC)
Author: Aaron Puchert
Date: 2021-05-03T14:03:17+02:00
New Revision: 530e074faafe995a560e80815f5af8306670ea7b
URL:
https://github.com/llvm/llvm-project/commit/530e074faafe995a560e80815f5af8306670ea7b
DIFF:
https://github.com/llvm/llvm-project/commit/530e074faafe995a560e80815f5af8306670ea7b.diff
LOG: Thread safety analysis: Replace flags in FactEntry by SourceKind (NFC)
The motivation here is to make it available in the base class whether a
fact is managed or not. That would have meant three flags on the base
class, so I had a look whether we really have 8 possible combinations.
It turns out we don't: asserted and declared are obviously mutually
exclusive. Managed facts are only created when we acquire a capability
through a scoped capability. Adopting an asserted or declared lock will
not (in fact can not, because Facts are immutable) make them managed.
We probably don't want to allow adopting an asserted lock (because then
the function should probably have a release attribute, and then the
assertion is pointless), but we might at some point decide to replace a
declared fact on adoption.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D100801
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index cbb992f40c32b..bb3a4f1616fca 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -105,32 +105,37 @@ class FactSet;
///
/// FIXME: this analysis does not currently support re-entrant locking.
class FactEntry : public CapabilityExpr {
+public:
+ /// Where a fact comes from.
+ enum SourceKind {
+Acquired, ///< The fact has been directly acquired.
+Asserted, ///< The fact has been asserted to be held.
+Declared, ///< The fact is assumed to be held by callers.
+Managed, ///< The fact has been acquired through a scoped capability.
+ };
+
private:
/// Exclusive or shared.
- LockKind LKind;
+ LockKind LKind : 8;
+
+ // How it was acquired.
+ SourceKind Source : 8;
/// Where it was acquired.
SourceLocation AcquireLoc;
- /// True if the lock was asserted.
- bool Asserted;
-
- /// True if the lock was declared.
- bool Declared;
-
public:
FactEntry(const CapabilityExpr , LockKind LK, SourceLocation Loc,
-bool Asrt, bool Declrd = false)
- : CapabilityExpr(CE), LKind(LK), AcquireLoc(Loc), Asserted(Asrt),
-Declared(Declrd) {}
+SourceKind Src)
+ : CapabilityExpr(CE), LKind(LK), Source(Src), AcquireLoc(Loc) {}
virtual ~FactEntry() = default;
LockKind kind() const { return LKind; }
SourceLocation loc() const { return AcquireLoc; }
- bool asserted() const { return Asserted; }
- bool declared() const { return Declared; }
- void setDeclared(bool D) { Declared = D; }
+ bool asserted() const { return Source == Asserted; }
+ bool declared() const { return Source == Declared; }
+ bool managed() const { return Source == Managed; }
virtual void
handleRemovalFromIntersection(const FactSet , FactManager ,
@@ -851,20 +856,16 @@ static void findBlockLocations(CFG *CFGraph,
namespace {
class LockableFactEntry : public FactEntry {
-private:
- /// managed by ScopedLockable object
- bool Managed;
-
public:
LockableFactEntry(const CapabilityExpr , LockKind LK, SourceLocation Loc,
-bool Mng = false, bool Asrt = false)
- : FactEntry(CE, LK, Loc, Asrt), Managed(Mng) {}
+SourceKind Src = Acquired)
+ : FactEntry(CE, LK, Loc, Src) {}
void
handleRemovalFromIntersection(const FactSet , FactManager ,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler ) const override {
-if (!Managed && !asserted() && !negative() && !isUniversal()) {
+if (!managed() && !asserted() && !negative() && !isUniversal()) {
Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
LEK);
}
@@ -903,7 +904,7 @@ class ScopedLockableFactEntry : public FactEntry {
public:
ScopedLockableFactEntry(const CapabilityExpr , SourceLocation Loc)
- : FactEntry(CE, LK_Exclusive, Loc, false) {}
+ : FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
void addLock(const CapabilityExpr ) {
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
@@ -983,7 +984,7 @@ class ScopedLockableFactEntry : public FactEntry {
} else {
FSet.removeLock(FactMan, !Cp);
FSet.addLock(FactMan,
- std::make_unique(Cp, kind, loc, true));
+ std::make_unique(Cp, kind, loc,
Managed));
}
}
@@ -1854,10 +1855,11 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
CapExprSet AssertLocks;
[clang] 572fe08 - Thread safety analysis: Simplify intersectAndWarn (NFC)
Author: Aaron Puchert
Date: 2021-04-23T23:19:15+02:00
New Revision: 572fe087765626529d63eaaefb2e1c5ba277f756
URL:
https://github.com/llvm/llvm-project/commit/572fe087765626529d63eaaefb2e1c5ba277f756
DIFF:
https://github.com/llvm/llvm-project/commit/572fe087765626529d63eaaefb2e1c5ba277f756.diff
LOG: Thread safety analysis: Simplify intersectAndWarn (NFC)
Instead of conditionally overwriting a nullptr and then branching on its
nullness, just branch directly on the original condition. Then we can
make both pointers (non-null) references instead.
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 00678c7129a3..cbb992f40c32 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2207,27 +2207,25 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet
,
// Find locks in FSet2 that conflict or are not in FSet1, and warn.
for (const auto : FSet2) {
-const FactEntry *LDat1 = nullptr;
-const FactEntry *LDat2 = [Fact];
-FactSet::iterator Iter1 = FSet1.findLockIter(FactMan, *LDat2);
-if (Iter1 != FSet1.end()) LDat1 = [*Iter1];
-
-if (LDat1) {
- if (LDat1->kind() != LDat2->kind()) {
-Handler.handleExclusiveAndShared("mutex", LDat2->toString(),
- LDat2->loc(), LDat1->loc());
-if (Modify && LDat1->kind() != LK_Exclusive) {
+const FactEntry = FactMan[Fact];
+
+FactSet::iterator Iter1 = FSet1.findLockIter(FactMan, LDat2);
+if (Iter1 != FSet1.end()) {
+ const FactEntry = FactMan[*Iter1];
+ if (LDat1.kind() != LDat2.kind()) {
+Handler.handleExclusiveAndShared("mutex", LDat2.toString(),
LDat2.loc(),
+ LDat1.loc());
+if (Modify && LDat1.kind() != LK_Exclusive) {
// Take the exclusive lock, which is the one in FSet2.
*Iter1 = Fact;
}
- }
- else if (Modify && LDat1->asserted() && !LDat2->asserted()) {
+ } else if (Modify && LDat1.asserted() && !LDat2.asserted()) {
// The non-asserted lock in FSet2 is the one we want to track.
*Iter1 = Fact;
}
} else {
- LDat2->handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
- Handler);
+ LDat2.handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
+ Handler);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] dfec26b - Thread safety analysis: Don't warn about managed locks on join points
Author: Aaron Puchert
Date: 2021-04-06T22:29:48+02:00
New Revision: dfec26b186d2f0c80f2b70901b7cc5747f5b377c
URL:
https://github.com/llvm/llvm-project/commit/dfec26b186d2f0c80f2b70901b7cc5747f5b377c
DIFF:
https://github.com/llvm/llvm-project/commit/dfec26b186d2f0c80f2b70901b7cc5747f5b377c.diff
LOG: Thread safety analysis: Don't warn about managed locks on join points
We already did so for scoped locks acquired in the constructor, this
change extends the treatment to deferred locks and scoped unlocking, so
locks acquired outside of the constructor. Obviously this makes things
more consistent.
Originally I thought this was a bad idea, because obviously it
introduces false negatives when it comes to double locking, but these
are typically easily found in tests, and the primary goal of the Thread
safety analysis is not to find double locks but race conditions.
Since the scoped lock will release the mutex anyway when the scope ends,
the inconsistent state is just temporary and probably fine.
Reviewed By: delesley
Differential Revision: https://reviews.llvm.org/D98747
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 84e0e91f597fe..00678c7129a31 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -983,7 +983,7 @@ class ScopedLockableFactEntry : public FactEntry {
} else {
FSet.removeLock(FactMan, !Cp);
FSet.addLock(FactMan,
- std::make_unique(Cp, kind, loc));
+ std::make_unique(Cp, kind, loc, true));
}
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index d1520b1decbd3..b837206138a67 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2595,6 +2595,7 @@ void Foo::test2() {
if (c) {// test join point -- held/not held during release
rlock.Release();
}
+ // No warning on join point because the lock will be released by the scope
object anyway.
}
void Foo::test3() {
@@ -2615,7 +2616,7 @@ void Foo::test5() {
if (c) {
rlock.Release();
}
- // no warning on join point for managed lock.
+ // No warning on join point because the lock will be released by the scope
object anyway.
rlock.Release(); // expected-warning {{releasing mutex 'mu_' that was not
held}}
}
@@ -2659,6 +2660,7 @@ class SCOPED_LOCKABLE RelockableMutexLock {
Mutex mu;
int x GUARDED_BY(mu);
+bool b;
void print(int);
@@ -2740,6 +2742,23 @@ void doubleLock2() {
scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already
held}}
}
+void lockJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ if (b)
+scope.Lock();
+ // No warning on join point because the lock will be released by the scope
object anyway.
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
+void unlockJoin() {
+ RelockableMutexLock scope(, DeferTraits{});
+ scope.Lock();
+ if (b)
+scope.Unlock();
+ // No warning on join point because the lock will be released by the scope
object anyway.
+ x = 2; // expected-warning {{writing variable 'x' requires holding mutex
'mu' exclusively}}
+}
+
void directUnlock() {
RelockableExclusiveMutexLock scope();
mu.Unlock();
@@ -2871,10 +2890,9 @@ void manual() EXCLUSIVE_LOCKS_REQUIRED(mu) {
void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
MutexUnlock scope();
- if (c) {
-scope.Lock(); // expected-note{{mutex acquired here}}
- }
- // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+ if (c)
+scope.Lock();
+ // No warning on join point because the lock will be released by the scope
object anyway.
scope.Lock();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] c61ae6e - Deduplicate branches and adjust comment [NFC]
Author: Aaron Puchert
Date: 2021-03-27T23:08:43+01:00
New Revision: c61ae6e6d597984e6ff7d012dce4dfd59c05d792
URL:
https://github.com/llvm/llvm-project/commit/c61ae6e6d597984e6ff7d012dce4dfd59c05d792
DIFF:
https://github.com/llvm/llvm-project/commit/c61ae6e6d597984e6ff7d012dce4dfd59c05d792.diff
LOG: Deduplicate branches and adjust comment [NFC]
Currently we want to allow calling non-const methods even when only a
shared lock is held, because -Wthread-safety-reference is already quite
sensitive and not all code is const-correct. Even if it is, this might
require users to add std::as_const around the implicit object argument.
See D52395 for a discussion.
Fixes PR46963.
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 21583e92c72d..84e0e91f597f 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2051,15 +2051,11 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
if (ME && MD) {
if (ME->isArrow()) {
-if (MD->isConst())
- checkPtAccess(CE->getImplicitObjectArgument(), AK_Read);
-else // FIXME -- should be AK_Written
- checkPtAccess(CE->getImplicitObjectArgument(), AK_Read);
+// Should perhaps be AK_Written if !MD->isConst().
+checkPtAccess(CE->getImplicitObjectArgument(), AK_Read);
} else {
-if (MD->isConst())
- checkAccess(CE->getImplicitObjectArgument(), AK_Read);
-else // FIXME -- should be AK_Written
- checkAccess(CE->getImplicitObjectArgument(), AK_Read);
+// Should perhaps be AK_Written if !MD->isConst().
+checkAccess(CE->getImplicitObjectArgument(), AK_Read);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a6a1c30 - Fix false negative in -Wthread-safety-attributes
Author: Aaron Puchert
Date: 2021-03-24T17:45:25+01:00
New Revision: a6a1c3051dbd2cc5ccc70272890cf38d11dca9c7
URL:
https://github.com/llvm/llvm-project/commit/a6a1c3051dbd2cc5ccc70272890cf38d11dca9c7
DIFF:
https://github.com/llvm/llvm-project/commit/a6a1c3051dbd2cc5ccc70272890cf38d11dca9c7.diff
LOG: Fix false negative in -Wthread-safety-attributes
The original implementation didn't fire on non-template classes when a
base class was an instantiation of a template with a dependent base.
In that case the base of the base is dependent as seen from the base,
but not from the class we're interested in, which isn't a template.
Also it simplifies the code a lot.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98724
Added:
Modified:
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaCXX/warn-thread-safety-parsing.cpp
Removed:
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c4901042c042..b39460d33214 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -513,16 +513,9 @@ static bool checkRecordDeclForAttr(const RecordDecl *RD) {
// Else check if any base classes have the attribute.
if (const auto *CRD = dyn_cast(RD)) {
-CXXBasePaths BPaths(false, false);
-if (CRD->lookupInBases(
-[](const CXXBaseSpecifier *BS, CXXBasePath &) {
- const auto = *BS->getType();
- // If it's type-dependent, we assume it could have the attribute.
- if (Ty.isDependentType())
-return true;
- return Ty.castAs()->getDecl()->hasAttr();
-},
-BPaths, true))
+if (!CRD->forallBases([](const CXXRecordDecl *Base) {
+ return !Base->hasAttr();
+}))
return true;
}
return false;
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 6ad0f877a11d..b6e9c052a241 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -1295,6 +1295,11 @@ struct SLDerived2 : public SLTemplateClass {
// expected-warning{{'unlock_function' attribute without capability
arguments refers to 'this', but 'SLDerived2' isn't annotated with 'capability'
or 'scoped_lockable' attribute}}
};
+struct SLDerived3 : public SLTemplateDerived {
+ ~SLDerived3() UNLOCK_FUNCTION(); // \
+// expected-warning{{'unlock_function' attribute without capability
arguments refers to 'this', but 'SLDerived3' isn't annotated with 'capability'
or 'scoped_lockable' attribute}}
+};
+
//-
// Parsing of member variables and function parameters
//--
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1cb15b1 - Correct Doxygen syntax for inline code
Author: Aaron Puchert
Date: 2021-03-16T15:17:45+01:00
New Revision: 1cb15b10ea370178871769929ff9690f461191fc
URL:
https://github.com/llvm/llvm-project/commit/1cb15b10ea370178871769929ff9690f461191fc
DIFF:
https://github.com/llvm/llvm-project/commit/1cb15b10ea370178871769929ff9690f461191fc.diff
LOG: Correct Doxygen syntax for inline code
There is no syntax like {@code ...} in Doxygen, @code is a block command
that ends with @endcode, and generally these are not enclosed in braces.
The correct syntax for inline code snippets is @c .
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98665
Added:
Modified:
clang/include/clang/Analysis/AnyCall.h
clang/include/clang/Analysis/RetainSummaryManager.h
clang/lib/Analysis/RetainSummaryManager.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
llvm/include/llvm/Support/GraphWriter.h
Removed:
diff --git a/clang/include/clang/Analysis/AnyCall.h
b/clang/include/clang/Analysis/AnyCall.h
index 16371eb1da18..846ff7719ce1 100644
--- a/clang/include/clang/Analysis/AnyCall.h
+++ b/clang/include/clang/Analysis/AnyCall.h
@@ -107,8 +107,8 @@ class AnyCall {
}
- /// If {@code E} is a generic call (to ObjC method /function/block/etc),
- /// return a constructed {@code AnyCall} object. Return None otherwise.
+ /// If @c E is a generic call (to ObjC method /function/block/etc),
+ /// return a constructed @c AnyCall object. Return None otherwise.
static Optional forExpr(const Expr *E) {
if (const auto *ME = dyn_cast(E)) {
return AnyCall(ME);
@@ -127,8 +127,8 @@ class AnyCall {
}
}
- /// If {@code D} is a callable (Objective-C method or a function), return
- /// a constructed {@code AnyCall} object. Return None otherwise.
+ /// If @c D is a callable (Objective-C method or a function), return
+ /// a constructed @c AnyCall object. Return None otherwise.
// FIXME: block support.
static Optional forDecl(const Decl *D) {
if (const auto *FD = dyn_cast(D)) {
@@ -186,7 +186,7 @@ class AnyCall {
}
/// \returns Function identifier if it is a named declaration,
- /// {@code nullptr} otherwise.
+ /// @c nullptr otherwise.
const IdentifierInfo *getIdentifier() const {
if (const auto *ND = dyn_cast_or_null(D))
return ND->getIdentifier();
diff --git a/clang/include/clang/Analysis/RetainSummaryManager.h
b/clang/include/clang/Analysis/RetainSummaryManager.h
index 6acefb563d8c..b7ccb0317830 100644
--- a/clang/include/clang/Analysis/RetainSummaryManager.h
+++ b/clang/include/clang/Analysis/RetainSummaryManager.h
@@ -613,8 +613,8 @@ class RetainSummaryManager {
const FunctionType *FT,
bool );
- /// Apply the annotation of {@code pd} in function {@code FD}
- /// to the resulting summary stored in out-parameter {@code Template}.
+ /// Apply the annotation of @c pd in function @c FD
+ /// to the resulting summary stored in out-parameter @c Template.
/// \return whether an annotation was applied.
bool applyParamAnnotationEffect(const ParmVarDecl *pd, unsigned parm_idx,
const NamedDecl *FD,
@@ -715,8 +715,8 @@ class RetainSummaryManager {
/// Set argument types for arguments which are not doing anything.
void updateSummaryForArgumentTypes(const AnyCall , const RetainSummary
*);
- /// Determine whether a declaration {@code D} of correspondent type (return
- /// type for functions/methods) {@code QT} has any of the given attributes,
+ /// Determine whether a declaration @c D of correspondent type (return
+ /// type for functions/methods) @c QT has any of the given attributes,
/// provided they pass necessary validation checks AND tracking the given
/// attribute is enabled.
/// Returns the object kind corresponding to the present attribute, or None,
diff --git a/clang/lib/Analysis/RetainSummaryManager.cpp
b/clang/lib/Analysis/RetainSummaryManager.cpp
index 00bc854a8804..ecda47a67c1d 100644
--- a/clang/lib/Analysis/RetainSummaryManager.cpp
+++ b/clang/lib/Analysis/RetainSummaryManager.cpp
@@ -881,8 +881,8 @@ RetainSummaryManager::getRetEffectFromAnnotations(QualType
RetTy,
return None;
}
-/// \return Whether the chain of typedefs starting from {@code QT}
-/// has a typedef with a given name {@code Name}.
+/// \return Whether the chain of typedefs starting from @c QT
+/// has a typedef with a given name @c
[clang] 825f80e - [Sema] Introduce function reference conversion, NFC
Author: Aaron Puchert
Date: 2020-11-22T20:51:57+01:00
New Revision: 825f80e111f2815a009084f65267be3b5bf0897a
URL:
https://github.com/llvm/llvm-project/commit/825f80e111f2815a009084f65267be3b5bf0897a
DIFF:
https://github.com/llvm/llvm-project/commit/825f80e111f2815a009084f65267be3b5bf0897a.diff
LOG: [Sema] Introduce function reference conversion, NFC
Technically 'noexcept' isn't a qualifier, so this should be a separate
conversion.
Also make the test a pure frontend test.
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D67112
Added:
clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
Modified:
clang/include/clang/AST/OperationKinds.def
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaInit.cpp
Removed:
clang/test/CodeGenCXX/implicit-function-conversion.cpp
diff --git a/clang/include/clang/AST/OperationKinds.def
b/clang/include/clang/AST/OperationKinds.def
index 6daab1ffcb0a..7c82ab6e57ef 100644
--- a/clang/include/clang/AST/OperationKinds.def
+++ b/clang/include/clang/AST/OperationKinds.def
@@ -77,9 +77,10 @@ CAST_OPERATION(LValueToRValueBitCast)
CAST_OPERATION(LValueToRValue)
/// CK_NoOp - A conversion which does not affect the type other than
-/// (possibly) adding qualifiers.
+/// (possibly) adding qualifiers or removing noexcept.
/// int-> int
/// char** -> const char * const *
+/// void () noexcept -> void ()
CAST_OPERATION(NoOp)
/// CK_BaseToDerived - A conversion from a C++ class pointer/reference
diff --git a/clang/include/clang/Sema/Initialization.h
b/clang/include/clang/Sema/Initialization.h
index 6976e7c95c8b..2245c1505001 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -840,6 +840,9 @@ class InitializationSequence {
/// Perform a qualification conversion, producing an lvalue.
SK_QualificationConversionLValue,
+/// Perform a function reference conversion, see [dcl.init.ref]p4.
+SK_FunctionReferenceConversion,
+
/// Perform a conversion adding _Atomic to a type.
SK_AtomicConversion,
@@ -1288,6 +1291,10 @@ class InitializationSequence {
void AddQualificationConversionStep(QualType Ty,
ExprValueKind Category);
+ /// Add a new step that performs a function reference conversion to the
+ /// given type.
+ void AddFunctionReferenceConversionStep(QualType Ty);
+
/// Add a new step that performs conversion from non-atomic to atomic
/// type.
void AddAtomicConversionStep(QualType Ty);
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 5131ce446d04..6d2e6094e79c 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -3442,6 +3442,7 @@ void InitializationSequence::Step::Destroy() {
case SK_QualificationConversionRValue:
case SK_QualificationConversionXValue:
case SK_QualificationConversionLValue:
+ case SK_FunctionReferenceConversion:
case SK_AtomicConversion:
case SK_ListInitialization:
case SK_UnwrapInitList:
@@ -3620,6 +3621,13 @@ void
InitializationSequence::AddQualificationConversionStep(QualType Ty,
Steps.push_back(S);
}
+void InitializationSequence::AddFunctionReferenceConversionStep(QualType Ty) {
+ Step S;
+ S.Kind = SK_FunctionReferenceConversion;
+ S.Type = Ty;
+ Steps.push_back(S);
+}
+
void InitializationSequence::AddAtomicConversionStep(QualType Ty) {
Step S;
S.Kind = SK_AtomicConversion;
@@ -4653,7 +4661,7 @@ static OverloadingResult TryRefInitWithConversionFunction(
else if (RefConv & Sema::ReferenceConversions::ObjC)
Sequence.AddObjCObjectConversionStep(cv1T1);
else if (RefConv & Sema::ReferenceConversions::Function)
-Sequence.AddQualificationConversionStep(cv1T1, VK);
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
else if (RefConv & Sema::ReferenceConversions::Qualification) {
if (!S.Context.hasSameType(cv1T4, cv1T1))
Sequence.AddQualificationConversionStep(cv1T1, VK);
@@ -4755,12 +4763,12 @@ static void TryReferenceInitializationCore(Sema ,
Sequence.AddDerivedToBaseCastStep(cv1T1, VK_LValue);
else
Sequence.AddObjCObjectConversionStep(cv1T1);
- } else if (RefConv & (Sema::ReferenceConversions::Qualification |
-Sema::ReferenceConversions::Function)) {
+ } else if (RefConv & Sema::ReferenceConversions::Qualification) {
// Perform a (possibly multi-level) qualification conversion.
-// FIXME: Should we use a
diff erent step kind for function conversions?
Sequence.AddQualificationConversionStep(cv1T1,
Initializer->getValueKind());
+ } else if (RefConv & Sema::ReferenceConversions::Function) {
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
}
// We only create a temporary
[clang] 6f84779 - [Sema] Improve notes for value category mismatch in overloading
Author: Aaron Puchert
Date: 2020-11-15T18:05:11+01:00
New Revision: 6f84779674a9764c6adee29b9a48ed3b3f0d5132
URL:
https://github.com/llvm/llvm-project/commit/6f84779674a9764c6adee29b9a48ed3b3f0d5132
DIFF:
https://github.com/llvm/llvm-project/commit/6f84779674a9764c6adee29b9a48ed3b3f0d5132.diff
LOG: [Sema] Improve notes for value category mismatch in overloading
When an overloaded member function has a ref-qualifier, like:
class X {
void f() &&;
void f(int) &;
};
we would print strange notes when the ref-qualifier doesn't fit the value
category:
X x;
x.f();
X().f(0);
would both print a note "no known conversion from 'X' to 'X' for object
argument" on their relevant overload instead of pointing out the
mismatch in value category.
At first I thought the solution is easy: just use the FailureKind member
of the BadConversionSequence struct. But it turns out that we weren't
properly setting this for function arguments. So I went through
TryReferenceInit to make sure we're doing that right, and found a number
of notes in the existing tests that improved as well.
Fixes PR47791.
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D90123
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/dr14xx.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/drs/dr6xx.cpp
clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
clang/test/SemaCXX/overload-member-call.cpp
clang/test/SemaCXX/rval-references-examples.cpp
Removed:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 554d5943a63a..a256bb9b562b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4287,9 +4287,9 @@ def note_ovl_candidate_bad_arc_conv : Note<
"cannot implicitly convert argument "
"%
diff {of type $ to $|type to parameter type}3,4 for "
"%select{%ordinal6 argument|object argument}5 under ARC">;
-def note_ovl_candidate_bad_lvalue : Note<
+def note_ovl_candidate_bad_value_category : Note<
"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-"expects an l-value for "
+"expects an %select{l-value|r-value}5 for "
"%select{%ordinal4 argument|object argument}3">;
def note_ovl_candidate_bad_addrspace : Note<
"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 277ef5af9130..847f0dd977b7 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -4832,8 +4832,11 @@ TryReferenceInit(Sema , Expr *Init, QualType DeclType,
// -- Otherwise, the reference shall be an lvalue reference to a
//non-volatile const type (i.e., cv1 shall be const), or the
reference
//shall be an rvalue reference.
- if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified()))
+ if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified())) {
+if (InitCategory.isRValue() && RefRelationship != Sema::Ref_Incompatible)
+ ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
return ICS;
+ }
// -- If the initializer expression
//
@@ -4923,9 +4926,11 @@ TryReferenceInit(Sema , Expr *Init, QualType DeclType,
// If T1 is reference-related to T2 and the reference is an rvalue
// reference, the initializer expression shall not be an lvalue.
- if (RefRelationship >= Sema::Ref_Related &&
- isRValRef && Init->Classify(S.Context).isLValue())
+ if (RefRelationship >= Sema::Ref_Related && isRValRef &&
+ Init->Classify(S.Context).isLValue()) {
+ICS.setBad(BadConversionSequence::rvalue_ref_to_lvalue, Init, DeclType);
return ICS;
+ }
// C++ [over.ics.ref]p2:
// When a parameter of reference type is not bound directly to
@@ -4963,11 +4968,8 @@ TryReferenceInit(Sema , Expr *Init, QualType DeclType,
// binding an rvalue reference to an lvalue other than a function
// lvalue.
// Note that the function case is not possible here.
-if (DeclType->isRValueReferenceType() && LValRefType) {
- // FIXME: This is the wrong BadConversionSequence. The problem is binding
- // an rvalue reference to a (non-function) lvalue, not binding an lvalue
- // reference to an rvalue!
- ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
+if (isRValRef && LValRefType) {
+ ICS.setBad(BadConversionSequence::no_conversion, Init, DeclType);
return ICS;
}
@@ -10458,7 +10460,7 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
}
unsigned CVR = FromQs.getCVRQualifiers() & ~ToQs.getCVRQualifiers();
-assert(CVR && "unexpected qualifiers mismatch");
[clang] bbed8cf - Thread safety analysis: Consider static class members as inaccessible
Author: Aaron Puchert
Date: 2020-10-30T00:35:14+01:00
New Revision: bbed8cfe80cd27d3a47d877c7608d9be4e487d97
URL:
https://github.com/llvm/llvm-project/commit/bbed8cfe80cd27d3a47d877c7608d9be4e487d97
DIFF:
https://github.com/llvm/llvm-project/commit/bbed8cfe80cd27d3a47d877c7608d9be4e487d97.diff
LOG: Thread safety analysis: Consider static class members as inaccessible
This fixes the issue pointed out in D84604#2363134. For now we exclude
static members completely, we'll take them into account later.
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-negative.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 65d410a65ba4..21583e92c72d 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1269,10 +1269,16 @@ bool ThreadSafetyAnalyzer::inCurrentScope(const
CapabilityExpr ) {
const threadSafety::til::SExpr *SExp = CapE.sexpr();
assert(SExp && "Null expressions should be ignored");
- // Global variables are always in scope.
if (const auto *LP = dyn_cast(SExp)) {
const ValueDecl *VD = LP->clangDecl();
-return VD->isDefinedOutsideFunctionOrMethod();
+// Variables defined in a function are always inaccessible.
+if (!VD->isDefinedOutsideFunctionOrMethod())
+ return false;
+// For now we consider static class members to be inaccessible.
+if (isa(VD->getDeclContext()))
+ return false;
+// Global variables are always in scope.
+return true;
}
// Members are in scope from methods of the same class.
diff --git a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
index d7db5f402d11..9eabd67e4fc7 100644
--- a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -118,6 +118,33 @@ void testNamespaceGlobals()
EXCLUSIVE_LOCKS_REQUIRED(!globalMutex) {
ns::fq(); // expected-warning {{calling function 'fq' requires negative
capability '!globalMutex'}}
}
+class StaticMembers {
+public:
+ void pub() EXCLUSIVE_LOCKS_REQUIRED(!publicMutex);
+ void prot() EXCLUSIVE_LOCKS_REQUIRED(!protectedMutex);
+ void priv() EXCLUSIVE_LOCKS_REQUIRED(!privateMutex);
+ void test() {
+pub();
+prot();
+priv();
+ }
+
+ static Mutex publicMutex;
+
+protected:
+ static Mutex protectedMutex;
+
+private:
+ static Mutex privateMutex;
+};
+
+void testStaticMembers() {
+ StaticMembers x;
+ x.pub();
+ x.prot();
+ x.priv();
+}
+
} // end namespace ScopeTest
namespace DoubleAttribute {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ebfc427 - [Sema] Let getters assert that trailing return type exists, NFCI
Author: Aaron Puchert
Date: 2020-10-28T23:32:57+01:00
New Revision: ebfc427bbe08f0c36af9721d5a4e6d3ffe2e4bf5
URL:
https://github.com/llvm/llvm-project/commit/ebfc427bbe08f0c36af9721d5a4e6d3ffe2e4bf5
DIFF:
https://github.com/llvm/llvm-project/commit/ebfc427bbe08f0c36af9721d5a4e6d3ffe2e4bf5.diff
LOG: [Sema] Let getters assert that trailing return type exists, NFCI
This was requested in the review of D90129.
Added:
Modified:
clang/include/clang/Sema/DeclSpec.h
Removed:
diff --git a/clang/include/clang/Sema/DeclSpec.h
b/clang/include/clang/Sema/DeclSpec.h
index 33b57db9548f..abbefc9d285e 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1502,10 +1502,14 @@ struct DeclaratorChunk {
bool hasTrailingReturnType() const { return HasTrailingReturnType; }
/// Get the trailing-return-type for this function declarator.
-ParsedType getTrailingReturnType() const { return TrailingReturnType; }
+ParsedType getTrailingReturnType() const {
+ assert(HasTrailingReturnType);
+ return TrailingReturnType;
+}
/// Get the trailing-return-type location for this function declarator.
SourceLocation getTrailingReturnTypeLoc() const {
+ assert(HasTrailingReturnType);
return SourceLocation::getFromRawEncoding(TrailingReturnTypeLoc);
}
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5dbccc6 - Better source location for -Wignored-qualifiers on trailing return types
Author: Aaron Puchert
Date: 2020-10-28T23:32:57+01:00
New Revision: 5dbccc6c89c0f6c6dc6277cc304057f6d50b298d
URL:
https://github.com/llvm/llvm-project/commit/5dbccc6c89c0f6c6dc6277cc304057f6d50b298d
DIFF:
https://github.com/llvm/llvm-project/commit/5dbccc6c89c0f6c6dc6277cc304057f6d50b298d.diff
LOG: Better source location for -Wignored-qualifiers on trailing return types
We collect the source location of a trailing return type in the parser,
improving the location for regular functions and providing a location
for lambdas, where previously there was none.
Fixes PR47732.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D90129
Added:
Modified:
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/SemaType.cpp
clang/test/SemaCXX/return.cpp
Removed:
diff --git a/clang/include/clang/Sema/DeclSpec.h
b/clang/include/clang/Sema/DeclSpec.h
index 93a912609655..33b57db9548f 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1363,6 +1363,10 @@ struct DeclaratorChunk {
/// type specified.
UnionParsedType TrailingReturnType;
+/// If HasTrailingReturnType is true, this is the location of the trailing
+/// return type.
+unsigned TrailingReturnTypeLoc;
+
/// Reset the parameter list to having zero parameters.
///
/// This is used in various places for error recovery.
@@ -1499,6 +1503,11 @@ struct DeclaratorChunk {
/// Get the trailing-return-type for this function declarator.
ParsedType getTrailingReturnType() const { return TrailingReturnType; }
+
+/// Get the trailing-return-type location for this function declarator.
+SourceLocation getTrailingReturnTypeLoc() const {
+ return SourceLocation::getFromRawEncoding(TrailingReturnTypeLoc);
+}
};
struct BlockPointerTypeInfo {
@@ -1633,6 +1642,8 @@ struct DeclaratorChunk {
Declarator ,
TypeResult TrailingReturnType =
TypeResult(),
+ SourceLocation TrailingReturnTypeLoc =
+SourceLocation(),
DeclSpec *MethodQualifiers = nullptr);
/// Return a DeclaratorChunk for a block.
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 01a16575c239..281cd6d67c48 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -6429,6 +6429,7 @@ void Parser::ParseFunctionDeclarator(Declarator ,
CachedTokens *ExceptionSpecTokens = nullptr;
ParsedAttributesWithRange FnAttrs(AttrFactory);
TypeResult TrailingReturnType;
+ SourceLocation TrailingReturnTypeLoc;
/* LocalEndLoc is the end location for the local FunctionTypeLoc.
EndLoc is the end location for the function declarator.
@@ -6539,6 +6540,7 @@ void Parser::ParseFunctionDeclarator(Declarator ,
SourceRange Range;
TrailingReturnType =
ParseTrailingReturnType(Range, D.mayBeFollowedByCXXDirectInit());
+TrailingReturnTypeLoc = Range.getBegin();
EndLoc = Range.getEnd();
}
} else if (standardAttributesAllowed()) {
@@ -6571,7 +6573,8 @@ void Parser::ParseFunctionDeclarator(Declarator ,
DynamicExceptionRanges.data(), DynamicExceptions.size(),
NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr,
ExceptionSpecTokens, DeclsInPrototype, StartLoc,
-LocalEndLoc, D, TrailingReturnType, ),
+LocalEndLoc, D, TrailingReturnType, TrailingReturnTypeLoc,
+),
std::move(FnAttrs), EndLoc);
}
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp
b/clang/lib/Parse/ParseDeclCXX.cpp
index a903896f172c..6e9ecedd6fb6 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -3898,6 +3898,7 @@ void Parser::ParseTrailingRequiresClause(Declarator ) {
auto = D.getFunctionTypeInfo();
FunctionChunk.HasTrailingReturnType = TrailingReturnType.isUsable();
FunctionChunk.TrailingReturnType = TrailingReturnType.get();
+ FunctionChunk.TrailingReturnTypeLoc = Range.getBegin().getRawEncoding();
} else
SkipUntil({tok::equal, tok::l_brace, tok::arrow, tok::kw_try,
tok::comma},
StopAtSemi | StopBeforeMatch);
diff --git a/clang/lib/Parse/ParseExprCXX.cpp
b/clang/lib/Parse/ParseExprCXX.cpp
index b225bb7c8b36..8181d2afb43a 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1293,6 +1293,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
}
[clang] b296c64 - Thread safety analysis: Nullability improvements in TIL, NFCI
Author: Aaron Puchert
Date: 2020-10-25T19:37:16+01:00
New Revision: b296c64e64a0bc600538c8bc67d4ccc2564e3c72
URL:
https://github.com/llvm/llvm-project/commit/b296c64e64a0bc600538c8bc67d4ccc2564e3c72
DIFF:
https://github.com/llvm/llvm-project/commit/b296c64e64a0bc600538c8bc67d4ccc2564e3c72.diff
LOG: Thread safety analysis: Nullability improvements in TIL, NFCI
The constructor of Project asserts that the contained ValueDecl is not
null, use that in the ThreadSafetyAnalyzer. In the case of LiteralPtr
it's the other way around.
Also dyn_cast<> is sufficient if we know something isn't null.
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
index c26d2ed99dd2..77a800c28754 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
@@ -634,7 +634,9 @@ typename V::R_SExpr Literal::traverse(V , typename
V::R_Ctx Ctx) {
/// At compile time, pointer literals are represented by symbolic names.
class LiteralPtr : public SExpr {
public:
- LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) {}
+ LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) {
+assert(D && "ValueDecl must not be null");
+ }
LiteralPtr(const LiteralPtr &) = default;
static bool classof(const SExpr *E) { return E->opcode() == COP_LiteralPtr; }
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index ef90fa175a6d..65d410a65ba4 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1279,9 +1279,8 @@ bool ThreadSafetyAnalyzer::inCurrentScope(const
CapabilityExpr ) {
if (const auto *P = dyn_cast(SExp)) {
if (!CurrentMethod)
return false;
-const auto *VD = P->clangDecl();
-if (VD)
- return VD->getDeclContext() == CurrentMethod->getDeclContext();
+const ValueDecl *VD = P->clangDecl();
+return VD->getDeclContext() == CurrentMethod->getDeclContext();
}
return false;
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp
b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 1b8c55e56d47..0c5d1857cc2b 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -185,7 +185,7 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr
*AttrExp,
return CapabilityExpr(nullptr, false);
// Hack to deal with smart pointers -- strip off top-level pointer casts.
- if (const auto *CE = dyn_cast_or_null(E)) {
+ if (const auto *CE = dyn_cast(E)) {
if (CE->castOpcode() == til::CAST_objToPtr)
return CapabilityExpr(CE->expr(), Neg);
}
@@ -274,7 +274,7 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const
DeclRefExpr *DRE,
const auto *VD = cast(DRE->getDecl()->getCanonicalDecl());
// Function parameters require substitution and/or renaming.
- if (const auto *PV = dyn_cast_or_null(VD)) {
+ if (const auto *PV = dyn_cast(VD)) {
unsigned I = PV->getFunctionScopeIndex();
const DeclContext *D = PV->getDeclContext();
if (Ctx && Ctx->FunArgs) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5250a03 - Thread safety analysis: Consider global variables in scope
Author: Aaron Puchert
Date: 2020-10-25T19:32:26+01:00
New Revision: 5250a03a9959c2701a8338fe1a1ffa8bd93d6173
URL:
https://github.com/llvm/llvm-project/commit/5250a03a9959c2701a8338fe1a1ffa8bd93d6173
DIFF:
https://github.com/llvm/llvm-project/commit/5250a03a9959c2701a8338fe1a1ffa8bd93d6173.diff
LOG: Thread safety analysis: Consider global variables in scope
Instead of just mutex members we also consider mutex globals.
Unsurprisingly they are always in scope. Now the paper [1] says that
> The scope of a class member is assumed to be its enclosing class,
> while the scope of a global variable is the translation unit in
> which it is defined.
But I don't think we should limit this to TUs where a definition is
available - a declaration is enough to acquire the mutex, and if a mutex
is really limited in scope to a translation unit, it should probably be
only declared there.
The previous attempt in 9dcc82f34ea was causing false positives because
I wrongly assumed that LiteralPtrs were always globals, which they are
not. This should be fixed now.
[1]
https://static.googleusercontent.com/media/research.google.com/en/us/pubs/archive/42958.pdf
Fixes PR46354.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D84604
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
clang/test/SemaCXX/warn-thread-safety-negative.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 64e0da9e64b1..ef90fa175a6d 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1266,13 +1266,24 @@ ClassifyDiagnostic(const AttrTy *A) {
}
bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr ) {
- if (!CurrentMethod)
+ const threadSafety::til::SExpr *SExp = CapE.sexpr();
+ assert(SExp && "Null expressions should be ignored");
+
+ // Global variables are always in scope.
+ if (const auto *LP = dyn_cast(SExp)) {
+const ValueDecl *VD = LP->clangDecl();
+return VD->isDefinedOutsideFunctionOrMethod();
+ }
+
+ // Members are in scope from methods of the same class.
+ if (const auto *P = dyn_cast(SExp)) {
+if (!CurrentMethod)
return false;
- if (const auto *P = dyn_cast_or_null(CapE.sexpr())) {
const auto *VD = P->clangDecl();
if (VD)
return VD->getDeclContext() == CurrentMethod->getDeclContext();
}
+
return false;
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 91bd15def577..d1520b1decbd 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5036,7 +5036,8 @@ void spawn_fake_flight_control_thread(void) {
}
extern const char *deque_log_msg(void)
__attribute__((requires_capability(Logger)));
-void logger_entry(void) __attribute__((requires_capability(Logger))) {
+void logger_entry(void) __attribute__((requires_capability(Logger)))
+__attribute__((requires_capability(!FlightControl))) {
const char *msg;
while ((msg = deque_log_msg())) {
@@ -5044,13 +5045,13 @@ void logger_entry(void)
__attribute__((requires_capability(Logger))) {
}
}
-void spawn_fake_logger_thread(void) {
+void spawn_fake_logger_thread(void)
__attribute__((requires_capability(!FlightControl))) {
acquire(Logger);
logger_entry();
release(Logger);
}
-int main(void) {
+int main(void) __attribute__((requires_capability(!FlightControl))) {
spawn_fake_flight_control_thread();
spawn_fake_logger_thread();
diff --git a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
index 456fe16e6574..d7db5f402d11 100644
--- a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -21,6 +21,12 @@ class LOCKABLE Mutex {
void AssertReaderHeld() ASSERT_SHARED_LOCK();
};
+class SCOPED_LOCKABLE MutexLock {
+public:
+ MutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+ MutexLock(Mutex *mu, bool adopt) EXCLUSIVE_LOCKS_REQUIRED(mu);
+ ~MutexLock() UNLOCK_FUNCTION();
+};
namespace SimpleTest {
@@ -77,10 +83,43 @@ class Foo {
mu.Unlock();
baz(); // no warning -- !mu in set.
}
+
+ void test4() {
+MutexLock lock(); // expected-warning {{acquiring mutex 'mu' requires
negative capability '!mu'}}
+ }
};
} // end namespace SimpleTest
+Mutex globalMutex;
+
+namespace ScopeTest {
+
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+void fq() EXCLUSIVE_LOCKS_REQUIRED(!::globalMutex);
+
+namespace ns {
+ Mutex globalMutex;
+ void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+ void fq() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex);
+}
+
+void testGlobals() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex) {
+ f();
