[cfe-commits] r126797 - in /cfe/trunk: lib/Analysis/CFG.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/misc-ps.m test/SemaCXX/array-bounds.cpp
Nico Weber
thakis at chromium.org
Sat Mar 5 20:53:30 PST 2011
Hi Ted,
when updating clang on the chrome buildbots, clang now crashes when
compiling webkit. I binary searched, and it looks like this change is
what caused it. It's a bit tricky to reproduce, since for some reason
the crash happens only on the buildbots, not on my local box. I'll try
to get a reliable repro, but the code this crashes on looks something
like this:
namespace {
enum MatchType {
Exact,
UpperBound,
LowerBound
};
template <MatchType type> int getScaledValue(const Vector<int>&
scaledValues, int valueToMatch, int searchStart)
{
if (scaledValues.isEmpty())
return valueToMatch;
const int* dataStart = scaledValues.data();
const int* dataEnd = dataStart + scaledValues.size();
const int* matched = std::lower_bound(dataStart + searchStart,
dataEnd, valueToMatch);
switch (type) {
case Exact:
return matched != dataEnd && *matched == valueToMatch ?
matched - dataStart : -1;
case LowerBound:
return matched != dataEnd && *matched == valueToMatch ?
matched - dataStart : matched - dataStart - 1;
case UpperBound:
default:
return matched != dataEnd ? matched - dataStart : -1;
}
}
}
// ...
int ImageDecoder::upperBoundScaledY(int origY, int searchStart)
{
return getScaledValue<UpperBound>(m_scaledRows, origY, searchStart);
}
int ImageDecoder::lowerBoundScaledY(int origY, int searchStart)
{
return getScaledValue<LowerBound>(m_scaledRows, origY, searchStart);
}
int ImageDecoder::scaledY(int origY, int searchStart)
{
return getScaledValue<Exact>(m_scaledRows, origY, searchStart);
}
Maybe it's obvious to you what's wrong?
The complete code is here:
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp
The stack this crashes with is:
0 clang 0x00000000017c964f
1 clang 0x00000000017cb8c2
2 libpthread.so.0 0x00002b3feb2528f0
3 clang 0x0000000000c65aa6
clang::CFGBlock::FilterEdge(clang::CFGBlock::FilterOptions const&,
clang::CFGBlock const*, clang::CFGBlock const*) + 38
4 clang 0x0000000000c82f83
clang::reachable_code::ScanReachableFromBlock(clang::CFGBlock const&,
llvm::BitVector&) + 323
5 clang 0x0000000000b55a2e
clang::sema::AnalysisBasedWarnings::IssueWarnings(clang::sema::AnalysisBasedWarnings::Policy,
clang::sema::FunctionScopeInfo*, clang::Decl const*, clang::BlockExpr
const*) + 1198
6 clang 0x0000000000909c04
clang::Sema::PopFunctionOrBlockScope(clang::sema::AnalysisBasedWarnings::Policy
const*, clang::Decl const*, clang::BlockExpr const*) + 180
7 clang 0x0000000000968d89
clang::Sema::ActOnFinishFunctionBody(clang::Decl*, clang::Stmt*, bool)
+ 553
8 clang 0x0000000000b307d1
clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation,
clang::FunctionDecl*, bool, bool) + 1121
9 clang 0x0000000000b3009d
clang::Sema::PerformPendingInstantiations(bool) + 365
10 clang 0x0000000000909d62
clang::Sema::ActOnEndOfTranslationUnit() + 306
11 clang 0x00000000008cac68
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&)
+ 200
12 clang 0x00000000008b161b clang::ParseAST(clang::Sema&, bool) + 139
13 clang 0x000000000078dcf4 clang::CodeGenAction::ExecuteAction() + 68
14 clang 0x000000000067322b
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 347
15 clang 0x000000000064da54
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 1268
16 clang 0x00000000006453df cc1_main(char const**, char
const**, char const*, void*) + 655
17 clang 0x000000000064caa7 main + 4679
18 libc.so.6 0x00002b3febe32c4d __libc_start_main + 253
19 clang 0x0000000000643a69
Stack dump:
0. Program arguments: /* very long command line */
1. <eof> parser at end of file
2. third_party/WebKit/Source/WebCore/platform/image-decoders/ImageDecoder.cpp:257:31:
instantiating function definition 'getScaledValue'
clang: error: unable to execute command: Segmentation fault
clang: error: clang frontend command failed due to signal 1 (use -v to
see invocation)
Thanks,
Nico
On Tue, Mar 1, 2011 at 3:12 PM, Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Tue Mar 1 17:12:55 2011
> New Revision: 126797
>
> URL: http://llvm.org/viewvc/llvm-project?rev=126797&view=rev
> Log:
> Teach CFGBuilder to prune trivially unreachable case statements.
>
> Modified:
> cfe/trunk/lib/Analysis/CFG.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> cfe/trunk/test/Analysis/misc-ps.m
> cfe/trunk/test/SemaCXX/array-bounds.cpp
>
> Modified: cfe/trunk/lib/Analysis/CFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=126797&r1=126796&r2=126797&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFG.cpp Tue Mar 1 17:12:55 2011
> @@ -210,6 +210,25 @@
> LocalScope::const_iterator scopePosition;
> };
>
> +/// TryResult - a class representing a variant over the values
> +/// 'true', 'false', or 'unknown'. This is returned by tryEvaluateBool,
> +/// and is used by the CFGBuilder to decide if a branch condition
> +/// can be decided up front during CFG construction.
> +class TryResult {
> + int X;
> +public:
> + TryResult(bool b) : X(b ? 1 : 0) {}
> + TryResult() : X(-1) {}
> +
> + bool isTrue() const { return X == 1; }
> + bool isFalse() const { return X == 0; }
> + bool isKnown() const { return X >= 0; }
> + void negate() {
> + assert(isKnown());
> + X ^= 0x1;
> + }
> +};
> +
> /// CFGBuilder - This class implements CFG construction from an AST.
> /// The builder is stateful: an instance of the builder should be used to only
> /// construct a single CFG.
> @@ -238,7 +257,7 @@
> CFGBlock* SwitchTerminatedBlock;
> CFGBlock* DefaultCaseBlock;
> CFGBlock* TryTerminatedBlock;
> -
> +
> // Current position in local scope.
> LocalScope::const_iterator ScopePos;
>
> @@ -257,12 +276,17 @@
>
> bool badCFG;
> CFG::BuildOptions BuildOpts;
> +
> + // State to track for building switch statements.
> + bool switchExclusivelyCovered;
> + Expr::EvalResult switchCond;
>
> public:
> explicit CFGBuilder() : cfg(new CFG()), // crew a new CFG
> Block(NULL), Succ(NULL),
> SwitchTerminatedBlock(NULL), DefaultCaseBlock(NULL),
> - TryTerminatedBlock(NULL), badCFG(false) {}
> + TryTerminatedBlock(NULL), badCFG(false),
> + switchExclusivelyCovered(false) {}
>
> // buildCFG - Used by external clients to construct the CFG.
> CFG* buildCFG(const Decl *D, Stmt *Statement, ASTContext *C,
> @@ -387,45 +411,34 @@
> B->addSuccessor(S, cfg->getBumpVectorContext());
> }
>
> - /// TryResult - a class representing a variant over the values
> - /// 'true', 'false', or 'unknown'. This is returned by tryEvaluateBool,
> - /// and is used by the CFGBuilder to decide if a branch condition
> - /// can be decided up front during CFG construction.
> - class TryResult {
> - int X;
> - public:
> - TryResult(bool b) : X(b ? 1 : 0) {}
> - TryResult() : X(-1) {}
> -
> - bool isTrue() const { return X == 1; }
> - bool isFalse() const { return X == 0; }
> - bool isKnown() const { return X >= 0; }
> - void negate() {
> - assert(isKnown());
> - X ^= 0x1;
> - }
> - };
> + /// Try and evaluate an expression to an integer constant.
> + bool tryEvaluate(Expr *S, Expr::EvalResult &outResult) {
> + if (!BuildOpts.PruneTriviallyFalseEdges)
> + return false;
> + return !S->isTypeDependent() &&
> + !S->isValueDependent() &&
> + S->Evaluate(outResult, *Context);
> + }
>
> /// tryEvaluateBool - Try and evaluate the Stmt and return 0 or 1
> /// if we can evaluate to a known value, otherwise return -1.
> TryResult tryEvaluateBool(Expr *S) {
> - if (!BuildOpts.PruneTriviallyFalseEdges)
> + Expr::EvalResult Result;
> + if (!tryEvaluate(S, Result))
> return TryResult();
> +
> + if (Result.Val.isInt())
> + return Result.Val.getInt().getBoolValue();
>
> - Expr::EvalResult Result;
> - if (!S->isTypeDependent() && !S->isValueDependent() &&
> - S->Evaluate(Result, *Context)) {
> - if (Result.Val.isInt())
> - return Result.Val.getInt().getBoolValue();
> - if (Result.Val.isLValue()) {
> - Expr *e = Result.Val.getLValueBase();
> - const CharUnits &c = Result.Val.getLValueOffset();
> - if (!e && c.isZero())
> - return false;
> - }
> + if (Result.Val.isLValue()) {
> + Expr *e = Result.Val.getLValueBase();
> + const CharUnits &c = Result.Val.getLValueOffset();
> + if (!e && c.isZero())
> + return false;
> }
> return TryResult();
> }
> +
> };
>
> // FIXME: Add support for dependent-sized array types in C++?
> @@ -2180,6 +2193,17 @@
> assert(Terminator->getBody() && "switch must contain a non-NULL body");
> Block = NULL;
>
> + // For pruning unreachable case statements, save the current state
> + // for tracking the condition value.
> + SaveAndRestore<bool> save_switchExclusivelyCovered(switchExclusivelyCovered,
> + false);
> + SaveAndRestore<Expr::EvalResult> save_switchCond(switchCond);
> +
> +
> + // Determine if the switch condition can be explicitly evaluated.
> + assert(Terminator->getCond() && "switch condition must be non-NULL");
> + tryEvaluate(Terminator->getCond(), switchCond);
> +
> // If body is not a compound statement create implicit scope
> // and add destructors.
> if (!isa<CompoundStmt>(Terminator->getBody()))
> @@ -2193,11 +2217,11 @@
>
> // If we have no "default:" case, the default transition is to the code
> // following the switch body.
> - addSuccessor(SwitchTerminatedBlock, DefaultCaseBlock);
> + addSuccessor(SwitchTerminatedBlock,
> + switchExclusivelyCovered ? 0 : DefaultCaseBlock);
>
> // Add the terminator and condition in the switch block.
> SwitchTerminatedBlock->setTerminator(Terminator);
> - assert(Terminator->getCond() && "switch condition must be non-NULL");
> Block = SwitchTerminatedBlock;
> Block = addStmt(Terminator->getCond());
>
> @@ -2213,6 +2237,44 @@
>
> return Block;
> }
> +
> +static bool shouldAddCase(bool &switchExclusivelyCovered,
> + const Expr::EvalResult &switchCond,
> + const CaseStmt *CS,
> + ASTContext &Ctx) {
> + bool addCase = false;
> +
> + if (!switchExclusivelyCovered) {
> + if (switchCond.Val.isInt()) {
> + // Evaluate the LHS of the case value.
> + Expr::EvalResult V1;
> + CS->getLHS()->Evaluate(V1, Ctx);
> + assert(V1.Val.isInt());
> + const llvm::APSInt &condInt = switchCond.Val.getInt();
> + const llvm::APSInt &lhsInt = V1.Val.getInt();
> +
> + if (condInt == lhsInt) {
> + addCase = true;
> + switchExclusivelyCovered = true;
> + }
> + else if (condInt < lhsInt) {
> + if (const Expr *RHS = CS->getRHS()) {
> + // Evaluate the RHS of the case value.
> + Expr::EvalResult V2;
> + RHS->Evaluate(V2, Ctx);
> + assert(V2.Val.isInt());
> + if (V2.Val.getInt() <= condInt) {
> + addCase = true;
> + switchExclusivelyCovered = true;
> + }
> + }
> + }
> + }
> + else
> + addCase = true;
> + }
> + return addCase;
> +}
>
> CFGBlock* CFGBuilder::VisitCaseStmt(CaseStmt* CS) {
> // CaseStmts are essentially labels, so they are the first statement in a
> @@ -2232,9 +2294,12 @@
> else
> TopBlock = currentBlock;
>
> - addSuccessor(SwitchTerminatedBlock, currentBlock);
> - LastBlock = currentBlock;
> + addSuccessor(SwitchTerminatedBlock,
> + shouldAddCase(switchExclusivelyCovered, switchCond,
> + CS, *Context)
> + ? currentBlock : 0);
>
> + LastBlock = currentBlock;
> CS = cast<CaseStmt>(Sub);
> Sub = CS->getSubStmt();
> }
> @@ -2256,7 +2321,10 @@
> // Add this block to the list of successors for the block with the switch
> // statement.
> assert(SwitchTerminatedBlock);
> - addSuccessor(SwitchTerminatedBlock, CaseBlock);
> + addSuccessor(SwitchTerminatedBlock,
> + shouldAddCase(switchExclusivelyCovered, switchCond,
> + CS, *Context)
> + ? CaseBlock : 0);
>
> // We set Block to NULL to allow lazy creation of a new block (if necessary)
> Block = NULL;
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=126797&r1=126796&r2=126797&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Mar 1 17:12:55 2011
> @@ -1043,6 +1043,10 @@
> bool defaultIsFeasible = I == EI;
>
> for ( ; I != EI; ++I) {
> + // Successor may be pruned out during CFG construction.
> + if (!I.getBlock())
> + continue;
> +
> const CaseStmt* Case = I.getCase();
>
> // Evaluate the LHS of the case value.
>
> Modified: cfe/trunk/test/Analysis/misc-ps.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=126797&r1=126796&r2=126797&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/misc-ps.m (original)
> +++ cfe/trunk/test/Analysis/misc-ps.m Tue Mar 1 17:12:55 2011
> @@ -1269,3 +1269,22 @@
> }
> }
>
> +void test_switch() {
> + switch (4) {
> + case 1: {
> + int *p = 0;
> + *p = 0xDEADBEEF; // no-warning
> + break;
> + }
> + case 4: {
> + int *p = 0;
> + *p = 0xDEADBEEF; // expected-warning {{null}}
> + break;
> + }
> + default: {
> + int *p = 0;
> + *p = 0xDEADBEEF; // no-warning
> + break;
> + }
> + }
> +}
> \ No newline at end of file
>
> Modified: cfe/trunk/test/SemaCXX/array-bounds.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/array-bounds.cpp?rev=126797&r1=126796&r2=126797&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/array-bounds.cpp (original)
> +++ cfe/trunk/test/SemaCXX/array-bounds.cpp Tue Mar 1 17:12:55 2011
> @@ -127,4 +127,23 @@
> return sizeof(char) == sizeof(char) ? arr[2] : arr[1]; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}}
> }
>
> +void test_switch() {
> + switch (4) {
> + case 1: {
> + int arr[2];
> + arr[2] = 1; // no-warning
> + break;
> + }
> + case 4: {
> + int arr[2]; // expected-note {{array 'arr' declared here}}
> + arr[2] = 1; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}}
> + break;
> + }
> + default: {
> + int arr[2];
> + arr[2] = 1; // no-warning
> + break;
> + }
> + }
> +}
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list