[PATCH] D124571: Avoid strict aliasing violation on type punning inside llvm::PointerIntPair

[Breno Rodrigues Guimaraes via Phabricator via llvm-commits]

brenoguim updated this revision to Diff 426881.
brenoguim added a comment.

Changed the underlying storage of PunnedPointer to an array of char. It doesn't trigger the bug in clang (v12, v13, v14) and seems safer with regards to UB.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124571/new/

https://reviews.llvm.org/D124571

Files:
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/unittests/ADT/PointerIntPairTest.cpp


Index: llvm/unittests/ADT/PointerIntPairTest.cpp
===================================================================
--- llvm/unittests/ADT/PointerIntPairTest.cpp
+++ llvm/unittests/ADT/PointerIntPairTest.cpp
@@ -106,4 +106,22 @@
       "trivially copyable");
 }
 
+TEST(PointerIntPairTest, TypePunning) {
+  int I = 0;
+  int *IntPtr = &I;
+
+  int **IntPtrBegin = &IntPtr;
+  int **IntPtrEnd = IntPtrBegin + 1;
+
+  PointerIntPair<int *, 1> Pair;
+  int **PairAddr = Pair.getAddrOfPointer();
+
+  while (IntPtrBegin != IntPtrEnd) {
+    *PairAddr = *IntPtrBegin;
+    ++PairAddr;
+    ++IntPtrBegin;
+  }
+  EXPECT_EQ(Pair.getPointer(), IntPtr);
+}
+
 } // end anonymous namespace
Index: llvm/include/llvm/ADT/PointerIntPair.h
===================================================================
--- llvm/include/llvm/ADT/PointerIntPair.h
+++ llvm/include/llvm/ADT/PointerIntPair.h
@@ -19,10 +19,38 @@
 #include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <cstdint>
+#include <cstring>
 #include <limits>
 
 namespace llvm {
 
+namespace detail {
+template <typename Ptr> struct PunnedPointer {
+  static_assert(sizeof(Ptr) == sizeof(intptr_t), "");
+
+  explicit constexpr PunnedPointer(intptr_t i = 0) { *this = i; }
+
+  constexpr intptr_t asInt() const {
+    intptr_t R = 0;
+    std::memcpy(&R, Data, sizeof(R));
+    return R;
+  }
+
+  constexpr operator intptr_t() const { return asInt(); }
+
+  constexpr PunnedPointer &operator=(intptr_t V) {
+    std::memcpy(Data, &V, sizeof(Data));
+    return *this;
+  }
+
+  Ptr *getPointerAddress() { return reinterpret_cast<Ptr *>(Data); }
+  const Ptr *getPointerAddress() const { return reinterpret_cast<Ptr *>(Data); }
+
+private:
+  alignas(Ptr) unsigned char Data[sizeof(Ptr)];
+};
+} // namespace detail
+
 template <typename T, typename Enable> struct DenseMapInfo;
 template <typename PointerT, unsigned IntBits, typename PtrTraits>
 struct PointerIntPairInfo;
@@ -46,7 +74,7 @@
 class PointerIntPair {
   // Used by MSVC visualizer and generally helpful for debugging/visualizing.
   using InfoTy = Info;
-  intptr_t Value = 0;
+  detail::PunnedPointer<PointerTy> Value;
 
 public:
   constexpr PointerIntPair() = default;
@@ -86,10 +114,12 @@
     assert(Value == reinterpret_cast<intptr_t>(getPointer()) &&
            "Can only return the address if IntBits is cleared and "
            "PtrTraits doesn't change the pointer");
-    return reinterpret_cast<PointerTy *>(&Value);
+    return Value.getPointerAddress();
   }
 
-  void *getOpaqueValue() const { return reinterpret_cast<void *>(Value); }
+  void *getOpaqueValue() const {
+    return reinterpret_cast<void *>(Value.asInt());
+  }
 
   void setFromOpaqueValue(void *Val) & {
     Value = reinterpret_cast<intptr_t>(Val);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124571.426881.patch
Type: text/x-patch
Size: 2763 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220504/fc1eefc5/attachment-0001.bin>