You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2023/09/11 14:39:52 UTC
[GitHub] [arrow-adbc] WillAyd opened a new pull request, #1049: ci: Try ASAN/UBSAN in CI
WillAyd opened a new pull request, #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049
ref https://github.com/apache/arrow-adbc/issues/1044
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] zeroshade commented on pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#issuecomment-1714136395
Do we want the CI to also use ASAN for the Golang CI?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321773744
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
Review Comment:
Sounds good. FYI when using this in coordination with Python if you don't ignore leaks you get a ton of (false?) positives from the CPython runtime.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm merged pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321704540
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
+ echo "LD_PRELOAD=$(gcc --print-file-name=libasan.so)" >> "$GITHUB_ENV"
- name: Build SQLite3 Driver
shell: bash -l {0}
run: |
- env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
+ env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ADBC_USE_ASAN=1 ADBC_USE_UBSAN=1 \
+ ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
- name: Test SQLite3 Driver
shell: bash -l {0}
run: |
env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_test.sh "$(pwd)/build"
- name: Build PostgreSQL Driver
Review Comment:
The driver manager uses the Postgres binary for a test, IIRC
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321702748
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
+ echo "LD_PRELOAD=$(gcc --print-file-name=libasan.so)" >> "$GITHUB_ENV"
- name: Build SQLite3 Driver
shell: bash -l {0}
run: |
- env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
+ env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ADBC_USE_ASAN=1 ADBC_USE_UBSAN=1 \
+ ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
- name: Test SQLite3 Driver
shell: bash -l {0}
run: |
env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_test.sh "$(pwd)/build"
- name: Build PostgreSQL Driver
Review Comment:
There is a build step here for postgres but not a test step - is that intentional?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321761693
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
+ echo "LD_PRELOAD=$(gcc --print-file-name=libasan.so)" >> "$GITHUB_ENV"
- name: Build SQLite3 Driver
shell: bash -l {0}
run: |
- env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
+ env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ADBC_USE_ASAN=1 ADBC_USE_UBSAN=1 \
+ ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
Review Comment:
So, since the default is to enable ASAN, these tests already were using ASAN/UBSAN.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1323094193
##########
c/cmake_modules/san-config.cmake:
##########
@@ -25,6 +25,7 @@ if(${ADBC_USE_ASAN})
OR (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION
VERSION_GREATER "4.8"))
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -DADDRESS_SANITIZER")
+ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -DADDRESS_SANITIZER")
Review Comment:
Oh, good catch...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321833221
##########
c/driver/common/utils.c:
##########
@@ -166,17 +166,23 @@ void AppendErrorDetail(struct AdbcError* error, const char* key, const uint8_t*
return;
}
- memcpy(new_keys, details->keys, sizeof(char*) * details->count);
- free(details->keys);
- details->keys = new_keys;
+ if (details->keys != NULL) {
+ memcpy(new_keys, details->keys, sizeof(char*) * details->count);
Review Comment:
TIL that C does have a way of declaring a pointer parameter that is not NULL. I couldn't reproduce locally but these were failing on CI
Guessing the headers on the CI systems look something like:
https://stackoverflow.com/a/48830228/621736
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321762503
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
+ echo "LD_PRELOAD=$(gcc --print-file-name=libasan.so)" >> "$GITHUB_ENV"
- name: Build SQLite3 Driver
shell: bash -l {0}
run: |
- env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
+ env BUILD_ALL=0 BUILD_DRIVER_SQLITE=1 ADBC_USE_ASAN=1 ADBC_USE_UBSAN=1 \
+ ./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
Review Comment:
What we need to do is change ADBC_USE_ASAN/UBSAN in drivers-build-conda above, then export these env vars in all the downstream jobs.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#issuecomment-1714527537
OK should be good now. Tested a few times in CI to make sure we received failures. All green with current config
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321759950
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
Review Comment:
I think we don't need to copy NumPy's config exactly. We can remove `detect_leaks=0` to keep LeakSanitizer enabled. The other flags seem fine enough.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#issuecomment-1714167243
We can enable ASAN for Golang in a followup.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #1049: ci: Try ASAN/UBSAN in CI
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1049:
URL: https://github.com/apache/arrow-adbc/pull/1049#discussion_r1321829488
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
Review Comment:
Ah, ok, then we should keep it - thanks for the explanation
##########
.github/workflows/native-unix.yml:
##########
@@ -169,22 +169,30 @@ jobs:
mamba install -c conda-forge \
--file ci/conda_env_cpp.txt
+ - name: Export ASAN/UBSAN environment variables
+ shell: bash -l {0}
+ run: |
+ echo "ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1" >> "$GITHUB_ENV"
Review Comment:
Ah, ok, then we should keep it - thanks for the explanation
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org