Skip to content

Conversation

@CSharperMantle
Copy link

@CSharperMantle CSharperMantle commented Sep 6, 2025

This PR marks MIPS jalr[.hb] $zero, $ra as a return instruction.

Resolves #7355.

Effects

Before:

before

After:

after

@CLAassistant
Copy link

CLAassistant commented Sep 6, 2025

CLA assistant check
All committers have signed the CLA.

@CSharperMantle CSharperMantle marked this pull request as ready for review September 6, 2025 06:51
@CSharperMantle CSharperMantle marked this pull request as draft September 6, 2025 08:26
@CSharperMantle CSharperMantle marked this pull request as ready for review September 6, 2025 08:44
@CSharperMantle
Copy link
Author

Hi @psifertex and @zznop, I've rebased the branch onto the latest dev branch. Any progress on getting this reviewed? :)

@zznop
Copy link
Member

zznop commented Dec 11, 2025

Yes, I'll get this tested and merged today or tomorrow.

@zznop
Copy link
Member

zznop commented Dec 11, 2025

Looks like this code change affects jalr branches that don't use the zero operand too, and it causes regressions

image image

Should be a simple fix. Looking into it now.

@zznop
Copy link
Member

zznop commented Dec 11, 2025

I'm going to go with:

		case MIPS_JALR_HB:
			if (instr.operands[0].reg == REG_ZERO && instr.operands[1].reg == REG_RA)
				result.AddBranch(FunctionReturn, 0, nullptr, hasBranchDelay);
			else
				result.delaySlots = 1;
			break;

I see that you were probably following the pattern for MIPS_JR/MIPS_JR_HB, but I think we want to continue to not do anything in cases where it's not a jalr $zero, $ra and let Binja figure out the branch type when lifting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MIPS64R6 jalr[.hb] $zero, $ra should be treated as a return variant

3 participants