Merge pull request #151 from fwsGonzo/bintr_validate_mappings

bintr: Add validation step to binary translation mappings
fwsGonzo · Jun 13, 2024 · b489010 · b489010
2 parents 41907e8 + f6194eb
commit b489010
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 21 deletions.
diff --git a/lib/libriscv/decode_bytecodes.cpp b/lib/libriscv/decode_bytecodes.cpp
@@ -15,13 +15,8 @@ namespace riscv {
 template <int W>
 size_t CPU<W>::computed_index_for(rv32i_instruction instr)
 {
-#ifdef RISCV_BINARY_TRANSLATION
-	if (instr.whole == RV32_INSTR_BLOCK_END)
-		return RV32I_BC_TRANSLATOR;
-#else
 	if (instr.whole == RV32_INSTR_BLOCK_END)
 		return RV32I_BC_FUNCBLOCK;
-#endif
 
 #ifdef RISCV_EXT_COMPRESSED
 	if (instr.length() == 2)

diff --git a/lib/libriscv/decoded_exec_segment.hpp b/lib/libriscv/decoded_exec_segment.hpp
@@ -65,6 +65,7 @@ namespace riscv
 		void add_mapping(bintr_block_func<W> handler) { m_translator_mappings.push_back(handler); }
 		bintr_block_func<W> mapping_at(unsigned i) const { return m_translator_mappings.at(i); }
 		bintr_block_func<W> unchecked_mapping_at(unsigned i) const { return m_translator_mappings[i]; }
+		size_t translator_mappings() const noexcept { return m_translator_mappings.size(); }
 #else
 		bool is_binary_translated() const noexcept { return false; }
 #endif

diff --git a/lib/libriscv/decoder_cache.cpp b/lib/libriscv/decoder_cache.cpp
@@ -4,6 +4,7 @@
 #include "rvc.hpp"
 #include "safe_instr_loader.hpp"
 #include "threaded_rewriter.cpp"
+#include "threaded_bytecodes.hpp"
 
 namespace riscv
 {
@@ -54,7 +55,9 @@ namespace riscv
 		address_type<W> base_pc, address_type<W> last_pc,
 		const uint8_t* exec_segment, DecoderData<W>* exec_decoder)
 	{
-		const auto translator_op = CPU<W>::computed_index_for(RV32_INSTR_BLOCK_END);
+#ifdef RISCV_BINARY_TRANSLATION
+		const auto translator_op = RV32I_BC_TRANSLATOR;
+#endif
 
 		if constexpr (compressed_enabled)
 		{
@@ -100,14 +103,18 @@ namespace riscv
 					// All opcodes that can modify PC
 					if (length == 2)
 					{
-						if (!is_regular_compressed<W>(instruction.half[0]) || entry.get_bytecode() == translator_op)
+						if (!is_regular_compressed<W>(instruction.half[0]))
 							break;
 					} else {
 						if (opcode == RV32I_BRANCH || is_stopping_system(instruction)
 							|| opcode == RV32I_JAL || opcode == RV32I_JALR
-							|| is_stopping_auipc(instruction) || entry.get_bytecode() == translator_op)
+							|| is_stopping_auipc(instruction))
 							break;
 					}
+				#ifdef RISCV_BINARY_TRANSLATION
+					if (entry.get_bytecode() == translator_op)
+						break;
+				#endif
 
 					// A last test for the last instruction, which should have been a block-ending
 					// instruction. Since it wasn't we must force-end the block here.
@@ -179,8 +186,12 @@ namespace riscv
 				// All opcodes that can modify PC and stop the machine
 				if (opcode == RV32I_BRANCH || is_stopping_system(instruction)
 					|| opcode == RV32I_JAL || opcode == RV32I_JALR
-					|| is_stopping_auipc(instruction) || entry.get_bytecode() == translator_op)
+					|| is_stopping_auipc(instruction))
+					idxend = 0;
+			#ifdef RISCV_BINARY_TRANSLATION
+				if (entry.get_bytecode() == translator_op)
 					idxend = 0;
+			#endif
 				// Ends at *one instruction before* the block ends
 				entry.idxend = idxend;
 				// Increment after, idx becomes block count - 1
@@ -288,16 +299,21 @@ namespace riscv
 
 #ifdef RISCV_BINARY_TRANSLATION
 			if (entry.get_bytecode() == block_ending_bytecode) {
-				if constexpr (compressed_enabled) {
-					dst += 2;
-					if (was_full_instruction) {
-						was_full_instruction = (instruction.length() == 2);
-					} else {
-						was_full_instruction = true;
-					}
-				} else
-					dst += 4;
-				continue;
+				// Translator activation uses a special bytecode
+				// but we must still validate the mapping index.
+				if (entry.instr < exec.translator_mappings()) {
+					entry.set_bytecode(RV32I_BC_TRANSLATOR);
+					if constexpr (compressed_enabled) {
+						dst += 2;
+						if (was_full_instruction) {
+							was_full_instruction = (instruction.length() == 2);
+						} else {
+							was_full_instruction = true;
+						}
+					} else
+						dst += 4;
+					continue;
+				}
 			}
 #endif // RISCV_BINARY_TRANSLATION
 

diff --git a/lib/libriscv/tr_tcc.cpp b/lib/libriscv/tr_tcc.cpp
@@ -9,7 +9,7 @@ namespace riscv
 		TCCState* state = tcc_new();
 		tcc_set_output_type(state, TCC_OUTPUT_MEMORY);
 
-		for (auto pair : cflags) {
+		for (const auto& pair : cflags) {
 			tcc_define_symbol(state, pair.first.c_str(), pair.second.c_str());
 		}
 

diff --git a/lib/libriscv/tr_translate.cpp b/lib/libriscv/tr_translate.cpp
@@ -567,9 +567,9 @@ void CPU<W>::activate_dylib(const MachineOptions<W>& options, DecodedExecuteSegm
 	const auto nmappings = *no_mappings;
 	exec.reserve_mappings(nmappings);
 	for (size_t i = 0; i < nmappings; i++) {
-		exec.add_mapping(mappings[i].handler);
 		const auto addr = mappings[i].addr;
 		if (exec.is_within(addr)) {
+			exec.add_mapping(mappings[i].handler);
 			auto& entry = decoder_entry_at(exec, addr);
 			if (mappings[i].handler != nullptr) {
 				entry.instr = i;
@@ -582,6 +582,9 @@ void CPU<W>::activate_dylib(const MachineOptions<W>& options, DecodedExecuteSegm
 				fprintf(stderr, "libriscv: Translation mapping 0x%lX outside execute area 0x%lX-0x%lX\n",
 					(long)addr, (long)exec.exec_begin(), (long)exec.exec_end());
 			}
+			exec.add_mapping([] (CPU<W>&, uint64_t, uint64_t, address_t) -> bintr_block_returns<W> {
+				throw MachineException(INVALID_PROGRAM, "Translation mapping outside execute area");
+			});
 			//throw MachineException(INVALID_PROGRAM, "Translation mapping outside execute area", addr);
 			// It might be technically possible to produce some valid instructions in the
 			// padding areas, but it's not worth the effort to support this. Ignore.