Skip to content

Prevent native crashes on invalid Lua calls in optimized builds - #842

Open
Gopmyc wants to merge 1 commit into
Overload-Technologies:mainfrom
Gopmyc:828
Open

Gopmyc wants to merge 1 commit into
Overload-Technologies:mainfrom
Gopmyc:828

Conversation

@Gopmyc

@Gopmyc Gopmyc commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description

sol2 only enables its safety checks on debug builds. Release and Publish compile with NDEBUG, so SOL_SAFE_USERTYPE is off and a member call with a bad self goes down the unchecked branch in call.hpp. That branch dereferences a null userdata pointer in stack_get_unqualified.hpp, so we get an access violation instead of a Lua error.

Scene:FindActorByName("...") is one way to reach it. The global Scene is the usertype metatable, not a scene instance (the instance comes from Scenes.GetCurrentScene()). Any bound member of the 36 registered usertypes does the same thing when self is wrong, and so do nil or mistyped arguments.

I fixed it with a build define rather than in the bindings because sol2 already has the check, we just compile it out. Doing it by hand would mean validating self on every bound member of every usertype, then remembering it on each new binding. sol2 points to build defines for this in sol/config.hpp, and patching the vendored header would be lost the next time we update the dependency.

One thing to know before merging: Release and Publish now run integer precision checks as well. Math.RandomInt(1, n / 2) raises an error instead of silently truncating, since / returns a float in Lua 5.3+. Debug builds have always behaved this way.

Related Issue(s)

Fixes #828

Review Guidance

  • Placed at workspace level to match SOL_NO_LUA_HPP from Fix lua integration by disabling lua.hpp usage in sol2 #692. All sol2 translation units currently sit in OvCore, so scoping the define to that project works just as well if you prefer it there.
  • Cost is one type check per self and per argument on every bound call, in all configurations.
  • If the integer strictness is unwanted, SOL_SAFE_USERTYPE and SOL_SAFE_FUNCTION_CALLS on their own remove the crash without touching number handling.
  • This does not cover crashes sol2 has no say in, such as Scenes.GetCurrentScene() dereferencing a null scene.

Screenshots/GIFs

N/A

AI Usage Disclosure

Debugging

Checklist

  • My code follows the project's code style guidelines
  • When applicable, I have commented my code, particularly in hard-to-understand areas
  • When applicable, I have updated the documentation accordingly
  • My changes don't generate new warnings or errors
  • I have reviewed and take responsibility for all code in this PR (including any AI-assisted contributions)

@Gopmyc Gopmyc left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The crash needs fixing, I'm sure of that. Less sure SOL_ALL_SAFETIES_ON is the tradeoff you want, given the cost in every config. Would rather have someone else weigh in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Crash in OvEditor calling Scene:FindActorByName from Lua : access violation in sol (null userdata)

1 participant