[clang][bytecode] Support overlapping regions in __builtin_memmove

[tbaederr]

Unfortunately, a few circumstances make the implementation here less than ideal, but we need to handle overlapping regions anyway.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Unfortunately, a few circumstances make the implementation here less than ideal, but we need to handle overlapping regions anyway.

---

Full diff: https://github.com/llvm/llvm-project/pull/132523.diff

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp (+29-12)
• (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+1-2)

diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
index 6b8860c09167c..239b3104e89f1 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
@@ -19,6 +19,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/TargetInfo.h"
 
+#include <variant>
+
 using namespace clang;
 using namespace clang::interp;
 
@@ -450,33 +452,48 @@ bool clang::interp::DoBitCastPtr(InterpState &S, CodePtr OpPC,
   return Success;
 }
 
+using PrimTypeVariant =
+    std::variant<Pointer, FunctionPointer, MemberPointer, FixedPoint,
+                 Integral<8, false>, Integral<8, true>, Integral<16, false>,
+                 Integral<16, true>, Integral<32, false>, Integral<32, true>,
+                 Integral<64, false>, Integral<64, true>, IntegralAP<true>,
+                 IntegralAP<false>, Boolean, Floating>;
+
+// NB: This implementation isn't exactly ideal, but:
+//   1) We can't just do a bitcast here since we need to be able to
+//      copy pointers.
+//   2) This also needs to handle overlapping regions.
+//   3) We currently have no way of iterating over the fields of a pointer
+//      backwards.
 bool clang::interp::DoMemcpy(InterpState &S, CodePtr OpPC,
                              const Pointer &SrcPtr, const Pointer &DestPtr,
                              Bits Size) {
   assert(SrcPtr.isBlockPointer());
   assert(DestPtr.isBlockPointer());
 
-  unsigned SrcStartOffset = SrcPtr.getByteOffset();
-  unsigned DestStartOffset = DestPtr.getByteOffset();
-
+  llvm::SmallVector<PrimTypeVariant> Values;
   enumeratePointerFields(SrcPtr, S.getContext(), Size,
                          [&](const Pointer &P, PrimType T, Bits BitOffset,
                              Bits FullBitWidth, bool PackedBools) -> bool {
-                           unsigned SrcOffsetDiff =
-                               P.getByteOffset() - SrcStartOffset;
-
-                           Pointer DestP =
-                               Pointer(DestPtr.asBlockPointer().Pointee,
-                                       DestPtr.asBlockPointer().Base,
-                                       DestStartOffset + SrcOffsetDiff);
+                           TYPE_SWITCH(T, { Values.push_back(P.deref<T>()); });
+                           return true;
+                         });
 
+  unsigned ValueIndex = 0;
+  enumeratePointerFields(DestPtr, S.getContext(), Size,
+                         [&](const Pointer &P, PrimType T, Bits BitOffset,
+                             Bits FullBitWidth, bool PackedBools) -> bool {
                            TYPE_SWITCH(T, {
-                             DestP.deref<T>() = P.deref<T>();
-                             DestP.initialize();
+                             P.deref<T>() = std::get<T>(Values[ValueIndex]);
+                             P.initialize();
                            });
 
+                           ++ValueIndex;
                            return true;
                          });
 
+  // We should've read all the values into DestPtr.
+  assert(ValueIndex == Values.size());
+
   return true;
 }
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp
index 3dd348031fec1..13a34f71a6354 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -1276,7 +1276,6 @@ namespace BuiltinMemcpy {
   static_assert(test_incomplete_array_type() == 1234); // both-error {{constant}} both-note {{in call}}
 
 
-  /// FIXME: memmove needs to support overlapping memory regions.
   constexpr bool memmoveOverlapping() {
     char s1[] {1, 2, 3};
     __builtin_memmove(s1, s1 + 1, 2 * sizeof(char));
@@ -1289,7 +1288,7 @@ namespace BuiltinMemcpy {
 
     return Result1 && Result2;
   }
-  static_assert(memmoveOverlapping()); // expected-error {{failed}}
+  static_assert(memmoveOverlapping());
 }
 
 namespace Memcmp {