interpreter: Raise SIGILL on undefined instruction
Test: berberis_run_host_tests
Bug: 300687876
Change-Id: If55154d1ff2597982f4d4f464b6b5373f6eb6e3d
diff --git a/interpreter/riscv64/interpreter.cc b/interpreter/riscv64/interpreter.cc
index 8906176..82a575b 100644
--- a/interpreter/riscv64/interpreter.cc
+++ b/interpreter/riscv64/interpreter.cc
@@ -33,6 +33,7 @@
#include "berberis/intrinsics/intrinsics_float.h"
#include "berberis/intrinsics/type_traits.h"
#include "berberis/kernel_api/run_guest_syscall.h"
+#include "berberis/runtime_primitives/interpret_helpers.h"
#include "berberis/runtime_primitives/memory_region_reservation.h"
#include "berberis/runtime_primitives/recovery_code.h"
@@ -694,18 +695,11 @@
void Nop() {}
void Unimplemented() {
- auto* addr = ToHostAddr<const uint16_t>(GetInsnAddr());
- uint8_t size = Decoder::GetInsnSize(addr);
- if (size == 2) {
- FATAL("Unimplemented riscv64 instruction 0x%" PRIx16 " at %p", *addr, addr);
- } else {
- CHECK_EQ(size, 4);
- // Warning: do not cast and dereference the pointer
- // since the address may not be 4-bytes aligned.
- uint32_t code;
- memcpy(&code, addr, sizeof(code));
- FATAL("Unimplemented riscv64 instruction 0x%" PRIx32 " at %p", code, addr);
- }
+ UndefinedInsn(GetInsnAddr());
+ // If there is a guest handler registered for SIGILL we'll delay its processing until the next
+ // sync point (likely the main dispatching loop) due to enabled pending signals. Thus we must
+ // ensure that insn_addr isn't automatically advanced in FinalizeInsn.
+ exception_raised_ = true;
}
//
@@ -805,8 +799,10 @@
ThreadState* state_;
bool branch_taken_;
- // This flag is set by faulted memory accesses. Faulted memory accesses may result in having more
- // operations with side effects called before the end of the current instruction:
+ // This flag is set by illegal instructions and faulted memory accesses. The former must always
+ // stop the playback of the current instruction, so we don't need to do anything special. The
+ // latter may result in having more operations with side effects called before the end of the
+ // current instruction:
// Load (faulted) -> SetReg
// LoadFp (faulted) -> NanBoxAndSetFpReg
// If an exception is raised before these operations, we skip them. For all other operations with
diff --git a/runtime_primitives/Android.bp b/runtime_primitives/Android.bp
index 954ba6d..bce02e5 100644
--- a/runtime_primitives/Android.bp
+++ b/runtime_primitives/Android.bp
@@ -102,6 +102,7 @@
srcs: [
"checks_riscv64.cc",
"host_call_frame_riscv64.cc",
+ "interpret_helpers_riscv64.cc",
],
whole_static_libs: [
"libberberis_runtime_primitives",
diff --git a/runtime_primitives/include/berberis/runtime_primitives/interpret_helpers.h b/runtime_primitives/include/berberis/runtime_primitives/interpret_helpers.h
new file mode 100644
index 0000000..355752d
--- /dev/null
+++ b/runtime_primitives/include/berberis/runtime_primitives/interpret_helpers.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef BERBERIS_RUNTIME_PRIMITIVES_INTERPRET_HELPERS_H_
+#define BERBERIS_RUNTIME_PRIMITIVES_INTERPRET_HELPERS_H_
+
+#include "berberis/guest_state/guest_addr.h"
+
+namespace berberis {
+
+void UndefinedInsn(GuestAddr pc);
+
+} // namespace berberis
+
+#endif // BERBERIS_RUNTIME_PRIMITIVES_INTERPRET_HELPERS_H_
diff --git a/runtime_primitives/interpret_helpers_riscv64.cc b/runtime_primitives/interpret_helpers_riscv64.cc
new file mode 100644
index 0000000..bce36e4
--- /dev/null
+++ b/runtime_primitives/interpret_helpers_riscv64.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "berberis/runtime_primitives/interpret_helpers.h"
+
+#include <csignal>
+#include <cstdint>
+
+#include "berberis/base/checks.h"
+#include "berberis/base/logging.h"
+#include "berberis/guest_state/guest_addr.h"
+
+namespace berberis {
+
+namespace {
+
+uint8_t GetRiscv64InsnSize(GuestAddr pc) {
+ constexpr uint16_t kInsnLenMask = uint16_t{0b11};
+ if ((*ToHostAddr<const uint16_t>(pc) & kInsnLenMask) != kInsnLenMask) {
+ return 2;
+ }
+ return 4;
+}
+
+} // namespace
+
+void UndefinedInsn(GuestAddr pc) {
+ auto* addr = ToHostAddr<const uint16_t>(pc);
+ uint8_t size = GetRiscv64InsnSize(pc);
+ if (size == 2) {
+ ALOGE("Unimplemented riscv64 instruction 0x%" PRIx16 " at %p", *addr, addr);
+ } else {
+ CHECK_EQ(size, 4);
+ // Warning: do not cast and dereference the pointer since the address may not be 4-bytes
+ // aligned.
+ uint32_t code;
+ memcpy(&code, addr, sizeof(code));
+ ALOGE("Unimplemented riscv64 instruction 0x%" PRIx32 " at %p", code, addr);
+ }
+ raise(SIGILL);
+}
+
+} // namespace berberis
diff --git a/tests/ndk_program_tests/Android.bp b/tests/ndk_program_tests/Android.bp
index eaffe43..3bbd593 100644
--- a/tests/ndk_program_tests/Android.bp
+++ b/tests/ndk_program_tests/Android.bp
@@ -51,6 +51,13 @@
],
}
+filegroup {
+ name: "berberis_ndk_program_tests_riscv64_srcs",
+ srcs: [
+ "riscv64/sigill_test.cc",
+ ],
+}
+
cc_defaults {
name: "berberis_ndk_program_tests_defaults",
native_bridge_supported: true,
@@ -63,6 +70,11 @@
"-Wno-deprecated-declarations",
],
srcs: [":berberis_ndk_program_tests_srcs"],
+ arch: {
+ riscv64: {
+ srcs: [":berberis_ndk_program_tests_riscv64_srcs"],
+ },
+ },
header_libs: ["libberberis_ndk_program_tests_headers"],
static_libs: ["libgtest"],
}
diff --git a/tests/ndk_program_tests/riscv64/sigill_test.cc b/tests/ndk_program_tests/riscv64/sigill_test.cc
new file mode 100644
index 0000000..c8b8d45
--- /dev/null
+++ b/tests/ndk_program_tests/riscv64/sigill_test.cc
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "gtest/gtest.h"
+
+#include <setjmp.h>
+#include <signal.h>
+#include <sys/ucontext.h>
+
+#include "berberis/ndk_program_tests/scoped_sigaction.h"
+
+namespace {
+
+sigjmp_buf g_recover_riscv64;
+
+extern "C" char g_illegal_instruction_riscv64;
+
+void SigillSignalHandlerRiscv64(int /* sig */, siginfo_t* /* info */, void* ctx) {
+ fprintf(stderr, "SIGILL caught\n");
+ // Warning: Do not use ASSERT, so that we recover with longjmp unconditionally.
+ // Otherwise we'll be looping executing illegal instruction.
+ EXPECT_EQ(static_cast<ucontext*>(ctx)->uc_mcontext.__gregs[REG_PC],
+ reinterpret_cast<greg_t>(&g_illegal_instruction_riscv64));
+ longjmp(g_recover_riscv64, 1);
+}
+
+TEST(Signal, SigillRiscv64) {
+ struct sigaction sa;
+ sa.sa_flags = SA_SIGINFO;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_sigaction = SigillSignalHandlerRiscv64;
+ ScopedSigaction scoped_sa(SIGILL, &sa);
+
+ if (setjmp(g_recover_riscv64) == 0) {
+ fprintf(stderr, "Executing invalid RISC-V instruction\n");
+ asm volatile(
+ ".align 8\n"
+ ".globl g_illegal_instruction_riscv64\n"
+ "g_illegal_instruction_riscv64:\n"
+ ".4byte 0x0\n");
+ fprintf(stderr, "Bug, recover from SIGILL shall come as longjmp()\n");
+ FAIL();
+ } else {
+ fprintf(stderr, "Recovered, test passed\n");
+ }
+}
+
+} // namespace
