Fix crash when evaluating constant expressions involving nullptr

[t.p.northover]

Summary

For the simple casts in the test, we were crashing because the ZeroInitialization function created an LValue with an unexpected SubobjectDesignator: it was valid but didn't actually refer to a valid MostDerivedType.

Since PointerExprEvaluator actually creates an LValue based notionally on what you'd get if you dereferenced the pointer (I think), the MostDerivedType should be set by stripping the pointer from the type as in this patch.

The theoretically simpler solution of providing an invalid SubobjectDesignator (as happens for an int to pointer cast) is incorrect because nullptr has more possible uses in constexprs than other casts of integers.

Does my reasoning look sound? I've been all over that file before I finally thought I knew what was going on and I'm still not entirely confident.

[rsmith]

clang/lib/AST/ExprConstant.cpp

5505–5507

This is the only caller of set() that passes more than three arguments (that is, the only caller that passes true as IsNullPtr_). It seems that such calls would always be unsafe / wrong, so I think we can do better than this.

How about this: split set() into two functions: for one of them, remove the last two parameters (IsNullPtr_ and Offset_), and for the other one, rename to setNull() and just pass a QualType and offset.

[t.p.northover]

Sounds very reasonable to me. I've uploaded a new diff.

[rsmith]

Thanks!

[t.p.northover]

Thanks Richard. I've committed this as r303957.

clang/lib/AST/ExprConstant.cpp

void setFrom(ASTContext &Ctx, const APValue &V) {
       Base = V.getLValueBase();
       Offset = V.getLValueOffset();
       InvalidBase = false;
       CallIndex = V.getLValueCallIndex();
       Designator = SubobjectDesignator(Ctx, V);
       IsNullPtr = V.isNullPointer();
     }
 
-    void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false,
-             bool IsNullPtr_ = false, uint64_t Offset_ = 0) {
+    void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
 #ifndef NDEBUG
       // We only allow a few types of invalid bases. Enforce that here.
       if (BInvalid) {
         const auto *E = B.get<const Expr *>();
         assert((isa<MemberExpr>(E) || tryUnwrapAllocSizeCall(E)) &&
                "Unexpected type of invalid base");
       }
 #endif
 
       Base = B;
-      Offset = CharUnits::fromQuantity(Offset_);
+      Offset = CharUnits::fromQuantity(0);
       InvalidBase = BInvalid;
       CallIndex = I;
       Designator = SubobjectDesignator(getType(B));
-      IsNullPtr = IsNullPtr_;
+      IsNullPtr = false;
+    }
+
+    void setNull(QualType PointerTy, uint64_t TargetVal) {
+      Base = (Expr *)nullptr;
+      Offset = CharUnits::fromQuantity(TargetVal);
+      InvalidBase = false;
+      CallIndex = 0;
+      Designator = SubobjectDesignator(PointerTy->getPointeeType());
+      IsNullPtr = true;
     }
 
     void setInvalid(APValue::LValueBase B, unsigned I = 0) {
       set(B, I, true);
     }
 
     // Check that this LValue is not based on a null pointer. If it is, produce
     // a diagnostic and mark the designator as invalid.
PointerExprEvaluator(EvalInfo &info, LValue &Result, bool InvalidBaseOK)
       : ExprEvaluatorBaseTy(info), Result(Result),
         InvalidBaseOK(InvalidBaseOK) {}
 
   bool Success(const APValue &V, const Expr *E) {
     Result.setFrom(Info.Ctx, V);
     return true;
   }
   bool ZeroInitialization(const Expr *E) {
-    auto Offset = Info.Ctx.getTargetNullPointerValue(E->getType());
-    Result.set((Expr*)nullptr, 0, false, true, Offset);
+    auto TargetVal = Info.Ctx.getTargetNullPointerValue(E->getType());
+    Result.setNull(E->getType(), TargetVal);
     return true;
   }
 
   bool VisitBinaryOperator(const BinaryOperator *E);
   bool VisitCastExpr(const CastExpr* E);
   bool VisitUnaryAddrOf(const UnaryOperator *E);
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
       { return Success(E); }
   bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {

clang/test/SemaCXX/null-cast.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {};
+struct B : virtual A {};
+
+void foo() {
+  (void)static_cast<A&>(*(B *)0); // expected-warning {{binding dereferenced null pointer to reference has undefined behavior}}
+}