Skip to content

[clang][bytecode] Fix bitcasting from null pointers #116999

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 5, 2024

Conversation

tbaederr
Copy link
Contributor

This looks a little ugly now, I should wait for #116843.

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

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

This looks a little ugly now, I should wait for #116843.


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

2 Files Affected:

  • (modified) clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp (+12-2)
  • (modified) clang/test/AST/ByteCode/builtin-bit-cast.cpp (+2)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
index 7e8853d3469317..a5de8c3f6dcae3 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltinBitCast.cpp
@@ -299,6 +299,9 @@ static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
                    llvm::sys::IsLittleEndianHost);
   bool BigEndianTarget = ASTCtx.getTargetInfo().isBigEndian();
 
+  uint64_t PointerSizeInBits =
+      ASTCtx.getTargetInfo().getPointerWidth(LangAS::Default);
+
   return enumeratePointerFields(
       FromPtr, Ctx,
       [&](const Pointer &P, PrimType T, size_t BitOffset,
@@ -310,8 +313,15 @@ static bool readPointerToBuffer(const Context &Ctx, const Pointer &FromPtr,
 
         assert(P.isInitialized());
         // nullptr_t is a PT_Ptr for us, but it's still not std::is_pointer_v.
-        if (T == PT_Ptr)
-          assert(false && "Implement casting to pointer types");
+        if (T == PT_Ptr) {
+          assert(P.getType()->isNullPtrType());
+          std::byte Zeroes[] = {std::byte{0}, std::byte{0}, std::byte{0},
+                                std::byte{0}, std::byte{0}, std::byte{0},
+                                std::byte{0}, std::byte{0}};
+          assert(PointerSizeInBits <= (8 * 8));
+          Buffer.pushData(Zeroes, PointerSizeInBits, BigEndianTarget);
+          return true;
+        }
 
         CharUnits ObjectReprChars = ASTCtx.getTypeSizeInChars(P.getType());
         unsigned BitWidth = ASTCtx.toBits(ObjectReprChars);
diff --git a/clang/test/AST/ByteCode/builtin-bit-cast.cpp b/clang/test/AST/ByteCode/builtin-bit-cast.cpp
index 60e8c3a615c5e6..fbaff4ba226d58 100644
--- a/clang/test/AST/ByteCode/builtin-bit-cast.cpp
+++ b/clang/test/AST/ByteCode/builtin-bit-cast.cpp
@@ -135,6 +135,8 @@ namespace simple {
   /// diagnoses it.
   static_assert(__builtin_bit_cast(intptr_t, nullptr) == 0); // ref-error {{not an integral constant expression}} \
                                                              // ref-note {{indeterminate value can only initialize an object}}
+
+  constexpr int test_from_nullptr_pass = (__builtin_bit_cast(unsigned char[8], nullptr), 0);
 }
 
 namespace Fail {

@tbaederr
Copy link
Contributor Author

tbaederr commented Nov 22, 2024

This needs to incorporate the resolution from #117166, but that doesn't work properly without #116843 merged first.

This looks a little ugly now, I should wait for llvm#116843.
@tbaederr tbaederr force-pushed the builtin-bit-cast-14 branch from c316394 to c9c39e8 Compare December 5, 2024 11:02
@tbaederr tbaederr merged commit b6217f6 into llvm:main Dec 5, 2024
8 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