Fix tailjump to relocs (#347)

Ghidra uses some scripts written in Java to fixup some analysis corner
cases (see Ghidra/Features/Base/ghidra_scripts), but currently rz-ghidra
is not able to use them, which causes some decompiling issues. This
change makes an attempt to fixup one of the corner cases. Details about
the issue can be found in #202.

We try to solve the issue by adding a hook before decompiling action,
but after the initialization of the RizinArchitecture. The hook function
uses rizin to analyse shared return calls to relocs, and fixup the
P-Code using setFlowOverride() API provided by Ghidra.

Fix #202

CMakeLists.txt

@@ -57,6 +57,8 @@ set(CORE_SOURCE
 		src/RzCoreMutex.cpp
 		src/PrettyXmlEncode.h
 		src/PrettyXmlEncode.cpp
+		src/PcodeFixupPreprocessor.h
+		src/PcodeFixupPreprocessor.cpp
 		src/rz_ghidra.h
 		src/rz_ghidra_internal.h)
 

src/PcodeFixupPreprocessor.cpp

@@ -0,0 +1,27 @@
+// SPDX-FileCopyrightText: 2024 Crabtux <[email protected]>
+// SPDX-License-Identifier: LGPL-3.0-or-later
+
+#include "RizinLoadImage.h"
+#include "RizinUtils.h"
+#include "PcodeFixupPreprocessor.h"
+
+#include <funcdata.hh>
+#include <flow.hh>
+#include <override.hh>
+
+using namespace ghidra;
+
+void PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(RzAnalysisFunction *function, Funcdata *func, RzCore *core, RizinArchitecture &arch)
+{
+	RzList *xrefs = rz_analysis_function_get_xrefs_from(function);
+	rz_list_foreach_cpp<RzAnalysisXRef>(xrefs, [&](RzAnalysisXRef *xref){
+		// To ensure the instruction is a `jmp` instruction
+		if (xref->type == RZ_ANALYSIS_XREF_TYPE_CODE)
+		{
+			// If the target location is a imported function, then do the patch.
+			RzBinReloc *reloc = rz_core_get_reloc_to(core, xref->to);
+			if (reloc != nullptr && reloc->import != nullptr)
+				func->getOverride().insertFlowOverride(Address(arch.getDefaultCodeSpace(), xref->from), Override::CALL_RETURN);
+		}
+	});
+}

src/PcodeFixupPreprocessor.h

@@ -0,0 +1,17 @@
+// SPDX-FileCopyrightText: 2024 Crabtux <[email protected]>
+// SPDX-License-Identifier: LGPL-3.0-or-later
+
+#ifndef PCODE_PREPROCESSOR_H
+#define PCODE_PREPROCESSOR_H
+
+#include "RizinArchitecture.h"
+
+#include <rz_core.h>
+
+class PcodeFixupPreprocessor
+{
+	public:
+		static void fixupSharedReturnJumpToRelocs(RzAnalysisFunction *function, ghidra::Funcdata *func, RzCore *core, RizinArchitecture &arch);
+};
+
+#endif

src/core_ghidra.cpp

@@ -8,6 +8,7 @@
 #include "CodeXMLParse.h"
 #include "ArchMap.h"
 #include "PrettyXmlEncode.h"
+#include "PcodeFixupPreprocessor.h"
 #include "rz_ghidra.h"
 #include "rz_ghidra_internal.h"
 
@@ -140,6 +141,12 @@ static void Decompile(RzCore *core, ut64 addr, DecompileMode mode, std::stringst
 	ApplyPrintCConfig(core->config, dynamic_cast<PrintC *>(arch.print));
 	if(!func)
 		throw LowlevelError("No function in Scope");
+
+	// Other archs are not tested
+	if (strcmp(core->analysis->arch_target->arch, "x86") == 0)
+		// Must be called after arch.init(), but before decompiling the function
+		PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(function, func, core, arch);
+
 	arch.getCore()->sleepBegin();
 	auto action = arch.allacts.getCurrent();
 	int res;

test/bins/Makefile

@@ -3,7 +3,7 @@ all:
 
 .PHONY: clean
 clean:
-	rm -fv dectest32 dectest64 rec
+	rm -fv dectest32 dectest64 rec sharedReturn.o
 
 dectest32: dectest.c types.h
 	gcc -m32 -fno-pic -no-pie -fno-omit-frame-pointer -o dectest32 -O0 dectest.c
@@ -22,3 +22,6 @@ rec: rec.c types_rec.h
 
 strings: strings.c
 	gcc -m64 -fno-pic -no-pie -fno-omit-frame-pointer -o strings -O0 strings.c
+
+sharedReturn.o: sharedReturn.c
+	gcc -m64 -fno-pic -no-pie -fno-omit-frame-pointer -fno-inline -o sharedReturn.o -O2 -c sharedReturn.c

test/bins/sharedReturn.c

@@ -0,0 +1,13 @@
+#include <stdio.h>
+
+int getNum(void);
+int calc(int a, int b);
+
+int main(void){
+    int c = getNum();
+
+    if (c > 2)
+        return calc(c, 2);
+    else
+        return 0;
+}

test/db/extras/ghidra

@@ -3364,3 +3364,27 @@ undefined8 main(void)
 }
 EOF
 RUN
+
+NAME=fixup shared return call to relocs
+FILE=bins/sharedReturn.o
+CMDS=<<EOF
+aaa
+pdg @ sym.main
+EOF
+EXPECT=<<EOF
+
+undefined8 sym.main(void)
+{
+    int32_t iVar1;
+    undefined8 uVar2;
+
+    // [04] -r-x section size 31 named .text.startup
+    iVar1 = getNum();
+    if (iVar1 < 3) {
+        return 0;
+    }
+    uVar2 = calc(iVar1, 2);
+    return uVar2;
+}
+EOF
+RUN