Skip to content

fix: deoptimize math-specialized functions on non-numeric arguments - #30

Merged
esrrhs merged 3 commits into
masterfrom
fix/spec-func-non-numeric-deopt
Oct 11, 2026
Merged

esrrhs merged 3 commits into
masterfrom
fix/spec-func-non-numeric-deopt

Conversation

@esrrhs

@esrrhs esrrhs commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Problem

A file-level math-specialized function whose math-position argument was non-numeric (string/bool/nil/table, including nil filled in for a missing argument) hard-failed directly in the TCC/GCC entry dispatcher. The exception escaped the generated dylib with no handler → terminate due to uncaught exception, aborting the whole process. It could not be caught by pcall, while the INTERP backend executed the function body normally:

function f(x)
    if type(x) ~= "number" then return -1 end  -- legal type guard
    return x + 1
end
f("abc")  -- interp: -1 ; TCC/GCC (master): process abort

Fix

  • Every specialized function now gets an additional <fn>__dyn full-dynamic variant compiled with CompileFuncBody(..., spec_bitmask=-1) — the mature non-specialized CVar path. The entry dispatcher routes to it when any math argument is neither VAR_INT nor VAR_FLOAT, instead of throwing. Genuine arithmetic errors inside the body still take the pcall-able Lua error path, and all three backends now report the same body-level message (attempt to perform arithmetic on non-numeric value).
  • New FakeluaSpecMissException (util/exception.h): the 7 tentative native-numeric compile sites (CompileNumericExp, raw native binop/unop) throw this instead of a logged hard error. TryCompileNativeExpr catches it as routine fallback; if it escapes Build(), Generate() converts it to the same Code generate failed, ... text as before.
  • __dyn is forward-declared in the decls section.
  • Recursion inside __dyn re-enters through the VM entry dispatcher; RTLD_LOCAL keeps same-named symbols isolated across files.

Tests

  • New test/lua/infer/test_spec_deopt.lua + infer.test_spec_deopt: type guard, multiple math params, heterogeneous return (string), pcall, recursion, C++ boundary catch — across TCC/GCC/INTERP × debug on/off.
  • exception.math_param_non_numeric_error updated to the unified message and now runs on AllJitTypes().
  • InferGetCCode strips __dyn bodies by default so the 100+ existing shape assertions keep targeting the specialized variants; the deopt test opts in with keep_dyn_variants=true.
  • Diff tests: 32 new cases vs Lua 5.4.9, 0 disagreements; prior 223 cases show no new regressions.
  • Full suite: infer 220/220, exception 105/105, all pass except the 8 test_mysql.integration_* tests that require a live MySQL server (pre-existing environment baseline).

File-level math-specialized functions hard-failed in the TCC/GCC entry
dispatcher when a math-position argument was non-numeric (string/bool/
nil/table, or nil from a missing argument). The exception escaped the
dylib with no handler and aborted the process; pcall could not catch it,
while the INTERP backend ran the function body normally (e.g. a type()
guard returning early).

- emit a <fn>__dyn full-dynamic variant for each specialized function
  (CompileFuncBody with spec_bitmask=-1, the mature non-specialized
  path); the entry dispatcher routes non-numeric math arguments to it
  instead of throwing, so body-level errors stay pcall-able Lua errors
- add FakeluaSpecMissException: tentative native numeric compilation
  (CompileNumericExp etc., 7 sites) now falls back silently as normal
  control flow; escaping Build() still becomes the same hard compile
  error text as before
- forward-declare __dyn in the decls section
- tests: add test_spec_deopt (type guard/multi-arg/heterogeneous
  return/pcall/recursion across all backends and debug modes); update
  math_param_non_numeric_error to the unified body-level message and
  run on AllJitTypes(); InferGetCCode strips __dyn bodies by default so
  existing shape assertions target the specialized variants
@codecov-commenter

codecov-commenter commented Oct 11, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 85.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.00%. Comparing base (2c89572) to head (30f7ba1).

Files with missing lines Patch % Lines
src/compile/c_gen.cpp 84.61% 6 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   87.99%   88.00%   +0.01%     
==========================================
  Files         124      124              
  Lines       25388    25414      +26     
==========================================
+ Hits        22339    22366      +27     
+ Misses       3049     3048       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@esrrhs
esrrhs merged commit 2aabdad into master Oct 11, 2026
9 checks passed
@esrrhs
esrrhs deleted the fix/spec-func-non-numeric-deopt branch October 11, 2026 23:38
esrrhs added a commit that referenced this pull request Oct 11, 2026
…xit code (#31)

* fix: reject redis pub/sub commands, auto-seed math.random, CLI exit code

Fixes three issues from the FakeLua Phase 16 integration report
(docs/fakelua-bugs-phase16.md, based on dafdac7).

redis (high-value safety fix): SUBSCRIBE/PSUBSCRIBE/UNSUBSCRIBE/
PUNSUBSCRIBE issued via the one-shot conn:command() channel left hiredis
holding a dangling CmdOp* as privdata after the first ack popped and
destroyed it; every subsequent published message wrote through the
freed pointer (use-after-free/heap corruption). Full pub/sub needs a
dedicated long-lived subscription context, so for now the command
verbs are rejected case-insensitively before the readiness check with
a clear error (pcall-catchable, works without a live server), and both
native READMEs document the limitation plus the polling workaround.

math.random: std::rand() defaulted to seed 1, so without an explicit
math.randomseed every process start produced the identical sequence.
The math library now auto-seeds once per process on first state init
(time + std::random_device + stack-address entropy), matching Lua 5.4;
explicit math.randomseed(x) still overrides.

flua CLI: an uncaught FakeluaException propagated to libc++abi and
killed the process with SIGABRT (exit 134). main() now catches
std::exception around compile/run, prints "flua: <msg>" to stderr and
exits 1, per standard Lua interpreter convention.

The report's Bug 1 (math-specialization entry guard aborting on
non-numeric args) is addressed by the independent PR #30.

Tests: new test_redis.subscribe_rejected (all backends, no server
required); full suite passes except the 8 test_mysql integration tests
that need a live MySQL server.

* fix: reject sharded pub/sub commands (ssubscribe/sunsubscribe)
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.

2 participants