[PATCH] D27854: [analyzer] Add check for mutex acquisition during interrupt context in Magenta kernel
khazem created this revision.
khazem added reviewers: dcoughlin, dergachev.a.
khazem added subscribers: cfe-commits, phosek, seanklein.
Herald added a subscriber: mgorny.
Acquiring a mutex during the Magenta kernel exception handler can cause
deadlocks and races. This patch adds a checker that ensures that no mutexes are
acquired during an interrupt context.
https://reviews.llvm.org/D27854
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MagentaInterruptContextChecker.cpp
test/Analysis/mutex-in-interrupt-context.cpp
Index: test/Analysis/mutex-in-interrupt-context.cpp
===
--- /dev/null
+++ test/Analysis/mutex-in-interrupt-context.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,optin.magenta.MutexInInterruptContext -std=c++11 -verify %s
+
+// Exit critical region
+void thread_preempt() {}
+void panic() {}
+void _panic() {}
+
+// Don't call these during critical region
+void mutex_acquire() {}
+void mutex_acquire_timeout() {}
+void mutex_acquire_timeout_internal() {}
+
+void call_ma() {
+ mutex_acquire(); // expected-warning {{Mutex acquired during interrupt context}}
+}
+
+void call_mat() {
+ mutex_acquire_timeout(); // expected-warning {{Mutex acquired during interrupt context}}
+}
+
+void call_mati() {
+ mutex_acquire_timeout_internal(); // expected-warning {{Mutex acquired during interrupt context}}
+}
+
+void call_ma_safe() {
+ mutex_acquire(); // no-warning
+}
+
+// The exception handler doesn't get called from C code. We start
+// exploring from the beginning of this function
+void x86_exception_handler(int foo) {
+ switch (foo) {
+case 0:
+ mutex_acquire(); // expected-warning {{Mutex acquired during interrupt context}}
+ break;
+
+case 1:
+ mutex_acquire_timeout(); // expected-warning {{Mutex acquired during interrupt context}}
+ break;
+
+case 2:
+ mutex_acquire_timeout_internal(); // expected-warning {{Mutex acquired during interrupt context}}
+ break;
+
+case 3:
+ call_ma();
+ break;
+
+case 4:
+ call_mat();
+ break;
+
+case 5:
+ call_mati();
+ break;
+
+case 6:
+ thread_preempt();
+ call_ma_safe();
+ break;
+
+case 7:
+ panic();
+ call_ma_safe();
+ break;
+
+case 8:
+ _panic();
+ call_ma_safe();
+ break;
+ }
+}
Index: lib/StaticAnalyzer/Checkers/MagentaInterruptContextChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/MagentaInterruptContextChecker.cpp
@@ -0,0 +1,148 @@
+//===-- MagentaInterruptContextChecker.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker checks for acquisitions of mutexes during an interrupt context
+// in the Magenta kernel, as such an acquisition can lead to race conditions and
+// deadlocks.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class MagentaInterruptContextChecker : public Checker {
+ std::unique_ptr MagentaInterruptContextBugType;
+ void reportMutexAcquisition(SymbolRef FileDescSym,
+const CallEvent &call,
+CheckerContext &C) const;
+
+ /// An interrupt context starts from this function
+ StringRef ExceptionHandler;
+
+ /// A list of functions that break out of an interrupt context
+ std::vector ExitFuns;
+
+ /// A list of functions that must not be called during an interrupt context
+ std::vector ProhibitedCalls;
+
+public:
+ MagentaInterruptContextChecker();
+
+ /// \brief Check if we've just started an interrupt context.
+ ///
+ /// We don't use PreCall for this because the interrupt context function is
+ /// never called from C code (thus there will never be a CallEvent for
+ /// x86_exception_handler). Therefore, we consider an interrupt context to
+ /// have started at the beginning of the body of ExceptionHandler, rather than
+ /// a call to that function.
+ void checkBeginFunction(CheckerContext &C) const;
+
+ /// \brief Check if we have exited from an interrupt context
+ ///
+ /// If the exception handler returns normally, then we're no longer in an
+ /// interrupt context and it it once again safe to call any of the
+ /// P
[PATCH] D26904: [astimporter] Support importing CXXDependentScopeMemberExpr and FunctionTemplateDecl
khazem updated this revision to Diff 79371.
khazem added a comment.
Thanks for the comments, Aleksei!
I've merged some aspects of the code you pointed me to, although I had to
change some of it because some of the function calls have changed since 2015.
In particular, I'm checking for already-imported functions, and adding the decl
to the imported map.
I'm not sure how to proceed with tests. I was looking at
test/ASTMerge/class-template-partial-spec.cpp, would it be something like this?
But I'm not sure what error I should throw if we have already imported the
function, there is no appropriate error in
include/clang/Basic/DiagnosticASTKinds.td. Should I define a new error, e.g.
"function %0 defined in multiple translation units"?
https://reviews.llvm.org/D26904
Files:
include/clang/ASTMatchers/ASTMatchers.h
lib/AST/ASTImporter.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -489,5 +489,54 @@
}
+TEST(ImportDecl, ImportFunctionTemplateDecl) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+testImport(
+ "template void declToImport() { };",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl()));
+}
+
+
+TEST(ImportExpr, ImportCXXDependentScopeMemberExpr) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+testImport(
+ "template class C { T t; };"
+ "template void declToImport() {"
+"C d;"
+"d.t = T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(
+has(
+ functionDecl(
+has(
+ compoundStmt(
+has(
+ binaryOperator(
+has(
+ cxxDependentScopeMemberExpr()));
+ EXPECT_TRUE(
+testImport(
+ "template class C { T t; };"
+ "template void declToImport() {"
+"C d;"
+"(&d)->t = T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(
+has(
+ functionDecl(
+has(
+ compoundStmt(
+has(
+ binaryOperator(
+has(
+ cxxDependentScopeMemberExpr()));
+}
+
+
} // end namespace ast_matchers
} // end namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -141,6 +141,7 @@
REGISTER_MATCHER(cxxCtorInitializer);
REGISTER_MATCHER(cxxDefaultArgExpr);
REGISTER_MATCHER(cxxDeleteExpr);
+ REGISTER_MATCHER(cxxDependentScopeMemberExpr);
REGISTER_MATCHER(cxxDestructorDecl);
REGISTER_MATCHER(cxxDynamicCastExpr);
REGISTER_MATCHER(cxxForRangeStmt);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -124,6 +124,9 @@
ImportDefinitionKind Kind = IDK_Default);
bool ImportDefinition(ObjCProtocolDecl *From, ObjCProtocolDecl *To,
ImportDefinitionKind Kind = IDK_Default);
+
+void ImportAttributes(Decl *From, Decl *To);
+
TemplateParameterList *ImportTemplateParameterList(
TemplateParameterList *Params);
TemplateArgument ImportTemplateArgument(const TemplateArgument &From);
@@ -138,6 +141,8 @@
bool Complain = true);
bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
+bool IsStructuralMatch(FunctionTemplateDecl *From,
+ FunctionTemplateDecl *To);
bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
Decl *VisitDecl(Decl *D);
@@ -160,6 +165,7 @@
Decl *VisitFieldDecl(FieldDecl *D);
Decl *VisitIndirectFieldDecl(IndirectFieldDecl *D);
Decl *VisitFriendDecl(FriendDecl *D);
+Decl *VisitFunctionTemplateDecl(FunctionTemplateDecl *D);
Decl *VisitObjCIvarDecl(ObjCIvarDecl *D);
Decl *VisitVarDecl(VarDecl *D);
Decl *VisitImplicitParamDecl(ImplicitParamDecl *D);
@@ -266,6 +272,7 @@
Expr *VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E);
Expr *VisitCXXNewExpr(CXXNewExpr *CE);
Expr *VisitCXXDeleteExpr(CXXDeleteExpr *E);
+Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
Expr *VisitCXXMemberCallExpr(CXXMemberC
[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl
khazem created this revision.
khazem added reviewers: spyffe, a.sidorin.
khazem added subscribers: cfe-commits, phosek, seanklein, klimek.
Some of this patch comes from Aleksei's branch [1], with minor revisions. I've
added unit tests and AST Matcher support. Copying in Manuel in case there is no
need for a dedicated usingShadowDecl() matcher, in which case I could define it
only in the test suite.
[1]
https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L3594
https://reviews.llvm.org/D27181
Files:
include/clang/AST/ASTImporter.h
include/clang/ASTMatchers/ASTMatchers.h
lib/AST/ASTImporter.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -558,5 +558,35 @@
}
+TEST(ImportDecl, ImportUsingDecl) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+testImport(
+ "namespace foo { int bar; }"
+ "int declToImport(){ using foo::bar; }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(
+has(
+ compoundStmt(
+has(
+ declStmt(
+has(
+ usingDecl();
+}
+
+
+TEST(ImportDecl, ImportUsingShadowDecl) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+testImport(
+ "namespace foo { int bar; }"
+ "namespace declToImport { using foo::bar; }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ namespaceDecl(
+has(
+ usingShadowDecl();
+}
+
+
} // end namespace ast_matchers
} // end namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -420,6 +420,7 @@
REGISTER_MATCHER(unresolvedUsingValueDecl);
REGISTER_MATCHER(userDefinedLiteral);
REGISTER_MATCHER(usingDecl);
+ REGISTER_MATCHER(usingShadowDecl);
REGISTER_MATCHER(usingDirectiveDecl);
REGISTER_MATCHER(valueDecl);
REGISTER_MATCHER(varDecl);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -175,6 +175,8 @@
Decl *VisitObjCCategoryDecl(ObjCCategoryDecl *D);
Decl *VisitObjCProtocolDecl(ObjCProtocolDecl *D);
Decl *VisitLinkageSpecDecl(LinkageSpecDecl *D);
+Decl *VisitUsingDecl(UsingDecl *D);
+Decl *VisitUsingShadowDecl(UsingShadowDecl *D);
ObjCTypeParamList *ImportObjCTypeParamList(ObjCTypeParamList *list);
Decl *VisitObjCInterfaceDecl(ObjCInterfaceDecl *D);
@@ -4288,6 +4290,68 @@
return ToLinkageSpec;
}
+Decl *ASTNodeImporter::VisitUsingDecl(UsingDecl *D) {
+ DeclContext *DC, *LexicalDC;
+ DeclarationName Name;
+ SourceLocation Loc;
+ NamedDecl *AlreadyImported;
+ if (ImportDeclParts(D, DC, LexicalDC, Name, AlreadyImported, Loc))
+return NULL;
+ assert(DC && "Null DeclContext after importing decl parts");
+ if (AlreadyImported)
+return AlreadyImported;
+
+ DeclarationNameInfo NameInfo(Name, Importer.Import(D->getNameInfo().getLoc()));
+ ImportDeclarationNameLoc(D->getNameInfo(), NameInfo);
+
+ // If a UsingShadowDecl has a pointer to its UsingDecl, then we may
+ // already have imported this UsingDecl. That should have been caught
+ // above when checking if ToD is non-null.
+ assert(!Importer.GetImported(D) &&
+ "Decl imported but not assigned to AlreadyImported");
+
+ UsingDecl *ToD = UsingDecl::Create(Importer.getToContext(), DC,
+ Importer.Import(D->getUsingLoc()),
+ Importer.Import(D->getQualifierLoc()),
+ NameInfo, D->hasTypename());
+ ImportAttributes(D, ToD);
+ ToD->setLexicalDeclContext(LexicalDC);
+ LexicalDC->addDeclInternal(ToD);
+ Importer.Imported(D, ToD);
+
+ for (UsingShadowDecl *I : D->shadows()) {
+UsingShadowDecl *SD = cast(Importer.Import(I));
+ToD->addShadowDecl(SD);
+ }
+ return ToD;
+}
+
+Decl *ASTNodeImporter::VisitUsingShadowDecl(UsingShadowDecl *D) {
+ DeclContext *DC, *LexicalDC;
+ DeclarationName Name;
+ SourceLocation Loc;
+ NamedDecl *AlreadyImported;
+ if (ImportDeclParts(D, DC, LexicalDC, Name, AlreadyImported, Loc))
+return NULL;
+ assert(DC && "Null DeclContext after importing decl parts");
+ if (AlreadyImported)
+return AlreadyImported;
+
+ UsingShadowDecl *ToD = UsingShadowDecl::Create(
+Importer.getToContext(), DC, Loc,
+cast(Importer.Import(D->getUsingDecl())),
+cast_or_null(Importer.Import(D->getTargetDecl(;
+
+ ImportAttributes(D, ToD);
+ ToD->setAccess(D->getAccess());
+ ToD->setLexicalDeclContext(LexicalDC);
+ Importer.Imported(D, ToD);
+
+ LexicalDC->addDeclInter
[PATCH] D26753: ASTImporter: improve support for C++ templates
khazem added a comment.
Sorry for the late comment, but one of the tests that this introduces is
breaking on my end:
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/class-template-partial-spec.cpp:9:11:
error: expected string not found in input
// CHECK:
/media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:21:32:
error: external variable 'X1' declared with incompatible types in different
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
^
:1:1: note: scanning from here
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:20:30:
error: external variable 'X0' defined in multiple translation units
^
:7:15: note: possible intended match here
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:21:32:
error: external variable 'X1' declared with incompatible types in different
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
^
Is this expected?
https://reviews.llvm.org/D26753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
