[ASan][libcxx] Annotating std::vector with all allocators

[AdvenamTacet]

Summary

This revision is a part of a series of patches extending
AddressSanitizer C++ container overflow detection
capabilities by adding annotations, similar to those existing
in std::vector, to std::string and std::deque collections.
These changes allow ASan to detect cases when the instrumented
program accesses memory which is internally allocated by
the collection but is still not in-use (accesses before or
after the stored elements for std::deque, or between the size and
capacity bounds for std::string).

The motivation for the 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.

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 anymore.
This patch removes the check in std::vector annotation member
function (__annotate_contiguous_container) to support
different allocators.

Additionally, this revision fixes unpoisoning in std::vector.
It guarantees that __alloc_traits::deallocate may access returned memory.
Originally suggested in D144155 revision.

If you have any questions, please email:

• [e-mail]
• [e-mail]

[AdvenamTacet]

Turning on old check for llvm without new annotation implementation.
(Check for _LIBCPP_CLANG_VER.)

For old llvm versions, it will work the same way.

Btw. I do not have a commiter access.

[philnik]

libcxx/include/vector

748–749

Could you explain here why this is possible in LLVM16, but not before?

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

39

Could you instead add a test that checks this case?

[EricWF]

libcxx/include/vector

748–749

Seconded.

Also, I really don't like having the condition be conditionally compiled. It smells weird, and it looks weird, and I think it'll lead to bugs.

Can you find another way to write this?

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

31–32

This seems like it's defeating the purpose of the test?

[AdvenamTacet]

libcxx/include/vector

748–749

Vectors annotations rely on __sanitizer_annotate_contiguous_container, its implementation from LLVM15 (and earlier) requires that beginning of a containers data buffer is aligned to shadow granularity. Now, in effort to extend ASans capabilities, implementation of the function changed with commit rGdd1b7b797a116eed588fd752fbe61d34deeb24e4 and supports:

• unaligned beginnings of buffers,
• shared first/last granule (if memory granule is shared with a different object just after the end of unaligned end/before the unaligned beginning, memory of that object won't be poisoned).

Therefore, check for standard allocator is not necessary.

I don't know how to change the condition, requirements for the functions (__sanitizer_annotate_contiguous_container) arguments were weakened now and if older LLVM versions are supported, that check has to be still present for them. There are no new variables, just implementation changed. @EricWF Do you have an idea how it may be done in a different way?

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

31–32

It does change the purpose of the test, because with that patch, that access will be caught. But I will add a new test instead, as suggested.

39

Sure, I will add a new test and extend if around that one.

[AdvenamTacet]

With that change, new tests were introduced instead of modifying the old one.

List of changes:

• Changed 16000 to 160000 in comparison with _LIBCPP_CLANG_VER
• Withdrawn removal of a test, the test is now behind _LIBCPP_CLANG_VER < 160000 guard.
• A tests for annotations with min_allocator added.
• New allocator (returning not aligned buffers for types smaller than 8) added.
• A new test for annotations on an unaligned buffer added.

Does it look good now?

[AdvenamTacet]

This update fixes incorrect version number and applies changes to new head (master).

Please, consider accepting those changes as that revision increases number of situations when ASan may be used (non-standard allocators in vector).

[AdvenamTacet]

Unit test updated.

[philnik]

libcxx/include/vector

748–749

Please make this a comment in the code. Maybe also a // TODO LLVM18: Remove the special-casing, so we don't forget removing it.

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

32–54

Let's use TEST_CLANG_VER instead.

libcxx/test/support/min_allocator.h

445 ↗

(On Diff #491215)

This is UB in general. Please add a static_assert(alignof(T) == 1) to make sure nobody tries to instantiate it with a type where this produces UB.

[AdvenamTacet]

Fixes suggested in code review.

• Comment "TODO LLVM18..." added.
• Explained why special case is necessary and why the change is possible.
• TEST_CLANG_VER used instead of _LIBCPP_CLANG_VER
• Static assert for type size added.

[AdvenamTacet]

Fixing issue with old C++.

error: 'static_assert' with no message is a C++17 extension

[AdvenamTacet]

One more attempt to fix problems with older C++ (this time, C++03).

• Fixed static_assert description.
• static_assert is modified for C++03, as alignof was introduced in C++11.

[philnik]

LGTM % nit.

libcxx/test/support/min_allocator.h

438–441 ↗

(On Diff #491587)

Sorry that I didn't say that before. We have TEST_ALIGNOF to support C++03. Then you don't need the special-casing here.

[AdvenamTacet]

libcxx/test/support/min_allocator.h

438–441 ↗

(On Diff #491587)

Should I change it? I see that you accepted the revision.

[AdvenamTacet]

I see that the revision is accepted, but based on the inline commit, I updated static_assert.
Now TEST_ALIGNOF is used instead of two cases with #if.

I do not have commiter permissions.

[philnik]

In D136765#4076621, @AdvenamTacet wrote:

I see that the revision is accepted, but based on the inline commit, I updated static_assert.
Now TEST_ALIGNOF is used instead of two cases with #if.

I do not have commiter permissions.

LGTM % nit means that it looks good when the remaining comments are addressed, which are usually just small changes where one can't do much wrong. Are the Name and E-Mail in the commit correct?

[AdvenamTacet]

LGTM % nit means that it looks good when the remaining comments are addressed, which are usually just small changes where one can't do much wrong

Thank you for explaining. I realized that it means LGTM modulo something, but I wasn't sure if I should fix it or not.

Are the Name and E-Mail in the commit correct?

I don't see those fields here, but I believe so. locally it's correct (Advenam Tacet <[e-mail]>) in git commit and I used arc to send it.

[hans]

Extending the check to non-standard allocators, appears to cause a false positive during copy-assignment of a vector with custom allocator in V8, see https://crbug.com/1410719#c6

[hans]

In D136765#4085406, @hans wrote:

Extending the check to non-standard allocators, appears to cause a false positive during copy-assignment of a vector with custom allocator in V8, see https://crbug.com/1410719#c6

Filed as https://github.com/llvm/llvm-project/issues/60384 with a stand-alone reproducer. I'll revert this until it can be fixed.

[AdvenamTacet]

This update should fix problems with zero allocators and add tests confirming it.
It adds fixes to std::vector annotations unpoisoning, originally suggested in D144155. It guarantees that __alloc_traits::deallocate may access returned memory. (Details below.)
It adds a new test allocator (safe_allocator), this allocator accesses memory during allocation and deallocation.
It extends existing and relevant tests to use that allocator as well. This should show that a similar problem to the one which happened won’t happen again.

Short description of the fix:
A call to __annotate_delete() in operator() (in __destroy_vector class) is moved after a call to clear(), because clear may poison memory (as container overflow - fc).
It also adds unpoisoning memory before passing it to the deallocator in __vdeallocate() and __copy_assign_alloc(const vector& __c, true_type).
Current implementation is not correct and works only because annotations are turned on for only the standard allocator, as well as its implementation does not access memory at all, it may change with implementation change. You can see details in D144155.

[AdvenamTacet]

This update applies the patch on the newest commit from master branch.
This hopefuly fixes:

No testing is possible because we couldn't apply the patch.

Src: https://buildkite.com/llvm-project/diff-checks/builds/153926

The patch applied without a conflict.

Important update notes are in the previous message.

[AdvenamTacet]

fix: test

Is there a way to send a quiet update? Without informing everyone following the revision?

Important update notes are in the previous message.

[AdvenamTacet]

fix 2: test

[AdvenamTacet]

This revision passed automatic tests. To confirm that it's working, I used those changes to compile a bunch of random harnesses (~45) from Chromium and I'm fuzzing using them right now. Everything works.
The problem with false positives was caused by incorrect use/missing use of __annotate_delete() and should be fixed with that revision. Just the fix to that problem was suggested in D144155 but moved here, look there for details.

I believe the patch is ready to be merged again. Please, let me know if there is anything else to do.

[philnik]

A couple small things, but nothing major. LGTM with nits addressed.

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

32–54

Could you also add a TODO here?

libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.pass.cpp

100 ↗

(On Diff #498726)

Why are you replacing min_allocator here? Everywhere else you added a test.

libcxx/test/std/containers/sequences/vector/vector.special/swap.pass.cpp

181 ↗

(On Diff #498726)

Could you make the allocator a template parameter instead to avoid the duplication? The other cases are small enough that I don't really care, but would of course be a plus to deduplicate them as well.

libcxx/test/support/min_allocator.h

447 ↗

(On Diff #498726)

This static_cast shouldn't be necessary, right?

475 ↗

(On Diff #498726)

The compiler probably knows that after freeing the memory it's values can't be read, so this memset could probably get optimized away. Let's add a DoNotOptimize(p) just to be safe.

[AdvenamTacet]

libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.pass.cpp

100 ↗

(On Diff #498726)

I believe, someone pasted that test (with min_allocator) twice by mistake. Same test is just above.

[AdvenamTacet]

This update answers requests from the code review:

• adds TODO,
• removes static_cast,
• adds DoNotOptimize.

I hope, this patch is ready now and may be upstreamed.

[AdvenamTacet]

libcxx/test/std/containers/sequences/vector/vector.special/swap.pass.cpp

181 ↗

(On Diff #498726)

Could you make the allocator a template parameter

I forgot to mention in the update note, I did that as well.

libcxx/test/support/min_allocator.h

447 ↗

(On Diff #498726)

It's not, fixed.

[AdvenamTacet]

To make sure that after changes this diff still works as intended, I again compiled a bunch of Chromium harnesses and I don't get any false positives during fuzzing.
I believe, the patch is ready for upstream.

[hans]

I'm afraid we did hit an issue with this in Chromium, but I think it points to a more fundamental problem with these annotations rather than this patch.

The issue is that we have some code using an arena allocator to allocate a bunch of objects, including vectors and their backing memory. Then instead of tracking the lifetimes of those objects, the arena is cleared, and the memory recycled for next time. That means the vector destructors don't run, so some memory is left in the arena with container annotations, causing errors when it's reused.

I've created a reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5

This could have happened before this patch too, if the user overloaded the global operator new and delete for example.

The question is what to do about it. I don't think there's an easy way for the allocator to remove the container annotations when recycling the memory?

[philnik]

In D136765#4155262, @hans wrote:

I'm afraid we did hit an issue with this in Chromium, but I think it points to a more fundamental problem with these annotations rather than this patch.

The issue is that we have some code using an arena allocator to allocate a bunch of objects, including vectors and their backing memory. Then instead of tracking the lifetimes of those objects, the arena is cleared, and the memory recycled for next time. That means the vector destructors don't run, so some memory is left in the arena with container annotations, causing errors when it's reused.

I've created a reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5

This could have happened before this patch too, if the user overloaded the global operator new and delete for example.

The question is what to do about it. I don't think there's an easy way for the allocator to remove the container annotations when recycling the memory?

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

[vitalybuka]

In D136765#4155285, @philnik wrote:

In D136765#4155262, @hans wrote:

I'm afraid we did hit an issue with this in Chromium, but I think it points to a more fundamental problem with these annotations rather than this patch.

The issue is that we have some code using an arena allocator to allocate a bunch of objects, including vectors and their backing memory. Then instead of tracking the lifetimes of those objects, the arena is cleared, and the memory recycled for next time. That means the vector destructors don't run, so some memory is left in the arena with container annotations, causing errors when it's reused.

I've created a reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5

This could have happened before this patch too, if the user overloaded the global operator new and delete for example.

The question is what to do about it. I don't think there's an easy way for the allocator to remove the container annotations when recycling the memory?

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

[hans]

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

I'm not sure it is UB actually. (At least not the part that recycles the vector storage memory.) In any case it's not an unreasonable pattern; for example Clang does the same for its AST with ASTContext::Allocator -- the AST nodes' destructors never run, and one could imagine a scenario where the memory is re-used.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

Yes, I think that would work for us. (You mean just with __asan_unpoison_memory_region right?)
I'll give it a try tomorrow.

[philnik]

In D136765#4156486, @hans wrote:

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

I'm not sure it is UB actually. (At least not the part that recycles the vector storage memory.) In any case it's not an unreasonable pattern; for example Clang does the same for its AST with ASTContext::Allocator -- the AST nodes' destructors never run, and one could imagine a scenario where the memory is re-used.

Since you never run the destructor the object is still within it's lifetime, i.e. you create multiple objects within the same memory region, which is UB. I agree that it's not that crazy of a thing to do, just like comparing unrelated pointers is. Still, both are UB and are reasonable to be detected by a tool that is there to find UB. Maybe there should be a standards-blessed mechanism to destroy all objects within a region of memory, but we don't have that.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

Yes, I think that would work for us. (You mean just with __asan_unpoison_memory_region right?)
I'll give it a try tomorrow.

I wasn't aware that there exists a magic way to remove any annotations within a region, interesting.

[vitalybuka]

In D136765#4156486, @hans wrote:

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

I'm not sure it is UB actually. (At least not the part that recycles the vector storage memory.) In any case it's not an unreasonable pattern; for example Clang does the same for its AST with ASTContext::Allocator -- the AST nodes' destructors never run, and one could imagine a scenario where the memory is re-used.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

Yes, I think that would work for us. (You mean just with __asan_unpoison_memory_region right?)
I'll give it a try tomorrow.

In D136765#4158559, @philnik wrote:

In D136765#4156486, @hans wrote:

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

I'm not sure it is UB actually. (At least not the part that recycles the vector storage memory.) In any case it's not an unreasonable pattern; for example Clang does the same for its AST with ASTContext::Allocator -- the AST nodes' destructors never run, and one could imagine a scenario where the memory is re-used.

Since you never run the destructor the object is still within it's lifetime, i.e. you create multiple objects within the same memory region, which is UB. I agree that it's not that crazy of a thing to do, just like comparing unrelated pointers is. Still, both are UB and are reasonable to be detected by a tool that is there to find UB. Maybe there should be a standards-blessed mechanism to destroy all objects within a region of memory, but we don't have that.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

Yes, I think that would work for us. (You mean just with __asan_unpoison_memory_region right?)
I'll give it a try tomorrow.

I wasn't aware that there exists a magic way to remove any annotations within a region, interesting.

In D136765#4158559, @philnik wrote:

In D136765#4156486, @hans wrote:

I think it's pretty clear that what you are doing is UB, so IMO we want to catch this generally. I don't think there is an easy way to unpoison the memory other than running the destructors. We could add an escape hatch through the allocator traits, but that would of course mean that you don't get the additional coverage for that allocator.

I'm not sure it is UB actually. (At least not the part that recycles the vector storage memory.) In any case it's not an unreasonable pattern; for example Clang does the same for its AST with ASTContext::Allocator -- the AST nodes' destructors never run, and one could imagine a scenario where the memory is re-used.

Since you never run the destructor the object is still within it's lifetime, i.e. you create multiple objects within the same memory region, which is UB. I agree that it's not that crazy of a thing to do, just like comparing unrelated pointers is. Still, both are UB and are reasonable to be detected by a tool that is there to find UB. Maybe there should be a standards-blessed mechanism to destroy all objects within a region of memory, but we don't have that.

@hans If Chromium knowingly drops destructors, can you just upoison the entire range there, by arena allocator?

Yes, I think that would work for us. (You mean just with __asan_unpoison_memory_region right?)
I'll give it a try tomorrow.

I wasn't aware that there exists a magic way to remove any annotations within a region, interesting.

You should be able to achieve the same with sanitizer_annotate_contiguous_container() using special set of arguments.
something like sanitizer_annotate_contiguous_container(alloc_begin, alloc_end, alloc_begin, alloc_end)
libc++ should use the __sanitizer_ api so we can support other sanitizers, e.g. HWASAN, or even MTE.

@hans For arena __sanitizer_annotate_contiguous_container looks like weird but probably better option for the same reason. But having ifdef per sanitizers seems fine as well.

[hans]

We've hit another issue after this in Chromium: http://crbug.com/1420967. Also, Google internal testing hit a similar issue in MySQL. That means at least three false positives after this patch. At the same time this patch has revealed zero real issues in our code.

I understand the desire to make the behavior with custom allocators match that of standard allocators, but perhaps these stats indicate that the new behavior is doing more harm than good? As opposed to standard allocators, it seems that with custom allocators, it's common to have accesses to the memory that fall outside the container range.

I think we should consider backing this out, and leaving the container annotations to the standard allocators only.

[philnik]

In D136765#4171295, @hans wrote:

We've hit another issue after this in Chromium: http://crbug.com/1420967. Also, Google internal testing hit a similar issue in MySQL. That means at least three false positives after this patch. At the same time this patch has revealed zero real issues in our code.

I understand the desire to make the behavior with custom allocators match that of standard allocators, but perhaps these stats indicate that the new behavior is doing more harm than good? As opposed to standard allocators, it seems that with custom allocators, it's common to have accesses to the memory that fall outside the container range.

I think we should consider backing this out, and leaving the container annotations to the standard allocators only.

3 issues doesn't seem like a lot of data to build on. I don't think that these should be described as "not real issues", since they are clearly UB, which is something ASan is supposed to flag. I guess these are three different custom allocators that try to access allocated memory for some reason? I'd be fine with an opt-out mechanism for these allocators, but I think we should enable poisoning by default. e.g. for the wink-out mechanism it makes sense to poison the memory and when winking-out the objects un-poison everything. This allows ASan to still detect any overflow issues and having (most of) the performance benefit. There are also super simple allocators out there for logging etc. which would benefit from this kind of poisoning.

[hans]

In D136765#4171473, @philnik wrote:

In D136765#4171295, @hans wrote:

We've hit another issue after this in Chromium: http://crbug.com/1420967. Also, Google internal testing hit a similar issue in MySQL. That means at least three false positives after this patch. At the same time this patch has revealed zero real issues in our code.

I understand the desire to make the behavior with custom allocators match that of standard allocators, but perhaps these stats indicate that the new behavior is doing more harm than good? As opposed to standard allocators, it seems that with custom allocators, it's common to have accesses to the memory that fall outside the container range.

I think we should consider backing this out, and leaving the container annotations to the standard allocators only.

3 issues doesn't seem like a lot of data to build on.

Combined with the fact that we've run tests for hundreds of millions of lines of code and found zero real issues, I think it's a good indication.

I don't think that these should be described as "not real issues", since they are clearly UB, which is something ASan is supposed to flag.

I don't think the UB situation is that clear. If an allocator has allocated a chunk of ints, it's perfectly well defined behavior for that allocator or other code to access the whole chunk, even if parts of it is being used by a vector.

Another indication that these are not real issues is that the fix for all of them is to remove the container annotations.

One way to think about the situation is that with the standard allocator, these annotations work because the std::vector is in practice in complete control of the memory. With custom allocators that's no longer true, since the standard doesn't say what those allocators might do, and then these checks become much less precise.

I guess these are three different custom allocators that try to access allocated memory for some reason? I'd be fine with an opt-out mechanism for these allocators, but I think we should enable poisoning by default. e.g. for the wink-out mechanism it makes sense to poison the memory and when winking-out the objects un-poison everything. This allows ASan to still detect any overflow issues and having (most of) the performance benefit. There are also super simple allocators out there for logging etc. which would benefit from this kind of poisoning.

I can also see the theoretical benefit of this, and part of me likes it, but with the issues that are showing up, I'm just not sure it works out in practice. Perhaps it should be opt-in instead?

[ayzhao]

FYI this is causing (yet another) bug on Chrome/Chrome OS: https://crbug.com/1422033

[AdvenamTacet]

The question is what to do about it. I don't think there's an easy way for the allocator to remove the container annotations when recycling the memory?

Depends on your implementation, but you can easily unpoison memory (there are macros for it in the ASan API, but it should be possible to also use interface functions (not recommended)), or turn off instrumentation in a function accessing poisoned area. At the same time, I'm happy to implement an easy way to turn off annotations for specific allocators. Below details.

1. Unpoisoning:
   • The best option may be using macros for manual poisoning, as described in sanitizers wiki, but I never used them, so I'm not sure.
   • @vitalybuka I think __sanitizer_annotate_contiguous_container cannot be used here, as the function checks state of shadow memory, it's a pain to work with and I would be happy to remove those checks, if you agree.

2. It's possible to access memory without checks by turning off instrumentation with __attribute__((__no_sanitize__("address"))).

we've run tests for hundreds of millions of lines of code and found zero real issues

with the issues that are showing up, I'm just not sure it works out in practice. Perhaps it should be opt-in instead?

I strongly believe, it shouldn't be opt-in. We would have to educate everyone about that possibility and they would have to maintain list of allocators. With opt-out, one may just react to visible errors. Also, if in "hundreds of millions of lines" only 3 allocators are problematic, turning off annotations for them (or unpoisoning memory in them) seems like a small effort compared to a potential benefit of finding a security bug early. I understand that you didn't find anything yet, but it's same logic as for default allocator, and implementations with custom allocators may have bugs as well.

---

If @philnik thinks that opt-out is ok, I'm happy to add something like:

template<class A>
struct libcxx_asan_allocator_annotate {
    const bool value = true;
}

and in vectors member function __annotate_contiguous_container add to the check && libcxx_asan_allocator_annotate<_Allocator>::value.

• @philnik what file would be best for that class definition? Probably std::basic_string and std::deque should have access to it as well.
• @hans do you have a minimal failing example? Would be good to add unit test and make sure that it solves problems you observe.

At the same time, if simply unpoisoning memory works for you, it is probably a better choice as that change increases chance of detecting bugs.

---

@ayzhao I'm not sure what exactly is happening in your case, but there is a write to memory.

Strangely enough, calling __asan_unpoison_memory_region(...) in deallocate(...) doesn't resolve this...

Is it the first thing you do in deallocator? Are you unpoisoning whole memory for sure?

I'm guessing the reason is because std::vector calls __annotate_delete() before deallocate()

It should leave memory poisoned in the very same way as before a constructor. Why do you think it's the reason?

[AntonBikineev]

Regarding the Standard terms, accessing destructed, but not released memory is well defined in basic.life#6 (in some limited ways, but exactly sufficient for the allocator needs):

after the lifetime of an object has ended and before the storage which the object occupied is reused or released, any pointer that represents the address of the storage location where the object will be or was located may be used but only in limited ways. ... such a pointer refers to allocated storage ([basic.stc.dynamic.allocation]), and using the pointer as if the pointer were of type void* is well-defined. Indirection through such a pointer is permitted...

Since you never run the destructor the object is still within it's lifetime, i.e. you create multiple objects within the same memory region, which is UB.

Even that may (suprisingly) be well-defined (as the paragraph above mentions):

A program may end the lifetime of an object of class type without invoking the destructor, by reusing or releasing the storage as described above.

[hans]

In D136765#4173661, @ayzhao wrote:

FYI this is causing (yet another) bug on Chrome/Chrome OS: https://crbug.com/1422033

We've root caused this to ChromeOS using a toolchain based on llvmorg-16-init-6711-g11897708c022, which is before the ASan runtime gained support for unaligned container memory in llvmorg-16-init-9006-gdd1b7b797a11, which this patch depends on. (https://crbug.com/1422033#c11 has a reproducer).

I see that the patch is already trying to handle this with the #if _LIBCPP_CLANG_VER >= 1600 check. However, that doesn't quite work since there were many revisions with that version number before the ASan change.

Should it be checking _LIBCPP_CLANG_VER >= 1700 instead, or is there some more precise way to handle this? (Does the ASan runtime have some way to check if it supports this for example?)

[manojgupta]

In D136765#4178089, @hans wrote:

In D136765#4173661, @ayzhao wrote:

FYI this is causing (yet another) bug on Chrome/Chrome OS: https://crbug.com/1422033

We've root caused this to ChromeOS using a toolchain based on llvmorg-16-init-6711-g11897708c022, which is before the ASan runtime gained support for unaligned container memory in llvmorg-16-init-9006-gdd1b7b797a11, which this patch depends on. (https://crbug.com/1422033#c11 has a reproducer).

I see that the patch is already trying to handle this with the #if _LIBCPP_CLANG_VER >= 1600 check. However, that doesn't quite work since there were many revisions with that version number before the ASan change.

Should it be checking _LIBCPP_CLANG_VER >= 1700 instead, or is there some more precise way to handle this? (Does the ASan runtime have some way to check if it supports this for example?)

Afaik, this is against libc++ policy. libc++ needs to work with at least 1 or 2 last clang releases so this patch needs to accomodate it.
@ldionne to confirm.

[philnik]

In D136765#4178116, @manojgupta wrote:

In D136765#4178089, @hans wrote:

In D136765#4173661, @ayzhao wrote:

FYI this is causing (yet another) bug on Chrome/Chrome OS: https://crbug.com/1422033

We've root caused this to ChromeOS using a toolchain based on llvmorg-16-init-6711-g11897708c022, which is before the ASan runtime gained support for unaligned container memory in llvmorg-16-init-9006-gdd1b7b797a11, which this patch depends on. (https://crbug.com/1422033#c11 has a reproducer).

I see that the patch is already trying to handle this with the #if _LIBCPP_CLANG_VER >= 1600 check. However, that doesn't quite work since there were many revisions with that version number before the ASan change.

Should it be checking _LIBCPP_CLANG_VER >= 1700 instead, or is there some more precise way to handle this? (Does the ASan runtime have some way to check if it supports this for example?)

Afaik, this is against libc++ policy. libc++ needs to work with at least 1 or 2 last clang releases so this patch needs to accomodate it.
@ldionne to confirm.

The last two official releases. It doesn't seem like ChromeOS uses that, given that LLVM16 isn't released yet. Anyways, I think we have to revert, since we clearly need some way to fix the annotation problems, and that will probably take some time to figure out.

[hans]

In D136765#4178156, @philnik wrote:

The last two official releases. It doesn't seem like ChromeOS uses that, given that LLVM16 isn't released yet. Anyways, I think we have to revert, since we clearly need some way to fix the annotation problems, and that will probably take some time to figure out.

FWIW I think just checking _LIBCPP_CLANG_VER >= 1700 would be a good fix.

[philnik]

In D136765#4178231, @hans wrote:

In D136765#4178156, @philnik wrote:

The last two official releases. It doesn't seem like ChromeOS uses that, given that LLVM16 isn't released yet. Anyways, I think we have to revert, since we clearly need some way to fix the annotation problems, and that will probably take some time to figure out.

FWIW I think just checking _LIBCPP_CLANG_VER >= 1700 would be a good fix.

I don't think that makes sense. We don't check that we have the version after the first one in which it's implemented anywhere else. Supporting this kind of stuff would be a nightmare, and 99% of people who don't use an official release are either on the bleeding edge, or are a vendor. If you are a vendor you probably have some downstream patches anyways, so having this too seems not that much of a problem.

[hans]

In D136765#4178312, @philnik wrote:

In D136765#4178231, @hans wrote:

In D136765#4178156, @philnik wrote:

The last two official releases. It doesn't seem like ChromeOS uses that, given that LLVM16 isn't released yet. Anyways, I think we have to revert, since we clearly need some way to fix the annotation problems, and that will probably take some time to figure out.

FWIW I think just checking _LIBCPP_CLANG_VER >= 1700 would be a good fix.

I don't think that makes sense. We don't check that we have the version after the first one in which it's implemented anywhere else. Supporting this kind of stuff would be a nightmare, and 99% of people who don't use an official release are either on the bleeding edge, or are a vendor. If you are a vendor you probably have some downstream patches anyways, so having this too seems not that much of a problem.

As we've seen, _LIBCPP_CLANG_VER >= 1600 is not sufficient to check that the ASan support is there, but _LIBCPP_CLANG_VER >= 1700 is sufficient. And since this patch (annotations for vector with custom allocator) is happening in the 17 release, there's no delay before they can be used. That's why I thought it would be a good fix.

An even nicer fix would of course be if ASan had some kind of feature checking mechanism or finer grained version we could check.

[AdvenamTacet]

I'm reopening because of the revert.

• Safe allocator and tests are moved to D145597, I will remove them from here.
• A way to turn off annotations for a specific allocator will be added in a separate patch.

[AdvenamTacet]

This update removes tests already added in D145597.

[AdvenamTacet]

This update includes change from D145628 and adds an easy way to turn off annotations for a specific allocator.
It also adds a test for that change, based on failing example showed above.

I still consider unpoisoning memory as a superior choice, but if it's impossible in some situations (like using allocator mantained by someone else), now it should be easy to turn off annotations for a specific allocator.

[AdvenamTacet]

Test update.

[AdvenamTacet]

This update tries to solve the problem with MacOS error (cannot test it locally).

It adds #include <new> to solve:

error: no matching 'operator new' function for non-allocating placement new expression

[hans]

In D136765#4179911, @AdvenamTacet wrote:

This update includes change from D145628 and adds an easy way to turn off annotations for a specific allocator.
It also adds a test for that change, based on failing example showed above.

I still consider unpoisoning memory as a superior choice, but if it's impossible in some situations (like using allocator mantained by someone else), now it should be easy to turn off annotations for a specific allocator.

Thanks! I added a comment on that. Besides allocators maintained by someone else, I think https://crbug.com/1420967 is also a good example of where unpoisoning isn't an option.

The current patch still has the _LIBCPP_CLANG_VER issue though.

[AdvenamTacet]

I think https://crbug.com/1420967 is also a good example of where unpoisoning isn't an option.

If access to poisoned memory happens only in a few functions, it's possible to just turn off instrumentation in them with __attribute__((__no_sanitize__("address"))).
But if the buffer is modified, it may be better to turn off annotations for that allocator. Therefore I made D145628.

The current patch still has the _LIBCPP_CLANG_VER issue though.

Here I agree with @philnik that waiting until LLVM17 (given that LLVM16 isn't released yet) makes no sense and testing changes would be very difficult. Also, rGdd1b7b797a116eed588fd752fbe61d34deeb24e4 was accepted more than 4 moths ago, so it's a big time gap between support in compiler-rt and libc++.
I also don't see any other reasonable way to check support in compiler-rt than checking version number.

[hans]

In D136765#4198690, @AdvenamTacet wrote:

I think https://crbug.com/1420967 is also a good example of where unpoisoning isn't an option.

If access to poisoned memory happens only in a few functions, it's possible to just turn off instrumentation in them with __attribute__((__no_sanitize__("address"))).
But if the buffer is modified, it may be better to turn off annotations for that allocator. Therefore I made D145628.

Thanks, I think turning off annotations for this allocator is the best option. Hunting down all functions that access the memory would be hard, and also disabling instrumentation in them seems like a too big hammer.

The current patch still has the _LIBCPP_CLANG_VER issue though.

Here I agree with @philnik that waiting until LLVM17 (given that LLVM16 isn't released yet) makes no sense and testing changes would be very difficult. Also, rGdd1b7b797a116eed588fd752fbe61d34deeb24e4 was accepted more than 4 moths ago, so it's a big time gap between support in compiler-rt and libc++.
I also don't see any other reasonable way to check support in compiler-rt than checking version number.

There would be no need to wait since 17 is already the current version at HEAD.

[ldionne]

Considering that this won't be cherry-picked back to release/16.x (right?), then whether we use _LIBCPP_CLANG_VER >= 1600 or _LIBCPP_CLANG_VER >= 1700 is irrelevant for all practical purposes, since libc++ gets released in sync with Clang and the next release this will get in will have Clang 17. I do agree however that if someone (in this case Chrome) is using a not-yet-released snapshot of trunk that happens to have _LIBCPP_CLANG_VER >= 1600 yet isn't something we've released as LLVM 16, then it's probably not reasonable for us to try to support that. Imagine coming up with that policy, I think it would mean that we can't ever rely on _LIBCPP_CLANG_VER >= 1600 during the libc++ 16 release, which would be pretty unfortunate.

So I have no strong opinion, either way is fine with me (even though the status quo is kind of unfortunate for Chrome until they bump their Clang version).

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

66–67

Since we're mostly interested in having good coverage for the Clang >= 16 behavior anyway, I would instead suggest keeping this test but removing the #if TEST_STD_VER >= 11 && TEST_CLANG_VER < 1600 test above. I don't really see much value in keeping it around, unless I missed something.

libcxx/test/libcxx/containers/sequences/vector/no_asan.pass.cpp

13 ↗

(On Diff #503649)

Can you please add an english description of what this test does? It's also nice to have the link, however the test should ideally be understandable just by reading it (if e.g. the link goes down or something).

57 ↗

(On Diff #503649)

Nitpick, but this is needed for freestanding.

[hans]

In D136765#4203247, @ldionne wrote:

I do agree however that if someone (in this case Chrome) is using a not-yet-released snapshot of trunk that happens to have _LIBCPP_CLANG_VER >= 1600 yet isn't something we've released as LLVM 16, then it's probably not reasonable for us to try to support that. Imagine coming up with that policy, I think it would mean that we can't ever rely on _LIBCPP_CLANG_VER >= 1600 during the libc++ 16 release, which would be pretty unfortunate.

(FWIW it's ChromeOS, not Chrome, that's using an older Clang version. And if necessary they can probably patch in the needed compiler-rt changes. Also, https://libcxx.llvm.org/ technically says that clang "16-git" is supported, which is what they are using ;)

I'll stop harping on about this, but I think gating functionality on _LIBCPP_CLANG_VER isn't great for the reason we've seen here. (And Eric predicted it in his "It smells weird, and it looks weird, and I think it'll lead to bugs." comment). I'm pleased to see that it's currently only used in two places in libc++, one of which is for a deprecation warning. As mentioned above, the best would be if compiler-rt exposed some kind of feature check for this, but if we can't have that, I think >= 17 is the way to go.

[philnik]

In D136765#4205962, @hans wrote:

In D136765#4203247, @ldionne wrote:

I do agree however that if someone (in this case Chrome) is using a not-yet-released snapshot of trunk that happens to have _LIBCPP_CLANG_VER >= 1600 yet isn't something we've released as LLVM 16, then it's probably not reasonable for us to try to support that. Imagine coming up with that policy, I think it would mean that we can't ever rely on _LIBCPP_CLANG_VER >= 1600 during the libc++ 16 release, which would be pretty unfortunate.

(FWIW it's ChromeOS, not Chrome, that's using an older Clang version. And if necessary they can probably patch in the needed compiler-rt changes. Also, https://libcxx.llvm.org/ technically says that clang "16-git" is supported, which is what they are using ;)

It's "16-git" as-in current trunk, not some random version from a few months ago.

I'll stop harping on about this, but I think gating functionality on _LIBCPP_CLANG_VER isn't great for the reason we've seen here. (And Eric predicted it in his "It smells weird, and it looks weird, and I think it'll lead to bugs." comment). I'm pleased to see that it's currently only used in two places in libc++, one of which is for a deprecation warning. As mentioned above, the best would be if compiler-rt exposed some kind of feature check for this, but if we can't have that, I think >= 17 is the way to go.

I don't think >= 17 is an option. You are basically asking us to keep the code around for another release because someone used some wacky unsupported configuration. Either live at head or use releases. They exist for a reason.

[hans]

In D136765#4205989, @philnik wrote:

I don't think >= 17 is an option. You are basically asking us to keep the code around for another release because someone used some wacky unsupported configuration. Either live at head or use releases. They exist for a reason.

I'm just trying to be pragmatic. Given the nasty failure mode - ASan silently malfunctioning - using a check which actually guarantees that the required functionality is there seems reasonable to me.

[AdvenamTacet]

This update:

• adds mains arguments,
• adds tests description,
• removes suggested test case.

[manojgupta]

In D136765#4205989, @philnik wrote:

In D136765#4205962, @hans wrote:

In D136765#4203247, @ldionne wrote:

I do agree however that if someone (in this case Chrome) is using a not-yet-released snapshot of trunk that happens to have _LIBCPP_CLANG_VER >= 1600 yet isn't something we've released as LLVM 16, then it's probably not reasonable for us to try to support that. Imagine coming up with that policy, I think it would mean that we can't ever rely on _LIBCPP_CLANG_VER >= 1600 during the libc++ 16 release, which would be pretty unfortunate.

(FWIW it's ChromeOS, not Chrome, that's using an older Clang version. And if necessary they can probably patch in the needed compiler-rt changes. Also, https://libcxx.llvm.org/ technically says that clang "16-git" is supported, which is what they are using ;)

It's "16-git" as-in current trunk, not some random version from a few months ago.

I'll stop harping on about this, but I think gating functionality on _LIBCPP_CLANG_VER isn't great for the reason we've seen here. (And Eric predicted it in his "It smells weird, and it looks weird, and I think it'll lead to bugs." comment). I'm pleased to see that it's currently only used in two places in libc++, one of which is for a deprecation warning. As mentioned above, the best would be if compiler-rt exposed some kind of feature check for this, but if we can't have that, I think >= 17 is the way to go.

I don't think >= 17 is an option. You are basically asking us to keep the code around for another release because someone used some wacky unsupported configuration. Either live at head or use releases. They exist for a reason.

809855b56f06dd7182685f88fbbc64111df9339a changed clang version in trunk to 16.
https://reviews.llvm.org/rGdd1b7b797a11 added the API that this patch is using.

This change means that any clang released in the range 809855b56f06dd7182685f88fbbc64111df9339a..dd1b7b797a11 is now BAD for building libc++.

$ git log 809855b56f06dd7182685f88fbbc64111df9339a..dd1b7b797a11 --oneline | wc -l
9006

IMO, this is an large number of commits and punishes people that made clang releases in this range.

I'd however like to steer out of the debate by cherry-picking https://reviews.llvm.org/rGdd1b7b797a11 to ChromeOS clang.

[AdvenamTacet]

I was thinking about a solution for the silent error.

API for contiguous containers was extended on Oct 28 2022, - rGdd1b7b797a116eed588fd752fbe61d34deeb24e4.

API for double ended containers was commited on Nov 22 2022 - rG1c5ad6d2c01294a0decde43a88e9c27d7437d157.
And extended on Nov 29 - rGe1657e322902d973aea606d3557f938ce0e0f06c

If we first land D132092 (annotations for std::deque with default allocator only), there will be no silent error, as a missing function in compiler-rt causes an error.

That solutions holds water only if D132092 will be accepted, but I hope it will be. The pach is ready for review.

[hans]

In D136765#4212222, @AdvenamTacet wrote:

If we first land D132092 (annotations for std::deque with default allocator only), there will be no silent error, as a missing function in compiler-rt causes an error.

Yes, that is indeed much nicer.

[AdvenamTacet]

This update removed #if checking compiler-rt to confirm support in API, the check was moved into __asan_annotate_container_with_allocator (check D145628) as suggested by @EricWF.

Additionally, test name is changed for one which (hopefuly) has a more precise description of the purpose.

[AdvenamTacet]

This update:

• adds use of Cpp17UnaryTypeTrait in a test case, same as in updated D145628.

[AdvenamTacet]

In D145628, an FTM was added. This update introduces that change to this revision.

[AdvenamTacet]

This update the base commit.

[AdvenamTacet]

Rebase to HEAD of master.

[thakis]

Somewhat early heads-up: This is again causing various test failures in Chromium land. Several of us are traveling this week, so it'll likely be a while until we can provide more details. (No action needed at this point, just as an FYI.) Our upstream bug is https://bugs.chromium.org/p/chromium/issues/detail?id=1444659 .

libcxx/include/vector

 #include <__functional/hash.h>
 #include <__functional/unary_function.h>
 #include <__iterator/advance.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
 #include <__iterator/wrap_iter.h>
 #include <__memory/addressof.h>
 #include <__memory/allocate_at_least.h>
+#include <__memory/allocator_traits.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/swap_allocator.h>
 #include <__memory/temp_value.h>
 #include <__memory/uninitialized_algorithms.h>
 #include <__memory_resource/polymorphic_allocator.h>
 #include <__split_buffer>
 #include <__type_traits/is_allocator.h>
 #include <__type_traits/is_constructible.h>
#endif
   vector(_ForwardIterator __first, _ForwardIterator __last, const allocator_type& __a);
 
 private:
   class __destroy_vector {
     public:
       _LIBCPP_CONSTEXPR __destroy_vector(vector& __vec) : __vec_(__vec) {}
 
       _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void operator()() {
-          __vec_.__annotate_delete();
           std::__debug_db_erase_c(std::addressof(__vec_));
 
           if (__vec_.__begin_ != nullptr) {
             __vec_.__clear();
+            __vec_.__annotate_delete();
             __alloc_traits::deallocate(__vec_.__alloc(), __vec_.__begin_, __vec_.capacity());
           }
       }
 
     private:
       vector& __vec_;
   };
 
__construct_at_end(_ForwardIterator __first, _ForwardIterator __last, size_type __n);
     // We call annotatations only for the default Allocator because other allocators
     // may not meet the AddressSanitizer alignment constraints.
     // See the documentation for __sanitizer_annotate_contiguous_container for more details.
 #ifndef _LIBCPP_HAS_NO_ASAN
     _LIBCPP_CONSTEXPR_SINCE_CXX20
     void __annotate_contiguous_container(const void *__beg, const void *__end,
                                          const void *__old_mid,
                                          const void *__new_mid) const
     {
-
-      if (!__libcpp_is_constant_evaluated() && __beg && is_same<allocator_type, __default_allocator_type>::value)
+      if (!__libcpp_is_constant_evaluated() && __beg != nullptr && __asan_annotate_container_with_allocator<_Allocator>::value)
         __sanitizer_annotate_contiguous_container(__beg, __end, __old_mid, __new_mid);
     }
 #else
     _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
     void __annotate_contiguous_container(const void*, const void*, const void*,
                                          const void*) const _NOEXCEPT {}
 #endif
     _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
#endif
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
   void __copy_assign_alloc(const vector& __c, true_type)
   {
     if (__alloc() != __c.__alloc())
     {
       __clear();
+      __annotate_delete();
       __alloc_traits::deallocate(__alloc(), this->__begin_, capacity());
       this->__begin_ = this->__end_ = __end_cap() = nullptr;
     }
     __alloc() = __c.__alloc();
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
   void __copy_assign_alloc(const vector&, false_type)

 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20
 void
 vector<_Tp, _Allocator>::__vdeallocate() _NOEXCEPT
 {
     if (this->__begin_ != nullptr)
     {
         clear();
+        __annotate_delete();
         __alloc_traits::deallocate(this->__alloc(), this->__begin_, capacity());
         this->__begin_ = this->__end_ = this->__end_cap() = nullptr;
     }
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20
 typename vector<_Tp, _Allocator>::size_type

libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

 
 extern "C" void __sanitizer_set_death_callback(void (*callback)(void));
 
 void do_exit() {
   exit(0);
 }
 
 int main(int, char**)
 {
-#if TEST_STD_VER >= 11
-    {
-        typedef int T;
-        typedef std::vector<T, min_allocator<T>> C;
-        const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
-        C c(std::begin(t), std::end(t));
-        c.reserve(2*c.size());
-        volatile T foo = c[c.size()];    // bad, but not caught by ASAN
-        ((void)foo);
-    }
+#if TEST_STD_VER >= 11 && TEST_CLANG_VER >= 1600
+  // TODO LLVM18: Remove the special-casing
+    typedef cpp17_input_iterator<T*> MyInputIter;
+    std::vector<T, min_allocator<T>> v;
+    v.reserve(1);
+    int i[] = {42};
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 1));
+    assert(v[0] == 42);
+    assert(is_contiguous_container_asan_correct(v));
+  }
+  {
+    typedef char T;
+    typedef cpp17_input_iterator<T*> MyInputIter;
+    std::vector<T, unaligned_allocator<T>> v;
+    v.reserve(1);
+    char i[] = {'a', 'b'};
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 2));
+    assert(v[0] == 'a');
+    assert(v[1] == 'b');
+    assert(is_contiguous_container_asan_correct(v));
 #endif
-
-    {
-        typedef cpp17_input_iterator<int*> MyInputIter;
-        // Sould not trigger ASan.
-        std::vector<int> v;
-        v.reserve(1);
-        int i[] = {42};
-        v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 1));
-        assert(v[0] == 42);
-        assert(is_contiguous_container_asan_correct(v));
-    }
 
-    __sanitizer_set_death_callback(do_exit);
-    {
-        typedef int T;
-        typedef std::vector<T> C;
-        const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
-        C c(std::begin(t), std::end(t));
-        c.reserve(2*c.size());
-        assert(is_contiguous_container_asan_correct(c));
-        assert(!__sanitizer_verify_contiguous_container( c.data(), c.data() + 1, c.data() + c.capacity()));
-        volatile T foo = c[c.size()]; // should trigger ASAN. Use volatile to prevent being optimized away.
-        assert(false);          // if we got here, ASAN didn't trigger
-        ((void)foo);
-    }
+    c.reserve(2 * c.size());
+    assert(!__sanitizer_verify_contiguous_container(c.data(), c.data() + 1, c.data() + c.capacity()));
+    assert(false);                // if we got here, ASAN didn't trigger
 }

libcxx/test/libcxx/containers/sequences/vector/asan_turning_off.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <vector>
+
+// Test based on: https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5
+// Some allocators during deallocation may not call destructors and just reuse memory.
+// In those situations, one may want to deactivate annotations for a specific allocator.
+// It's possible with __asan_annotate_container_with_allocator template class.
+// This test confirms that those allocators work after turning off annotations.
+
+#include <assert.h>
+#include <stdlib.h>
+#include <vector>
+#include <new>
+
+struct reuse_allocator {
+  static size_t const N = 100;
+  reuse_allocator() {
+    for (size_t i = 0; i < N; ++i)
+      __buffers[i] = malloc(8*1024);
+  }
+  ~reuse_allocator() {
+    for (size_t i = 0; i < N; ++i)
+      free(__buffers[i]);
+  }
+  void* alloc() {
+    assert(__next_id < N);
+    return __buffers[__next_id++];
+  }
+  void reset() { __next_id = 0; }
+  void* __buffers[N];
+  size_t __next_id = 0;
+} reuse_buffers;
+
+template <typename T>
+struct user_allocator {
+  using value_type = T;
+  user_allocator() = default;
+  template <class U>
+  user_allocator(user_allocator<U>) {}
+  friend bool operator==(user_allocator, user_allocator) {return true;}
+  friend bool operator!=(user_allocator x, user_allocator y) {return !(x == y);}
+
+  T* allocate(size_t) { return (T*)reuse_buffers.alloc(); }
+  void deallocate(T*, size_t) noexcept {}
+};
+
+#ifdef _LIBCPP_HAS_ASAN_CONTAINER_ANNOTATIONS_FOR_ALL_ALLOCATORS
+template <class T>
+struct std::__asan_annotate_container_with_allocator<user_allocator<T>> : false_type {};
+#endif
+
+int main(int, char**) {
+  using V = std::vector<int, user_allocator<int>>;
+
+  {
+    V* v = new (reuse_buffers.alloc()) V();
+    for (int i = 0; i < 100; i++)
+      v->push_back(i);
+  }
+  reuse_buffers.reset();
+  {
+    V v;
+    for (int i = 0; i < 1000; i++)
+      v.push_back(i);
+  }
+
+  return 0;
+}