Skip to content

Commit

Permalink
[analyzer] Enable analysis of WebKit "unified sources".
Browse files Browse the repository at this point in the history
Normally the analyzer begins path-sensitive analysis from functions within
the main file, even though the path is allowed to go through any functions
within the translation unit.

When a recent version of WebKit is compiled, the "unified sources" technique
is used, that assumes #including multiple code files into a single main file.
Such file would have no functions defined in it, so the analyzer wouldn't be
able to find any entry points for path-sensitive analysis.

This patch pattern-matches unified file names that are similar to those
used by WebKit and allows the analyzer to find entry points in the included
code files. A more aggressive/generic approach is being planned as well.

Differential Revision: https://reviews.llvm.org/D45839

llvm-svn: 330876
  • Loading branch information
haoNoQ committed Apr 25, 2018
1 parent 75fda2e commit 516837f
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 13 deletions.
Expand Up @@ -126,6 +126,36 @@ class AnalysisManager : public BugReporterData {
AnalysisDeclContext *getAnalysisDeclContext(const Decl *D) {
return AnaCtxMgr.getContext(D);
}

static bool isInCodeFile(SourceLocation SL, const SourceManager &SM) {
if (SM.isInMainFile(SL))
return true;

// Support the "unified sources" compilation method (eg. WebKit) that
// involves producing non-header files that include other non-header files.
// We should be included directly from a UnifiedSource* file
// and we shouldn't be a header - which is a very safe defensive check.
SourceLocation IL = SM.getIncludeLoc(SM.getFileID(SL));
if (!IL.isValid() || !SM.isInMainFile(IL))
return false;
// Should rather be "file name starts with", but the current .getFilename
// includes the full path.
if (SM.getFilename(IL).contains("UnifiedSource")) {
// It might be great to reuse FrontendOptions::getInputKindForExtension()
// but for now it doesn't discriminate between code and header files.
return llvm::StringSwitch<bool>(SM.getFilename(SL).rsplit('.').second)
.Cases("c", "m", "mm", "C", "cc", "cp", true)
.Cases("cpp", "CPP", "c++", "cxx", "cppm", true)
.Default(false);
}

return false;
}

bool isInCodeFile(SourceLocation SL) {
const SourceManager &SM = getASTContext().getSourceManager();
return isInCodeFile(SL, SM);
}
};

} // enAnaCtxMgrspace
Expand Down
9 changes: 4 additions & 5 deletions clang/lib/StaticAnalyzer/Core/CallEvent.cpp
Expand Up @@ -939,15 +939,14 @@ const ObjCPropertyDecl *ObjCMethodCall::getAccessedProperty() const {
bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,
Selector Sel) const {
assert(IDecl);
const SourceManager &SM =
getState()->getStateManager().getContext().getSourceManager();

AnalysisManager &AMgr =
getState()->getStateManager().getOwningEngine()->getAnalysisManager();
// If the class interface is declared inside the main file, assume it is not
// subcassed.
// TODO: It could actually be subclassed if the subclass is private as well.
// This is probably very rare.
SourceLocation InterfLoc = IDecl->getEndOfDefinitionLoc();
if (InterfLoc.isValid() && SM.isInMainFile(InterfLoc))
if (InterfLoc.isValid() && AMgr.isInCodeFile(InterfLoc))
return false;

// Assume that property accessors are not overridden.
Expand All @@ -969,7 +968,7 @@ bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,
return false;

// If outside the main file,
if (D->getLocation().isValid() && !SM.isInMainFile(D->getLocation()))
if (D->getLocation().isValid() && !AMgr.isInCodeFile(D->getLocation()))
return true;

if (D->isOverriding()) {
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
Expand Up @@ -806,8 +806,9 @@ static bool isCXXSharedPtrDtor(const FunctionDecl *FD) {
/// This checks static properties of the function, such as its signature and
/// CFG, to determine whether the analyzer should ever consider inlining it,
/// in any context.
static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
AnalyzerOptions &Opts) {
static bool mayInlineDecl(AnalysisManager &AMgr,
AnalysisDeclContext *CalleeADC) {
AnalyzerOptions &Opts = AMgr.getAnalyzerOptions();
// FIXME: Do not inline variadic calls.
if (CallEvent::isVariadic(CalleeADC->getDecl()))
return false;
Expand All @@ -830,7 +831,7 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
// Conditionally control the inlining of methods on objects that look
// like C++ containers.
if (!Opts.mayInlineCXXContainerMethods())
if (!Ctx.getSourceManager().isInMainFile(FD->getLocation()))
if (!AMgr.isInCodeFile(FD->getLocation()))
if (isContainerMethod(Ctx, FD))
return false;

Expand Down Expand Up @@ -891,7 +892,7 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
} else {
// We haven't actually checked the static properties of this function yet.
// Do that now, and record our decision in the function summaries.
if (mayInlineDecl(CalleeADC, Opts)) {
if (mayInlineDecl(getAnalysisManager(), CalleeADC)) {
Engine.FunctionSummaries->markMayInline(D);
} else {
Engine.FunctionSummaries->markShouldNotInline(D);
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
Expand Up @@ -29,6 +29,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
#include "llvm/ADT/ArrayRef.h"
Expand Down Expand Up @@ -148,11 +149,11 @@ getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
if (CallLoc.isMacroID())
return nullptr;

assert(SMgr.isInMainFile(CallLoc) &&
"The call piece should be in the main file.");
assert(AnalysisManager::isInCodeFile(CallLoc, SMgr) &&
"The call piece should not be in a header file.");

// Check if CP represents a path through a function outside of the main file.
if (!SMgr.isInMainFile(CP->callEnterWithin.asLocation()))
if (!AnalysisManager::isInCodeFile(CP->callEnterWithin.asLocation(), SMgr))
return CP;

const PathPieces &Path = CP->path;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Expand Up @@ -678,7 +678,7 @@ AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
SourceLocation SL = Body ? Body->getLocStart() : D->getLocation();
SL = SM.getExpansionLoc(SL);

if (!Opts->AnalyzeAll && !SM.isWrittenInMainFile(SL)) {
if (!Opts->AnalyzeAll && !Mgr->isInCodeFile(SL)) {
if (SL.isInvalid() || SM.isInSystemHeader(SL))
return AM_None;
return Mode & ~AM_Path;
Expand Down
5 changes: 5 additions & 0 deletions clang/test/Analysis/unified-sources/UnifiedSource-1.cpp
@@ -0,0 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s

// There should still be diagnostics within included files.
#include "source1.cpp"
#include "source2.cpp"
10 changes: 10 additions & 0 deletions clang/test/Analysis/unified-sources/container.h
@@ -0,0 +1,10 @@
class ContainerInHeaderFile {
class Iterator {
};

public:
Iterator begin() const;
Iterator end() const;

int method() { return 0; }
};
15 changes: 15 additions & 0 deletions clang/test/Analysis/unified-sources/source1.cpp
@@ -0,0 +1,15 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s

// This test tests that the warning is here when it is included from
// the unified sources file. The run-line in this file is there
// only to suppress LIT warning for the complete lack of run-line.
int foo(int x) {
if (x) {}
return 1 / x; // expected-warning{{}}
}

// Let's see if the container inlining heuristic still works.
#include "container.h"
int testContainerMethodInHeaderFile(ContainerInHeaderFile Cont) {
return 1 / Cont.method(); // no-warning
}
25 changes: 25 additions & 0 deletions clang/test/Analysis/unified-sources/source2.cpp
@@ -0,0 +1,25 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s

// This test tests that the warning is here when it is included from
// the unified sources file. The run-line in this file is there
// only to suppress LIT warning for the complete lack of run-line.
int testNullDereference() {
int *x = 0;
return *x; // expected-warning{{}}
}

// Let's see if the container inlining heuristic still works.
class ContainerInCodeFile {
class Iterator {
};

public:
Iterator begin() const;
Iterator end() const;

int method() { return 0; }
};

int testContainerMethodInCodeFile(ContainerInCodeFile Cont) {
return 1 / Cont.method(); // expected-warning{{}}
}

0 comments on commit 516837f

Please sign in to comment.