Bug 1374024 - add checker to prevent dangling pointers returned by method calls

on temporaries. r=mystor

MozReview-Commit-ID: 9khNt59ONFE
This commit is contained in:
Tristan Bourvon 2017-07-05 16:14:21 +02:00
Родитель 350ab58390
Коммит 20068f7ba3
12 изменённых файлов: 527 добавлений и 0 удалений

Просмотреть файл

@ -6,6 +6,7 @@
CHECK(ArithmeticArgChecker, "arithmetic-argument")
CHECK(AssertAssignmentChecker, "assignment-in-assert")
CHECK(DanglingOnTemporaryChecker, "dangling-on-temporary")
CHECK(ExplicitImplicitChecker, "implicit-constructor")
CHECK(ExplicitOperatorBoolChecker, "explicit-operator-bool")
CHECK(KungFuDeathGripChecker, "kungfu-death-grip")

Просмотреть файл

@ -7,6 +7,7 @@
#include "ArithmeticArgChecker.h"
#include "AssertAssignmentChecker.h"
#include "DanglingOnTemporaryChecker.h"
#include "ExplicitImplicitChecker.h"
#include "ExplicitOperatorBoolChecker.h"
#include "KungFuDeathGripChecker.h"

Просмотреть файл

@ -7,6 +7,7 @@
#include "MemMoveAnnotation.h"
#include "Utils.h"
#include "VariableUsageHelpers.h"
namespace clang {
namespace ast_matchers {
@ -29,6 +30,78 @@ AST_MATCHER(CXXRecordDecl, hasTrivialCtorDtor) {
return hasCustomAnnotation(&Node, "moz_trivial_ctor_dtor");
}
/// This matcher will match lvalue-ref-qualified methods.
AST_MATCHER(CXXMethodDecl, isLValueRefQualified) {
return Node.getRefQualifier() == RQ_LValue;
}
/// This matcher will match rvalue-ref-qualified methods.
AST_MATCHER(CXXMethodDecl, isRValueRefQualified) {
return Node.getRefQualifier() == RQ_RValue;
}
/// This matcher will match any method declaration that is marked as returning
/// a pointer deleted by the destructor of the class.
AST_MATCHER(CXXMethodDecl, noDanglingOnTemporaries) {
return hasCustomAnnotation(&Node, "moz_no_dangling_on_temporaries");
}
/// This matcher will match an expression if it escapes the scope of the callee
/// of a parent call expression (skipping trivial parents).
/// The first inner matcher matches the statement where the escape happens, and
/// the second inner matcher corresponds to the declaration through which it
/// happens.
AST_MATCHER_P2(Expr, escapesParentFunctionCall, \
internal::Matcher<Stmt>, EscapeStmtMatcher, \
internal::Matcher<Decl>, EscapeDeclMatcher) {
auto Call =
IgnoreParentTrivials(Node, &Finder->getASTContext()).get<CallExpr>();
if (!Call) {
return false;
}
auto FunctionEscapeData = escapesFunction(&Node, Call);
assert(FunctionEscapeData && "escapesFunction() returned NoneType: there is a"
" logic bug in the matcher");
const Stmt* EscapeStmt;
const Decl* EscapeDecl;
std::tie(EscapeStmt, EscapeDecl) = *FunctionEscapeData;
return EscapeStmt && EscapeDecl
&& EscapeStmtMatcher.matches(*EscapeStmt, Finder, Builder)
&& EscapeDeclMatcher.matches(*EscapeDecl, Finder, Builder);
}
/// This is the custom matcher class corresponding to hasNonTrivialParent.
template <typename T, typename ParentT>
class HasNonTrivialParentMatcher : public internal::WrapperMatcherInterface<T> {
static_assert(internal::IsBaseType<ParentT>::value,
"has parent only accepts base type matcher");
public:
explicit HasNonTrivialParentMatcher(
const internal::Matcher<ParentT> &NonTrivialParentMatcher)
: HasNonTrivialParentMatcher::WrapperMatcherInterface(
NonTrivialParentMatcher) {}
bool matches(const T &Node, internal::ASTMatchFinder *Finder,
internal::BoundNodesTreeBuilder *Builder) const override {
auto NewNode = IgnoreParentTrivials(Node, &Finder->getASTContext());
// We return the result of the inner matcher applied to the new node.
return this->InnerMatcher.matches(NewNode, Finder, Builder);
}
};
/// This matcher acts like hasParent, except it skips trivial constructs by
/// traversing the AST tree upwards.
const internal::ArgumentAdaptingMatcherFunc<
HasNonTrivialParentMatcher,
internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>,
internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>>
LLVM_ATTRIBUTE_UNUSED hasNonTrivialParent = {};
/// This matcher will match any function declaration that is marked to prohibit
/// calling AddRef or Release on its return value.
AST_MATCHER(FunctionDecl, hasNoAddRefReleaseOnReturnAttr) {

Просмотреть файл

@ -0,0 +1,149 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "DanglingOnTemporaryChecker.h"
#include "CustomMatchers.h"
void DanglingOnTemporaryChecker::registerMatchers(MatchFinder *AstMatcher) {
////////////////////////////////////////
// Quick annotation conflict checkers //
////////////////////////////////////////
AstMatcher->addMatcher(
// This is a matcher on a method declaration,
cxxMethodDecl(
// which is marked as no dangling on temporaries,
noDanglingOnTemporaries(),
// and which is && ref-qualified.
isRValueRefQualified(),
decl().bind("invalidMethodRefQualified")),
this);
AstMatcher->addMatcher(
// This is a matcher on a method declaration,
cxxMethodDecl(
// which is marked as no dangling on temporaries,
noDanglingOnTemporaries(),
// and which doesn't return a pointer.
unless(returns(pointerType())),
decl().bind("invalidMethodPointer")),
this);
//////////////////
// Main checker //
//////////////////
AstMatcher->addMatcher(
// This is a matcher on a method call,
cxxMemberCallExpr(
// which is performed on a temporary,
onImplicitObjectArgument(materializeTemporaryExpr()),
// and which is marked as no dangling on temporaries.
callee(cxxMethodDecl(noDanglingOnTemporaries())),
anyOf(
// We care only about the cases where the method call is NOT an
// argument in a call expression. If it is in a call expression,
// the temporary lives long enough so that it's valid to use the
// pointer.
unless(hasNonTrivialParent(callExpr())),
// Unless the argument somehow escapes the function scope through
// globals/statics/black magic.
escapesParentFunctionCall(
stmt().bind("escapeStatement"),
decl().bind("escapeDeclaration"))),
expr().bind("memberCallExpr")),
this);
}
void DanglingOnTemporaryChecker::check(const MatchFinder::MatchResult &Result) {
///////////////////////////////////////
// Quick annotation conflict checker //
///////////////////////////////////////
const char *ErrorInvalidRefQualified = "methods annotated with "
"MOZ_NO_DANGLING_ON_TEMPORARIES "
"cannot be && ref-qualified";
const char *ErrorInvalidPointer = "methods annotated with "
"MOZ_NO_DANGLING_ON_TEMPORARIES must "
"return a pointer";
if (auto InvalidRefQualified
= Result.Nodes.getNodeAs<CXXMethodDecl>("invalidMethodRefQualified")) {
diag(InvalidRefQualified->getLocation(), ErrorInvalidRefQualified,
DiagnosticIDs::Error);
return;
}
if (auto InvalidPointer
= Result.Nodes.getNodeAs<CXXMethodDecl>("invalidMethodPointer")) {
diag(InvalidPointer->getLocation(), ErrorInvalidPointer,
DiagnosticIDs::Error);
return;
}
//////////////////
// Main checker //
//////////////////
const char *Error = "calling `%0` on a temporary, potentially allowing use "
"after free of the raw pointer";
const char *EscapeStmtNote
= "the raw pointer escapes the function scope here";
const CXXMemberCallExpr *MemberCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>("memberCallExpr");
// If we escaped the a parent function call, we get the statement and the
// associated declaration.
const Stmt *EscapeStmt =
Result.Nodes.getNodeAs<Stmt>("escapeStatement");
const Decl *EscapeDecl =
Result.Nodes.getNodeAs<Decl>("escapeDeclaration");
// Just in case.
if (!MemberCall) {
return;
}
// We emit the error diagnostic indicating that we are calling the method
// temporary.
diag(MemberCall->getExprLoc(), Error, DiagnosticIDs::Error)
<< MemberCall->getMethodDecl()->getName()
<< MemberCall->getSourceRange();
// If we didn't escape a parent function, we're done.
if (!EscapeStmt || !EscapeDecl) {
return;
}
diag(EscapeStmt->getLocStart(), EscapeStmtNote, DiagnosticIDs::Note)
<< EscapeStmt->getSourceRange();
StringRef EscapeDeclNote;
SourceRange EscapeDeclRange;
if (isa<ParmVarDecl>(EscapeDecl)) {
EscapeDeclNote = "through the parameter declared here";
EscapeDeclRange = EscapeDecl->getSourceRange();
} else if (isa<VarDecl>(EscapeDecl)) {
EscapeDeclNote = "through the variable declared here";
EscapeDeclRange = EscapeDecl->getSourceRange();
} else if (auto FuncDecl = dyn_cast<FunctionDecl>(EscapeDecl)) {
EscapeDeclNote = "through the return value of the function declared here";
EscapeDeclRange = FuncDecl->getReturnTypeSourceRange();
} else {
return;
}
diag(EscapeDecl->getLocation(), EscapeDeclNote, DiagnosticIDs::Note)
<< EscapeDeclRange;
}

Просмотреть файл

@ -0,0 +1,19 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef DanglingOnTemporaryChecker_h__
#define DanglingOnTemporaryChecker_h__
#include "plugin.h"
class DanglingOnTemporaryChecker : public BaseCheck {
public:
DanglingOnTemporaryChecker(StringRef CheckName,
ContextType *Context = nullptr)
: BaseCheck(CheckName, Context) {}
void registerMatchers(MatchFinder *AstMatcher) override;
void check(const MatchFinder::MatchResult &Result) override;
};
#endif

Просмотреть файл

@ -307,6 +307,13 @@ inline bool typeIsRefPtr(QualType Q) {
return false;
}
// This method returns true if the statement is trivial.
inline bool IsTrivial(const Stmt *s) {
return s && (isa<ExprWithCleanups>(s) || isa<MaterializeTemporaryExpr>(s) ||
isa<CXXBindTemporaryExpr>(s) || isa<ImplicitCastExpr>(s) ||
isa<ParenExpr>(s));
}
// The method defined in clang for ignoring implicit nodes doesn't work with
// some AST trees. To get around this, we define our own implementation of
// IgnoreTrivials.
@ -329,6 +336,7 @@ inline const Stmt *IgnoreTrivials(const Stmt *s) {
}
}
assert(!IsTrivial(s));
return s;
}
@ -336,6 +344,29 @@ inline const Expr *IgnoreTrivials(const Expr *e) {
return cast<Expr>(IgnoreTrivials(static_cast<const Stmt *>(e)));
}
// This method is like IgnoreTrivials but ignores the nodes upwards instead of
// downwards.
template <typename T>
inline ast_type_traits::DynTypedNode IgnoreParentTrivials(const T &Node,
ASTContext *Context) {
// We traverse the AST upward until we encounter a non-trivial node.
auto CurrentNode = ast_type_traits::DynTypedNode::create(Node);
do {
// We get the parents of the current node from the AST context.
auto Parents = Context->getParents(CurrentNode);
// Not implemented yet, but probably not very useful for the cases where
// we use this matcher.
if (Parents.size() != 1) {
break;
}
CurrentNode = Parents[0];
} while (IsTrivial(CurrentNode.template get<Stmt>()));
return CurrentNode;
}
const FieldDecl *getBaseRefCntMember(QualType T);
inline const FieldDecl *getBaseRefCntMember(const CXXRecordDecl *D) {

Просмотреть файл

@ -0,0 +1,163 @@
#include "VariableUsageHelpers.h"
#include "Utils.h"
std::vector<const Stmt*>
getUsageAsRvalue(const ValueDecl* ValueDeclaration,
const FunctionDecl* FuncDecl) {
std::vector<const Stmt*> UsageStatements;
// We check the function declaration has a body.
auto Body = FuncDecl->getBody();
if (!Body) {
return std::vector<const Stmt*>();
}
// We build a Control Flow Graph (CFG) fron the body of the function
// declaration.
std::unique_ptr<CFG> StatementCFG
= CFG::buildCFG(FuncDecl, Body, &FuncDecl->getASTContext(),
CFG::BuildOptions());
// We iterate through all the CFGBlocks, which basically means that we go over
// all the possible branches of the code and therefore cover all statements.
for (auto& Block : *StatementCFG) {
// We iterate through all the statements of the block.
for (auto& BlockItem : *Block) {
Optional<CFGStmt> CFGStatement = BlockItem.getAs<CFGStmt>();
if (!CFGStatement) {
continue;
}
// FIXME: Right now this function/if chain is very basic and only covers
// the cases we need for escapesFunction()
if (auto BinOp = dyn_cast<BinaryOperator>(CFGStatement->getStmt())) {
// We only care about assignments.
if (BinOp->getOpcode() != BO_Assign) {
continue;
}
// We want our declaration to be used on the right hand side of the
// assignment.
auto DeclRef = dyn_cast<DeclRefExpr>(IgnoreTrivials(BinOp->getRHS()));
if (!DeclRef) {
continue;
}
if (DeclRef->getDecl() != ValueDeclaration) {
continue;
}
} else if (auto Return = dyn_cast<ReturnStmt>(CFGStatement->getStmt())) {
// We want our declaration to be used as the expression of the return
// statement.
auto DeclRef = dyn_cast_or_null<DeclRefExpr>(
IgnoreTrivials(Return->getRetValue()));
if (!DeclRef) {
continue;
}
if (DeclRef->getDecl() != ValueDeclaration) {
continue;
}
} else {
continue;
}
// We didn't early-continue, so we add the statement to the list.
UsageStatements.push_back(CFGStatement->getStmt());
}
}
return UsageStatements;
}
Optional<std::tuple<const Stmt*, const Decl*>>
escapesFunction(const Expr* Arg, const CallExpr* Call) {
// We get the function declaration corresponding to the call.
auto FuncDecl = Call->getDirectCallee();
if (!FuncDecl) {
return NoneType();
}
// We find the argument number corresponding to the Arg expression.
unsigned ArgNum = 0;
for (auto CallArg : Call->arguments()) {
if (IgnoreTrivials(Arg) == IgnoreTrivials(CallArg)) {
break;
}
++ArgNum;
}
// If we don't find it, we early-return NoneType.
if (ArgNum >= Call->getNumArgs()) {
return NoneType();
}
// Now we get the associated parameter.
if (ArgNum >= FuncDecl->getNumParams()) {
return NoneType();
}
auto Param = FuncDecl->getParamDecl(ArgNum);
// We want both the argument and the parameter to be of pointer type.
// FIXME: this is enough for the DanglingOnTemporaryChecker, because the
// analysed methods only return pointers, but more cases should probably be
// handled when we want to use this function more broadly.
if (!Arg->getType()->isPointerType()
|| !Param->getType()->isPointerType()) {
return NoneType();
}
// We retrieve the usages of the parameter in the function.
auto Usages = getUsageAsRvalue(Param, FuncDecl);
// For each usage, we check if it doesn't allow the parameter to escape the
// function scope.
for (auto Usage : Usages) {
// In the case of an assignment.
if (auto BinOp = dyn_cast<BinaryOperator>(Usage)) {
// We retrieve the declaration the parameter is assigned to.
auto DeclRef = dyn_cast<DeclRefExpr>(BinOp->getLHS());
if (!DeclRef) {
continue;
}
if (auto ParamDeclaration = dyn_cast<ParmVarDecl>(DeclRef->getDecl())) {
// This is the case where the parameter escapes through another
// parameter.
// FIXME: for now we only care about references because we only detect
// trivial LHS with just a DeclRefExpr, and not more complex cases like:
// void func(Type* param1, Type** param2) {
// *param2 = param1;
// }
// This should be fixed when we have better/more helper functions to
// help deal with this kind of lvalue expressions.
if (!ParamDeclaration->getType()->isReferenceType()) {
continue;
}
return {{Usage, ParamDeclaration}};
} else if (auto VarDeclaration = dyn_cast<VarDecl>(DeclRef->getDecl())) {
// This is the case where the parameter escapes through a global/static
// variable.
if (!VarDeclaration->hasGlobalStorage()) {
continue;
}
return {{Usage, VarDeclaration}};
}
} else if (auto Return = dyn_cast<ReturnStmt>(Usage)) {
// This is the case where the parameter escapes through the return value
// of the function.
if (!FuncDecl->getReturnType()->isPointerType()
&& !FuncDecl->getReturnType()->isReferenceType()) {
continue;
}
return {{Usage, FuncDecl}};
}
}
// No early-return, this means that we haven't found any case of funciton
// escaping and that therefore the parameter remains in the function scope.
return {{nullptr, nullptr}};
}

Просмотреть файл

@ -0,0 +1,36 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include <vector>
#ifndef VariableUsageHelpers_h__
#define VariableUsageHelpers_h__
#include "plugin.h"
/// Returns a list of the statements where the given declaration is used as an
/// rvalue (within the provided function).
///
/// WARNING: incomplete behaviour/implementation for general-purpose use outside
/// of escapesFunction(). This only detects very basic usages (see
/// implementation for more details).
std::vector<const Stmt*>
getUsageAsRvalue(const ValueDecl* ValueDeclaration,
const FunctionDecl* FuncDecl);
/// Returns a (statement, decl) tuple if an argument from a call expression
/// escapes the function scope through globals/statics/other things. The
/// statement is where the value escapes the function, while the declaration
/// points to what it escapes through. If the argument doesn't escape the
/// function, the tuple will only contain nullptrs.
/// If the analysis runs into an unexpected error or into an unimplemented
/// configuration, this returns NoneType.
///
/// WARNING: incomplete behaviour/implementation for general-purpose use outside
/// of DanglingOnTemporaryChecker. This only covers a limited set of cases,
/// mainly in terms of arguments and parameter types.
Optional<std::tuple<const Stmt*, const Decl*>>
escapesFunction(const Expr* Arg, const CallExpr* Call);
#endif

Просмотреть файл

@ -12,6 +12,7 @@ UNIFIED_SOURCES += [
'ArithmeticArgChecker.cpp',
'AssertAssignmentChecker.cpp',
'CustomTypeAnnotation.cpp',
'DanglingOnTemporaryChecker.cpp',
'DiagnosticsMatcher.cpp',
'ExplicitImplicitChecker.cpp',
'ExplicitOperatorBoolChecker.cpp',
@ -36,6 +37,7 @@ UNIFIED_SOURCES += [
'ScopeChecker.cpp',
'SprintfLiteralChecker.cpp',
'TrivialCtorDtorChecker.cpp',
'VariableUsageHelpers.cpp',
]
GENERATED_FILES += ['ThirdPartyPaths.cpp']

Просмотреть файл

@ -0,0 +1,41 @@
#define MOZ_NO_DANGLING_ON_TEMPORARIES \
__attribute__((annotate("moz_no_dangling_on_temporaries")))
class AnnotateConflict {
MOZ_NO_DANGLING_ON_TEMPORARIES int *get() && { return nullptr; } // expected-error {{methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES cannot be && ref-qualified}}
MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // expected-error {{methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES must return a pointer}}
};
class NS_ConvertUTF8toUTF16 {
public:
MOZ_NO_DANGLING_ON_TEMPORARIES int *get() { return nullptr; }
};
NS_ConvertUTF8toUTF16 TemporaryFunction() { return NS_ConvertUTF8toUTF16(); }
void UndefinedFunction(int* test);
void NoEscapeFunction(int *test) {}
int *glob; // expected-note {{through the variable declared here}}
void EscapeFunction1(int *test) { glob = test; } // expected-note {{the raw pointer escapes the function scope here}}
void EscapeFunction2(int *test, int *&escape) { escape = test; } // expected-note {{the raw pointer escapes the function scope here}} \
expected-note {{through the parameter declared here}}
int *EscapeFunction3(int *test) { return test; } // expected-note {{the raw pointer escapes the function scope here}} \
expected-note {{through the return value of the function declared here}}
int main() {
int *test = TemporaryFunction().get(); // expected-error {{calling `get` on a temporary, potentially allowing use after free of the raw pointer}}
int *test2 = NS_ConvertUTF8toUTF16().get(); // expected-error {{calling `get` on a temporary, potentially allowing use after free of the raw pointer}}
UndefinedFunction(NS_ConvertUTF8toUTF16().get());
NoEscapeFunction(TemporaryFunction().get());
EscapeFunction1(TemporaryFunction().get()); // expected-error {{calling `get` on a temporary, potentially allowing use after free of the raw pointer}}
int *escape;
EscapeFunction2(TemporaryFunction().get(), escape); // expected-error {{calling `get` on a temporary, potentially allowing use after free of the raw pointer}}
int *escape2 = EscapeFunction3(TemporaryFunction().get()); // expected-error {{calling `get` on a temporary, potentially allowing use after free of the raw pointer}}
}

Просмотреть файл

@ -11,6 +11,7 @@ SOURCES += [
'TestAssertWithAssignment.cpp',
'TestBadImplicitConversionCtor.cpp',
'TestCustomHeap.cpp',
'TestDanglingOnTemporary.cpp',
'TestExplicitOperatorBool.cpp',
'TestGlobalClass.cpp',
'TestHeapClass.cpp',

Просмотреть файл

@ -526,6 +526,14 @@
* MOZ_NEEDS_MEMMOVABLE_MEMBERS: Applies to class declarations where each member
* must be safe to move in memory using memmove(). MOZ_NON_MEMMOVABLE types
* used in members of these classes are compile time errors.
* MOZ_NO_DANGLING_ON_TEMPORARIES: Applies to method declarations which return
* a pointer that is freed when the destructor of the class is called. This
* prevents these methods from being called on temporaries of the class,
* reducing risks of use-after-free.
* This attribute cannot be applied to && methods.
* In some cases, adding a deleted &&-qualified overload is too restrictive as
* this method should still be callable as a non-escaping argument to another
* function. This annotation can be used in those cases.
* MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS: Applies to template class
* declarations where an instance of the template should be considered, for
* static analysis purposes, to inherit any type annotations (such as
@ -587,6 +595,7 @@
# define MOZ_NON_MEMMOVABLE __attribute__((annotate("moz_non_memmovable")))
# define MOZ_NEEDS_MEMMOVABLE_TYPE __attribute__((annotate("moz_needs_memmovable_type")))
# define MOZ_NEEDS_MEMMOVABLE_MEMBERS __attribute__((annotate("moz_needs_memmovable_members")))
# define MOZ_NO_DANGLING_ON_TEMPORARIES __attribute__((annotate("moz_no_dangling_on_temporaries")))
# define MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS \
__attribute__((annotate("moz_inherit_type_annotations_from_template_args")))
# define MOZ_NON_AUTOABLE __attribute__((annotate("moz_non_autoable")))
@ -632,6 +641,7 @@
# define MOZ_NON_MEMMOVABLE /* nothing */
# define MOZ_NEEDS_MEMMOVABLE_TYPE /* nothing */
# define MOZ_NEEDS_MEMMOVABLE_MEMBERS /* nothing */
# define MOZ_NO_DANGLING_ON_TEMPORARIES /* nothing */
# define MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS /* nothing */
# define MOZ_INIT_OUTSIDE_CTOR /* nothing */
# define MOZ_IS_CLASS_INIT /* nothing */