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

Fix socket in --luadebug mode. #518

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

elliottcarlson
Copy link

Currently, when launching with --luadebug and using require("socket") will cause Repentogon to crash. This is due to the scripts/socket/core.dll being built for the base TBOI, and it will not work with Lua 5.4.

The fix for this is to build a new luasocket linked to Lua 5.4, however, there are some nuances to this approach.

Since scripts/socket.lua ships with TBOI, and it directly references scripts/socket/core.dll, we can not cleanly override when socket is being required. As I don't think there should be any permanent changes to the default files (which would get reset on a Steam integrity check anyhow), we have to hack around this slightly.

The route I have taken is to add luasocket as a new libs submodule. I have created a patch for src/luasocket.c which will rename the entry point to repentogon_socket.core. A new folder is added under resources/scripts called repentogon_socket which includes the same base files from the socket folder, but gets the new core.dll that is built during the standard build process.

The one hacky element to this approach, which I am not really happy about, but don't see a solid way of working around (definitely open to suggestions), is the interception of require("socket") inside of main_ex.lua. I took advantage of the fact that require is already being caught and re-addressed to oldrequire, and add a check for if the modname is socket.core then it should load in the repentogon_socket.core instance instead. While I am not happy about this, it allows luasocket to be loaded without altering any of the base files in Isaac.

I have tested this extensively with Repentogon, as well as via -repentogonoff, and performing an uninstall of Repentogon to ensure it continues to work as expected.

No changes in the build process; in the cleanup process there would be the additional removal of the repentogon_socket folder within scripts, however it has no side effects if not removed either.

@elliottcarlson elliottcarlson mentioned this pull request Aug 7, 2024
@elliottcarlson
Copy link
Author

In reference to @jsgnextortex's valid concern in #517 - this does not remove the need for --luadebug - when the flag is not present it will still not load this library. This is a fix for when --luadebug has been specified, as require("socket") will currently crash Repentogon.

@Sylmir
Copy link
Collaborator

Sylmir commented Aug 7, 2024

The Lua 5.3 DLL is still loaded by the game if I'm not mistaken. We only rewrite its import table to point towards the exports of Lua 5.4 (after loading 5.4 of course).

What exactly is the reason for the crash ? The only thing that comes to my mind is that Windows may remap Lua5.3.3.r.dll cleanly in the address space of the socket lib, meaning our redirections towards Lua 5.4 are not effective in there

@elliottcarlson
Copy link
Author

I could be mistaken, but I believe that since requiring socket is referencing the resources/scripts/socket.lua, which in turn does the require for socket.core i,e, resources/scripts/socket/core.dll, it is loading the 5.3 linked DLL in the 5.4 runtime. This is happening outside of internal game code.

@Sylmir
Copy link
Collaborator

Sylmir commented Aug 7, 2024

image
I'm running Repentogon on a debug build of commit 56ce0dd and I can require socket just fine from the console, even iterating over its function, so I'm even more confused as to how you got a crash

@Sylmir
Copy link
Collaborator

Sylmir commented Aug 7, 2024

And we can see that most of the addresses given by Lua do match the address range where core.dll was mapped in memory :
image

@elliottcarlson
Copy link
Author

Let me poke around and see where the issue is.

@elliottcarlson
Copy link
Author

elliottcarlson commented Aug 7, 2024

So I am getting a clear crash when using a clean Repentogon 1.0.11b install (without my changes)

Tested via:

  • require from within a mod
  • doing a require in the debug console as you did
  • altering to do a pcall

The stack trace in the logs:

[INFO] - Lua stack trace:
[INFO] - [C](-1): ?
[INFO] - [C](-1): oldrequire
[INFO] - resources/scripts/main_ex.lua(52): require
[INFO] - .\resources\scripts\socket.lua(12): ?
[INFO] - [C](-1): oldrequire
[INFO] - resources/scripts/main_ex.lua(52): require
[INFO] - lua(1): ?
[INFO] - Caught exception, writing minidump...

When loading without Repentogon it works (both scenarios using the --luadebug flag).

I'll try a fresh Isaac install with the above scenarios, as well as a dev build on the commit you mentioned and will report back.

Note: I had this reported to me by some users of my Twitch Integration mod - hence why I am digging in deeper.

@Sylmir
Copy link
Collaborator

Sylmir commented Aug 7, 2024

Some testing on our end seems to indicate that running Isaac from a debugger (as I did in my screenshots) instead of from Steam prevents the crash. Are you running from Steam or from a debugger directly (and bypassing Steam when launching from the debugger) ?

@elliottcarlson
Copy link
Author

From Steam directly whether with or without Repentogon.

@Sylmir
Copy link
Collaborator

Sylmir commented Aug 7, 2024

What if you try to launch it through a debugger while bypassing Steam (add a steam_appid.txt file next to isaac-ng.exe and write 250900 in it) ? If it doesn't crash then we're looking at a funny Steam interaction, and massive headaches

@elliottcarlson
Copy link
Author

I'll have to try later tonight; I'll get back to you with my findings.

@elliottcarlson
Copy link
Author

Real life came in the way; I should be able to try this weekend.

@youngtaek-id
Copy link

Is there any progress on this issue?

@SlawekNowy
Copy link

SlawekNowy commented Oct 13, 2024

I made the clean installation of Repentance via DepotDownloader, plopped in steam emulator... and luadebug functioned just fine.
Can't run x86dbg on vanilla exe without stripping SteamStub.

EDIT: Ok after adding StartDebug() (and stripping SteamStub) on dummy mod I got DIVIDE_BY_ZERO exception. Tested with release. I'll try with master.
EDIT2: debugger screenshot
luadebug

@SlawekNowy
Copy link

Same thing with master.

@SlawekNowy
Copy link

SlawekNowy commented Oct 15, 2024

Here's the link to x96dbg minidump
https://drive.google.com/file/d/1gooiwGi6zof8195VFPgibMqsfDrwSSHn/view?usp=drive_link
using master

EDIT: Should have written precise commit of the repentogon: 01a3bf3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants