Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Lua’s longjmp-based error handling to better cooperate with AddressSanitizer by invoking an ASan “no return” hook before performing longjmp.
Changes:
- Adds a declaration for
__asan_handle_no_return. - Wraps
LUAI_THROW(for_longjmp/_setjmpandlongjmp/setjmpconfigurations) to call__asan_handle_no_return()beforelua_do_longjmp.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| void __asan_handle_no_return(void); | ||
|
|
There was a problem hiding this comment.
__asan_handle_no_return is declared unconditionally, but the symbol is only provided when linking with the ASan runtime. In non-ASan builds this will either cause a link error once referenced, or at minimum introduces an undocumented dependency. Consider wrapping the declaration (and any calls) in an ASan feature check (e.g. __SANITIZE_ADDRESS__ or __has_feature(address_sanitizer)), and define a no-op fallback when ASan is not enabled; alternatively include sanitizer/asan_interface.h conditionally when available.
| /* in Unix, try _longjmp/_setjmp (more efficient) */ | ||
| #define LUAI_THROW(L,c) lua_do_longjmp((c)->b, 1) | ||
| #define LUAI_THROW(L,c) do { __asan_handle_no_return(); lua_do_longjmp((c)->b, 1); } while(0) | ||
| #define LUAI_TRY(L,c,a) if (lua_do_setjmp((c)->b) == 0) { a } |
There was a problem hiding this comment.
LUAI_THROW now unconditionally calls __asan_handle_no_return(). Without guarding this call behind an AddressSanitizer-enabled compile check (or providing a no-op fallback), non-ASan builds can fail to link or may inadvertently require the ASan runtime. Gate the call behind an __SANITIZE_ADDRESS__/__has_feature(address_sanitizer) check or similar.
| /* default handling with long jumps */ | ||
| #define LUAI_THROW(L,c) lua_do_longjmp((c)->b, 1) | ||
| #define LUAI_THROW(L,c) do { __asan_handle_no_return(); lua_do_longjmp((c)->b, 1); } while(0) | ||
| #define LUAI_TRY(L,c,a) if (lua_do_setjmp((c)->b) == 0) { a } |
There was a problem hiding this comment.
Same issue as above: this LUAI_THROW variant calls __asan_handle_no_return() unconditionally, which can break non-ASan builds. Please guard it behind an ASan-enabled macro check or provide a no-op fallback when ASan is not active.
|
DON'T DELETE |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 27c45ad. Configure here.
| */ | ||
| #if defined(__SANITIZE_ADDRESS__) | ||
| # define lua_do_setjmp setjmp | ||
| # define lua_do_longjmp longjmp |
There was a problem hiding this comment.
ASAN detection misses Clang before version 21
Low Severity
The ASAN check uses only __SANITIZE_ADDRESS__, which GCC has supported since 4.8 but Clang only added in version 21 (2026). Clang versions before 21 use __has_feature(address_sanitizer) instead. Builds with older Clang and -fsanitize=address will silently skip this block and continue using the custom asm setjmp/longjmp, defeating the purpose of the ASAN fix. The common portable pattern checks both: defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)).
Reviewed by Cursor Bugbot for commit 27c45ad. Configure here.


Note
Low Risk
Change is gated to ASan builds and only affects how non-local jumps are implemented under sanitizer; production behavior remains unchanged, but it touches core error/exception jump plumbing.
Overview
When
__SANITIZE_ADDRESS__is enabled,src/thrlua.hnow bypasses the custom arch-specific asmlua_setjmp_*/lua_longjmp_*wrappers and mapslua_do_setjmp/lua_do_longjmpdirectly to libcsetjmp/longjmp.This allows ASan to intercept jumps and correctly unpoison skipped stack frames during Lua error handling, reducing ASan false positives while keeping the existing asm fast paths for non-ASan builds.
Reviewed by Cursor Bugbot for commit 511acc4. Bugbot is set up for automated code reviews on this repo. Configure here.