Lephenixnoir
/
fxos
3
2
Fork 1
Code Issues 2 Pull Requests 1 Releases Wiki Activity

Move vspace to option in isc command, adding custom addresses as positional arguments #6

Merged
Lephenixnoir merged 6 commits from Dr-Carlos/fxos:isc into master 2022-04-20 10:17:17 +02:00
Conversation 8 Commits 6 Files Changed 1 +62 -18
Dr-Carlos commented 2022-04-17 06:12:18 +02:00
Collaborator
Copy Link

Hello!

In these two commits, I have changed the syntax of isc from isc [sort=true] [<vspace>] to isc [sort=true] [vspace=<virtual_space>] [<address>...].

vspace behaves as it should (there was a comment saying that it didn't work, but it works now), specifying the virtual space to calculate information in.

By default, isc still prints info for all syscalls, but if address(es) are provided, the command prints information on these instead. The original use case is that you can find out what syscall an adddress from ic comes from, but there are many other uses.

Example outputs:

cg_3.60 @ 0x80000000> isc %1
  0x8002c550  %001
cg_3.60 @ 0x80000000> isc %1 %2
  0x8002c550  %001
  0x8002c55e  %002
cg_3.60 @ 0x80000000> isc 0x8002c550
  0x8002c550  %001
cg_3.60 @ 0x80000000> isc 0x8002c550 (0x8002c550+0xe)
  0x8002c550  %001
  0x8002c55e  %002
cg_3.60 @ 0x80000000> isc 0x8002c550 0x8002c55d
  0x8002c550  %001
cg_3.60 @ 0x80000000> isc $
cg_3.60 @ 0x80000000>

I think this makes isc much more useful - future ideas that could be implemented are being able to print the syscall name, and being able to select which piece(s) of information you want.

Hello! In these two commits, I have changed the syntax of `isc` from `isc [sort=true] [<vspace>]` to `isc [sort=true] [vspace=<virtual_space>] [<address>...]`. vspace behaves as it should (there was a comment saying that it didn't work, but it works now), specifying the virtual space to calculate information in. By default, `isc` still prints info for all syscalls, but if address(es) are provided, the command prints information on these instead. The original use case is that you can find out what syscall an adddress from `ic` comes from, but there are many other uses. Example outputs: ``` cg_3.60 @ 0x80000000> isc %1 0x8002c550 %001 cg_3.60 @ 0x80000000> isc %1 %2 0x8002c550 %001 0x8002c55e %002 cg_3.60 @ 0x80000000> isc 0x8002c550 0x8002c550 %001 cg_3.60 @ 0x80000000> isc 0x8002c550 (0x8002c550+0xe) 0x8002c550 %001 0x8002c55e %002 cg_3.60 @ 0x80000000> isc 0x8002c550 0x8002c55d 0x8002c550 %001 cg_3.60 @ 0x80000000> isc $ cg_3.60 @ 0x80000000> ``` I think this makes `isc` much more useful - future ideas that could be implemented are being able to print the syscall name, and being able to select which piece(s) of information you want.
👍 1
Dr-Carlos force-pushed isc from 57e41ad913 to 4d44180a27 2022-04-17 06:31:42 +02:00 Compare
Dr-Carlos commented 2022-04-18 00:05:50 +02:00
Author
Collaborator
Copy Link

Although the errors raised in isc are not in the parsing function, I have changed them to use FxOS_log at ERR level as well - as you said you were trying to get rid of errors in fxos.

Although the errors raised in `isc` are not in the parsing function, I have changed them to use FxOS_log at ERR level as well - as you said you were trying to get rid of errors in fxos.
Lephenixnoir commented 2022-04-18 13:00:53 +02:00
Owner
Copy Link

This is indeed super useful, that's exactly the direction these commands should take. Other options include filtering by name or wildcard, which would also be useful for symbols (I guess we could have a generic symbol query tool, is would use it, and isc would use it + filter syscalls only).

I can test this later today. Maybe a nitpick, but I'd rather perform symbol queries to fill the vector than materialize every entry and then remove the useless ones. This would be faster in almost every case, and also enrich the symbol query API, which can only be useful later.

This is indeed super useful, that's exactly the direction these commands should take. Other options include filtering by name or wildcard, which would also be useful for symbols (I guess we could have a generic symbol query tool, `is` would use it, and `isc` would use it + filter syscalls only). I can test this later today. Maybe a nitpick, but I'd rather perform symbol queries to fill the vector than materialize every entry and then remove the useless ones. This would be faster in almost every case, and also enrich the symbol query API, which can only be useful later.
Dr-Carlos force-pushed isc from 2eb893b77a to e037d5889c 2022-04-18 23:45:39 +02:00 Compare
Dr-Carlos added 1 commit 2022-04-19 00:02:58 +02:00
Dr-Carlos commented 2022-04-19 00:04:20 +02:00
Author
Collaborator
Copy Link

This is indeed super useful, that's exactly the direction these commands should take. Other options include filtering by name or wildcard, which would also be useful for symbols (I guess we could have a generic symbol query tool, is would use it, and isc would use it + filter syscalls only).

I can test this later today. Maybe a nitpick, but I'd rather perform symbol queries to fill the vector than materialize every entry and then remove the useless ones. This would be faster in almost every case, and also enrich the symbol query API, which can only be useful later.

I agree with changing the remove. I have changed this to use os->syscall_at to find the syscall entry for every item instead of removing the useless ones.

It also now prints %1 instead of %001 or %0001 when given addresses. I thought it made more sense, but what do you think?

> This is indeed super useful, that's exactly the direction these commands should take. Other options include filtering by name or wildcard, which would also be useful for symbols (I guess we could have a generic symbol query tool, `is` would use it, and `isc` would use it + filter syscalls only). > > I can test this later today. Maybe a nitpick, but I'd rather perform symbol queries to fill the vector than materialize every entry and then remove the useless ones. This would be faster in almost every case, and also enrich the symbol query API, which can only be useful later. I agree with changing the remove. I have changed this to use os->syscall_at to find the syscall entry for every item instead of removing the useless ones. It also now prints %1 instead of %001 or %0001 when given addresses. I thought it made more sense, but what do you think?
Lephenixnoir reviewed 2022-04-19 13:18:07 +02:00
shell/i.cpp Outdated
@ -175,3 +178,2 @@
return;
// TODO: is <vspace_name> doesn't work
if(!space) {
Lephenixnoir commented 2022-04-19 13:18:07 +02:00
Owner
Copy Link

This condition could also be triggered by isc with no argument and no current vspace. I'm not sure how legitimate "no current vspace" is, maybe we should just guarantee that there is always one. Currently the only way to not have one is to not load a file and have a useless fxosrc

This condition could also be triggered by `isc` with no argument and no current vspace. I'm not sure how legitimate "no current vspace" is, maybe we should just guarantee that there is always one. Currently the only way to not have one is to not load a file and have a useless fxosrc
Dr-Carlos commented 2022-04-19 13:53:49 +02:00
Author
Collaborator
Copy Link

Valid point - I added a check after getting the current vspace. If there is no space there, nothing is selected and we should exit.

Valid point - I added a check after getting the current vspace. If there is no space there, nothing is selected and we should exit.
Dr-Carlos marked this conversation as resolved
Lephenixnoir reviewed 2022-04-19 13:18:56 +02:00
shell/i.cpp Outdated
@ -182,0 +185,4 @@
if(!os) {
if(!vspace_name.empty())
FxOS_log(ERR, "OS analysis on '%s' failed", vspace_name);
FxOS_log(ERR, "OS analysis failed");
Lephenixnoir commented 2022-04-19 13:18:56 +02:00
Owner
Copy Link

I guess an else is missing here?

I guess an `else` is missing here?
Dr-Carlos marked this conversation as resolved
Lephenixnoir commented 2022-04-19 13:23:09 +02:00
Owner
Copy Link

Very cool! Thanks again. Another minor detail is sort=true is not honored with explicit aguments; but that's not blocking.

I'll merge the PR probably tonight, and "fix" the details left at the same time.

Edit: you should resolve the vspace name at parsing time like in e so that symbol names can be interpreted properly:

# This is a cool feature
fx_3.10 @ 0x80000000> isc Bdisp_PutDisp_DD
  0x80013642  %28
# But it doesn't work from another vspace:
fx_3.10 @ 0x80000000> vs cg_3.60
cg_3.60 @ 0x80000000> isc vspace=fx_3.10 Bdisp_PutDisp_DD
cg_3.60 @ 0x80000000>
Very cool! Thanks again. Another minor detail is `sort=true` is not honored with explicit aguments; but that's not blocking. I'll merge the PR probably tonight, and "fix" the details left at the same time. Edit: you should resolve the vspace name at parsing time like in `e` so that symbol names can be interpreted properly: ``` # This is a cool feature fx_3.10 @ 0x80000000> isc Bdisp_PutDisp_DD 0x80013642 %28 # But it doesn't work from another vspace: fx_3.10 @ 0x80000000> vs cg_3.60 cg_3.60 @ 0x80000000> isc vspace=fx_3.10 Bdisp_PutDisp_DD cg_3.60 @ 0x80000000> ```
Dr-Carlos added 1 commit 2022-04-19 13:52:53 +02:00
Dr-Carlos commented 2022-04-19 13:54:39 +02:00
Author
Collaborator
Copy Link

Very cool! Thanks again. Another minor detail is sort=true is not honored with explicit aguments; but that's not blocking.

I'll merge the PR probably tonight, and "fix" the details left at the same time.

Edit: you should resolve the vspace name at parsing time like in e so that symbol names can be interpreted properly:

# This is a cool feature
fx_3.10 @ 0x80000000> isc Bdisp_PutDisp_DD
  0x80013642  %28
# But it doesn't work from another vspace:
fx_3.10 @ 0x80000000> vs cg_3.60
cg_3.60 @ 0x80000000> isc vspace=fx_3.10 Bdisp_PutDisp_DD
cg_3.60 @ 0x80000000>

Yep, fixed. What are you referring to with sort=true?

> Very cool! Thanks again. Another minor detail is `sort=true` is not honored with explicit aguments; but that's not blocking. > > I'll merge the PR probably tonight, and "fix" the details left at the same time. > > Edit: you should resolve the vspace name at parsing time like in `e` so that symbol names can be interpreted properly: > > ``` > # This is a cool feature > fx_3.10 @ 0x80000000> isc Bdisp_PutDisp_DD > 0x80013642 %28 > # But it doesn't work from another vspace: > fx_3.10 @ 0x80000000> vs cg_3.60 > cg_3.60 @ 0x80000000> isc vspace=fx_3.10 Bdisp_PutDisp_DD > cg_3.60 @ 0x80000000> > ``` Yep, fixed. What are you referring to with `sort=true`?
Lephenixnoir commented 2022-04-19 14:16:33 +02:00
Owner
Copy Link

Sweet. Here's what I mean with sort=true:

cg_3.60 @ 0x80000000> isc sort=true %1f65 %1f64
  0x802e1358  %1f65
  0x802e1116  %1f64

As you can see despite setting sort=true the entries are presented in argument order and not sorted. ^^

Sweet. Here's what I mean with `sort=true`: ``` cg_3.60 @ 0x80000000> isc sort=true %1f65 %1f64 0x802e1358 %1f65 0x802e1116 %1f64 ``` As you can see despite setting `sort=true` the entries are presented in argument order and not sorted. ^^
Dr-Carlos added 1 commit 2022-04-19 14:22:06 +02:00
Dr-Carlos commented 2022-04-19 14:22:45 +02:00
Author
Collaborator
Copy Link

Sweet. Here's what I mean with sort=true:

cg_3.60 @ 0x80000000> isc sort=true %1f65 %1f64
  0x802e1358  %1f65
  0x802e1116  %1f64

As you can see despite setting sort=true the entries are presented in argument order and not sorted. ^^

Ah, thanks. I forgot about this when I changed it to search the addresses instead of remove_if. Fixed!

> Sweet. Here's what I mean with `sort=true`: > > ``` > cg_3.60 @ 0x80000000> isc sort=true %1f65 %1f64 > 0x802e1358 %1f65 > 0x802e1116 %1f64 > ``` > As you can see despite setting `sort=true` the entries are presented in argument order and not sorted. ^^ Ah, thanks. I forgot about this when I changed it to search the addresses instead of remove_if. Fixed!
Lephenixnoir commented 2022-04-20 10:17:09 +02:00
Owner
Copy Link

Slightly late, but there you go!

Slightly late, but there you go!
Lephenixnoir merged commit 0bdb98562d into master 2022-04-20 10:17:17 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Lephenixnoir/fxos#6
No description provided.
Delete Branch "Dr-Carlos/fxos:isc"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?