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

Add anal.cc & anal.syscc and remove the hardcoded conventions in debug ##anal #17960

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

trufae
Copy link
Collaborator

@trufae trufae commented Nov 22, 2020

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 added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the documentation and the radare2 book with the relevant information (if needed)

Detailed description

This PR introduces two new eval variables:

  • anal.cc
  • anal.syscc

those are used to set the sdb keys default.cc and default.syscc, which is used tod efine the default calling convention for analysing functions and syscalls. This change also lets the #17954 to allow to write tests for it

Test plan

Fix/review all the tests that are failing and make CI green.

Expect #17954 to add a test after rebasing

Closing issues

not sure if there's one, but the src had comments about it, and it was planned to kill all the hardcoded calling conventions. so i did it.

@@ -5978,7 +5979,7 @@ static void r_anal_aefa(RCore *core, const char *arg) {
int i;
eprintf ("NARGS %d (%s)\n", nargs, key);
for (i = 0; i < nargs; i++) {
ut64 v = r_debug_arg_get (core->dbg, R_ANAL_CC_TYPE_STDCALL, i);
ut64 v = r_debug_arg_get (core->dbg, "reg", i);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure about stdcall.. maybe just using cc is fine

@github-actions github-actions bot added the command New commands requests, behaviour changes, removal label Nov 23, 2020
@XVilka XVilka requested a review from ret2libc November 23, 2020 04:39
@pelijah
Copy link
Contributor

pelijah commented Nov 23, 2020

Then you have to kill afc= command.
Upd: and modify afcR.

@trufae
Copy link
Collaborator Author

trufae commented Nov 23, 2020

No. The functions take the calling convention defined by default. But you can specify a different one on every function. So afc shouldnt die. But the problem is that befofe the cc had no way to specify which was the default. Also with this im allowing to define different convwntions for syscalls and userland code

@pelijah
Copy link
Contributor

pelijah commented Nov 23, 2020

No. The functions take the calling convention defined by default. But you can specify a different one on every function. So afc shouldnt die. But the problem is that befofe the cc had no way to specify which was the default. Also with this im allowing to define different convwntions for syscalls and userland code

See afc= https://github.com/radareorg/radare2/blob/master/libr/core/cmd_anal.c#L372

@pelijah
Copy link
Contributor

pelijah commented Nov 23, 2020

@pelijah
Copy link
Contributor

pelijah commented Nov 23, 2020

Well in case of afcR it would be better to replace this string command with API call. But afc= duplicates e anal.cc.

@karliss
Copy link
Contributor

karliss commented Nov 23, 2020

Is there a way for storing known os-arch-bits to syscall cc mapping so that for combinations with known syscall numbers which r2 currently defines in syscall/d/*.sdb.txt files, so that it isn't necessary to manually set anal.syscc each time?

@XVilka XVilka added the RAnal label Nov 24, 2020
@trufae
Copy link
Collaborator Author

trufae commented Dec 14, 2020

afc= is suposed to set the calling convention for the given function. anal.cc should be the default if none specified. so.. if afc= is just doing the same that must be fixed i guess. anyway what do you think about this change? adding this new variable makes sense to me, what about you?

@codecov-io
Copy link

Codecov Report

Merging #17960 (855bca9) into master (113eb52) will decrease coverage by 0.00%.
The diff coverage is 57.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17960      +/-   ##
==========================================
- Coverage   40.53%   40.52%   -0.01%     
==========================================
  Files        1140     1140              
  Lines      360693   360738      +45     
==========================================
+ Hits       146193   146199       +6     
- Misses     214500   214539      +39     
Impacted Files Coverage Δ
libr/core/carg.c 34.59% <0.00%> (-0.19%) ⬇️
libr/include/r_anal.h 100.00% <ø> (ø)
libr/include/r_debug.h 100.00% <ø> (ø)
libr/debug/arg.c 24.24% <21.87%> (+5.01%) ⬆️
libr/core/cconfig.c 88.87% <77.77%> (-0.08%) ⬇️
libr/core/cmd_anal.c 54.43% <83.33%> (+0.01%) ⬆️
libr/anal/cc.c 90.00% <100.00%> (+1.02%) ⬆️
libr/core/canal.c 64.15% <100.00%> (+0.01%) ⬆️
libr/core/disasm.c 77.17% <100.00%> (+<0.01%) ⬆️
libr/socket/r2pipe.c 45.21% <0.00%> (-10.64%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 113eb52...855bca9. Read the comment docs.

@pelijah
Copy link
Contributor

pelijah commented Dec 14, 2020

afc (not afc=) is suposed to set the calling convention for the given function.
Unfortunatelly, I am not active r2 user so I can't say how useful are these changes.

@trufae trufae added this to the 5.0.0 milestone Dec 15, 2020
@trufae trufae merged commit ce48120 into radareorg:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command New commands requests, behaviour changes, removal RAnal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants