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

Capstone@next TriCore #3463

Merged
merged 9 commits into from
Jun 8, 2023
Merged

Capstone@next TriCore #3463

merged 9 commits into from
Jun 8, 2023

Conversation

imbillow
Copy link
Contributor

@imbillow imbillow commented Apr 17, 2023

REBASE ONLY

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

  • Update TriCore asm plugin
  • Improve TriCore analysis plugin

Test plan

CI is green

Closing issues

Partially addresses #2704
Closes #3540

@github-actions github-actions bot added documentation Improvements or additions to documentation infrastructure rz-test labels Apr 17, 2023
XVilka

This comment was marked as resolved.

@imbillow

This comment was marked as resolved.

@ret2libc

This comment was marked as resolved.

@XVilka

This comment was marked as resolved.

@wargio

This comment was marked as resolved.

@ret2libc

This comment was marked as resolved.

@imbillow

This comment was marked as resolved.

@XVilka
Copy link
Member

XVilka commented Apr 20, 2023

@imbillow you could temporarily disable it in your PR, and address them later

@imbillow

This comment was marked as resolved.

@github-actions github-actions bot added the API label Apr 20, 2023
@XVilka XVilka added this to the 0.6.0 milestone Apr 23, 2023
@XVilka XVilka mentioned this pull request Apr 23, 2023
5 tasks
@imbillow imbillow mentioned this pull request Apr 23, 2023
5 tasks
XVilka

This comment was marked as resolved.

@XVilka

This comment was marked as resolved.

XVilka

This comment was marked as resolved.

@imbillow imbillow force-pushed the capstone-tricore branch from 8b4c418 to f719ba5 Compare May 10, 2023 07:50
XVilka

This comment was marked as resolved.

@XVilka

This comment was marked as resolved.

@imbillow
Copy link
Contributor Author

imbillow commented Jun 6, 2023

I found that there are some variables with different names and types in the DWARF information but with the same location. What I do in the code is to let it do not overwrite the arguments of function at least.

@XVilka
Copy link
Member

XVilka commented Jun 7, 2023

I found that there are some variables with different names and types in the DWARF information but with the same location. What I do in the code is to let it do not overwrite the arguments of function at least.

Interesting. This is better be addressed during the DWARF 5 implementation though. Since the only issue in this PR is the System Z, could you please extract variable changes (after addressing my feedback first) into a separate PR again, so that we could merge them again and rebase tricore branches?

@XVilka
Copy link
Member

XVilka commented Jun 7, 2023

One more nitpick @imbillow, on the same file erika3app.elf:

[0x80000020]> afv @ dbg.osEE_alarm_get
var OsEE_CounterDB *p_counter_db @ a12
var OsEE_TriggerCB * const  p_trigger_cb @ a13
var OsEE_spin_lock *p_lock @ a15
var OsEE_CDB * const  p_cdb @ a2
arg OsEE_AlarmDB *p_alarm_db @ a4
var CoreIdType counter_core_id @ d10
arg TickType *p_tick @ d11
var TickType delta @ d15
var TickType when @ d2
var StatusType ev @ d8

Note the * const and then two spaces before p_cdb. I think it should be var OsEE_CDB *const p_cdb @ a2 instead, and for other similar cases.

Please also rebase on top of the latest dev

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Why do you try to fix it as the endianness of the Tricore? It's the endianness of the host. I think the bug is somewhere in the Capstone, and you could try just running capstone tests for Tricore on the System Z VM, I bet it will show the problem

@imbillow
Copy link
Contributor Author

imbillow commented Jun 7, 2023

Why do you try to fix it as the endianness of the Tricore? It's the endianness of the host. I think the bug is somewhere in the Capstone, and you could try just running capstone tests for Tricore on the System Z VM, I bet it will show the problem

Just trying it out.

I've actually tried debug in a s390x qemu Ubuntu22.04 VM, but found that gdb/lldb doesn't seem to debug properly.

I'll do more experimentation when I can operate my pc later.

@imbillow

This comment was marked as off-topic.

@imbillow imbillow force-pushed the capstone-tricore branch from c28fe4b to ae6561d Compare June 7, 2023 18:29
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Looks green now; well done. Please rearrange commits, and extract non-tricore part into a separate PR. Once its merged, we can rebase and merge this one too.

@@ -6466,3 +6466,43 @@ RZ_API void rz_core_perform_auto_analysis(RZ_NONNULL RzCore *core, RzCoreAnalysi
rz_cons_break_pop();
RZ_FREE(debugger);
}

RZ_API char *rz_core_analysis_var_to_string(RZ_NONNULL RzCore *core, RZ_NONNULL RzAnalysisVar *var) {
Copy link
Member

Choose a reason for hiding this comment

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

@imbillow you forgot this

@@ -1281,6 +1281,8 @@ RZ_API RZ_OWN RzList /*<RzRegItem *>*/ *rz_core_reg_filter_items_sync(RZ_NONNULL
RZ_API void rz_core_cmd_show_analysis_help(RZ_NONNULL RzCore *core);
RZ_API void rz_core_rtr_enable(RZ_NONNULL RzCore *core, const char *cmdremote);

RZ_API char *rz_core_analysis_var_to_string(RZ_NONNULL RzCore *core, RZ_NONNULL RzAnalysisVar *var);
Copy link
Member

Choose a reason for hiding this comment

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

And this

@imbillow
Copy link
Contributor Author

imbillow commented Jun 8, 2023

#3561

I will fix these places in a new PR

@XVilka
Copy link
Member

XVilka commented Jun 8, 2023

@imbillow merged the PR: 6f3969f

Please rebase capstone-ng and this PR on top of dev, then we can finally merge this.

@imbillow imbillow force-pushed the capstone-tricore branch from ae6561d to 81c8b16 Compare June 8, 2023 09:44
@XVilka XVilka merged commit 01ea5d6 into capstone-ng Jun 8, 2023
@XVilka XVilka deleted the capstone-tricore branch June 8, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tricore DWARF function argument extraction problem
6 participants