Fix control-flow recovery relocation handling

GJDuck · GJDuck · commit 4323517890d4 · 2022-12-06T20:40:03.000+08:00
Previous version would miss some functions only
reachable via indirect calls.
diff --git a/src/e9tool/e9cfg.cpp b/src/e9tool/e9cfg.cpp
@@ -50,8 +50,6 @@ extern bool option_debug;
     }                                                                   \
     while (false)
 
-typedef std::set<intptr_t> RelaInfo;
-
 /*
  * Insert target information.
  */
@@ -251,7 +249,7 @@ static void CFGCodeAnalysis(const ELF *elf, bool pic, const Instr *Is,
  * Section analysis pass: find potential code pointers in data.
  */
 static void CFGSectionAnalysis(const ELF *elf, bool pic, const char *name,
-    const Elf64_Shdr *shdr, const Instr *Is, size_t size, const RelaInfo relas,
+    const Elf64_Shdr *shdr, const Instr *Is, size_t size,
     const std::set<intptr_t> &tables, Targets &targets)
 {
     if ((shdr->sh_flags & SHF_EXECINSTR) != 0 || shdr->sh_addr == 0x0)
@@ -316,26 +314,6 @@ static void CFGSectionAnalysis(const ELF *elf, bool pic, const char *name,
                 }
             }
         }
-
-        if (shdr->sh_type == SHT_PROGBITS)
-        {
-            // Scan for code pointers using relocation information.
-            auto bounds = getBounds<int64_t>(sh_data, sh_data + sh_size);
-            for (const int64_t *p = bounds.first; p < bounds.second; p++)
-            {
-                intptr_t offset = (intptr_t)shdr->sh_addr +
-                    ((intptr_t)p - (intptr_t)sh_data);
-                auto i = relas.find(offset);
-                if (i == relas.end())
-                    continue;
-                
-                intptr_t target = *p;
-                if (findInstr(Is, size, target) < 0)
-                    continue;
-                DEBUG(targets, target, "Reloc : %p (F)", (void *)target);
-                addTarget(target, TARGET_INDIRECT | TARGET_FUNCTION, targets);
-            }
-        }
     }
 }
 
@@ -347,7 +325,6 @@ static void CFGDataAnalysis(const ELF *elf, bool pic, const Instr *Is,
 {
     // Gather relocation information:
     const SectionInfo &sections = getELFSectionInfo(elf);
-    RelaInfo relas;
     for (const auto &entry: sections)
     {
         const Elf64_Shdr *shdr = entry.second;
@@ -359,16 +336,19 @@ static void CFGDataAnalysis(const ELF *elf, bool pic, const Instr *Is,
         const Elf64_Rela *rela_end = rela + sh_size / sizeof(Elf64_Rela);
         for (; rela < rela_end; rela++)
         {
-            if (ELF64_R_TYPE(rela->r_info) == R_X86_64_RELATIVE &&
-                    rela->r_addend == 0)
-                relas.insert(rela->r_offset);
+            if (ELF64_R_TYPE(rela->r_info) == R_X86_64_RELATIVE)
+            {
+                intptr_t target = (intptr_t)rela->r_addend;
+                DEBUG(targets, target, "Reloc : %p (F)", (void *)target);
+                addTarget(target, TARGET_INDIRECT | TARGET_FUNCTION, targets);
+            }
         }
     }
 
     // Analyze each data section:
     for (const auto &entry: sections)
         CFGSectionAnalysis(elf, pic, entry.first, entry.second, Is, size,
-            relas, tables, targets);
+            tables, targets);
 }
 
 /*
diff --git a/test/regtest/Makefile b/test/regtest/Makefile
@@ -18,6 +18,7 @@ all:
 	gcc -x assembler-with-cpp -shared -o libtest.so libtest.s 
 	gcc -O2 -fPIC $(FCF_NONE) -pie -o test_c test_c.c \
 		-Wl,--export-dynamic -U_FORTIFY_SOURCE
+	strip test_c
 	../../e9compile.sh inst.c -I ../../examples/ -D NO_GLIBC
 	../../e9compile.sh patch.cpp -std=c++11 -I ../../examples/ -D NO_GLIBC
 	../../e9compile.sh dl.c -I ../../examples/
diff --git a/test/regtest/call_func.exp b/test/regtest/call_func.exp
@@ -0,0 +1,16 @@
+Hello world!
+Hello world!
+fib = 89
+prime(121) = 0
+prime(131) = 1
+         *         
+        ***        
+       *****       
+      *******      
+     *********     
+    *         *    
+   ***       ***   
+  *****     *****  
+ *******   ******* 
+********* *********
+invoke data_func()
diff --git a/test/regtest/call_func.in b/test/regtest/call_func.in
@@ -0,0 +1 @@
+./test_c -M 'mnemonic==/xchg.*/ && reg[0] == %r15 && reg[0] == reg[1] && addr == F.addr' -P 'exit(0)'
diff --git a/test/regtest/repair_fib.exp b/test/regtest/repair_fib.exp
@@ -13,3 +13,5 @@ prime(131) = 1
   *****     *****  
  *******   ******* 
 ********* *********
+invoke data_func()
+invoked data_func()
diff --git a/test/regtest/repair_fib_2.exp b/test/regtest/repair_fib_2.exp
@@ -13,3 +13,5 @@ prime(131) = 1
   *****     *****  
  *******   ******* 
 ********* *********
+invoke data_func()
+invoked data_func()
diff --git a/test/regtest/test_c.c b/test/regtest/test_c.c
@@ -66,6 +66,34 @@ __attribute__((__noinline__)) void triforce(ssize_t n)
     }
 }
 
+void data_func_2(void)
+{
+    printf("invoked data_func()\n");
+    asm volatile ("nop");
+}
+
+static void data_func(void)
+{
+    asm volatile (
+        "xchg %r15, %r15\n"
+        "callq data_func_2");
+}
+
+struct call_s
+{
+    void (*f)(void);
+    const char *name;
+};
+
+static const struct call_s call_info = {data_func, "data_func"};
+
+__attribute__((__noinline__)) void invoke(const struct call_s *info)
+{
+    printf("invoke %s()\n", info->name);
+    fflush(stdout);
+    info->f();
+}
+
 int main(void)
 {
     printf("Hello world!\n");
@@ -88,6 +116,9 @@ int main(void)
     printf("prime(131) = %d\n", is_prime(131));
     triforce(9);
 
+    fflush(stdout);
+    invoke(&call_info);
+
     return 0;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+./test_c -M 'mnemonic==/xchg.*/ && reg[0] == %r15 && reg[0] == reg[1] && addr == F.addr' -P 'exit(0)'`