[cfe-commits] r51918 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/AST/Expr.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp
Steve Naroff
snaroff at apple.com
Tue Jun 3 14:46:19 PDT 2008
Absolutely.
Feel free to commit the code I forwarded,
snaroff
On Jun 3, 2008, at 2:34 PM, Ted Kremenek wrote:
> 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