Skip to content
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.

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

RISC-V JALR: Handle cases where rd==rs1, and rd!=zero #6064

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

jeanmicheldeva
Copy link
Contributor

Solves #6003

@CLAassistant
Copy link

CLAassistant commented Oct 30, 2024

CLA assistant check
All committers have signed the CLA.

@galenbwill galenbwill self-assigned this Oct 30, 2024
@galenbwill galenbwill added Lifting issues related to LLIL lifting Arch: RISC-V Issues with the RISC-V architecture plugin Type: Bug Issue is a non-crashing bug with repro steps Component: Architecture Issue needs changes to an architecture plugin labels Oct 30, 2024
@galenbwill galenbwill modified the milestones: Frogstar, Gallifrey Oct 30, 2024
@emesare emesare assigned emesare and unassigned galenbwill Nov 30, 2024
@emesare
Copy link
Member

emesare commented Nov 30, 2024

Adding the unresolved branch

Before:
image
After:
image

@emesare
Copy link
Member

emesare commented Dec 1, 2024

So half of this PR was merged with #6213. Your other change, adding the unresolved branch to JALR, is being considered. I am not entirely sure if there was a reason why the branch was left out which is why I can't merge that part now.

@emesare
Copy link
Member

emesare commented Dec 4, 2024

image

For an example this is the issue with not adding a branch and what this PR would fix.

@emesare
Copy link
Member

emesare commented Dec 5, 2024

Talking to @rssor

so it was intentional, and when rd is ra especially we don't want it (those are, in fact, calls)
plt stubs are always going to be super bespoke and, generally speaking, we don't want calls to them to be showing up anyways (since they can't be meaningfully analyzed)

If you look at #6064 (comment) you can see an example of the function calls.

Looking at your comment #6003 (comment) it seems you are running into a different set of issues resulting from the omitted branch info, if you can share more information about what was fixed by this change maybe their is some alternative solution.

@emesare emesare added the State: Blocked (Customer) Issue is blocked on waiting for a response from a customer label Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: RISC-V Issues with the RISC-V architecture plugin Component: Architecture Issue needs changes to an architecture plugin Lifting issues related to LLIL lifting State: Blocked (Customer) Issue is blocked on waiting for a response from a customer Type: Bug Issue is a non-crashing bug with repro steps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants