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

[tbaeder]

Summary

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.

[aaron.ballman]

Generally looks correct to me.

clang/lib/AST/Interp/PrimType.h

65

I think Size should be something more like Value -- I was thrown off by the pointer-based aligned that was treating the pointer as an integer value for Size. :-D

69

Pedantically, this should be casting through uintptr_t rather than size_t.

[tbaeder]

Ping

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/ByteCodeEmitter.cpp

void ByteCodeEmitter::emitLabel(LabelTy Label) {
   const size_t Target = Code.size();
   LabelOffsets.insert({Label, Target});
   auto It = LabelRelocs.find(Label);
   if (It != LabelRelocs.end()) {
     for (unsigned Reloc : It->second) {
       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);
     }
     LabelRelocs.erase(It);
   }
 }
 
 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);
   if (It != LabelOffsets.end()) {
     return It->second - Position;
   }
 
   // Otherwise, record relocation and return dummy offset.
static void emit(Program &P, std::vector<char> &Code, const T &Val,
   else
     Size = sizeof(T);
 
   if (Code.size() + Size > std::numeric_limits<unsigned>::max()) {
     Success = false;
     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);
   }
 }
 
 template <typename... Tys>
 bool ByteCodeEmitter::emitOp(Opcode Op, const Tys &... Args, const SourceInfo &SI) {
   bool Success = true;
 
   /// The opcode is followed by arguments. The source info is

clang/lib/AST/Interp/PrimType.h

 /// Returns the size of a primitive type in bytes.
 size_t primSize(PrimType Type);
 
 /// Aligns a size to the pointer alignment.
 constexpr size_t align(size_t Size) {
   return ((Size + alignof(void *) - 1) / alignof(void *)) * alignof(void *);
 }
 
+constexpr bool aligned(uintptr_t Value) { return Value == align(Value); }
+static_assert(aligned(sizeof(void *)));
+
+static inline bool aligned(const void *P) {
+  return aligned(reinterpret_cast<uintptr_t>(P));
+}
+
 inline bool isPrimitiveIntegral(PrimType Type) {
   switch (Type) {
   case PT_Bool:
   case PT_Sint8:
   case PT_Uint8:
   case PT_Sint16:
   case PT_Uint16:
   case PT_Sint32:

clang/lib/AST/Interp/Source.h

 //===--- Source.h - Source location provider for the VM  --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Defines a program which organises and links multiple bytecode functions.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 
 namespace clang {
 namespace interp {
 class Function;
 
public:
   }
 
   bool operator!=(const CodePtr &RHS) const { return Ptr != RHS.Ptr; }
 
   operator bool() const { return Ptr; }
 
   /// 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;
   }
 
 private:
   /// Constructor used by Function to generate pointers.
   CodePtr(const char *Ptr) : Ptr(Ptr) {}
 
 private: