Skip to content

Conversation

@bjosv
Copy link

@bjosv bjosv commented Dec 9, 2025

The flag can be enabled in CMake using -DENABLE_UB_SANITIZER=ON similar to the existing flag for the address sanitizer.
Building aws-sdk-cpp (and its dependencies) with this flag indicates a couple of issues, like:

aws-sdk-cpp/src/aws-cpp-sdk-core/include/aws/core/client/AWSClientAsyncCRTP.h:51:40: runtime error: downcast of address 0x7ffe7bff5b90 which does not point to an object of type 'Aws::S3::S3Client'
aws-sdk-cpp/src/aws-cpp-sdk-core/include/aws/core/utils/stream/StreamBufProtectedWriter.h:46:67: runtime error: downcast of address 0x7ffd80fa21e8 which does not point to an object of type 'StreamBufProtectedWriter'
aws-sdk-cpp/crt/aws-crt-cpp/crt/aws-c-common/source/hash_table.c:68:12: runtime error: call to function aws_byte_cursor_eq through pointer to incorrect function type 'bool (*)(const void *, const void *)'
aws-sdk-cpp/crt/aws-crt-cpp/crt/aws-c-sdkutils/source/endpoints_util.c:50:24: runtime error: addition of unsigned offset to 0x595033296ca0 overflowed to 0x595033296c9f
aws-sdk-cpp/tests/aws-cpp-sdk-core-tests/utils/crypto/SymmetricCiphersTest.cpp:689:12: runtime error: null pointer passed as argument 1, which is declared to never be null
aws-sdk-cpp/tests/aws-cpp-sdk-core-tests/utils/crypto/SymmetricCiphersTest.cpp:689:43: runtime error: null pointer passed as argument 2, which is declared to never be null

Existing PRs to fix above issues are: awslabs/aws-c-http#531 and awslabs/aws-c-sdkutils#58
Related issue for one issue: #3464

Some of these issues are also seen when a SDK-user builds with UBSan.
Maybe we should add a CI-step building with this flag?

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (Can be added to CI?)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The flag can be enabled in CMake using -DENABLE_UB_SANITIZER=ON

Signed-off-by: Björn Svensson <[email protected]>
@sbiscigl
Copy link
Contributor

sbiscigl commented Dec 9, 2025

So i have two high level thoughts on this

I know ENABLE_ADDRESS_SANITIZER exists and this would just be adding a mirror to that. However we've been trying to move away from making a cmake option for everything and only making knobs for things that cannot be provided to cmake as a argument. for instance instead of creating this flag one could just use

cmake -DCMAKE_CXX_FLAGS="-ggdb -fsanitize=undefined"

when running cmake generation instead of relying on a option we define ourselves. infact in our CI step we actually use address sanatizer that way instead of providing the cmake option directly.

Maybe we should add a CI-step building with this flag?

I whole heartily agree on this sentiment, but until your PRs are merged and the other issue dealt with the build will actually fail so i would be against adding a flag until its passing.

so summarizing:

  • i would prefer to stop the adding of specific cmake options for things that can be provided at generation time.
  • we definitely need to add a CI step. likely just updating this argument in our CI scripts once your changes are merged.

@bjosv
Copy link
Author

bjosv commented Dec 9, 2025

  • i would prefer to stop the adding of specific cmake options for things that can be provided at generation time.

Sounds reasonable. The problem I faced was how to get the same flags in crt dependencies when building with the default BUILD_DEPS=ON. We want to enable UBSan on dependencies to avoid false positives.
This was solved in this PR by using cached CMake variables, but maybe there is some other way.. I will take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants