Skip to content

Commit

Permalink
FFI examples use address sanitizer (#447)
Browse files Browse the repository at this point in the history
## What changes are proposed in this pull request?

Enable ASAN for the FFI examples, to automatically detect many memory
errors. On linux, the address sanitizer automatically includes leak
detection as well, but the leak sanitizer is unsupported on darwin:
```
cc: error: unsupported option '-fsanitize=leak' for target 'arm64-apple-darwin23.6.0'
```
(which is weird, given that the [leak sanitizer
docs](https://clang.llvm.org/docs/LeakSanitizer.html) claim support for
"macOS aarch64/i386/x86_64"

## How was this change tested?

NOTE: Leak detection is temporarily disabled on the `read-table`
example, because it has many memory leaks that need to be fixed first.
Tracked as
#448.

Address sanitizer tested locally by injecting the following code (which
was successfully caught and caused test failures):
```c
// intentional use-after-free access
char* dangling = malloc(1);
free(dangling);
*dangling = 0;
```

Leak sanitizer tested by temporarily adding the following code and
letting github actions detect it:
```c
// intentional leak
char* leaked =  malloc(10);
*leaked = 0;
```
  • Loading branch information
scovich authored Oct 30, 2024
1 parent 266e9b4 commit b61c76f
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 17 deletions.
3 changes: 2 additions & 1 deletion ffi/examples/read-table/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ if(MSVC)
target_compile_options(read_table PRIVATE /W4 /WX)
else()
# no-strict-prototypes because arrow headers have fn defs without prototypes
target_compile_options(read_table PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes)
target_compile_options(read_table PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes -g -fsanitize=address)
target_link_options(read_table PRIVATE -g -fsanitize=address)
endif()

if(PRINT_DATA)
Expand Down
1 change: 1 addition & 0 deletions ffi/examples/read-table/arrow.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ void c_read_parquet_file(
};
ExternResultHandleExclusiveFileReadResultIterator read_res =
read_parquet_file(context->engine, &meta, context->read_schema);
free(full_path);
if (read_res.tag != OkHandleExclusiveFileReadResultIterator) {
printf("Couldn't read data\n");
return;
Expand Down
4 changes: 4 additions & 0 deletions ffi/examples/read-table/read_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
#include "read_table.h"
#include "schema.h"

// TODO - We cannot enable leak detection until existing leaks are fixed.
// See https://github.com/delta-incubator/delta-kernel-rs/issues/448
const char* __asan_default_options() { return "detect_leaks=0"; }

// some diagnostic functions
void print_diag(char* fmt, ...)
{
Expand Down
22 changes: 6 additions & 16 deletions ffi/examples/visit-expression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,16 @@ target_link_directories(visit_expression PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../
target_link_libraries(visit_expression PUBLIC delta_kernel_ffi)
target_compile_options(visit_expression PUBLIC)

target_compile_options(visit_expression PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes)

# Get info on the OS and platform
if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(MACOSX TRUE)
endif()
if(UNIX AND NOT APPLE)
set(LINUX TRUE)
if(MSVC)
target_compile_options(read_table PRIVATE /W4 /WX)
else()
# no-strict-prototypes because arrow headers have fn defs without prototypes
target_compile_options(visit_expression PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes -g -fsanitize=address)
target_link_options(visit_expression PRIVATE -g -fsanitize=address)
endif()

# Add the kernel expresion -> engine expression test
include(CTest)
set(ExprTestRunner "../../../tests/test-expression-visitor/run_test.sh")
set(ExprExpectedPath "../../../tests/test-expression-visitor/expected.txt")
add_test(NAME test_expression_visitor COMMAND ${ExprTestRunner} ${ExprExpectedPath})
if(LINUX)
add_test(NAME test_expression_visitor_leaks COMMAND valgrind
--error-exitcode=1
--tool=memcheck
--leak-check=full
--errors-for-leak-kinds=definite
--show-leak-kinds=definite ./visit_expression)
endif()

0 comments on commit b61c76f

Please sign in to comment.