Merge pull request #83 from dword64/master

Fix tracer recursion stack overflows (#52, #78) by combining #79/#81; update Capstone/Keystone deps

Author: can1357

Dependencies/capstone/CMakeLists.txt

@@ -11,7 +11,7 @@ message(STATUS "Fetching capstone (this might take a while)...")
 FetchContent_Declare(
     capstone
     GIT_REPOSITORY https://github.com/aquynh/capstone
-    GIT_TAG        d71c95b09b1fa478c0ab9294b07fd6f2efaa1b31
+    GIT_TAG        52c66920fc7bfa15fd9626dfd9f646c698aaa99b
     GIT_SHALLOW    false
 )
 FetchContent_MakeAvailable(capstone)
@@ -22,9 +22,27 @@ FetchContent_MakeAvailable(capstone)
 #
 # TODO: Maybe contribute a fix to these upstream projects?
 #
-get_target_property(capstone_SOURCE_DIR capstone-static SOURCE_DIR)
-set_property(TARGET capstone-static PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${capstone_SOURCE_DIR}/include)
+# capstone target naming differs by version/toolchain; resolve the first valid one cus lazy
+set(CAPSTONE_TARGET "")
+foreach(_candidate capstone-static capstone_static capstone)
+    if(TARGET ${_candidate})
+        set(CAPSTONE_TARGET ${_candidate})
+        break()
+    endif()
+endforeach()
+
+if(NOT CAPSTONE_TARGET)
+    message(FATAL_ERROR "Capstone target was not created (checked: capstone-static, capstone_static, capstone)")
+endif()
+
+get_target_property(capstone_SOURCE_DIR ${CAPSTONE_TARGET} SOURCE_DIR)
+set_property(TARGET ${CAPSTONE_TARGET} PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${capstone_SOURCE_DIR}/include)
 
 # Downgrade C++ standard on these targets since they depend on some removed/deprecated features
-set_property(TARGET capstone-static PROPERTY CXX_STANDARD 11)
-set_property(TARGET capstone-static PROPERTY CXX_STANDARD_REQUIRED ON)
+set_property(TARGET ${CAPSTONE_TARGET} PROPERTY CXX_STANDARD 11)
+set_property(TARGET ${CAPSTONE_TARGET} PROPERTY CXX_STANDARD_REQUIRED ON)
+
+# Keep downstream link line stable across Capstone naming variants.
+if(NOT TARGET capstone-static)
+    add_library(capstone-static ALIAS ${CAPSTONE_TARGET})
+endif()

Dependencies/keystone/CMakeLists.txt

@@ -2,7 +2,7 @@ message(STATUS "Fetching keystone (this might take a while)...")
 FetchContent_Declare(
     keystone
     GIT_REPOSITORY https://github.com/keystone-engine/keystone
-    GIT_TAG        e1547852d9accb9460573eb156fc81645b8e1871
+    GIT_TAG        dc7932ef2b2c4a793836caec6ecab485005139d6
     GIT_SHALLOW    false
 )
 

VTIL-Architecture/trace/tracer.cpp

@@ -29,9 +29,39 @@
 #include <vtil/io>
 #include "../vm/lambda.hpp"
 #include <vtil/utility>
+#include <unordered_map>
 
 namespace vtil
 {
+	// Re entry guards for cycle heavy traces.
+	// - #52: direct/indirect reentry in tracer::trace via VM callbacks.
+	// - #78: recursive rtrace propagation over cyclic path expansions.
+	//
+	template<typename key_t>
+	struct recursion_guard
+	{
+		std::unordered_map<key_t, size_t>& depth_map;
+		key_t key;
+		bool reentrant = false;
+
+		recursion_guard( std::unordered_map<key_t, size_t>& depth_map, const key_t& key )
+			: depth_map( depth_map ), key( key )
+		{
+			auto& depth = depth_map[ key ];
+			reentrant = depth != 0;
+			depth++;
+		}
+
+		~recursion_guard()
+		{
+			auto it = depth_map.find( key );
+			if ( it != depth_map.end() && --it->second == 0 )
+				depth_map.erase( it );
+		}
+	};
+
+	inline static thread_local std::unordered_map<symbolic::variable, size_t> active_trace;
+	inline static thread_local std::unordered_map<symbolic::variable, size_t> active_rtrace;
 	// Internal type definitions.
 	//
 	using path_map_t = std::map<std::pair<const basic_block*, const basic_block*>, int>;
@@ -284,6 +314,14 @@ namespace vtil
 	{
 		using namespace logger;
 
+		recursion_guard guard_rtrace{ active_rtrace, lookup };
+		if ( guard_rtrace.reentrant )
+		{
+			auto cyclic = lookup;
+			cyclic.is_branch_dependant = true;
+			return cyclic.to_expression();
+		}
+
 		// Save whether this is the call whose result will reach the user.
 		//
 		bool initial_call = path_map.empty();
@@ -355,9 +393,9 @@ namespace vtil
 						if ( counter >= 2 )
 						{
 #if VTIL_OPT_TRACE_VERBOSE
-							// Log skipping of path.
-							//
-							log<CON_CYN>( "Path [%llx->%llx] is not taken as it's n-looping.\n", lookup.at.block->entry_vip, it.block->entry_vip );
+						// Log skipping of path.
+						//
+						log<CON_CYN>( "Path [%llx->%llx] is not taken as it's n-looping.\n", lookup.at.block->entry_vip, it.block->entry_vip );
 #endif
 							return enumerator::ocontinue;
 						}
@@ -446,6 +484,10 @@ namespace vtil
 	{
 		using namespace logger;
 
+		recursion_guard guard_trace{ active_trace, lookup };
+		if ( guard_trace.reentrant )
+			return lookup.to_expression();
+
 		// If invalid/.begin() iterator or register with "no-trace" flags, return as is.
 		//
 		if ( lookup.at.is_begin() || ( lookup.is_register() && ( lookup.reg().flags & ( register_volatile | register_readonly ) ) ) )

VTIL-Common/util/copy_on_write.hpp

@@ -30,6 +30,7 @@
 #include <functional>
 #include <type_traits>
 #include <atomic>
+#include <utility>
 #include "../io/asserts.hpp"
 #include "object_pool.hpp"
 #include "../util/intrinsics.hpp"

VTIL-Common/util/task.hpp

@@ -148,7 +148,7 @@ namespace vtil::task
 #if VTIL_USE_THREAD_POOLING
 			handle = std::async( std::launch::async, std::move( f ) );
 #else
-			handle = { f };
+			handle = std::thread( std::move( f ) );
 #endif
 		}
 
@@ -171,4 +171,4 @@ namespace vtil::task
 #endif
 		}
 	};
-};
+};

VTIL-Compiler/optimizer/register_renaming_pass.cpp

@@ -111,6 +111,11 @@ namespace vtil::optimizer
 					//
 					if ( i->is_volatile() )
 						return fail();
+
+					// If source is being accessed by a vxcall instruction, fail.
+					//
+					if ( mask && i->base == &ins::vxcall )
+						return fail();
 				}
 
 				// If destination is used by the instruction, fail.

VTIL-Tests/dummy.cpp

@@ -19,6 +19,58 @@ namespace registers
 }
 
 
+DOCTEST_TEST_CASE("Tracer stack overflow protection (#52 VM callback reentry)")
+{
+    vtil::logger::log("\n\n>> %s \n", __FUNCTION__);
+
+    // Create a scenario where trace -> trace (via VM hooks) could recurse.
+    // This simulates the #52 pattern where read_register calls back into trace.
+    auto block = vtil::basic_block::begin(0x1000);
+    vtil::register_desc reg_ax(vtil::register_physical, registers::ax, vtil::arch::bit_count, 0);
+
+    // mov eax, eax  (self-reference that would cause reentry during VM execution)
+    block->mov(reg_ax, reg_ax);
+    block->vexit(0ull);
+
+    // Trace the register at the end - should not stack overflow
+    vtil::tracer tracer;
+    auto result = tracer.trace({ std::prev(block->end()), reg_ax });
+
+    // Result should be valid and stable (either original reg or simplified constant)
+    CHECK(result.get() != nullptr);
+    vtil::logger::log("  Trace result: %s\n", result->to_string().c_str());
+}
+
+DOCTEST_TEST_CASE("Tracer stack overflow protection (#78 cyclic path propagation)")
+{
+    vtil::logger::log("\n\n>> %s \n", __FUNCTION__);
+
+    // Create a CFG with back edge to trigger #78 pattern:
+    // Block1 -> Block2 -> Block1 (cycle)
+    // Trace should terminate safely when encountering symbolic cycles.
+    auto block1 = vtil::basic_block::begin(0x1000);
+    vtil::register_desc reg_ax(vtil::register_physical, registers::ax, vtil::arch::bit_count, 0);
+
+    block1->mov(reg_ax, 0x100);
+    block1->js(vtil::REG_FLAGS, 0x2000ull, 0x3000ull);
+
+    auto block2 = block1->fork(0x2000);
+    block2->add(reg_ax, 1);
+    block2->jmp(0x1000ull);  // Back edge to block1
+    block2->fork(0x1000ull);
+
+    auto block3 = block1->fork(0x3000);
+    block3->vexit(0ull);
+
+    // rtrace should terminate without stack overflow despite the cycle
+    vtil::tracer tracer;
+    auto result = tracer.rtrace({ block3->begin(), reg_ax });
+
+    // Result should be valid (either concrete or branch ddependent)
+    CHECK(result.get() != nullptr);
+    vtil::logger::log("  RTrace result: %s\n", result->to_string().c_str());
+}
+
 DOCTEST_TEST_CASE("dummy")
 {
     vtil::logger::log("\n\n>> %s \n", __FUNCTION__);
@@ -285,6 +337,46 @@ DOCTEST_TEST_CASE("Optimization register_renaming_pass")
     CHECK(ins.operands[1].imm().ival == 0x1);
 }
 
+DOCTEST_TEST_CASE("Optimization register_renaming_pass vxcall")
+{
+	vtil::logger::log("\n\n>> %s \n", __FUNCTION__);
+
+	auto block = vtil::basic_block::begin(0x1337);
+
+	vtil::register_desc reg_ecx(vtil::register_physical, registers::cx, vtil::arch::bit_count, 0);
+
+	auto sr0 = block->owner->alloc(vtil::arch::bit_count);
+
+	// The ecx register here is a potential function argument, register_renaming_pass should not work here.
+	block->mov(reg_ecx, (uintptr_t)0x880000);
+	block->vxcall((uintptr_t)0x10000);
+
+	auto block2 = block->fork(0x2000);
+	block2->mov(sr0, reg_ecx);
+	block2->mov(reg_ecx, (uintptr_t)1);
+	block2->mov(reg_ecx, sr0);
+	block2->vxcall((uintptr_t)0x10000);
+
+	auto block3 = block2->fork(0x3000);
+	block3->vexit(0ull); // marks the end of a basic_block
+
+	vtil::logger::log(":: Before:\n");
+	vtil::debug::dump(block->owner);
+
+	vtil::optimizer::register_renaming_pass{}(block->owner);
+
+	vtil::logger::log(":: After:\n");
+	vtil::debug::dump(block->owner);
+
+	auto ins = (*block)[0];
+
+	//  mov ecx, 0x880000
+	CHECK(ins.base == &vtil::ins::mov);
+	CHECK(ins.operands.size() == 2);
+	CHECK(ins.operands[0].reg().local_id == registers::cx);
+	CHECK(ins.operands[1].imm().ival == 0x880000);
+}
+
 DOCTEST_TEST_CASE("Optimization symbolic_rewrite_pass<true>")
 {
     vtil::logger::log("\n\n>> %s \n", __FUNCTION__);