fgerlits commented on code in PR #2254:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2254#discussion_r3970525270
##########
cmake/BuildTests.cmake:
##########
@@ -17,6 +17,22 @@
include(GetCatch2)
+if (MINIFI_ADVANCED_ASAN_BUILD)
+ file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/asan_logs")
+
+ # Route each test's AddressSanitizer/LeakSanitizer output to
<build>/asan_logs/<test-name>.<pid>
+ # and suppress odr-violation warnings (only if the two symbols have the
same size). Hundreds of global
+ # symbols are present in two or more .so's, so we don't want to suppress
each individually.
+ function(add_test)
+ _add_test(${ARGV})
+ cmake_parse_arguments(MINIFI_TEST "" "NAME" "COMMAND" ${ARGV})
+ if (MINIFI_TEST_NAME)
+ set_property(TEST "${MINIFI_TEST_NAME}" APPEND PROPERTY
+ ENVIRONMENT
"ASAN_OPTIONS=detect_odr_violation=1:log_path=${CMAKE_BINARY_DIR}/asan_logs/${MINIFI_TEST_NAME}")
+ endif()
+ endfunction()
Review Comment:
If you redefine a built-in CMake function, the original function will be
available inside the new implementation with an underscore prepended, so this
does work. This is a widely used CMake technique (so CMake is unlikely to drop
it, since that would break many projects), but it is not officially supported,
and has some issues (e.g., redefining the same function a second time will not
work).
I don't think Copilot's solutions would work without creating an infinite
call loop. We could write a `minifi_add_test` wrapper which adds the ASAN
options when needed, but then all `add_test` calls would need to be changed to
`minifi_add_test` and all future test writers will need to remember to use
`minifi_add_test` instead of `add_test`. Or we could define the ASAN_OPTIONS
globally, but then ASan results would be written to files named `asan.<pid>`
(without the name of the test), which is not great.
I still think redefining `add_test` as above is the cleanest way to do this,
but I will keep this comment open so others can comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
