Skip to content

[clang][bytecode] Fix memcmp/bcmp failures on big-endian hosts #119851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

tbaederr
Copy link
Contributor

See the discussion in
#119678 (comment) and
#119544 (comment)

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

See the discussion in
#119678 (comment) and
#119544 (comment)


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

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+8)
  • (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp (-5)
  • (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.h (+5)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index a343280b5ce50f..172e26332474c0 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1943,10 +1943,18 @@ static bool interp__builtin_memcmp(InterpState &S, CodePtr OpPC,
   BitcastBuffer BufferA(
       Bits(S.getASTContext().getTypeSize(PtrA.getFieldDesc()->getType())));
   readPointerToBuffer(S.getContext(), PtrA, BufferA, false);
+  // FIXME: The swapping here is UNDOING something we do when reading the
+  // data into the buffer.
+  if (S.getASTContext().getTargetInfo().isBigEndian())
+    swapBytes(BufferA.Data.get(), BufferA.byteSize().getQuantity());
 
   BitcastBuffer BufferB(
       Bits(S.getASTContext().getTypeSize(PtrB.getFieldDesc()->getType())));
   readPointerToBuffer(S.getContext(), PtrB, BufferB, false);
+  // FIXME: The swapping here is UNDOING something we do when reading the
+  // data into the buffer.
+  if (S.getASTContext().getTargetInfo().isBigEndian())
+    swapBytes(BufferB.Data.get(), BufferB.byteSize().getQuantity());
 
   size_t MinBufferSize = std::min(BufferA.byteSize().getQuantity(),
                                   BufferB.byteSize().getQuantity());
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
index c87993b8739a77..07f76943708216 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
@@ -73,11 +73,6 @@ using DataFunc =
     }                                                                          \
   } while (0)
 
-static void swapBytes(std::byte *M, size_t N) {
-  for (size_t I = 0; I != (N / 2); ++I)
-    std::swap(M[I], M[N - 1 - I]);
-}
-
 /// We use this to recursively iterate over all fields and elements of a pointer
 /// and extract relevant data for a bitcast.
 static bool enumerateData(const Pointer &P, const Context &Ctx, Bits Offset,
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
index 08c207c7415dfa..b45613b2f21e20 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.h
@@ -19,6 +19,11 @@ class InterpState;
 class CodePtr;
 class Context;
 
+inline static void swapBytes(std::byte *M, size_t N) {
+  for (size_t I = 0; I != (N / 2); ++I)
+    std::swap(M[I], M[N - 1 - I]);
+}
+
 bool DoBitCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
                std::byte *Buff, Bits BitWidth, Bits FullBitWidth,
                bool &HasIndeterminateBits);

@tbaederr tbaederr merged commit 30cbd09 into llvm:main Dec 13, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants