Merge pull request #57 from HighlanderLab:gregorgorjanc/issue52

#52 Adding debug and sanitiser flags

Author: gregorgorjanc

RcppTskit/R/RcppExports.R

@@ -111,3 +111,7 @@ test_tsk_trace_error_cpp <- function() {
     invisible(.Call(`_RcppTskit_test_tsk_trace_error_cpp`))
 }
 
+tsk_trace_errors_defined <- function() {
+    .Call(`_RcppTskit_tsk_trace_errors_defined`)
+}
+

RcppTskit/notes_pkg_dev.Rmd

@@ -144,13 +144,13 @@ use_release_issue(version = NULL) # https://usethis.r-lib.org/reference/use_rele
 # just follow tasks in there;)
 ```
 
-## Setup
+## Dev tools setup / use
 
 ```
 install.packages(c("usethis", "devtools"))
 ```
 
-## Code testing coverage with covr
+### Code testing coverage with covr
 
 https://covr.r-lib.org
 
@@ -187,7 +187,37 @@ To ignore multiple lines use `# nocov start/stop`.
 
 In `Rcpp` code use `// # nocov ...`.
 
-## Air formatter of R code
+### Debugging C/C++
+
+In `src/Makevars.in` uncomment debug flags ...
+
+From terminal run:
+
+```
+# Go to package folder
+R_HOME="/Library/Frameworks/R.framework/Resources"
+R_BIN="$R_HOME/bin/exec/R"
+ASAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.asan_osx_dynamic.dylib"
+UBSAN="/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/17/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib"
+
+env R_HOME="$R_HOME" \
+  DYLD_INSERT_LIBRARIES="$ASAN:$UBSAN" \
+  ASAN_OPTIONS="abort_on_error=1,detect_leaks=0,verbosity=1" \
+  UBSAN_OPTIONS="print_stacktrace=1" \
+  "$R_BIN" --vanilla
+```
+
+In the R session (not from Rstudio/Positron!):
+
+```
+# Load + compile into the current ASan session (no child R load)
+devtools::load_all()
+
+# Run tests in the same process
+devtools::test()
+```
+
+### Air formatter of R code
 
 https://usethis.r-lib.org/reference/use_air.html
 ```
@@ -205,7 +235,7 @@ air format .
 air format --check .
 ```
 
-## Jarl linter of R code
+### Jarl linter of R code
 
 https://jarl.etiennebacher.com
 
@@ -216,7 +246,7 @@ jarl check .
 jarl check --fix .
 ```
 
-## clang-format of C/C++ code
+### clang-format of C/C++ code
 
 https://clang.llvm.org/docs/ClangFormat.html
 
@@ -250,7 +280,7 @@ pre-commit run clang-format src/RcppTskit.cpp
 pre-commit run clang-format --all-files
 ```
 
-## clang-tidy linter of C/C++ code
+### clang-tidy linter of C/C++ code
 
 https://clang.llvm.org/extra/clang-tidy/
 
@@ -270,7 +300,7 @@ pre-commit run clang-tidy src/RcppTskit.cpp
 pre-commit run clang-tidy --all-files
 ```
 
-## GitHub actions
+### GitHub actions
 
 ```
 install.packages("usethis")

RcppTskit/src/Makevars.in

@@ -30,7 +30,7 @@ RCPPTSKIT_OBJECTS = $(RCPPTSKIT_C_SOURCES:.c=.o) $(RCPPTSKIT_CPP_SOURCES:.cpp=.o
 OBJECTS = $(TSKIT_OBJECTS) $(RCPPTSKIT_OBJECTS)
 # $(info OBJECTS = $(OBJECTS)) # for debugging, info is also a GNU extension
 
-# Include paths and preprocessor defines
+# *Preprocessor (CPP)* flags for include paths and defines
 # * A bit complicated include/tskit structure due to how we include tskit headers
 # * $(R_INCLUDE_DIR) is for <R_ext/Error.h>
 PKG_CPPFLAGS = \
@@ -39,14 +39,29 @@ PKG_CPPFLAGS = \
 	-I../inst/include/tskit/tskit \
 	-I$(R_INCLUDE_DIR)
 
-# Compiler flags
-PKG_CFLAGS = -DNDEBUG # to remove calls to assert() as per the R extensions manual
-# PKG_CFLAGS = -DNDEBUG -DTSK_TRACE_ERRORS # to also enable error tracing in tskit C
-# PKG_CXXFLAGS = -DTSK_TRACE_ERRORS # to also enable error tracing in tskit C as called from Rcpp
+# *Compiler (C/CXX)* flags
+# Uncomment for local debugging
+# SAN_FLAGS enables ASan/UBSan to catch memory/UB errors in dev builds (only run R in terminal!)
+# TSK_TRACE_ERRORS enables tskit C error tracing
+# DEBUG_OPT = -g -O1 # -O1 is needed for many of the below flags
+# WARN_FLAGS = $(DEBUG_OPT) -Wall -Wextra -Wpedantic -Wuninitialized
+# SAN_FLAGS = $(DEBUG_OPT) -fno-omit-frame-pointer -fsanitize=address,undefined -fno-common
+# DEV_FLAGS = $(SAN_FLAGS) $(WARN_FLAGS) -DTSK_TRACE_ERRORS
+# RCPPTSKIT_CFLAGS = $(DEV_FLAGS)
+# RCPPTSKIT_CXXFLAGS = $(DEV_FLAGS)
+# RCPPTSKIT_LDFLAGS = -fsanitize=address,undefined
+# Uncomment for release builds
+# NDEBUG removes calls to assert()
+RCPPTSKIT_CFLAGS = -DNDEBUG
+RCPPTSKIT_CXXFLAGS = -DNDEBUG
+RCPPTSKIT_LDFLAGS =
+# Use the above choices
+PKG_CFLAGS = $(RCPPTSKIT_CFLAGS)
+PKG_CXXFLAGS = $(RCPPTSKIT_CXXFLAGS)
 
 # Explicit compile rule for tskit C files
 tskit/%.o: tskit/%.c
 	$(CC) $(ALL_CPPFLAGS) $(PKG_CFLAGS) $(CFLAGS) $(CPICFLAGS) -c $< -o $@
 
 # Linking
-PKG_LIBS = @RCPPTSKIT_LIB@
+PKG_LIBS = @RCPPTSKIT_LIB@ $(RCPPTSKIT_LDFLAGS)

RcppTskit/src/RcppExports.cpp

@@ -266,6 +266,16 @@ BEGIN_RCPP
     return R_NilValue;
 END_RCPP
 }
+// tsk_trace_errors_defined
+bool tsk_trace_errors_defined();
+RcppExport SEXP _RcppTskit_tsk_trace_errors_defined() {
+BEGIN_RCPP
+    Rcpp::RObject rcpp_result_gen;
+    Rcpp::RNGScope rcpp_rngScope_gen;
+    rcpp_result_gen = Rcpp::wrap(tsk_trace_errors_defined());
+    return rcpp_result_gen;
+END_RCPP
+}
 
 static const R_CallMethodDef CallEntries[] = {
     {"_RcppTskit_kastore_version", (DL_FUNC) &_RcppTskit_kastore_version, 0},
@@ -292,6 +302,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"_RcppTskit_test_tsk_bug_assert_cpp", (DL_FUNC) &_RcppTskit_test_tsk_bug_assert_cpp, 0},
     {"_RcppTskit_test_tsk_trace_error_c", (DL_FUNC) &_RcppTskit_test_tsk_trace_error_c, 0},
     {"_RcppTskit_test_tsk_trace_error_cpp", (DL_FUNC) &_RcppTskit_test_tsk_trace_error_cpp, 0},
+    {"_RcppTskit_tsk_trace_errors_defined", (DL_FUNC) &_RcppTskit_tsk_trace_errors_defined, 0},
     {NULL, NULL, 0}
 };
 

RcppTskit/src/test_tsk_abort_stderr.cpp

@@ -18,3 +18,12 @@ void test_tsk_trace_error_c() { RcppTskit_trace_error_c(); } // # nocov
 // This is tested if we compile with -DTSK_TRACE_ERRORS
 // [[Rcpp::export]]
 void test_tsk_trace_error_cpp() { (void)tsk_trace_error(-1); } // # nocov
+
+// [[Rcpp::export]]
+bool tsk_trace_errors_defined() {
+#ifdef TSK_TRACE_ERRORS
+  return true;
+#else
+  return false;
+#endif
+}

RcppTskit/tests/testthat/test_misc.R

@@ -11,13 +11,13 @@ test_that("tskit_version() works", {
 })
 
 test_that("tsk_bug_assert() works", {
-  expect_error(test_tsk_bug_assert_c())
-  expect_error(test_tsk_bug_assert_cpp())
+  expect_error(RcppTskit:::test_tsk_bug_assert_c())
+  expect_error(RcppTskit:::test_tsk_bug_assert_cpp())
 })
 
 test_that("tsk_trace_error() works", {
   t <- "You have to compile with -DTSK_TRACE_ERRORS to run these tests. See src/Makevars.in."
-  skip(t)
-  expect_warning(test_tsk_trace_error_c())
-  expect_warning(test_tsk_trace_error_cpp())
+  skip_if_not(RcppTskit:::tsk_trace_errors_defined(), t)
+  expect_warning(RcppTskit:::test_tsk_trace_error_c())
+  expect_warning(RcppTskit:::test_tsk_trace_error_cpp())
 })