Merge pull request #150 from fwsGonzo/bintr_safe_tcc

bintr: Make libtcc pass all unit tests

.github/workflows/unittests_libtcc.yml

@@ -0,0 +1,34 @@
+name: libtcc Bintr Unit Tests
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: ${{github.workspace}}/tests/unit
+
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Install dependencies
+      run: |
+        sudo apt update
+        sudo apt install -y gcc-12-riscv64-linux-gnu g++-12-riscv64-linux-gnu libtcc-dev tcc
+        git submodule update --init ${{github.workspace}}/tests/Catch2
+        git submodule update --init ${{github.workspace}}/tests/unit/ext/lodepng
+
+    - name: Configure
+      run: cmake -B ${{github.workspace}}/build -DRISCV_THREADED=ON -DRISCV_BINARY_TRANSLATION=ON -DRISCV_LIBTCC=ON -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}
+
+    - name: Build the unittests
+      run: cmake --build ${{github.workspace}}/build --parallel 4
+
+    - name: Run tests
+      working-directory: ${{github.workspace}}/build
+      run: CFLAGS=-O0 ctest --verbose . -j4

lib/CMakeLists.txt

@@ -46,7 +46,7 @@ if (RISCV_EXPERIMENTAL)
 	# executing RISC-V guest functions in parallel using a custom API.
 	option(RISCV_MULTIPROCESS        "Enable multiprocessing" OFF)
 endif()
-if (RISCV_EXPERIMENTAL AND RISCV_BINARY_TRANSLATION)
+if (RISCV_BINARY_TRANSLATION)
 	# LIBTCC will embed the TCC compiler library, using it for binary translation.
 	option(RISCV_LIBTCC              "Enable binary translation with libtcc" OFF)
 endif()

lib/libriscv/cpu.hpp

@@ -143,6 +143,12 @@ namespace riscv
 		// Retrieve default handler for unimplemented instructions (can be returned in on_unimplemented_instruction)
 		static const instruction_t& get_unimplemented_instruction() noexcept;
 
+		// Set current exception
+		void set_current_exception(unsigned exception) noexcept { m_current_exception = exception + 1; }
+		void clear_current_exception() noexcept { m_current_exception = 0; }
+		bool has_current_exception() const noexcept { return m_current_exception != 0; }
+		exceptions current_exception() const noexcept { return exceptions(m_current_exception-1); }
+
 	private:
 		Registers<W> m_regs;
 		Machine<W>&  m_machine;
@@ -155,6 +161,9 @@ namespace riscv
 
 		const unsigned m_cpuid;
 
+		// The current exception (used by eg. TCC which doesn't create unwinding tables)
+		unsigned m_current_exception = 0;
+
 		// The default execute fault simply triggers the exception
 		execute_fault_t m_fault = [] (auto& cpu, auto&) {
 			trigger_exception(EXECUTION_SPACE_PROTECTION_FAULT, cpu.pc());

lib/libriscv/cpu_dispatch.cpp

@@ -345,6 +345,18 @@ INSTRUCTION(RV32I_BC_STOP, rv32i_stop) {
 		goto new_execute_segment;
 
 counter_overflow:
+	if constexpr (libtcc_enabled)
+	{
+		// We need to check if we have a current exception
+		if (CPU().has_current_exception())
+		{
+			// We have an exception, so we need to handle it
+			const auto except_num = CPU().current_exception();
+			CPU().clear_current_exception();
+			CPU().trigger_exception(except_num, CPU().pc());
+		}
+	}
+
 	registers().pc = pc;
 	MACHINE().set_instruction_counter(counter.value());
 

lib/libriscv/tr_api.cpp

@@ -141,6 +141,7 @@ static struct CallbackTable {
 	void (*vec_load)(const CPU*, int, addr_t);
 	void (*vec_store)(const CPU*, addr_t, int);
 	syscall_t* syscalls;
+	int  (*system_call)(CPU*, addr_t);
 	void (*unknown_syscall)(CPU*, addr_t);
 	void (*system)(CPU*, uint32_t);
 	unsigned (*execute)(CPU*, uint32_t);
@@ -168,13 +169,17 @@ static inline int do_syscall(CPU* cpu, uint64_t counter, uint64_t max_counter, a
 {
 	INS_COUNTER(cpu) = counter; // Reveal instruction counters
 	MAX_COUNTER(cpu) = max_counter;
+#ifdef __TINYC__
+	return api.system_call(cpu, sysno);
+#else
 	addr_t old_pc = cpu->pc;
 	if (LIKELY(sysno < RISCV_MAX_SYSCALLS))
 		api.syscalls[SPECSAFE(sysno)](cpu);
 	else
 		api.unknown_syscall(cpu, sysno);
 	// Resume if the system call did not modify PC, or hit a limit
 	return (cpu->pc != old_pc || counter >= MAX_COUNTER(cpu));
+#endif
 }
 
 #define JUMP_TO(cpu, addr) \

lib/libriscv/tr_api.hpp

@@ -12,6 +12,7 @@ namespace riscv {
 		void (*vec_load)(CPU<W>&, int vd, address_type<W> addr);
 		void (*vec_store) (CPU<W>&, address_type<W> addr, int vd);
 		syscall_t<W>* syscalls;
+		int  (*system_call)(CPU<W>&, int);
 		void (*unknown_syscall)(CPU<W>&, address_type<W>);
 		void (*system)(CPU<W>&, uint32_t);
 		unsigned (*execute)(CPU<W>&, uint32_t);

lib/libriscv/tr_emit.cpp

@@ -19,11 +19,17 @@
 // Since it is possible to send a program to another machine, we don't exactly know
 // the order of intruction handlers, so we need to lazily get the handler index by
 // calling the execute function the first time.
+#ifdef RISCV_LIBTCC
+#define UNKNOWN_INSTRUCTION() \
+	code += "if (api.execute(cpu, " + std::to_string(instr.whole) + "))\n" \
+		"  return (ReturnValues){0, 0};\n"; // Exception thrown
+#else
 #define UNKNOWN_INSTRUCTION() { \
 	code += "{ static int handler_idx = 0;\n"; \
 	code += "if (handler_idx) api.handlers[handler_idx](cpu, " + std::to_string(instr.whole) + ");\n"; \
 	code += "else handler_idx = api.execute(cpu, " + std::to_string(instr.whole) + "); }\n"; \
 	}
+#endif
 
 namespace riscv {
 static const std::string LOOP_EXPRESSION = "counter < max_counter";
@@ -1449,7 +1455,7 @@ CPU<W>::emit(std::string& code, const TransInfo<W>& tinfo) const
 			code += "case " + std::to_string(entry.addr) + ": goto " + label + ";\n";
 		}
 		if (tinfo.trace_instructions)
-			code += "default: api.exception(cpu, pc, 3);\n";
+			code += "default: api.exception(cpu, pc, 3); return (ReturnValues){0, 0};\n";
 		code += "}\n";
 	}
 

lib/libriscv/tr_translate.cpp

@@ -605,15 +605,38 @@ bool CPU<W>::initialize_translated_segment(DecodedExecuteSegment<W>&, void* dyli
 	if (ptr == nullptr) {
 		return false;
 	}
+	// 4x8-byte aligned scratch buffer for returning a valid pointer to
+	// the memory read/write functions in case of a fault.
+	static uint64_t scratch_buffer[4];
 
 	// Map the API callback table
 	auto func = (void (*)(const CallbackTable<W>&)) ptr;
 	func(CallbackTable<W>{
 		.mem_read = [] (CPU<W>& cpu, address_type<W> addr) -> const void* {
-			return cpu.machine().memory.cached_readable_page(addr, 1).buffer8.data();
+			if constexpr (libtcc_enabled) {
+				try {
+					return cpu.machine().memory.cached_readable_page(addr, 1).buffer8.data();
+				} catch (MachineException& e) {
+					cpu.set_current_exception(e.type());
+					cpu.machine().stop();
+					return &scratch_buffer[0];
+				}
+			} else {
+				return cpu.machine().memory.cached_readable_page(addr, 1).buffer8.data();
+			}
 		},
 		.mem_write = [] (CPU<W>& cpu, address_type<W> addr) -> void* {
-			return cpu.machine().memory.cached_writable_page(addr).buffer8.data();
+			if constexpr (libtcc_enabled) {
+				try {
+					return cpu.machine().memory.cached_writable_page(addr).buffer8.data();
+				} catch (MachineException& e) {
+					cpu.set_current_exception(e.type());
+					cpu.machine().stop();
+					return &scratch_buffer[0];
+				}
+			} else {
+				return cpu.machine().memory.cached_writable_page(addr).buffer8.data();
+			}
 		},
 		.vec_load = [] (CPU<W>& cpu, int vd, address_type<W> addr) {
 #ifdef RISCV_EXT_VECTOR
@@ -632,22 +655,72 @@ bool CPU<W>::initialize_translated_segment(DecodedExecuteSegment<W>&, void* dyli
 #endif
 		},
 		.syscalls = machine().syscall_handlers.data(),
+		.system_call = [] (CPU<W>& cpu, int sysno) -> int {
+			if constexpr (libtcc_enabled) {
+				try {
+					const auto current_pc = cpu.registers().pc;
+					cpu.machine().system_call(sysno);
+					return cpu.registers().pc != current_pc || cpu.machine().stopped();
+				} catch (const MachineException& e) {
+					cpu.set_current_exception(e.type());
+					cpu.machine().stop();
+					return false;
+				} catch (const std::exception& e) {
+					cpu.set_current_exception(SYSTEM_CALL_FAILED);
+					cpu.machine().stop();
+					return false;
+				}
+			} else {
+				return false;
+			}
+		},
 		.unknown_syscall = [] (CPU<W>& cpu, address_type<W> sysno) {
 			cpu.machine().on_unhandled_syscall(cpu.machine(), sysno);
 		},
 		.system = [] (CPU<W>& cpu, uint32_t instr) {
-			cpu.machine().system(rv32i_instruction{instr});
+			if constexpr (libtcc_enabled) {
+				try {
+					cpu.machine().system(rv32i_instruction{instr});
+				} catch (const MachineException& e) {
+					cpu.set_current_exception(e.type());
+					cpu.machine().stop();
+				} catch (const std::exception& e) {
+					cpu.set_current_exception(SYSTEM_CALL_FAILED);
+					cpu.machine().stop();
+				}
+			} else {
+				cpu.machine().system(rv32i_instruction{instr});
+			}
 		},
 		.execute = [] (CPU<W>& cpu, uint32_t instr) -> unsigned {
 			const rv32i_instruction rvi{instr};
-			auto* handler = cpu.decode(rvi).handler;
-			handler(cpu, rvi);
-			return DecoderData<W>::handler_index_for(handler);
+			if constexpr (libtcc_enabled) {
+				try {
+					cpu.decode(rvi).handler(cpu, rvi);
+					return 0;
+				} catch (const MachineException& e) {
+					cpu.set_current_exception(e.type());
+					return 1;
+				}
+			} else {
+				auto* handler = cpu.decode(rvi).handler;
+				handler(cpu, rvi);
+				return DecoderData<W>::handler_index_for(handler);
+			}
 		},
 		.handlers = (void (**)(CPU<W>&, uint32_t)) DecoderData<W>::get_handlers(),
 		.trigger_exception = [] (CPU<W>& cpu, address_type<W> pc, int e) {
 			cpu.registers().pc = pc; // XXX: Set PC to the failing instruction (?)
-			cpu.trigger_exception(e);
+			if constexpr (libtcc_enabled) {
+				// If we're using libtcc, we can't throw C++ exceptions because
+				// there's no unwinding support. But we can mark an exception
+				// in the CPU state and return back to dispatch.
+				cpu.set_current_exception(e);
+				// Trigger a slow-path in dispatch (which will check for exceptions)
+				cpu.machine().stop();
+			} else {
+				cpu.trigger_exception(e);
+			}
 		},
 		.trace = [] (CPU<W>& cpu, const char* msg, address_type<W> addr, uint32_t instr) {
 			(void)cpu;

tests/unit/native.cpp

@@ -135,7 +135,9 @@ TEST_CASE("Free unknown causes exception", "[Native]")
 	try {
 		machine.simulate(MAX_INSTRUCTIONS);
 	} catch (const std::exception& e) {
-		REQUIRE(std::string(e.what()) == "Possible double-free for freed pointer");
+		// Libtcc does not forward the real exception (instead throws a generic SYSTEM_CALL_FAILED)
+		if constexpr (!libtcc_enabled)
+			REQUIRE(std::string(e.what()) == "Possible double-free for freed pointer");
 		error = true;
 	}
 	REQUIRE(error);

tests/unit/protections.cpp

@@ -102,8 +102,12 @@ TEST_CASE("Writes to read-only segment", "[Memory]")
 
 	const auto binary = build_and_load(R"M(
 	static const int array[4] = {1, 2, 3, 4};
+	__attribute__((optimize("-O0")))
 	int main() {
 		*(volatile int *)array = 1234;
+
+		if (array[0] != 1234)
+			return -1;
 		return 666;
 	}
 	void write_to(char* dst) {