[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:25:32 PDT 2008
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
More information about the cfe-commits
mailing list