[libcxx-commits] [PATCH] D136765: [ASan][libcxx] Annotating std::vector with all allocators

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D136765#4156486 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136765/new/

https://reviews.llvm.org/D136765