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

Set "asm.arch" before the call to SleighIDFromCore #330

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

Conversation

DMaroo
Copy link
Member

@DMaroo DMaroo commented Sep 19, 2023

This way, the strcmp check in SleighIDFromCore function results as true, and in the corresponding branch, we extract the correct architecture using SleighIdFromSleighAsmConfig.

Before the fix:

[0x00000000]> pdga
WARNING: It is mandatory to have at least one argument register defined in the register profile.
WARNING: (null)
ERROR: core: cannot derive CC from reg profile.
WARNING: core: missing calling conventions for 'ghidra'. Deriving it from the regprofile.
[0x00000000]>

After the fix

[0x00006160]> pdga
[0x00006160]>

The above error is architecture and binary agnostic. Fixes https://im.rizin.re/rizinorg/pl/3xiabg1ah3naurrxmoq1ubxqca.

@DMaroo DMaroo requested a review from thestr4ng3r September 19, 2023 22:19
@DMaroo
Copy link
Member Author

DMaroo commented Sep 20, 2023

CI failure is because Cutter fails to compile, totally unrelated to the changes made in this PR. I have reproduced it and submitted the fix at rizinorg/cutter#3247.

@XVilka

This comment was marked as resolved.

@XVilka XVilka closed this Sep 26, 2023
@XVilka XVilka reopened this Sep 26, 2023
@XVilka
Copy link
Member

XVilka commented Sep 26, 2023

@DMaroo please resend from different branch and new PR. Can't restart all jobs in this one.

@DMaroo
Copy link
Member Author

DMaroo commented Sep 26, 2023

Don't I need to update the tests? If I push a new commit with the updated tests, the CI will restart, right?

@XVilka
Copy link
Member

XVilka commented Sep 26, 2023

Don't I need to update the tests? If I push a new commit with the updated tests, the CI will restart, right?

That would work too.

    * This way, the `strcmp` check in `SleighIDFromCore` function
      results as true, and in the corresponding branch, we extract the
      correct architecture using `SleighIdFromSleighAsmConfig`
@DMaroo
Copy link
Member Author

DMaroo commented Sep 26, 2023

I have restarted the build with an empty commit amend. Many of the test failures previously were unrelated. I will have a look at the new failures.

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.

3 participants