[cfe-commits] r138372 - in /cfe/trunk: lib/Analysis/CFG.cpp lib/StaticAnalyzer/Core/ExprEngineC.cpp test/Analysis/outofbound-notwork.c test/Analysis/outofbound.c test/Sema/uninit-variables.c

[Matthieu Monrocq]

Le 20 janvier 2012 02:04, Eli Friedman <eli.friedman at gmail.com> a écrit :

> On Tue, Aug 23, 2011 at 1:30 PM, Ted Kremenek <kremenek at apple.com> 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
>
>
> Ah interesting, we had a discussion about the unevaluated contexts and the
presence of null pointers yesterday on stackoverflow and the conclusion was
that also not so clear, the intent of the Standard seemed to be that values
were of no important in unevaluated contexts, except for `typeid(x)` where
`x` points to a polymorphic class.

And in fact, the C++11 Standard says in 5.3.3p1 [expr.sizeof]:

The sizeof operator yields the number of bytes in the object representation
of its operand. The operand is either an expression, which is an
**unevaluated operand** (Clause 5), or a parenthesized type-id.

(I know, VLA are not part of the C++ Standard)

It seems that the Standards are at odd was it revised in C11 ?

-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120120/3a1f864b/attachment.html>