KEMBAR78
Reactivate Clang UBSan test coverage by StephanTLavavej · Pull Request #5746 · microsoft/STL · GitHub
Skip to content

Conversation

@StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Sep 29, 2025

Fixes #3568.

This was briefly enabled for Clang 15 by #3452 on 2023-03-13, then disabled for Clang 16 by #3711 on 2023-05-18. After updating to Clang 20 by #5717 on 2025-09-15, we can re-enable this test coverage.

Note that tests/utils/stl/test/features.py disabled the configurations. We accumulated a couple of changes to tests/std/tests/VSO_0000000_vector_algorithms_floats/env.lst and tests/std/tests/usual_matrix.lst commenting out configurations, but I believe those changes were unnecessary. This PR updates features.py to enable the feature, uncomments the commented-out configurations, and doesn't change the large number of already-uncommented configurations across other files.

I audited our env.lst and related files, and I believe our UBSan coverage is reasonably comprehensive now. For the tests it's missing from, they either have good reasons (e.g. compile-only, /clr specific, incompatible with Clang for other reasons, etc.) or already have a blizzard of configurations where UBSan wouldn't be worth the extra cost.

Notably, UBSan has never been enabled for libcxx, and I am not attempting to do so here. libcxx configurations are extremely expensive (that's why we have only three). It may be reasonable to explore this in the future, but I don't want to spend developer and machine time on that now.

I'll leave Clang UBSan disabled for the MSVC-internal test harness (this was src/qa/VC/shared/testenv/bin/run.pl in MSVC-PR-457627, for future reference). I don't know if it works now, but it would provide negative value by burning a lot of machine time when GitHub is providing sufficient coverage of the STL's product and test code.

We've already merged a couple of fixes for bugs in vector<bool> and independent_bits_engine found by this impending PR:

⚙️ Commits

  • Activate UBSan.
  • Avoid duration_cast() UB, tracked by LWG-3921.
    • D:\GitHub\STL\out\x64\out\inc\__msvc_chrono.hpp:451:52: runtime error: signed integer overflow: 9223372036854775806 * 1000000 cannot be represented in type '_CR' (aka 'long long')
  • Avoid UB caused by deliberate misalignment with #pragma pack.
    • D:\GitHub\STL\tests\std\tests\VSO_0000000_vector_algorithms_mismatch_and_lex_compare\test.cpp:177:9: runtime error: reference binding to misaligned address 0x002ec5cfe1bb for type 'short[1]', which requires 2 byte alignment
  • Permanently avoid wmemcmp() UB, reported as VSO-2583476 "wmemcmp() triggers UB with a misaligned load".
    • C:\Program Files (x86)\Windows Kits\10\include\10.0.26100.0\ucrt\wchar.h:458:37: runtime error: load of misaligned address 0x7ff647c3b7bc for type 'unsigned long long', which requires 8 byte alignment
    • The offending line is: unsigned __int64 __v1 = *(unsigned __int64*)__s1;
  • Properly detect UBSan in test code.
    • This is necessary because MSVC doesn't understand __has_feature. (I initially missed this because I was testing Clang only.)

StephanTLavavej and others added 4 commits September 25, 2025 15:50
D:\GitHub\STL\out\x64\out\inc\__msvc_chrono.hpp:451:52: runtime error:
signed integer overflow: 9223372036854775806 * 1000000 cannot be represented in type '_CR' (aka 'long long')
D:\GitHub\STL\tests\std\tests\VSO_0000000_vector_algorithms_mismatch_and_lex_compare\test.cpp:177:9:
runtime error: reference binding to misaligned address 0x002ec5cfe1bb for type 'short[1]', which requires 2 byte alignment
…iggers UB with a misaligned load".

C:\Program Files (x86)\Windows Kits\10\include\10.0.26100.0\ucrt\wchar.h:458:37:
runtime error: load of misaligned address 0x7ff647c3b7bc for type 'unsigned long long', which requires 8 byte alignment

This line is:

unsigned __int64 __v1 = *(unsigned __int64*)__s1;
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 29, 2025 20:24
@StephanTLavavej StephanTLavavej added the test Related to test code label Sep 29, 2025
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Sep 29, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in STL Code Reviews Sep 29, 2025
Copy link
Member

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

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

LGTM, ty

@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in STL Code Reviews Oct 1, 2025
@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Oct 3, 2025
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 834a2dc into microsoft:main Oct 4, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Oct 4, 2025
@StephanTLavavej StephanTLavavej deleted the ubsan branch October 4, 2025 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Related to test code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Investigate why Clang/LLVM UBSan doesn't link

4 participants