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

Wed Mar 8 09:05:11 PST 2023

hans added a comment.

In D136765#4178312 <https://reviews.llvm.org/D136765#4178312>, @philnik wrote:

> In D136765#4178231 <https://reviews.llvm.org/D136765#4178231>, @hans wrote:
>
>> In D136765#4178156 <https://reviews.llvm.org/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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136765