Please file a PR. I have no time to look at this right now, and we shouldn't forget about fixing this.
On Jan 19, 2012, at 5:04 PM, Eli Friedman wrote: > 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
