Fix callback chaining for _initterm function table walking#277
Fix callback chaining for _initterm function table walking#277williballenthin wants to merge 5 commits intomasterfrom
Conversation
Co-authored-by: Moritz <mr-tz@users.noreply.github.com>
The callback handler in winemu.py only executed the first queued callback before returning to the original caller, silently dropping any remaining callbacks. This made setup_callback() unusable for APIs that need to invoke multiple function pointers in sequence (like _initterm). The handler now properly chains callbacks: when one completes, it sets up the next instead of returning early. _initterm and _initterm_e now parse the function pointer table and report the entries in the API event args for analyst visibility. Closes #107
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
williballenthin
force-pushed
the
worktree-fix-initterm-107
branch
from
9b33979 to
0c35735
Compare
williballenthin
force-pushed
the
worktree-fix-initterm-107
branch
from
c5c464b to
a08e984
Compare
| elif address == winemu.API_CALLBACK_HANDLER_ADDR: | ||
| run = self.get_current_run() | ||
| if run.api_callbacks: | ||
| pc, orig_func, args = run.api_callbacks.pop(0) | ||
| self.do_call_return(len(args), pc) | ||
| ret, orig_func, args = run.api_callbacks.pop(0) | ||
| if run.api_callbacks: | ||
| next_ret, next_func, next_args = run.api_callbacks[0] | ||
| if next_ret is None: | ||
| run.api_callbacks[0] = (ret, next_func, next_args) | ||
| sp = self.get_stack_ptr() | ||
| self.set_func_args(sp, winemu.API_CALLBACK_HANDLER_ADDR, *next_args) | ||
| self.set_pc(next_func) | ||
| else: | ||
| self.do_call_return(len(args), ret) | ||
| self._unset_emu_hooks() | ||
| return True |
There was a problem hiding this comment.
this nesting doesn't seem correct. why is if run.api_callbacks tested twice?
| if address == winemu.SEH_RETURN_ADDR: | ||
| self.continue_seh() |
There was a problem hiding this comment.
i wonder if maybe we should pull this whole block into its own self._handle_invalid_mem_exec so we can document its behavior a bit better. this function is getting a bit long.
| return rv | ||
| funcs = self._read_func_table(pfbegin, pfend) | ||
| if funcs: | ||
| argv.append(", ".join(f"0x{f:x}" for f in funcs)) |
There was a problem hiding this comment.
what is this doing?? its extending argv with a string? at least need some inline documentation cause i don't get it
There was a problem hiding this comment.
i don't think these tests add enough value, lets remove the file.
2 participants
Summary
winemu.py: TheAPI_CALLBACK_HANDLER_ADDRhandler previously popped the first callback and returned to the original caller, silently dropping remaining queued callbacks. It now properly chains: when one callback completes, it sets up the next callback's stack frame and redirects execution there, only returning to the original caller after the last callback finishes._initterm/_initterm_etable parsing: Both functions now read the function pointer table betweenpfbeginandpfendand report the non-NULL entries in the API event args. The functions still return immediately (CRT init functions generally rely on environment state the emulator doesn't provide), but the table contents are now visible to analysts.Closes #107
Test plan
test_initterm_reports_function_table— verifies function pointers appear in API event argstest_initterm_does_not_crash_emulation— verifies main() still executes after _initterm🤖 Generated with Claude Code