On Tue, Aug 23, 2011 at 1:30 PM, Ted Kremenek <[email protected]> wrote: > Author: kremenek > Date: Tue Aug 23 15:30:50 2011 > New Revision: 138372 > > URL: http://llvm.org/viewvc/llvm-project?rev=138372&view=rev > Log: > Fix regression in -Wuninitialized involving VLAs. It turns out that we were > modeling sizeof(VLAs) > incorrectly in the CFG, and also the static analyzer. This patch regresses > the analyzer a bit, but > that needs to be followed up with a better solution. > > Fixes <rdar://problem/10008112>. > > Added: > cfe/trunk/test/Analysis/outofbound-notwork.c > Modified: > cfe/trunk/lib/Analysis/CFG.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp > cfe/trunk/test/Analysis/outofbound.c > cfe/trunk/test/Sema/uninit-variables.c > > Modified: cfe/trunk/lib/Analysis/CFG.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=138372&r1=138371&r2=138372&view=diff > ============================================================================== > --- cfe/trunk/lib/Analysis/CFG.cpp (original) > +++ cfe/trunk/lib/Analysis/CFG.cpp Tue Aug 23 15:30:50 2011 > @@ -2203,16 +2203,7 @@ > for (const VariableArrayType *VA > =FindVA(E->getArgumentType().getTypePtr()); > VA != 0; VA = FindVA(VA->getElementType().getTypePtr())) > lastBlock = addStmt(VA->getSizeExpr()); > - } else { > - // For sizeof(x), where 'x' is a VLA, we should include the computation > - // of the lvalue of 'x'. > - Expr *subEx = E->getArgumentExpr(); > - if (subEx->getType()->isVariableArrayType()) { > - assert(subEx->isLValue()); > - lastBlock = addStmt(subEx); > - } > } > - > return lastBlock; > }
A bit late, but I think this commit is wrong. The subexpression of a sizeof() expression is in fact evaluated per C99 6.5.3.4p2. So strictly speaking, this code has undefined behavior. The fact that we're getting this wrong is leading to a crash in a patch I'm working on to model the evaluated-ness of sizeof() correctly in Sema. (That said, we can use the following reasoning to suppress the warning for the given testcase: in "sizeof(*memory)", the code doesn't actually use the loaded value, so it doesn't matter that it's an uninitialized load.) -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
