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

Sync master branches #750

Merged
merged 2 commits into from
Jul 3, 2024
Merged

Conversation

JorisVanEijden
Copy link
Contributor

@JorisVanEijden JorisVanEijden commented Jun 26, 2024

Description of Changes

A bunch of changes I had lying around my branch for ages.

  • Some formatting
  • Recording and dumping the mopst recently executed addresses. Helpt to debug Cpu Failures.
  • Record and allow dumping of callstack, also helps a lot in debugging
  • LogLevel changes to reduce noise
  • An ArgumentFetcher to help getting the arguments passed into overridden functions.
  • A "DoOnMemoryWrite" breakpoint setter for the CSharpOverrideHelper
  • Add a Peek function for a byte.
  • Mayeb some other small changes.

Rationale behind Changes

It's mostly small tweaks to make my life easier during reverse engineering of Krondor and debugging of Spice86.
Feel free to comment on stuff that should definitly not go into Spice86 and I'll keep those in my own branch.
Otherwise it'd be nice if we all work from mostly the same core codebase.

Suggested Testing Steps

Run games, do debugging, try out stuff.

Copy link
Member

@maximilien-noal maximilien-noal left a comment

Choose a reason for hiding this comment

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

Looks good overall, I've requested only a few changes.

@maximilien-noal maximilien-noal added reverse engineering Related to reverse enginneering (features, APIs, ...) DOS Related to DOS CPU Related to the CPU labels Jun 27, 2024
@maximilien-noal maximilien-noal self-requested a review June 28, 2024 08:13
int bufferLength = _buffer.Length;
for (int i = _writeIndex; i < _writeIndex + bufferLength; i++) {
int bufferIndex = i % bufferLength;
sb.AppendLine(_buffer[bufferIndex]?.ToString());

Check notice

Code scanning / CodeQL

Redundant ToString() call Note

Redundant call to 'ToString' on a String object.
@@ -46,6 +45,8 @@ public class ExecutionFlowRecorder {
private readonly HashSet<uint> _instructionsEncountered = new(200000);
private readonly HashSet<uint> _executableCodeAreasEncountered = new(200000);

private readonly CircularBuffer<string> _callStack = new(20);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already recorded by FunctionHandler which already provides a method to dump the call stack as string.
DumpCallStack
Isnt it what you need?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite. Comparing the output:

.....1834:7259 -> 1834:6182
.....1834:7313 -> 1834:6253
......1834:626F -> 1834:623E
.......1834:624A -> 1000:13C8
........1000:13D0 -> 1000:2E10
.........1000:2E27 -> 1000:2E2D
..........1000:2E62 -> 1000:27A9
......1834:6281 -> 1000:15F2
......1834:62A6 -> 1834:623E
.......1834:624A -> 1000:13C8
........1000:13D0 -> 1000:2E10
.........1000:2E27 -> 1000:2E2D
..........1000:2E62 -> 1000:27A9
....4C9B:028D -> 1834:3EB7
.....1834:3EC4 -> 1834:39D0
.....1834:3ED8 -> 1000:06D9
......1000:070F -> 1000:3544
....4C9B:0297 -> 4C9B:047E
...2083:03D2 -> 38A2:002F

vs

- unknown_38A2_002F_38A4F expected to return to address 2083:03D7
- interrupt_handler_0x3F_36BC_04F6_370B6 expected to return to address 38AD:0027
- unknown_38AD_0025_38AF5 expected to return to address 4C9B:003F
- unknown_4C9B_0030_4C9E0 expected to return to address 4C9B:0197
- interrupt_handler_0x3F_36BC_04F6_370B6 expected to return to address 38A2:002C
- unknown_38A2_002A_38A4A expected to return to address 2083:03CB
- unknown_2083_038C_20BBC expected to return to address 2083:0E4E
- interrupt_handler_0x3F_36BC_04F6_370B6 expected to return to address 382F:0002
- interrupt_handler_0x3F_36BC_04F6_370B6 expected to return to address 396F:0022
- unknown_396F_0020_39710 expected to return to address 4B5A:057C
- unknown_382F_005C_3834C expected to return to address 2083:0E33
- unknown_2083_0E15_21645 expected to return to address 1000:015D
- unknown_1000_3298_13298 expected to return to address 1000:0260
- unknown_1000_0220_10220 expected to return to address 1000:014C
- entry_1000_0000_10000 expected to return to address

I did name my helper wrong. It's not the call stack, but the last 20 executes function calls.

It helps a lot in figuring out how you got to a certain location in the code. Especially to figure out where the value for an argument in the current function comes from, since it's often the return value of a previous function call.

I'm also totally fine with just keeping these in my branch if they're not of general use.

Copy link
Member

@maximilien-noal maximilien-noal Jun 30, 2024

Choose a reason for hiding this comment

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

I'm OK with merging all of this. Once this _callStack object is renamed to (for example) _lastTwentyExecutedFunctionCalls ?

@kevinferrare kevinferrare merged commit 4fa0f63 into OpenRakis:master Jul 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CPU Related to the CPU DOS Related to DOS reverse engineering Related to reverse enginneering (features, APIs, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants