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

Convert i commands to rzshell #1238

Merged
merged 37 commits into from
Sep 4, 2021
Merged

Convert i commands to rzshell #1238

merged 37 commits into from
Sep 4, 2021

Conversation

ret2libc
Copy link
Member

@ret2libc ret2libc commented Jun 23, 2021

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 rizin book with the relevant information (if needed)
  • Run on dist-cmd-info-api branch for additional CI: see TEST cmd-info-api branch #1581 , everything is alright except netbsd.. not sure if it's related though.

Detailed description

Convert i commands to rzshell. In the process, move most of the output to RzTable for consistency.

Test plan

CI green + a lot of manual testing to ensure stuff still works and are displayed well.
Make sure the issues listed below were really addressed:

Closing issues

Closes #1516
Closes #786
Closes #1120
Closes #1027

@XVilka

This comment has been minimized.

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.

A few code style nitpicks

@XVilka

This comment has been minimized.

@wargio

This comment has been minimized.

@XVilka
Copy link
Member

XVilka commented Aug 18, 2021

Regarding:

[XX] db/cmd/cmd_i iIj
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc iIj~{} bins/elf/analysis/x86-helloworld-gcc
-- stdout
--- expected
+++ actual
@@ -5,11 +5,8 @@
   "bintype": "elf",
   "bits": 32,
   "class": "ELF32",
-  "compiled": "",
   "compiler": "GCC: (GNU) 4.8.2",
-  "dbg_file": "",
   "endian": "LE",
-  "guid": "",
   "intrp": "/lib/ld-linux.so.2",
   "laddr": 0,
   "lang": "c",
@@ -17,7 +14,6 @@
   "maxopsz": 16,
   "minopsz": 1,
   "os": "linux",
-  "cc": "",
   "pcalign": 0,
   "relro": "no",
   "rpath": "NONE",
@@ -33,8 +29,5 @@
   "canary": false,
   "PIE": false,
   "RELROCS": true,
-  "NX": true,
-  "checksums": {
-    
-  }
+  "NX": true
 }

I think it's necessary to print empty values still, for the sake of parsers of JSON output

[XX] db/cmd/cmd_i ic*
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc ic* bins/elf/libmagic.so
-- stdout
--- expected
+++ actual
@@ -1,111 +1,0 @@
-fs classes
-"f class.JNIEnv = 0x6060"
-"f method._JNIEnv.FindClass_char_const = 0x6060"
-"f method._JNIEnv.DeleteLocalRef__jobject = 0x606a"
-"f method._JNIEnv.NewObject__jclass____jmethodID___... = 0x6074"
-"f method._JNIEnv.GetObjectClass__jobject = 0x608e"
-"f method._JNIEnv.GetMethodID__jclass___char_const___char_const = 0x6098"
-"f method._JNIEnv.CallObjectMethod__jobject____jmethodID___... = 0x60a4"
-"f method._JNIEnv.CallVoidMethod__jobject____jmethodID___... = 0x60c0"
-"f method._JNIEnv.GetStaticMethodID__jclass___char_const___char_const = 0x60dc"
-"f method._JNIEnv.CallStaticObjectMethod__jclass____jmethodID___... = 0x60ea"
-"f method._JNIEnv.CallStaticIntMethod__jclass____jmethodID___... = 0x695e"
-"f method._JNIEnv.CallStaticVoidMethod__jclass____jmethodID___... = 0x697c"
-td "struct _JNIEnv { char empty[0];};"
-"f class.SystemClassLoaderInjector = 0x699c"

ic* is clearly a bug. In the future we should remove this command, but for now it's necessary still

@XVilka

This comment has been minimized.

@ret2libc
Copy link
Member Author

ret2libc commented Sep 3, 2021

Please ignore the failing tests for now. Just look at the output and see if the changes make sense. It is just a matter of updating tests to use obR instead of ib and then fix some tests... but please tell me if code and behaviours make sense!

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.

I like the new output 👍

@ret2libc ret2libc changed the title Convert i commands to the newshell Convert i commands to rzshell Sep 3, 2021
@XVilka XVilka merged commit 1c78aee into dev Sep 4, 2021
@XVilka XVilka mentioned this pull request Sep 6, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants