Skip to content

fix(bq_driver): Asan CI check fix #1521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Khushikathuria008 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(bq_driver): Asan CI check fix #1521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1154ebd
fix(bq_driver): Asan CI check fix
Khushikathuria008 May 7, 2026
c6cd9ea
test chnages
NeerajDwivedii Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ci/cloudbuild/builds/integration-bq-driver-asan.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ git checkout "$VCPKG_VERSION"
./bootstrap-vcpkg.sh -disableMetrics

cd "$WORKSPACE_DIR"

# This runs all the unit tests
mapfile -t args < <(bazel::common_args)
mapfile -t unit_tests_args < <(unit_tests::bazel_args)
Expand All @@ -56,6 +55,7 @@ mapfile -t cmake_args < <(cmake::common_args)
BUILD_DIR="/opt/odbc-driver"
# This is the name of DSN set in odbc.ini
export ODBC_TESTS_DSN="SampleDSNGoogleDriver"
export ASAN_OPTIONS="detect_container_overflow=0:detect_leaks=1"

@shivamd-gpartner shivamd-gpartner May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this detect_container_overflow=0 flag needed?

@Khushikathuria008 Khushikathuria008 May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detect_container_overflow=0 is currently required to avoid ASAN false positives coming from external dependencies/driver manager internals during integration tests. Leak detection remains enabled (detect_leaks=1) so real leaks are still caught.

@shivamd-gpartner shivamd-gpartner May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you give exact error?

@Khushikathuria008 Khushikathuria008 May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error here

export LSAN_OPTIONS="use_tls=0:suppressions=/opt/odbc-driver/lsan.supp:print_suppressions=0"

export CPP_BIGQUERY_ODBC_TEST_TABLE_PREFIX=${TRIGGER_NAME//[-:;.,?]/_}_${BRANCH_NAME//[-:;.,?]/_}
Expand Down Expand Up @@ -84,8 +84,9 @@ io::run cmake -B "$BUILD_DIR" \
-DODBC_EXAMPLES=ON \
-DODBC_UNIT_TESTING=OFF \
-DCLIENT_LIBRARY_INTEGRATION_TESTING=OFF

io::run cmake --build cmake-out

# Copy the roots.pem file to the .so directory to run test cases.
cp /opt/odbc-driver/roots.pem "cmake-out/google/cloud/odbc/roots.pem"
mapfile -t ctest_args < <(ctest::common_args)
io::run env -C cmake-out ctest "${ctest_args[@]}"
33 changes: 12 additions & 21 deletions ci/cloudbuild/dockerfiles/ubuntu-22.04-install.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ RUN apt-get update && \
build-essential \
# Dependency for arrow
bison \
clang-12 \
lld-12 \
clang \

@shivamd-gpartner shivamd-gpartner Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changes needed here @Khushikathuria008 ?

cmake \
curl \
# Dependency for arrow
Expand All @@ -32,10 +31,8 @@ RUN apt-get update && \
git \
gcc \
g++ \
# Required by Ubsan in Ubuntu 22.04
libunwind-12-dev \
libc++-12-dev \
libc++abi-12-dev \
libc++-dev \
libc++abi-dev \
libcurl4-openssl-dev \
# Needed to use autoreconf
libltdl-dev \
Expand All @@ -61,28 +58,22 @@ RUN apt-get update && \
apt-utils \
ca-certificates \
apt-transport-https \
clang-tidy-12
clang-tidy

# Set Clang 12 as default
RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 && \
update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100

# Set the compiler environment variables
ENV CC=/usr/bin/clang
ENV CXX=/usr/bin/clang++

# Needed for the existing driver v3.1.2.1004+
RUN locale-gen en_US.UTF-8
ENV LANG en_US.UTF-8
ENV LANGUAGE en_US.UTF-8
ENV LC_ALL en_US.UTF-8

# Set clang as default
RUN update-alternatives --install /usr/bin/clang clang /usr/bin/clang-12 100 && \
update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-12 100

ENV CC=clang
ENV CXX=clang++

# Install modern CMake locally
RUN mkdir -p /opt/cmake && \
curl -fsSL https://github.com/Kitware/CMake/releases/download/v3.30.1/cmake-3.30.1-linux-x86_64.tar.gz \
| tar -xz --strip-components=1 -C /opt/cmake

ENV PATH=/opt/cmake/bin:$PATH

# clang-tidy-cache needs python
RUN update-alternatives --install /usr/bin/python python $(which python3) 10

Expand Down
6 changes: 6 additions & 0 deletions ci/gha/builds/lib/lsan.supp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,9 @@ leak:Diagnostics::AddStatusRecord
leak:StatusRecord
leak:std::__cxx11::basic_string
leak:_iodbcdm_SetConnectOption_init
leak:ld-linux-x86-64.so.2

@shivamd-gpartner shivamd-gpartner May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we suppressing these new leaks?

@Khushikathuria008 Khushikathuria008 May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ld-linux-x86-64.so.2 is a runtime loader allocation from glibc/process teardown, not from our driver logic. This is a known non-actionable LSAN report, so it is being suppressed after verifying our handle cleanup paths are correct.

leak:_dl_exception_create_format
leak:libc.so.6
leak:CRYPTO_zalloc
leak:grpc_jwt_encode_and_sign
leak:_iodbcdm_pushsqlerr
8 changes: 6 additions & 2 deletions google/cloud/odbc/integration_tests/odbc_driver_tests/connection_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1175,9 +1175,13 @@ TEST(ConnectionTest, SQLBrowseConnect_InvalidConnectionAttribute) {
HasSubstr("Catalog:Catalog=?;OAuthMechanism:OAuthMechanism=?"));
#endif // _WIN32
}
// Pass `false` to indicate that the Driver Manager (DM) will automatically
// free the environment handle when the last connection handle is released.
// Pass `false` to indicate that the Driver Manager (DM) will automatically
// free the environment handle when the last connection handle is released.
#ifdef _WIN32
CleanupODBCHandles(*conn, false);
#else
CleanupODBCHandles(*conn);
#endif
}

TEST(ConnectionTest, SQLBrowseConnect_InvalidConnectionString) {
Expand Down
1 change: 0 additions & 1 deletion vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
},
{ "name": "grpc", "version>=": "1.68.2" },
{ "name": "opentelemetry-cpp", "version>=": "1.18.0" },
{ "name": "protobuf", "version>=": "5.29.2" },
"nlohmann-json",
"benchmark",
{ "name": "gtest", "version>=": "1.15.2" }
Expand Down
Loading