This is an archive of the discontinued LLVM Phabricator instance.

[ASan][libc++] Annotating std::basic_string with all allocators

Authored by AdvenamTacet on Mar 16 2023, 4:07 AM.


Group Reviewers
Restricted Project

This patch is part of our efforts to support container annotations with (almost) every allocator.
Annotating std::basic_string with default allocator is implemented in D132769.

In revision D132522, support for non-aligned memory buffers (sharing first/last granule with other objects) was added, therefore the check for standard allocator is not necessary.
This patch removes the check in std::basic_string annotation member function (__annotate_contiguous_container) to support different allocators and also includes changes from D145628, creating an easy way to turn off annotations for a specific allocator.

The motivation for a research and those changes was a bug, found by Trail of Bits, in a real code where an out-of-bounds read could happen as two strings were compared via a std::equals function that took iter1_begin, iter1_end, iter2_begin iterators (with a custom comparison function).
When object iter1 was longer than iter2, read out-of-bounds on iter2 could happen. Container sanitization would detect it.

If you have any questions, please email:


Diff Detail

Event Timeline

AdvenamTacet created this revision.Mar 16 2023, 4:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2023, 4:07 AM
AdvenamTacet requested review of this revision.Mar 16 2023, 4:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2023, 4:07 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
EricWF added a subscriber: EricWF.Mar 20 2023, 12:11 PM

Has this patch been tested against Chromium? Has it been tested elsewhere?
The more testing it's undergone, the more confident I can be.


conditional compilation blocks are the source of a lot of bugs.

Instead of an #if and #else block here. Why not make it so that __asan_annonate_container_with_allocator only returns true when the other conditions _LIBCPP_CLANG_VER > 1600 etc.

Then you could simply write the condition

if (!__libcpp_is_constant_evaluated() && __begin != nullptr &&  __asan_annotate_container_with_allocator<allocator_type, __default_allocator_type>)

Where __asan_annotate_container_allocator_type can return true in the case that the allocator is the default.


It's not clear to me what this test is supposed to do.

Does the name mean it should only run without address sanitizer?


Again, I would much rather see zero code inside this conditional compilation block.

If need-be, please write something sorta like this:


if (SHORT_STRING_ALLOWED || !is_short_string(s))

This update adds a description to the test case.


Maybe I should change the name, but I added a description. If you have any suggestion, please let me know. This test turns off annotations for a specific allocator.

AdvenamTacet added inline comments.Mar 22 2023, 4:20 AM

Moving those checks into __asan_annonate_container_with_allocator is a nice idea. I will try to do it Tomorrow. Thanks!

AdvenamTacet marked 3 inline comments as done.

This update removes conditional code as much as possible, as suggested in inline comments.
One #if was moved to __asan_annotate_container_with_allocator, check D145628 for details.

Previous update removed 4 lines too much, this reverted this part of the previous one.
It also adds __asan_annotate_container_with_allocator check there.

This update adds a simple asan libcxx test, similar to one in vector.