Skip to content
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

[clang-repl] Fix unboxing of va_args Value on 32-bit ARM (#94994) #96900

Conversation

weliveindetail
Copy link
Contributor

todo

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

llvmbot commented Jun 27, 2024

@llvm/pr-subscribers-clang

Author: Stefan Gränitz (weliveindetail)

Changes

todo


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

3 Files Affected:

  • (modified) clang/include/clang/Interpreter/Value.h (+2)
  • (modified) clang/lib/Interpreter/Interpreter.cpp (+30-1)
  • (modified) clang/unittests/Interpreter/InterpreterTest.cpp (-2)
diff --git a/clang/include/clang/Interpreter/Value.h b/clang/include/clang/Interpreter/Value.h
index d70e8f8719026..f1dcae2d672c0 100644
--- a/clang/include/clang/Interpreter/Value.h
+++ b/clang/include/clang/Interpreter/Value.h
@@ -98,6 +98,8 @@ class REPL_EXTERNAL_VISIBILITY Value {
     void *m_Ptr;
   };
 
+  static_assert(sizeof(Storage) <= 2 * sizeof(void *), "va_args");
+
 public:
   enum Kind {
 #define X(type, name) K_##name,
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 7a95278914276..11f3273dff45f 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -877,7 +877,36 @@ extern "C" void REPL_EXTERNAL_VISIBILITY __clang_Interpreter_SetValueNoAlloc(
   } else {
     if (const auto *ET = QT->getAs<EnumType>())
       QT = ET->getDecl()->getIntegerType();
-    switch (QT->castAs<BuiltinType>()->getKind()) {
+
+    BuiltinType::Kind K = QT->castAs<BuiltinType>()->getKind();
+  #ifdef __arm__
+    switch (K) {
+    default:
+      llvm_unreachable("unknown type kind!");
+    case BuiltinType::Bool:
+    case BuiltinType::SChar:
+    case BuiltinType::Char_S:
+    case BuiltinType::Char_U:
+    case BuiltinType::UChar:
+    case BuiltinType::Short:
+    case BuiltinType::UShort:
+    case BuiltinType::Int:
+    case BuiltinType::UInt:
+    case BuiltinType::Long:
+    case BuiltinType::ULong:
+    case BuiltinType::LongLong:
+    case BuiltinType::ULongLong:
+    case BuiltinType::Float:
+    case BuiltinType::Double:
+      // Consume unused leading 32-bit of storage
+      va_arg(args, int);
+
+    case BuiltinType::LongDouble:
+      break;
+    }
+  #endif
+
+    switch (K) {
     default:
       llvm_unreachable("unknown type kind!");
       break;
diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp
index bbd854149d5f5..37978b09adad2 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -284,7 +284,6 @@ TEST_F(InterpreterTest, InstantiateTemplate) {
 
 // This test exposes an ARM specific problem in the interpreter, see
 // https://github.com/llvm/llvm-project/issues/94994.
-#ifndef __arm__
 TEST_F(InterpreterTest, Value) {
   std::vector<const char *> Args = {"-fno-sized-deallocation"};
   std::unique_ptr<Interpreter> Interp = createInterpreter(Args);
@@ -383,6 +382,5 @@ TEST_F(InterpreterTest, Value) {
   EXPECT_EQ(V9.getKind(), Value::K_PtrOrObj);
   EXPECT_TRUE(V9.isManuallyAlloc());
 }
-#endif /* ifndef __arm__ */
 
 } // end anonymous namespace

@weliveindetail
Copy link
Contributor Author

Superseded by #97071

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