[cfe-commits] r51918 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/AST/Expr.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp
Ted Kremenek
kremenek at apple.com
Tue Jun 3 14:34:16 PDT 2008
Steve,
Maybe that should be added to the test suite (if it hasn't already).
Ted
On Jun 3, 2008, at 2:25 PM, Steve Naroff wrote:
>
> On Jun 3, 2008, at 2:01 PM, Eli Friedman wrote:
>
>> Author: efriedma
>> Date: Tue Jun 3 16:01:11 2008
>> New Revision: 51918
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=51918&view=rev
>> Log:
>> Re-fix r51907 in a way which doesn't affect valid code. This
>> essentially
>> moves the check for the invalid construct to a point where it doesn't
>> affect other uses of isIntegerConstantExpr, and we can warn properly
>> when the extension is used. This makes it a bit more complicated,
>> but
>> it's a lot cleaner.
>>
>> Steve, please tell me if this check is sufficient to handle the
>> relevant system header. I know it's enough to handle the testcase,
>> but
>> I don't know what exactly the original looks like.
>>
>
> The error has come back...
>
> [steve-naroffs-imac:llvm/tools/clang] snaroff% ../../Debug/bin/clang
> eli.m
> eli.m:10:10: error: arrays with static storage duration must have
> constant integer length
> char control[(((__darwin_size_t)((char *)(sizeof(struct cmsghdr))
> + (sizeof(__darwin_size_t) - 1)) &~ (sizeof(__darwin_size_t) - 1)) +
> ((__darwin_size_t)((char *)(sizeof(int)) + (sizeof(__darwin_size_t) -
> 1)) &~ (sizeof(__darwin_size_t) - 1)))];
> ^~~~~~~
>
> Here is a small test case...
>
> typedef long unsigned int __darwin_size_t;
> typedef long __darwin_ssize_t;
> typedef __darwin_size_t size_t;
> typedef __darwin_ssize_t ssize_t;
>
> struct cmsghdr {};
>
> ssize_t sendFileDescriptor(int fd, void *data, size_t nbytes, int
> sendfd) {
> union {
> char control[(((__darwin_size_t)((char *)(sizeof(struct cmsghdr))
> + (sizeof(__darwin_size_t) - 1)) &~ (sizeof(__darwin_size_t) - 1)) +
> ((__darwin_size_t)((char *)(sizeof(int)) + (sizeof(__darwin_size_t) -
> 1)) &~ (sizeof(__darwin_size_t) - 1)))];
> } control_un;
> }
>
> As you can imagine, this mess comes from a macro. Here is a brief
> excerpt from <sys/socket.h>:
>
> (kremenek at grue:include)$ grep -r CMSG_SPACE *
> sys/socket.h:#define CMSG_SPACE(l) (__DARWIN_ALIGN(sizeof(struct
> cmsghdr)) + __DARWIN_ALIGN(l))
> (kremenek at grue:include)$ grep -r DARWIN_ALIGN *
> i386/_param.h:#define __DARWIN_ALIGN(p) ((__darwin_size_t)((char *)(p)
> + __DARWIN_ALIGNBYTES) &~ __DARWIN_ALIGNBYTES)
>
> Thanks,
>
> snaroff
>
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticKinds.def
>> cfe/trunk/lib/AST/Expr.cpp
>> cfe/trunk/lib/Sema/Sema.h
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=51918&r1=51917&r2=51918&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Tue Jun 3
>> 16:01:11 2008
>> @@ -721,6 +721,8 @@
>> "enumeration values exceed range of largest integer")
>> DIAG(err_case_label_not_integer_constant_expr, ERROR,
>> "case label does not reduce to an integer constant")
>> +DIAG(warn_illegal_constant_array_size, EXTENSION,
>> + "size of static array must be an integer constant expression")
>> DIAG(err_typecheck_illegal_vla, ERROR,
>> "arrays with static storage duration must have constant integer
>> length")
>> DIAG(err_typecheck_negative_array_size, ERROR,
>>
>> Modified: cfe/trunk/lib/AST/Expr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=51918&r1=51917&r2=51918&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/AST/Expr.cpp (original)
>> +++ cfe/trunk/lib/AST/Expr.cpp Tue Jun 3 16:01:11 2008
>> @@ -918,11 +918,6 @@
>> if (!SubExpr->getType()->isArithmeticType() ||
>> !getType()->isIntegerType()) {
>> if (Loc) *Loc = SubExpr->getLocStart();
>> - // GCC accepts pointers as an extension.
>> - // FIXME: check getLangOptions().NoExtensions. At the moment,
>> it doesn't
>> - // appear possible to get langOptions() from the Expr.
>> - if (SubExpr->getType()->isPointerType()) // && !NoExtensions
>> - return true;
>> return false;
>> }
>>
>>
>> Modified: cfe/trunk/lib/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=51918&r1=51917&r2=51918&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/Sema.h (original)
>> +++ cfe/trunk/lib/Sema/Sema.h Tue Jun 3 16:01:11 2008
>> @@ -896,6 +896,11 @@
>>
>>
>> void InitBuiltinVaListType();
>> +
>> + // Helper method to turn variable array types into
>> + // constant array types in certain situations which would
>> otherwise
>> + // be errors
>> + QualType TryFixInvalidVariablyModifiedType(QualType T);
>>
>> //
>> =
>> =
>> =--------------------------------------------------------------------
>> ===//
>> // Extra semantic analysis beyond the C type system
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=51918&r1=51917&r2=51918&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun 3 16:01:11 2008
>> @@ -1842,6 +1842,52 @@
>> return New;
>> }
>>
>> +static bool CalcFakeICEVal(const Expr* Expr,
>> + llvm::APSInt& Result,
>> + ASTContext& Context) {
>> + // Calculate the value of an expression that has a calculatable
>> + // value, but isn't an ICE. Currently, this only supports
>> + // a very narrow set of extensions, but it can be expanded if
>> needed.
>> + if (const ParenExpr *PE = dyn_cast<ParenExpr>(Expr))
>> + return CalcFakeICEVal(PE->getSubExpr(), Result, Context);
>> +
>> + if (const CastExpr *CE = dyn_cast<CastExpr>(Expr)) {
>> + QualType CETy = CE->getType();
>> + if ((CETy->isIntegralType() && !CETy->isBooleanType()) ||
>> + CETy->isPointerType()) {
>> + if (CalcFakeICEVal(CE->getSubExpr(), Result, Context)) {
>> + Result.extOrTrunc(Context.getTypeSize(CETy));
>> + // FIXME: This assumes pointers are signed.
>> + Result.setIsSigned(CETy->isSignedIntegerType() ||
>> + CETy->isPointerType());
>> + return true;
>> + }
>> + }
>> + }
>> +
>> + if (Expr->getType()->isIntegralType())
>> + return Expr->isIntegerConstantExpr(Result, Context);
>> +
>> + return false;
>> +}
>> +
>> +QualType Sema::TryFixInvalidVariablyModifiedType(QualType T) {
>> + // This method tries to turn a variable array into a constant
>> + // array even when the size isn't an ICE. This is necessary
>> + // for compatibility with code that depends on gcc's buggy
>> + // constant expression folding, like struct {char x[(int)
>> (char*)2];}
>> + if (const VariableArrayType* VLATy =
>> dyn_cast<VariableArrayType>(T)) {
>> + llvm::APSInt Result(32);
>> + if (VLATy->getSizeExpr() &&
>> + CalcFakeICEVal(VLATy->getSizeExpr(), Result, Context) &&
>> + Result > llvm::APSInt(Result.getBitWidth(),
>> Result.isUnsigned())) {
>> + return Context.getConstantArrayType(VLATy->getElementType(),
>> + Result,
>> ArrayType::Normal, 0);
>> + }
>> + }
>> + return QualType();
>> +}
>> +
>> /// ActOnField - Each field of a struct/union/class is passed into
>> this in order
>> /// to create a FieldDecl object for it.
>> Sema::DeclTy *Sema::ActOnField(Scope *S,
>> @@ -1877,9 +1923,15 @@
>> // C99 6.7.2.1p8: A member of a structure or union may have any
>> type other
>> // than a variably modified type.
>> if (T->isVariablyModifiedType()) {
>> - // FIXME: This diagnostic needs work
>> - Diag(Loc, diag::err_typecheck_illegal_vla, Loc);
>> - InvalidDecl = true;
>> + QualType FixedTy = TryFixInvalidVariablyModifiedType(T);
>> + if (!FixedTy.isNull()) {
>> + Diag(Loc, diag::warn_illegal_constant_array_size, Loc);
>> + T = FixedTy;
>> + } else {
>> + // FIXME: This diagnostic needs work
>> + Diag(Loc, diag::err_typecheck_illegal_vla, Loc);
>> + InvalidDecl = true;
>> + }
>> }
>> // FIXME: Chain fielddecls together.
>> FieldDecl *NewFD = FieldDecl::Create(Context, Loc, II, T, BitWidth);
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
> _______________________________________________
> 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