[PATCH] D139185: [clang][Interp] Use placement new to construct opcode args into vector

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: sepavloff, aaron.ballman.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The last version of this patch created problems because it was using the placement new with unaligned addresses.

This version tries to address this issue. I've enabled ubsan locally and not seen problems when running the interpreter test suite.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139185

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Source.h


Index: clang/lib/AST/Interp/Source.h
===================================================================
--- clang/lib/AST/Interp/Source.h
+++ clang/lib/AST/Interp/Source.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_CLANG_AST_INTERP_SOURCE_H
 #define LLVM_CLANG_AST_INTERP_SOURCE_H
 
+#include "PrimType.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
 #include "llvm/Support/Endian.h"
@@ -47,9 +48,10 @@
 
   /// Reads data and advances the pointer.
   template <typename T> std::enable_if_t<!std::is_pointer<T>::value, T> read() {
+    assert(aligned(Ptr));
     using namespace llvm::support;
     T Value = endian::read<T, endianness::native, 1>(Ptr);
-    Ptr += sizeof(T);
+    Ptr += align(sizeof(T));
     return Value;
   }
 
Index: clang/lib/AST/Interp/PrimType.h
===================================================================
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -62,6 +62,13 @@
   return ((Size + alignof(void *) - 1) / alignof(void *)) * alignof(void *);
 }
 
+constexpr bool aligned(size_t Size) { return Size == align(Size); }
+static_assert(aligned(sizeof(void *)));
+
+static inline bool aligned(const void *P) {
+  return aligned(reinterpret_cast<size_t>(P));
+}
+
 inline bool isPrimitiveIntegral(PrimType Type) {
   switch (Type) {
   case PT_Bool:
Index: clang/lib/AST/Interp/ByteCodeEmitter.cpp
===================================================================
--- clang/lib/AST/Interp/ByteCodeEmitter.cpp
+++ clang/lib/AST/Interp/ByteCodeEmitter.cpp
@@ -116,7 +116,8 @@
       using namespace llvm::support;
 
       /// Rewrite the operand of all jumps to this label.
-      void *Location = Code.data() + Reloc - sizeof(int32_t);
+      void *Location = Code.data() + Reloc - align(sizeof(int32_t));
+      assert(aligned(Location));
       const int32_t Offset = Target - static_cast<int64_t>(Reloc);
       endian::write<int32_t, endianness::native, 1>(Location, Offset);
     }
@@ -126,7 +127,9 @@
 
 int32_t ByteCodeEmitter::getOffset(LabelTy Label) {
   // Compute the PC offset which the jump is relative to.
-  const int64_t Position = Code.size() + sizeof(Opcode) + sizeof(int32_t);
+  const int64_t Position =
+      Code.size() + align(sizeof(Opcode)) + align(sizeof(int32_t));
+  assert(aligned(Position));
 
   // If target is known, compute jump offset.
   auto It = LabelOffsets.find(Label);
@@ -162,13 +165,17 @@
     return;
   }
 
+  // Access must be aligned!
+  size_t ValPos = align(Code.size());
+  Size = align(Size);
+  assert(aligned(ValPos + Size));
+  Code.resize(ValPos + Size);
+
   if constexpr (!std::is_pointer_v<T>) {
-    const char *Data = reinterpret_cast<const char *>(&Val);
-    Code.insert(Code.end(), Data, Data + Size);
+    new (Code.data() + ValPos) T(Val);
   } else {
     uint32_t ID = P.getOrCreateNativePointer(Val);
-    const char *Data = reinterpret_cast<const char *>(&ID);
-    Code.insert(Code.end(), Data, Data + Size);
+    new (Code.data() + ValPos) uint32_t(ID);
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139185.479570.patch
Type: text/x-patch
Size: 3003 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221202/5386f81c/attachment.bin>