Skip to content

Change jump tables to use switch instead of array #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AngheloAlf merged 1 commit into from
Feb 27, 2025
Merged

Change jump tables to use switch instead of array #80

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 31 additions & 49 deletions recomp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct Insn {
uint32_t jtbl_addr;
uint32_t num_cases;
rabbitizer::Registers::Cpu::GprO32 index_reg;
bool jtbl_is_pic;
Copy link
Contributor

@AngheloAlf AngheloAlf Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any non-pic jumptable anywhere in IDO?

Copy link
Contributor Author

@cadmic cadmic Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not currently, but there are in ld and strip as you know. I don't think it's N32-specific though, and technically the current logic can detect IDO non-PIC jumptables anyway but it wasn't wired up

Copy link
Contributor

@AngheloAlf AngheloAlf Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, I was thinking on the n32 programs


// graph
vector<Edge> successors;
Expand All @@ -126,6 +127,7 @@ struct Insn {
this->jtbl_addr = 0;
this->num_cases = 0;
this->index_reg = rabbitizer::Registers::Cpu::GprO32::GPR_O32_zero;
this->jtbl_is_pic = false;

this->b_liveout = 0;
this->b_livein = 0;
Expand Down Expand Up @@ -531,6 +533,27 @@ rabbitizer::Registers::Cpu::GprO32 get_dest_reg(const Insn& insn) {
return rabbitizer::Registers::Cpu::GprO32::GPR_O32_zero;
}

uint32_t jump_table_target(Insn& insn, uint32_t case_index) {
assert(insn.jtbl_addr != 0);

uint32_t case_addr = insn.jtbl_addr + case_index * 4;
uint32_t target_addr;

if (rodata_vaddr <= case_addr && case_addr < rodata_vaddr + rodata_section_len) {
target_addr = read_u32_be(rodata_section + (case_addr - rodata_vaddr));
} else if (srdata_vaddr <= case_addr && case_addr < srdata_vaddr + srdata_section_len) {
target_addr = read_u32_be(srdata_section + (case_addr - srdata_vaddr));
Comment on lines +544 to +545
Copy link
Contributor

@AngheloAlf AngheloAlf Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, are there actually any jumptables in srdata? Are those non-pic?

Copy link
Contributor Author

@cadmic cadmic Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there's one in strip:

    /* 016C4C 10016C4C 8F868024 */  lw          $a2, %got(jtbl_1002F8B8)($gp)
    /* 016C50 10016C50 DCA50050 */  ld          $a1, 0x50($a1)
    /* 016C54 10016C54 24C6F8B8 */  addiu       $a2, $a2, %lo(jtbl_1002F8B8)
    /* 016C58 10016C58 30A50007 */  andi        $a1, $a1, 0x7
    /* 016C5C 10016C5C 00052800 */  sll         $a1, $a1, 0
    /* 016C60 10016C60 2CA10008 */  sltiu       $at, $a1, 0x8
    /* 016C64 10016C64 00052880 */  sll         $a1, $a1, 2
    /* 016C68 10016C68 10200026 */  beqz        $at, .L10016D04
    /* 016C6C 10016C6C 00A62821 */   addu       $a1, $a1, $a2
    /* 016C70 10016C70 8CA60000 */  lw          $a2, 0x0($a1)
    /* 016C74 10016C74 00C00008 */  jr          $a2
    /* 016C78 10016C78 00000000 */   nop

It's not PIC, the jump table targets are absolute addresses (even though the jump table itself loaded via the GOT)

Copy link
Contributor

@AngheloAlf AngheloAlf Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, it is pretty weird given the generated code should be position independent...

} else {
assert(0 && "jump table not in .rodata or .srdata");
}

if (insn.jtbl_is_pic) {
target_addr += gp_value;
}

return target_addr;
}

// try to find a matching LUI for a given register
void link_with_lui(int offset, rabbitizer::Registers::Cpu::GprO32 reg, int mem_imm) {
#define MAX_LOOKBACK 128
Expand Down Expand Up @@ -852,6 +875,7 @@ void pass1(void) {
insn.jtbl_addr = jtbl_addr;
insn.num_cases = num_cases;
insn.index_reg = insns[addu_index - 1].instruction.GetO32_rt();
insn.jtbl_is_pic = is_pic;
insns[lw].patchInstruction(rabbitizer::InstrId::UniqueId::cpu_nop);

insns[addu_index].patchInstruction(rabbitizer::InstrId::UniqueId::cpu_nop);
Expand All @@ -862,19 +886,8 @@ void pass1(void) {
insns[addu_index - 2].patchInstruction(rabbitizer::InstrId::UniqueId::cpu_nop);
}

if (jtbl_addr < rodata_vaddr ||
jtbl_addr + num_cases * sizeof(uint32_t) > rodata_vaddr + rodata_section_len) {
fprintf(stderr, "jump table outside rodata\n");
exit(EXIT_FAILURE);
}

for (uint32_t case_index = 0; case_index < num_cases; case_index++) {
uint32_t target_addr = read_u32_be(rodata_section + (jtbl_addr - rodata_vaddr) +
case_index * sizeof(uint32_t));

target_addr += gp_value;
// printf("%08X\n", target_addr);
label_addresses.insert(target_addr);
label_addresses.insert(jump_table_target(insn, case_index));
}
}
skip:;
Expand Down Expand Up @@ -1232,13 +1245,8 @@ void pass3(void) {
add_edge(i, i + 1);

if (insn.jtbl_addr != 0) {
uint32_t jtbl_pos = insn.jtbl_addr - rodata_vaddr;

assert(jtbl_pos < rodata_section_len &&
jtbl_pos + insn.num_cases * sizeof(uint32_t) <= rodata_section_len);

for (uint32_t j = 0; j < insn.num_cases; j++) {
uint32_t dest_addr = read_u32_be(rodata_section + jtbl_pos + j * sizeof(uint32_t)) + gp_value;
uint32_t dest_addr = jump_table_target(insn, j);

add_edge(i + 1, addr_to_i(dest_addr));
}
Expand Down Expand Up @@ -2756,43 +2764,17 @@ void dump_instr(int i) {
break;

case rabbitizer::InstrId::UniqueId::cpu_jr:
// TODO: understand why the switch version fails, and why only it needs the nop
if (insn.jtbl_addr != 0) {
uint32_t jtbl_pos = insn.jtbl_addr - rodata_vaddr;

assert(jtbl_pos < rodata_section_len &&
jtbl_pos + insn.num_cases * sizeof(uint32_t) <= rodata_section_len);
#if 1
printf(";static void *const Lswitch%x[] = {\n", insn.jtbl_addr);

for (uint32_t case_index = 0; case_index < insn.num_cases; case_index++) {
uint32_t dest_addr =
read_u32_be(rodata_section + jtbl_pos + case_index * sizeof(uint32_t)) + gp_value;
printf("&&L%x,\n", dest_addr);
label_addresses.insert(dest_addr);
}

printf("};\n");
printf("dest = Lswitch%x[(uint32_t)%s];\n", insn.jtbl_addr, r((int)insn.index_reg));
printf("jtbl_index = %s;\n", r((int)insn.index_reg));
dump_instr(i + 1);
printf("goto *dest;\n");
#else
// This block produces a switch instead of an array of labels.
// It is not being used because currently it is a bit bugged.
// It has been keep as a reference and with the main intention to fix it

assert(insns[i + 1].id == MIPS_INS_NOP);
printf("switch ((uint32_t)%s) {\n", r(insn.index_reg));
printf("switch (jtbl_index) {\n");

for (uint32_t case_index = 0; case_index < insn.num_cases; case_index++) {
uint32_t dest_addr =
read_u32_be(rodata_section + jtbl_pos + case_index * sizeof(uint32_t)) + gp_value;
printf("case %u: goto L%x;\n", case_index, dest_addr);
label_addresses.insert(dest_addr);
printf("case %u: goto L%x;\n", case_index, jump_table_target(insn, case_index));
}

printf("default: __builtin_unreachable();\n");
printf("}\n");
#endif
} else {
if (insn.instruction.GetO32_rs() != rabbitizer::Registers::Cpu::GprO32::GPR_O32_ra) {
printf("UNSUPPORTED JR %s (no jumptable available)\n", r((int)insn.instruction.GetO32_rs()));
Expand Down Expand Up @@ -3459,7 +3441,7 @@ void dump_c(void) {
printf("struct ReturnValue tempret;\n");
printf("double tempf64;\n");
printf("uint32_t fp_dest;\n");
printf("void *dest;\n");
printf("uint32_t jtbl_index;\n");

if (!f.v0_in) {
printf("uint64_t v0 = 0;\n");
Expand Down
Loading