Add proper abstraction for the inlined dynamic array inside the AbstractMemoryLocationImpl

Author: fabianbs96

include/phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocation.h

@@ -24,6 +24,7 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Value.h"
+#include "llvm/Support/TrailingObjects.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "phasar/PhasarLLVM/DataFlowSolver/IfdsIde/LLVMZeroValue.h"
@@ -34,21 +35,14 @@ namespace psr {
 
 namespace detail {
 
-struct AbstractMemoryLoactionStorage : public llvm::FoldingSetNode {
+struct AbstractMemoryLocationStorage : public llvm::FoldingSetNode {
   const llvm::Value *Baseptr;
   uint32_t Lifetime;
   uint32_t NumOffsets;
-  /// The actual length of Offsets is a runtime-constant determined by
-  /// NumOffsets. Note, that NumOffsets can be larger than 1
-  ptrdiff_t Offsets[1]; // NOLINT
 
 protected:
-  AbstractMemoryLoactionStorage(
-      const llvm::Value *Baseptr, uint32_t Lifetime,
-      const llvm::ArrayRef<ptrdiff_t> &Offsets) noexcept;
-
-  AbstractMemoryLoactionStorage(const llvm::Value *Baseptr,
-                                uint32_t Lifetime) noexcept;
+  AbstractMemoryLocationStorage(const llvm::Value *Baseptr, uint32_t Lifetime,
+                                uint32_t NumOffsets = 0) noexcept;
 };
 
 /// \brief A Memorylocation abstraction represented by a base-pointer and an
@@ -57,7 +51,14 @@ struct AbstractMemoryLoactionStorage : public llvm::FoldingSetNode {
 /// The byte offsets are applied to the base-pointer alternating
 /// with memory loads in order to represent indirect memory locations in a
 /// canonical way.
-class AbstractMemoryLocationImpl final : public AbstractMemoryLoactionStorage {
+class AbstractMemoryLocationImpl final
+    : public AbstractMemoryLocationStorage,
+      private llvm::TrailingObjects<AbstractMemoryLocationImpl, ptrdiff_t> {
+
+  // The factory is responsible for allocation, so it needs access to the
+  // private inherited members from llvm::TrailingObjects
+  friend class AbstractMemoryLocationFactoryBase;
+  friend TrailingObjects;
 
   [[nodiscard]] bool
   equivalentOffsets(const AbstractMemoryLocationImpl &TV) const;

include/phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocationFactory.h

@@ -17,6 +17,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/TrailingObjects.h"
 
 #include "phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocation.h"
 
@@ -35,9 +36,9 @@ class AbstractMemoryLocationFactoryBase {
 
 private:
   struct Allocator {
-    struct Block {
+    struct Block final : public llvm::TrailingObjects<Block, void *> {
+
       Block *Next = nullptr;
-      void *Data[1]; // NOLINT
 
       static Block *create(Block *Next, size_t NumPointerEntries);
       static void destroy(Block *Blck);
@@ -65,7 +66,7 @@ class AbstractMemoryLocationFactoryBase {
   private:
     constexpr static size_t ExpectedNumAmLsPerBlock = 1024;
     constexpr static size_t MinNumPointersPerAML =
-        offsetof(AbstractMemoryLocationImpl, Offsets) / sizeof(void *);
+        sizeof(AbstractMemoryLocationImpl) / sizeof(void *);
     constexpr static size_t NumPointersPerBlock =
         (MinNumPointersPerAML + 3) * ExpectedNumAmLsPerBlock;
   };

lib/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocation.cpp

@@ -10,6 +10,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include <cstddef>
 
 #include "phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocation.h"
 #include "phasar/PhasarLLVM/Utils/BasicBlockOrdering.h"
@@ -19,42 +20,37 @@
 
 namespace psr::detail {
 
-AbstractMemoryLoactionStorage::AbstractMemoryLoactionStorage(
-    const llvm::Value *Baseptr, uint32_t Lifetime,
-    const llvm::ArrayRef<ptrdiff_t> &Offsets) noexcept
-    : Baseptr(Baseptr), Lifetime(Lifetime),
-      NumOffsets(uint32_t(Offsets.size())) {
-  assert(Baseptr && "The baseptr must not be null!");
-  memcpy(this->Offsets, Offsets.begin(), Offsets.size() * sizeof(ptrdiff_t));
-}
-
-AbstractMemoryLoactionStorage::AbstractMemoryLoactionStorage(
-    const llvm::Value *Baseptr, uint32_t Lifetime) noexcept
-    : Baseptr(Baseptr), Lifetime(Lifetime), NumOffsets(0) {
+AbstractMemoryLocationStorage::AbstractMemoryLocationStorage(
+    const llvm::Value *Baseptr, uint32_t Lifetime, uint32_t NumOffsets) noexcept
+    : Baseptr(Baseptr), Lifetime(Lifetime), NumOffsets(NumOffsets) {
   assert(Baseptr && "The baseptr must not be null!");
 }
 
 AbstractMemoryLocationImpl::AbstractMemoryLocationImpl()
-    : AbstractMemoryLoactionStorage(LLVMZeroValue::getInstance(), 0) {}
+    : AbstractMemoryLocationStorage(LLVMZeroValue::getInstance(), 0) {}
 
 AbstractMemoryLocationImpl::AbstractMemoryLocationImpl(
     const llvm::Value *Baseptr, unsigned Lifetime) noexcept
-    : AbstractMemoryLoactionStorage(Baseptr, Lifetime) {}
+    : AbstractMemoryLocationStorage(Baseptr, Lifetime) {}
 AbstractMemoryLocationImpl::AbstractMemoryLocationImpl(
     const llvm::Value *Baseptr, llvm::SmallVectorImpl<ptrdiff_t> &&Offsets,
     unsigned Lifetime) noexcept
-    : AbstractMemoryLoactionStorage(Baseptr, Lifetime, Offsets) {}
+    : AbstractMemoryLocationImpl(Baseptr, llvm::makeArrayRef(Offsets),
+                                 Lifetime) {}
 AbstractMemoryLocationImpl::AbstractMemoryLocationImpl(
     const llvm::Value *Baseptr, llvm::ArrayRef<ptrdiff_t> Offsets,
     unsigned Lifetime) noexcept
-    : AbstractMemoryLoactionStorage(Baseptr, Lifetime, Offsets) {}
+    : AbstractMemoryLocationStorage(Baseptr, Lifetime, Offsets.size()) {
+  memcpy(this->getTrailingObjects<ptrdiff_t>(), Offsets.data(),
+         Offsets.size() * sizeof(ptrdiff_t));
+}
 
 bool AbstractMemoryLocationImpl::isZero() const {
   return LLVMZeroValue::getInstance()->isLLVMZeroValue(Baseptr);
 }
 
 llvm::ArrayRef<ptrdiff_t> AbstractMemoryLocationImpl::offsets() const {
-  return llvm::makeArrayRef(Offsets, NumOffsets);
+  return llvm::makeArrayRef(this->getTrailingObjects<ptrdiff_t>(), NumOffsets);
 }
 
 auto AbstractMemoryLocationImpl::computeOffset(

lib/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/ExtendedTaintAnalysis/AbstractMemoryLocationFactory.cpp

@@ -9,6 +9,7 @@
 
 #include <exception>
 #include <limits>
+#include <new>
 
 #include "llvm/IR/Instructions.h"
 
@@ -17,9 +18,6 @@
 
 namespace psr::detail {
 
-/// We intentionally don't initialize the Data member as it will be a dynamic
-/// array at runtime
-// NOLINTNEXTLINE (cppcoreguidelines-pro-type-member-init)
 AbstractMemoryLocationFactoryBase::Allocator::Block::Block(Block *Next)
     : Next(Next) {}
 
@@ -37,19 +35,16 @@ auto AbstractMemoryLocationFactoryBase::Allocator::Block::create(
     std::terminate();
   }
 
-  static_assert(
-      alignof(AbstractMemoryLocationImpl) == alignof(size_t),
-      "The alignment of the AbstractMemoryLocationImpl allocation cannot be "
-      "guaranteed as it differs from the alignment of size_t");
-
-  auto *Ret = reinterpret_cast<Block *>(new size_t[1 + NumPointerEntries]);
+  auto *Ret = reinterpret_cast<Block *>(new (std::align_val_t{
+      alignof(AbstractMemoryLocationImpl)}) size_t[1 + NumPointerEntries]);
 
   new (Ret) Block(Next);
   return Ret;
 }
 
 void AbstractMemoryLocationFactoryBase::Allocator::Block::destroy(Block *Blck) {
-  delete[] reinterpret_cast<size_t *>(Blck);
+  ::operator delete (Blck,
+                     std::align_val_t{alignof(AbstractMemoryLocationImpl)});
 }
 
 AbstractMemoryLocationFactoryBase::Allocator::Allocator(
@@ -61,7 +56,7 @@ AbstractMemoryLocationFactoryBase::Allocator::Allocator(
   const auto NumPointersPerInitialBlock =
       (MinNumPointersPerAML + 3) * InitialCapacity;
   Root = Block::create(nullptr, NumPointersPerInitialBlock);
-  Pos = Root->Data;
+  Pos = Root->getTrailingObjects<void *>();
   End = Pos + NumPointersPerInitialBlock;
 }
 
@@ -98,13 +93,16 @@ AbstractMemoryLocationFactoryBase::Allocator::create(
     llvm::ArrayRef<ptrdiff_t> Offsets) {
 
   // All fields inside AML have pointer size, so there is no padding at all
-  auto NumPointersRequired = MinNumPointersPerAML + Offsets.size();
+
+  auto NumPointersRequired =
+      AbstractMemoryLocationImpl::totalSizeToAlloc<ptrdiff_t>(Offsets.size()) /
+      sizeof(void *);
   auto *Rt = Root;
   auto *Curr = Pos;
 
   if (End - Curr < ptrdiff_t(NumPointersRequired)) {
     Root = Rt = Block::create(Rt, NumPointersPerBlock);
-    Pos = Curr = Rt->Data;
+    Pos = Curr = Rt->getTrailingObjects<void *>();
     End = Curr + NumPointersPerBlock;
   }