Conversation
68f27b8 to
9ac267a
Compare
311b2a4 to
4f0666e
Compare
f20a737 to
60903f8
Compare
|
Hi @ligurio Thanks for taking over this. I just tested on macOS ARM64 (M4), Darwin 24.6.0 Toolchain: LLVM 22.1.1, LuaJIT 2.1.1772619647, cmake 4.3.0 (all from homebrew) used: cmake -DCMAKE_C_COMPILER=/opt/homebrew/opt/llvm/bin/clang \
-DCMAKE_CXX_COMPILER=/opt/homebrew/opt/llvm/bin/clang++ \
-DENABLE_LUAJIT=ON \
-DLUA_INCLUDE_DIR=/opt/homebrew/Cellar/luajit/2.1.1772619647/include/luajit-2.1 \
-DLUA_LIBRARIES=/opt/homebrew/Cellar/luajit/2.1.1772619647/lib/libluajit-5.1.dylib \
-DCMAKE_BUILD_TYPE=Release -S . -B build
cmake --build build --parallelEverything worked cleanly
So LGTM :) |
46616ba to
a61a76b
Compare
ff83304 to
ac695df
Compare
|
Define Brew prefixes only once: diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 304b61c..168a2db 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -69,14 +69,20 @@ jobs:
-DCMAKE_C_COMPILER=clang-15 -DCMAKE_CXX_COMPILER=clang++-15
${{ matrix.CMAKEFLAGS }} ${{ matrix.FLAVORFLAGS }}
+ - name: Define Brew prefixes (macOS)
+ if: runner.os == 'macOS'
+ run: |
+ echo "BREW_LLVM_PREFIX=`brew --prefix llvm`" >> $GITHUB_ENV
+ echo "BREW_LUAJIT_PREFIX=`brew --prefix luajit`" >> $GITHUB_ENV
+
- name: Running CMake (macOS)
if: runner.os == 'macOS'
run: |
cmake -S . -B build -G Ninja \
- -DCMAKE_C_COMPILER=/opt/homebrew/opt/llvm/bin/clang \
- -DCMAKE_CXX_COMPILER=/opt/homebrew/opt/llvm/bin/clang++ \
- -DLUA_INCLUDE_DIR=/opt/homebrew/include/luajit-2.1 \
- -DLUA_LIBRARIES=/opt/homebrew/lib/libluajit-5.1.dylib \
+ -DCMAKE_C_COMPILER=$BREW_LLVM_PREFIX/bin/clang \
+ -DCMAKE_CXX_COMPILER=$BREW_LLVM_PREFIX/bin/clang++ \
+ -DLUA_INCLUDE_DIR=$BREW_LUAJIT_PREFIX/include/luajit-2.1 \
+ -DLUA_LIBRARIES=$BREW_LUAJIT_PREFIX/lib/libluajit-5.1.dylib \
-DENABLE_LUAJIT=ON \
-DLUAJIT_FRIENDLY_MODE=ON \
${{ matrix.CMAKEFLAGS }} |
10307e1 to
eb63251
Compare
|
The code related to getting symbols location was refactored and squashed to the commit diff --git a/luzer/luzer.c b/luzer/luzer.c
index e7b494e..1ed0eb8 100644
--- a/luzer/luzer.c
+++ b/luzer/luzer.c
@@ -120,45 +120,25 @@ __sanitizer_print_stack_trace(void)
} /* extern "C" */
#endif
-NO_SANITIZE const char *
-get_libFuzzer_symbols_location(void) {
+NO_SANITIZE char *
+get_symbol_path(void *addr) {
Dl_info dl_info;
- if (!dladdr((void*)&LLVMFuzzerRunDriver, &dl_info)) {
- return "<Not a shared object>";
+ if (dladdr(addr, &dl_info)) {
+ if (!dl_info.dli_sname)
+ return NULL;
}
- return (dl_info.dli_fname);
-}
-
-NO_SANITIZE const char *
-get_coverage_symbols_location(void) {
- Dl_info dl_info;
- if (!dladdr((void*)&__sanitizer_cov_8bit_counters_init, &dl_info)) {
- return "<Not a shared object>";
+ char *path = realpath(dl_info.dli_fname, NULL);
+ if (!path) {
+ perror("realpath");
+ return NULL;
}
- return (dl_info.dli_fname);
+ return path;
}
const char *dso_path_lf_asan;
const char *dso_path_lf_ubsan;
const char *dso_path_libcustom_mutator;
-NO_SANITIZE const char *
-search_module_path(char *base_path) {
- Dl_info dlinfo;
- int rc = dladdr((void *)&get_coverage_symbols_location, &dlinfo);
- if (!rc) {
- fprintf(stderr, "%s\n", dlerror());
- return NULL;
- }
- char *path = realpath(dlinfo.dli_fname, base_path);
- if (!path) {
- perror("realpath");
- return NULL;
- }
- const char *dir = dirname(path);
- return dir;
-}
-
NO_SANITIZE void
init(void)
{
@@ -169,34 +149,43 @@ init(void)
assert(NULL);
}
- if (strcmp(get_coverage_symbols_location(), get_libFuzzer_symbols_location()) != 0) {
+ char *coverage_symbols_location =
+ get_symbol_path((void*)&__sanitizer_cov_8bit_counters_init);
+ char *libFuzzer_symbols_location =
+ get_symbol_path((void*)&LLVMFuzzerRunDriver);
+ if (strcmp(libFuzzer_symbols_location, coverage_symbols_location) != 0) {
fprintf(stderr,
"WARNING: Coverage symbols are being provided by a library other than "
"libFuzzer. This will result in a broken Lua code coverage and "
"severely impacted native extension code coverage. Symbols are coming "
- "from this library: %s\n", get_coverage_symbols_location());
+ "from this library: %s\n", coverage_symbols_location);
}
+ free(coverage_symbols_location);
+ free(libFuzzer_symbols_location);
- char base_path[PATH_MAX], path[PATH_MAX];
- const char *base_so_path = search_module_path(base_path);
- if (!base_so_path) {
+ char path[PATH_MAX];
+ char *base_path = get_symbol_path((void *)&get_symbol_path);
+ if (!base_path) {
return;
}
+ const char *base_dir = dirname(base_path);
- snprintf(path, PATH_MAX, "%s/%s", base_so_path, CUSTOM_MUTATOR_LIB);
+ snprintf(path, PATH_MAX, "%s/%s", base_dir, CUSTOM_MUTATOR_LIB);
dso_path_libcustom_mutator = strdup(path);
if (access(dso_path_libcustom_mutator, F_OK))
perror("access");
- snprintf(path, PATH_MAX, "%s/%s", base_so_path, dso_asan_string());
+ snprintf(path, PATH_MAX, "%s/%s", base_dir, dso_asan_string());
dso_path_lf_asan = strdup(path);
if (access(dso_path_lf_asan, F_OK))
perror("access");
- snprintf(path, PATH_MAX, "%s/%s", base_so_path, dso_ubsan_string());
+ snprintf(path, PATH_MAX, "%s/%s", base_dir, dso_ubsan_string());
dso_path_lf_ubsan = strdup(path);
if (access(dso_path_lf_ubsan, F_OK))
perror("access");
+
+ free(base_path);
}
NO_SANITIZE static void |
eb63251 to
aa5898e
Compare
Buristan
left a comment
There was a problem hiding this comment.
[PATCH]: refactor get_symbol_path
Sergey, thanks for the patch.
Please, consider my comments below.
| Dl_info dl_info; | ||
| if (!dladdr((void*)&LLVMFuzzerRunDriver, &dl_info)) { | ||
| return "<Not a shared object>"; | ||
| if (dladdr(addr, &dl_info)) { |
There was a problem hiding this comment.
It would be nice to check !dladdr() case as well, as it was before.
There was a problem hiding this comment.
Updated:
--- a/luzer/luzer.c
+++ b/luzer/luzer.c
@@ -134,7 +134,12 @@ get_symbol_path(void *addr) {
if (dladdr(addr, &dl_info)) {
if (!dl_info.dli_sname)
return NULL;
- }
+ } else
+ /*
+ * The specified address in `addr` could not be matched to
+ * a shared object.
+ */
+ return NULL;
char *path = realpath(dl_info.dli_fname, NULL);
if (!path) {
perror("realpath");There was a problem hiding this comment.
Minor: Please, use matching style of braces:
if () {
} else {
}There was a problem hiding this comment.
Updated:
--- a/luzer/luzer.c
+++ b/luzer/luzer.c
@@ -134,12 +134,13 @@ get_symbol_path(void *addr) {
if (dladdr(addr, &dl_info)) {
if (!dl_info.dli_sname)
return NULL;
- } else
+ } else {
/*
* The specified address in `addr` could not be matched to
* a shared object.
*/
return NULL;
+ }
char *path = realpath(dl_info.dli_fname, NULL);
if (!path) {
perror("realpath");73b3b57 to
89b6811
Compare
89b6811 to
623a556
Compare
Buristan
left a comment
There was a problem hiding this comment.
[PATCH]: luzer: suppress an error due to unused jit_status
LGTM, also may conditionaly declare it (using #ifdef directive), but it's up to you.
|
Update code for diff --git a/luzer/tests/CMakeLists.txt b/luzer/tests/CMakeLists.txt
index 8efae47..82e3f11 100644
--- a/luzer/tests/CMakeLists.txt
+++ b/luzer/tests/CMakeLists.txt
@@ -14,6 +14,14 @@ make_lua_path(LUA_PATH
if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
SetClangLibPath("libclang_rt.asan_osx_dynamic.dylib" ASAN_LIBRARY_PATH)
+ # By default, `abort_on_error` is set to 0 on Linux (disabled),
+ # but on macOS it is set to 1 (enabled) by default. When
+ # `abort_on_error` is enabled, the AddressSanitizer calls
+ # `abort()` instead of `_exit()` after printing the error report
+ # and CTest considers such tests to be failed.
+ string(JOIN ":" ASAN_OPTIONS
+ abort_on_error=0
+ )
endif()
add_test(
@@ -238,7 +246,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=HEAP_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_heap_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -249,7 +257,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=STACK_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_stack_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -260,7 +268,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=DYNAMIC_STACK_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_dynamic_stack_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -271,7 +279,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=GLOBAL_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_global_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -282,7 +290,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=MEMSET_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_memset_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -293,7 +301,7 @@ string(JOIN ";" TEST_ENVIRONMENT
LD_PRELOAD=${ASAN_DSO_PATH}
LIB_NAME=luac_asan
ERR_INJECTION=MEMCPY_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_luac_test(luzer_luac_asan_memcpy_buffer_overflow
"${TEST_ENVIRONMENT}"
@@ -383,7 +391,7 @@ if (LUA_HAS_JIT)
LD_PRELOAD=${ASAN_DSO_PATH}
FFI_LIB_NAME=testlib_asan${CMAKE_SHARED_LIBRARY_SUFFIX}
ERR_INJECTION=HEAP_BUFFER_OVERFLOW
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_ffi_test(luzer_ffi_asan_heap_overflow
"${TEST_ENVIRONMENT}"
@@ -394,7 +402,7 @@ if (LUA_HAS_JIT)
LD_PRELOAD=${UBSAN_DSO_PATH}
FFI_LIB_NAME=testlib_ubsan${CMAKE_SHARED_LIBRARY_SUFFIX}
ERR_INJECTION=NULL_POINTER_DEREF
- ASAN_OPTIONS=abort_on_error=0
+ ASAN_OPTIONS=${ASAN_OPTIONS}
)
generate_ffi_test(luzer_ffi_ubsan_null_pointer_deref
"${TEST_ENVIRONMENT}" |
f8d4e58 to
39d7b73
Compare
Updated: diff --git a/luzer/luzer.c b/luzer/luzer.c
index 8a9925c..f82222a 100644
--- a/luzer/luzer.c
+++ b/luzer/luzer.c
@@ -37,7 +37,9 @@
#define DEBUG_HOOK_FUNC "luzer_custom_hook"
static lua_State *LL;
-static int jit_status __attribute__((unused)) = 0;
+#if defined(LUA_HAS_JIT) && defined(LUAJIT_FRIENDLY_MODE)
+static int jit_status = 0;
+#endif /* LUA_HAS_JIT && LUAJIT_FRIENDLY_MODE */
int internal_hook_disabled = 0;
@@ -208,10 +210,10 @@ sig_handler(int sig)
}
/* Returns the current status of the JIT compiler. */
+#if defined(LUA_HAS_JIT) && defined(LUAJIT_FRIENDLY_MODE)
NO_SANITIZE static int
luajit_has_enabled_jit(lua_State *L)
{
-#if defined(LUA_HAS_JIT)
lua_getglobal(L, "jit");
lua_getfield(L, -1, "status");
/*
@@ -223,10 +225,8 @@ luajit_has_enabled_jit(lua_State *L)
*/
lua_pcall(L, 0, 1, 0);
return lua_toboolean(L, -1);
-#else
- return 0;
-#endif
}
+#endif /* LUA_HAS_JIT && LUAJIT_FRIENDLY_MODE */
NO_SANITIZE int
luaL_mutate(lua_State *L)
@@ -524,7 +524,9 @@ luaL_fuzz(lua_State *L)
}
lua_pop(L, -1);
+#if defined(LUA_HAS_JIT) && defined(LUAJIT_FRIENDLY_MODE)
jit_status = luajit_has_enabled_jit(L);
+#endif
set_global_lua_state(L);
int rc = LLVMFuzzerRunDriver(&argc, &argv, &TestOneInput);
|
The patch simplifies the implementation of the function `SetHwArchString()` for obtaining the architecture name. Needed for the following patch with macOS support and Ubuntu 24.04 AArch64 (ARM64) support. Needed for #59
The replaces long strings with test environment variables with a list of environment variables joined to a single string. Needed for #59
The luzer module relies on other shared libraries: library with custom mutator and sanitizer libraries built with libFuzzer. These libraries are located in the directory with luzer_impl.so and on module loading we search a path to this shared library to found other shared libraries. It is done by the function `search_module_path()` - it iterates through the directories specified in the environment variable LUA_CPATH and tries to find the directory with luzer_impl.so. However, there's a simpler way to find a directory path with luzer_impl.so - using the `dladdr(3)` function. The patch introduce a function `get_symbol_path()` that replaces the functions `get_libFuzzer_symbols_location()`, `get_coverage_symbols_location()`, `search_module_path()` and makes implementation easier for support. The patch also renames `base_so_path` to `base_dir`. Related to #67 Needed for #59
The C compiler produce an error because variable `jit_status` and the related code are unused when PUC Rio Lua library is used. The patch declared variable conditionally to fix the error.
The patch replace static suffix ".so" for shared libraries with CMAKE_SHARED_LIBRARY_SUFFIX [1]. Needed for the following patch with macOS support. Needed for #59 1. https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LIBRARY_SUFFIX.html
There are three ways to set Lua libraries, headers and executable: 1. Setting CMake option ENABLE_LUAJIT will trigger searching LuaJIT library, headers and executable. 2. Setting CMake options LUA_LIBRARIES, LUA_INCLUDE_DIR and optional LUA_EXECUTABLE with a path Lua library, directory with headers and to a path to Lua executable explicitly. 3. Searching PUC Rio Lua library, headers and executable automatically. LUA_EXECUTABLE is required for regression testing only, so if this CMake variable was not specified explicitly or was not found automatically the testing will not be enabled. The patch simplify a code for searching Lua library and makes the behavior the same for all three cases described above: - Move a code for searching `luajit` binary executable to CMake module. - Remove fatal message for a case when PUC Rio Lua executable was not found. - Remove excess `unset()` for ENABLE_TESTING. Needed for #59
The patch adds macOS ARM64 support. Notable changes are: - Fixed building sanitizers libraries. There's no need to strip libraries because they don't have the same object files as Linux. - Added the -Wno-builtin-memcpy-chk-size option to build the test library; otherwise, the build won't work. - Use the DYLD_INSERT_LIBRARIES environment variable to load the ASAN library. - Added an environment variable ASAN_OPTIONS=abort_on_error=0 for regression regression tests. Without option ASAN aborts Lua runtime and CTest fails the test. The behaviour is macOS-specific: if the option `abort_on_error` is set, the AddressSanitizer calls `abort()` instead of `_exit()` after printing the error report [2]. By default, `abort_on_error` is set to 0 on Linux, but on macOS it is set to 1 [3]. - Added a directory with LuaJIT binary executable to HINTS for find_program(). Otherwise, `luajit` is not found automatically. - Added macOS 26 ARM64 [1] to the regression testing on Github Actions. 1. https://docs.github.com/en/actions/reference/runners/github-hosted-runners 2. https://github.com/google/sanitizers/wiki/SanitizerCommonFlags 3. https://reviews.llvm.org/D7203 Closes #59 Co-authored-by: Sergey Bronnikov <estetus@gmail.com>
39d7b73 to
4e6d4c1
Compare
Original changes in the PR #58
Closes #59
Related to #67