[cfe-commits] r146236 - in /cfe/trunk: lib/AST/ExprConstant.cpp test/Sema/builtins.c

Richard Smith richard-llvm at metafoo.co.uk
Thu Dec 8 18:04:48 PST 2011


Author: rsmith
Date: Thu Dec  8 20:04:48 2011
New Revision: 146236

URL: http://llvm.org/viewvc/llvm-project?rev=146236&view=rev
Log:
Replace the implementation of __builtin_constant_p (which was based on the GCC
documentation) with one based on what GCC's __builtin_constant_p is actually
intended to do (discovered by asking a friendly GCC developer).

In particular, an expression which folds to a pointer is now only considered to
be a "constant" by this builtin if it refers to the first character in a string
literal.

This fixes a rather subtle wrong-code issue when building with glibc. Given:

const char cs[4] = "abcd";
int f(const char *p) { return strncmp(p, cs, 4); }

... the macro magic for strncmp produces a (potentially crashing) call to
strlen(cs), because it expands to an expression starting with:

  __builtin_constant_p(cs) && strlen(cs) < 4 ? /* ... */

Under the secret true meaning of __builtin_constant_p, this is guaranteed to be
safe!

Modified:
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/Sema/builtins.c

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=146236&r1=146235&r2=146236&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Dec  8 20:04:48 2011
@@ -3035,11 +3035,53 @@
   case Builtin::BI__builtin_classify_type:
     return Success(EvaluateBuiltinClassifyType(E), E);
 
-  case Builtin::BI__builtin_constant_p:
-    // __builtin_constant_p always has one operand: it returns true if that
-    // operand can be folded, false otherwise.
-    return Success(E->getArg(0)->isEvaluatable(Info.Ctx), E);
-      
+  case Builtin::BI__builtin_constant_p: {
+    const Expr *Arg = E->getArg(0);
+    QualType ArgType = Arg->getType();
+    // __builtin_constant_p always has one operand. The rules which gcc follows
+    // are not precisely documented, but are as follows:
+    //
+    //  - If the operand is of integral, floating, complex or enumeration type,
+    //    and can be folded to a known value of that type, it returns 1.
+    //  - If the operand and can be folded to a pointer to the first character
+    //    of a string literal (or such a pointer cast to an integral type), it
+    //    returns 1.
+    //
+    // Otherwise, it returns 0.
+    //
+    // FIXME: GCC also intends to return 1 for literals of aggregate types, but
+    // its support for this does not currently work.
+    int IsConstant = 0;
+    if (ArgType->isIntegralOrEnumerationType()) {
+      // Note, a pointer cast to an integral type is only a constant if it is
+      // a pointer to the first character of a string literal.
+      Expr::EvalResult Result;
+      if (Arg->EvaluateAsRValue(Result, Info.Ctx) && !Result.HasSideEffects) {
+        APValue &V = Result.Val;
+        if (V.getKind() == APValue::LValue) {
+          if (const Expr *E = V.getLValueBase().dyn_cast<const Expr*>())
+            IsConstant = isa<StringLiteral>(E) && V.getLValueOffset().isZero();
+        } else {
+          IsConstant = 1;
+        }
+      }
+    } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
+      IsConstant = Arg->isEvaluatable(Info.Ctx);
+    } else if (ArgType->isPointerType() || Arg->isGLValue()) {
+      LValue LV;
+      // Use a separate EvalInfo: ignore constexpr parameter and 'this' bindings
+      // during the check.
+      Expr::EvalStatus Status;
+      EvalInfo SubInfo(Info.Ctx, Status);
+      if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, SubInfo)
+                            : EvaluatePointer(Arg, LV, SubInfo)) &&
+          !Status.HasSideEffects)
+        if (const Expr *E = LV.getLValueBase().dyn_cast<const Expr*>())
+          IsConstant = isa<StringLiteral>(E) && LV.getLValueOffset().isZero();
+    }
+
+    return Success(IsConstant, E);
+  }
   case Builtin::BI__builtin_eh_return_data_regno: {
     int Operand = E->getArg(0)->EvaluateKnownConstInt(Info.Ctx).getZExtValue();
     Operand = Info.Ctx.getTargetInfo().getEHDataRegisterNumber(Operand);

Modified: cfe/trunk/test/Sema/builtins.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=146236&r1=146235&r2=146236&view=diff
==============================================================================
--- cfe/trunk/test/Sema/builtins.c (original)
+++ cfe/trunk/test/Sema/builtins.c Thu Dec  8 20:04:48 2011
@@ -102,3 +102,63 @@
   return __builtin_constant_p() + // expected-error{{too few arguments}}
          __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
+
+const int test17_n = 0;
+const char test17_c[] = {1, 2, 3, 0};
+const char test17_d[] = {1, 2, 3, 4};
+typedef int __attribute__((vector_size(16))) IntVector;
+struct Aggregate { int n; char c; };
+enum Enum { EnumValue1, EnumValue2 };
+
+typedef __typeof(sizeof(int)) size_t;
+size_t strlen(const char *);
+
+void test17() {
+#define ASSERT(...) { int arr[(__VA_ARGS__) ? 1 : -1]; }
+#define T(...) ASSERT(__builtin_constant_p(__VA_ARGS__))
+#define F(...) ASSERT(!__builtin_constant_p(__VA_ARGS__))
+
+  // __builtin_constant_p returns 1 if the argument folds to:
+  //  - an arithmetic constant with value which is known at compile time
+  T(test17_n);
+  T(&test17_c[3] - test17_c);
+  T(3i + 5); // expected-warning {{imaginary constant}}
+  T(4.2 * 7.6);
+  T(EnumValue1);
+  T((enum Enum)(int)EnumValue2);
+
+  //  - the address of the first character of a string literal, losslessly cast
+  //    to any type
+  T("string literal");
+  T((double*)"string literal");
+  T("string literal" + 0);
+  T((long)"string literal");
+
+  // ... and otherwise returns 0.
+  F("string literal" + 1);
+  F(&test17_n);
+  F(test17_c);
+  F(&test17_c);
+  F(&test17_d);
+  F((struct Aggregate){0, 1});
+  F((IntVector){0, 1, 2, 3});
+
+  // Ensure that a technique used in glibc is handled correctly.
+#define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
+  // FIXME: These are incorrectly treated as ICEs because strlen is treated as
+  // a builtin.
+  ASSERT(OPT("abc"));
+  ASSERT(!OPT("abcd"));
+  // In these cases, the strlen is non-constant, but the __builtin_constant_p
+  // is 0: the array size is not an ICE but is foldable.
+  ASSERT(!OPT(test17_c));        // expected-warning {{folded}}
+  ASSERT(!OPT(&test17_c[0]));    // expected-warning {{folded}}
+  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
+  ASSERT(!OPT(test17_d));        // expected-warning {{folded}}
+  ASSERT(!OPT(&test17_d[0]));    // expected-warning {{folded}}
+  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
+
+#undef OPT
+#undef T
+#undef F
+}





More information about the cfe-commits mailing list