[libcxx-commits] [PATCH] D132090: [1a/3][ASan][compiler-rt] API for double ended containers

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet marked an inline comment as done.
AdvenamTacet added inline comments.


================
Comment at: libcxx/include/__config:883
     __sanitizer_annotate_contiguous_container(const void*, const void*, const void*, const void*);
+extern "C" _LIBCPP_FUNC_VIS void __sanitizer_annotate_double_ended_contiguous_container(
+    const void*, const void*, const void*, const void*, const void*, const void*);
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > this should be in a separate patch, probably D132092
> > 
> > however we need to support the case when we build libcxx with older clang/compiler-rt when this function is not available
> > maybe you land D132090 and after some time, if it's not reverted
> > you land libcxx patch with  _LIBCPP_CLANG_VER check?
> > 
> > how about gcc? it uses the same runtime
> Note: I assume that compiler-rt/clang version mismatch is possible, but is not required to support. I assume that libcxx clang mismatch is required to be supported.
> 
> this should be in a separate patch, probably D132092

Right, I'm going to move it to D132092.

> however we need to support the case when we build libcxx with older clang/compiler-rt

We do use checks for clang version there (`_LIBCPP_CLANG_VER >= 16000`) and I am going to guard that as well. It does work with old versions of llvm.

> how about gcc? it uses the same runtime

I do not know about using libc++ with gcc, I think we can wait with supporting annotations for deque there. If I understand correctly, there should be no problems generated by that change in that scenario, just no annotations for deque.

If you ask about libstdc++, annotations for that implementation have to be created. We do have working PoC, but we are focused on upstreaming those changes for llvm at the moment. I'm not sure if we are going to work further on libstdc++, but we may consider making them public "as is".

> maybe you land D132090 and after some time, if it's not reverted

Landing D132090 and D132522 (I'm about to make an update there) before libcxx patches is my plan, but I want to land D132092 and D132769 soon after to avoid further changes in std::deque or std::basic_string implementations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132090