[clang][bytecode] Propagate IsVolatile bit to subobjects (llvm#137293)

For
```c++
  struct S {
    constexpr S(int=0) : i(1) {}
    int i;
  };
  constexpr volatile S vs;
```

reading from `vs.i` is not allowed, even though `i` is not volatile
qualified. Propagate the IsVolatile bit down the hierarchy, so we know
reading from `vs.i` is a volatile read.

Author: tbaederr

clang/lib/AST/ByteCode/Compiler.cpp

@@ -364,8 +364,7 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
         Desc = P.createDescriptor(SubExpr, *T);
       else
         Desc = P.createDescriptor(SubExpr, PointeeType.getTypePtr(),
-                                  std::nullopt, true, false,
-                                  /*IsMutable=*/false, nullptr);
+                                  std::nullopt, /*IsConst=*/true);
     }
 
     uint64_t Val = Ctx.getASTContext().getTargetNullPointerValue(CE->getType());
@@ -417,8 +416,7 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
       Desc = nullptr;
     else
       Desc = P.createDescriptor(CE, PtrType->getPointeeType().getTypePtr(),
-                                Descriptor::InlineDescMD, true, false,
-                                /*IsMutable=*/false, nullptr);
+                                Descriptor::InlineDescMD, /*IsConst=*/true);
 
     if (!this->emitGetIntPtr(T, Desc, CE))
       return false;
@@ -3400,14 +3398,13 @@ bool Compiler<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) {
         Desc = nullptr; // We're not going to use it in this case.
       else
         Desc = P.createDescriptor(E, *ElemT, /*SourceTy=*/nullptr,
-                                  Descriptor::InlineDescMD,
-                                  /*IsConst=*/false, /*IsTemporary=*/false,
-                                  /*IsMutable=*/false);
+                                  Descriptor::InlineDescMD);
     } else {
       Desc = P.createDescriptor(
           E, ElementType.getTypePtr(),
           E->isArray() ? std::nullopt : Descriptor::InlineDescMD,
-          /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false, Init);
+          /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false,
+          /*IsVolatile=*/false, Init);
     }
   }
 
@@ -4355,7 +4352,7 @@ Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty,
 
   Descriptor *D = P.createDescriptor(
       Src, Ty.getTypePtr(), Descriptor::InlineDescMD, Ty.isConstQualified(),
-      IsTemporary, /*IsMutable=*/false, Init);
+      IsTemporary, /*IsMutable=*/false, /*IsVolatile=*/false, Init);
   if (!D)
     return std::nullopt;
   D->IsConstexprUnknown = IsConstexprUnknown;
@@ -4377,7 +4374,7 @@ std::optional<unsigned> Compiler<Emitter>::allocateTemporary(const Expr *E) {
 
   Descriptor *D = P.createDescriptor(
       E, Ty.getTypePtr(), Descriptor::InlineDescMD, Ty.isConstQualified(),
-      /*IsTemporary=*/true, /*IsMutable=*/false, /*Init=*/nullptr);
+      /*IsTemporary=*/true);
 
   if (!D)
     return std::nullopt;

clang/lib/AST/ByteCode/Descriptor.cpp

@@ -22,7 +22,7 @@ using namespace clang;
 using namespace clang::interp;
 
 template <typename T>
-static void ctorTy(Block *, std::byte *Ptr, bool, bool, bool, bool,
+static void ctorTy(Block *, std::byte *Ptr, bool, bool, bool, bool, bool,
                    const Descriptor *) {
   new (Ptr) T();
 }
@@ -41,7 +41,7 @@ static void moveTy(Block *, std::byte *Src, std::byte *Dst,
 }
 
 template <typename T>
-static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool, bool,
+static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool, bool, bool,
                         const Descriptor *D) {
   new (Ptr) InitMapPtr(std::nullopt);
 
@@ -82,8 +82,8 @@ static void moveArrayTy(Block *, std::byte *Src, std::byte *Dst,
 }
 
 static void ctorArrayDesc(Block *B, std::byte *Ptr, bool IsConst,
-                          bool IsMutable, bool IsActive, bool InUnion,
-                          const Descriptor *D) {
+                          bool IsMutable, bool IsVolatile, bool IsActive,
+                          bool InUnion, const Descriptor *D) {
   const unsigned NumElems = D->getNumElems();
   const unsigned ElemSize =
       D->ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
@@ -104,9 +104,10 @@ static void ctorArrayDesc(Block *B, std::byte *Ptr, bool IsConst,
     Desc->IsFieldMutable = IsMutable || D->IsMutable;
     Desc->InUnion = InUnion;
     Desc->IsArrayElement = true;
+    Desc->IsVolatile = IsVolatile;
 
     if (auto Fn = D->ElemDesc->CtorFn)
-      Fn(B, ElemLoc, Desc->IsConst, Desc->IsFieldMutable, IsActive,
+      Fn(B, ElemLoc, Desc->IsConst, Desc->IsFieldMutable, IsVolatile, IsActive,
          Desc->InUnion || SD->isUnion(), D->ElemDesc);
   }
 }
@@ -149,8 +150,8 @@ static void moveArrayDesc(Block *B, std::byte *Src, std::byte *Dst,
 }
 
 static void initField(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
-                      bool IsActive, bool IsUnionField, bool InUnion,
-                      const Descriptor *D, unsigned FieldOffset) {
+                      bool IsVolatile, bool IsActive, bool IsUnionField,
+                      bool InUnion, const Descriptor *D, unsigned FieldOffset) {
   auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + FieldOffset) - 1;
   Desc->Offset = FieldOffset;
   Desc->Desc = D;
@@ -160,15 +161,17 @@ static void initField(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
   Desc->InUnion = InUnion;
   Desc->IsConst = IsConst || D->IsConst;
   Desc->IsFieldMutable = IsMutable || D->IsMutable;
+  Desc->IsVolatile = IsVolatile || D->IsVolatile;
 
   if (auto Fn = D->CtorFn)
     Fn(B, Ptr + FieldOffset, Desc->IsConst, Desc->IsFieldMutable,
-       Desc->IsActive, InUnion || D->isUnion(), D);
+       Desc->IsVolatile, Desc->IsActive, InUnion || D->isUnion(), D);
 }
 
 static void initBase(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
-                     bool IsActive, bool InUnion, const Descriptor *D,
-                     unsigned FieldOffset, bool IsVirtualBase) {
+                     bool IsVolatile, bool IsActive, bool InUnion,
+                     const Descriptor *D, unsigned FieldOffset,
+                     bool IsVirtualBase) {
   assert(D);
   assert(D->ElemRecord);
   assert(!D->ElemRecord->isUnion()); // Unions cannot be base classes.
@@ -183,28 +186,32 @@ static void initBase(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
   Desc->IsConst = IsConst || D->IsConst;
   Desc->IsFieldMutable = IsMutable || D->IsMutable;
   Desc->InUnion = InUnion;
+  Desc->IsVolatile = false;
 
   for (const auto &V : D->ElemRecord->bases())
-    initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, InUnion,
-             V.Desc, V.Offset, false);
+    initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsVolatile, IsActive,
+             InUnion, V.Desc, V.Offset, false);
   for (const auto &F : D->ElemRecord->fields())
-    initField(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, InUnion,
-              InUnion, F.Desc, F.Offset);
+    initField(B, Ptr + FieldOffset, IsConst, IsMutable, IsVolatile, IsActive,
+              InUnion, InUnion, F.Desc, F.Offset);
 }
 
 static void ctorRecord(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
-                       bool IsActive, bool InUnion, const Descriptor *D) {
+                       bool IsVolatile, bool IsActive, bool InUnion,
+                       const Descriptor *D) {
   for (const auto &V : D->ElemRecord->bases())
-    initBase(B, Ptr, IsConst, IsMutable, IsActive, InUnion, V.Desc, V.Offset,
-             false);
+    initBase(B, Ptr, IsConst, IsMutable, IsVolatile, IsActive, InUnion, V.Desc,
+             V.Offset,
+             /*IsVirtualBase=*/false);
   for (const auto &F : D->ElemRecord->fields()) {
     bool IsUnionField = D->isUnion();
-    initField(B, Ptr, IsConst, IsMutable, IsActive, IsUnionField,
+    initField(B, Ptr, IsConst, IsMutable, IsVolatile, IsActive, IsUnionField,
               InUnion || IsUnionField, F.Desc, F.Offset);
   }
   for (const auto &V : D->ElemRecord->virtual_bases())
-    initBase(B, Ptr, IsConst, IsMutable, IsActive, InUnion, V.Desc, V.Offset,
-             true);
+    initBase(B, Ptr, IsConst, IsMutable, IsVolatile, IsActive, InUnion, V.Desc,
+             V.Offset,
+             /*IsVirtualBase=*/true);
 }
 
 static void destroyField(Block *B, std::byte *Ptr, const Descriptor *D,
@@ -332,12 +339,12 @@ static BlockMoveFn getMoveArrayPrim(PrimType Type) {
 /// Primitives.
 Descriptor::Descriptor(const DeclTy &D, const Type *SourceTy, PrimType Type,
                        MetadataSize MD, bool IsConst, bool IsTemporary,
-                       bool IsMutable)
+                       bool IsMutable, bool IsVolatile)
     : Source(D), SourceType(SourceTy), ElemSize(primSize(Type)), Size(ElemSize),
       MDSize(MD.value_or(0)), AllocSize(align(Size + MDSize)), PrimT(Type),
       IsConst(IsConst), IsMutable(IsMutable), IsTemporary(IsTemporary),
-      CtorFn(getCtorPrim(Type)), DtorFn(getDtorPrim(Type)),
-      MoveFn(getMovePrim(Type)) {
+      IsVolatile(IsVolatile), CtorFn(getCtorPrim(Type)),
+      DtorFn(getDtorPrim(Type)), MoveFn(getMovePrim(Type)) {
   assert(AllocSize >= Size);
   assert(Source && "Missing source");
 }
@@ -396,12 +403,13 @@ Descriptor::Descriptor(const DeclTy &D, const Descriptor *Elem, MetadataSize MD,
 
 /// Composite records.
 Descriptor::Descriptor(const DeclTy &D, const Record *R, MetadataSize MD,
-                       bool IsConst, bool IsTemporary, bool IsMutable)
+                       bool IsConst, bool IsTemporary, bool IsMutable,
+                       bool IsVolatile)
     : Source(D), ElemSize(std::max<size_t>(alignof(void *), R->getFullSize())),
       Size(ElemSize), MDSize(MD.value_or(0)), AllocSize(Size + MDSize),
       ElemRecord(R), IsConst(IsConst), IsMutable(IsMutable),
-      IsTemporary(IsTemporary), CtorFn(ctorRecord), DtorFn(dtorRecord),
-      MoveFn(moveRecord) {
+      IsTemporary(IsTemporary), IsVolatile(IsVolatile), CtorFn(ctorRecord),
+      DtorFn(dtorRecord), MoveFn(moveRecord) {
   assert(Source && "Missing source");
 }
 

clang/lib/AST/ByteCode/Descriptor.h

@@ -33,8 +33,8 @@ using InitMapPtr = std::optional<std::pair<bool, std::shared_ptr<InitMap>>>;
 /// inline descriptors of all fields and array elements. It also initializes
 /// all the fields which contain non-trivial types.
 using BlockCtorFn = void (*)(Block *Storage, std::byte *FieldPtr, bool IsConst,
-                             bool IsMutable, bool IsActive, bool InUnion,
-                             const Descriptor *FieldDesc);
+                             bool IsMutable, bool IsVolatile, bool IsActive,
+                             bool InUnion, const Descriptor *FieldDesc);
 
 /// Invoked when a block is destroyed. Invokes the destructors of all
 /// non-trivial nested fields of arrays and records.
@@ -104,6 +104,8 @@ struct InlineDescriptor {
   /// Flag indicating if the field is an element of a composite array.
   LLVM_PREFERRED_TYPE(bool)
   unsigned IsArrayElement : 1;
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsVolatile : 1;
 
   Lifetime LifeState;
 
@@ -112,7 +114,8 @@ struct InlineDescriptor {
   InlineDescriptor(const Descriptor *D)
       : Offset(sizeof(InlineDescriptor)), IsConst(false), IsInitialized(false),
         IsBase(false), IsActive(false), IsFieldMutable(false),
-        IsArrayElement(false), LifeState(Lifetime::Started), Desc(D) {}
+        IsArrayElement(false), IsVolatile(false), LifeState(Lifetime::Started),
+        Desc(D) {}
 
   void dump() const { dump(llvm::errs()); }
   void dump(llvm::raw_ostream &OS) const;
@@ -164,6 +167,7 @@ struct Descriptor final {
   const bool IsMutable = false;
   /// Flag indicating if the block is a temporary.
   const bool IsTemporary = false;
+  const bool IsVolatile = false;
   /// Flag indicating if the block is an array.
   const bool IsArray = false;
   /// Flag indicating if this is a dummy descriptor.
@@ -177,7 +181,8 @@ struct Descriptor final {
 
   /// Allocates a descriptor for a primitive.
   Descriptor(const DeclTy &D, const Type *SourceTy, PrimType Type,
-             MetadataSize MD, bool IsConst, bool IsTemporary, bool IsMutable);
+             MetadataSize MD, bool IsConst, bool IsTemporary, bool IsMutable,
+             bool IsVolatile);
 
   /// Allocates a descriptor for an array of primitives.
   Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD, size_t NumElems,
@@ -198,7 +203,7 @@ struct Descriptor final {
 
   /// Allocates a descriptor for a record.
   Descriptor(const DeclTy &D, const Record *R, MetadataSize MD, bool IsConst,
-             bool IsTemporary, bool IsMutable);
+             bool IsTemporary, bool IsMutable, bool IsVolatile);
 
   /// Allocates a dummy descriptor.
   Descriptor(const DeclTy &D, MetadataSize MD = std::nullopt);

clang/lib/AST/ByteCode/DynamicAllocator.cpp

@@ -84,6 +84,7 @@ Block *DynamicAllocator::allocate(const Descriptor *D, unsigned EvalID,
   ID->IsFieldMutable = false;
   ID->IsConst = false;
   ID->IsInitialized = false;
+  ID->IsVolatile = false;
 
   B->IsDynamic = true;
 

clang/lib/AST/ByteCode/Interp.cpp

@@ -634,32 +634,35 @@ static bool CheckVolatile(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
                           AccessKinds AK) {
   assert(Ptr.isLive());
 
-  // FIXME: This check here might be kinda expensive. Maybe it would be better
-  // to have another field in InlineDescriptor for this?
-  if (!Ptr.isBlockPointer())
-    return true;
-
-  QualType PtrType = Ptr.getType();
-  if (!PtrType.isVolatileQualified())
+  if (!Ptr.isVolatile())
     return true;
 
   if (!S.getLangOpts().CPlusPlus)
     return Invalid(S, OpPC);
 
+  // The reason why Ptr is volatile might be further up the hierarchy.
+  // Find that pointer.
+  Pointer P = Ptr;
+  while (!P.isRoot()) {
+    if (P.getType().isVolatileQualified())
+      break;
+    P = P.getBase();
+  }
+
   const NamedDecl *ND = nullptr;
   int DiagKind;
   SourceLocation Loc;
-  if (const auto *F = Ptr.getField()) {
+  if (const auto *F = P.getField()) {
     DiagKind = 2;
     Loc = F->getLocation();
     ND = F;
-  } else if (auto *VD = Ptr.getFieldDesc()->asValueDecl()) {
+  } else if (auto *VD = P.getFieldDesc()->asValueDecl()) {
     DiagKind = 1;
     Loc = VD->getLocation();
     ND = VD;
   } else {
     DiagKind = 0;
-    if (const auto *E = Ptr.getFieldDesc()->asExpr())
+    if (const auto *E = P.getFieldDesc()->asExpr())
       Loc = E->getExprLoc();
   }
 

clang/lib/AST/ByteCode/InterpBlock.h

@@ -114,6 +114,7 @@ class Block final {
     std::memset(rawData(), 0, Desc->getAllocSize());
     if (Desc->CtorFn) {
       Desc->CtorFn(this, data(), Desc->IsConst, Desc->IsMutable,
+                   Desc->IsVolatile,
                    /*isActive=*/true, /*InUnion=*/false, Desc);
     }
     IsInitialized = true;

clang/lib/AST/ByteCode/InterpBuiltin.cpp

@@ -1595,11 +1595,9 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
 
   assert(!ElemT);
   // Structs etc.
-  const Descriptor *Desc = S.P.createDescriptor(
-      NewCall, ElemType.getTypePtr(),
-      IsArray ? std::nullopt : Descriptor::InlineDescMD,
-      /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false,
-      /*Init=*/nullptr);
+  const Descriptor *Desc =
+      S.P.createDescriptor(NewCall, ElemType.getTypePtr(),
+                           IsArray ? std::nullopt : Descriptor::InlineDescMD);
 
   if (IsArray) {
     Block *B =

clang/lib/AST/ByteCode/Pointer.h

@@ -577,6 +577,13 @@ class Pointer {
     return isRoot() ? getDeclDesc()->IsConst : getInlineDesc()->IsConst;
   }
 
+  /// Checks if an object or a subfield is volatile.
+  bool isVolatile() const {
+    if (!isBlockPointer())
+      return false;
+    return isRoot() ? getDeclDesc()->IsVolatile : getInlineDesc()->IsVolatile;
+  }
+
   /// Returns the declaration ID.
   std::optional<unsigned> getDeclID() const {
     if (isBlockPointer()) {