[libcxx-commits] [PATCH] D145628: [ASan][libcxx] A way to turn off annotations for containers with a specific allocator

[Hans Wennborg via Phabricator via libcxx-commits]

hans added inline comments.


================
Comment at: libcxx/docs/UsingLibcxx.rst:521
+
+Turning off ASan annotation in containers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
----------------
When this lands, it should probably have a link from the release notes.


================
Comment at: libcxx/docs/UsingLibcxx.rst:524
+
+Struct template ``__asan_annotate_container_with_allocator`` may be used to turn off ASan annotations for containers
+with a specific allocator.
----------------
Perhaps the "ASan annotations for containers" could be a link to https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow for those who are not familiar with the feature.


================
Comment at: libcxx/docs/UsingLibcxx.rst:530
+If you are creating allocator not working correctly with container annotations from libc++,
+a better choice may be unpoisoning memory, if possible. This way, ASan benefits are present in the program.
+
----------------
It would be good to have an example (not necessarily in code) of why an allocator would not work with this, and either code or a link to how to unpoison the memory.


================
Comment at: libcxx/docs/UsingLibcxx.rst:527
+If ``__asan_annotate_container_with_allocator<_Alloc>::value == false``, container won't be poisoned at all.
+Value may be changed by template specialization. Variable ``value`` is of type ``bool``.
+
----------------
AdvenamTacet wrote:
> hans wrote:
> > Could you please include a code example which shows how to do the specialization?
> > 
> > Also, will it be possible to write this in a way that works both with libc++ versions that have and don't have this change?
> I added examples. I hope those are helpful. If you believe that another example is necessary as well, let me know.
> 
> > Also, will it be possible to write this in a way that works both with libc++ versions that have and don't have this change?
> 
> Possibly a macro would be answer to that, I suggested one in an update comment. But it should be possible to solve easily with libc++ version number as well. I don't really like the idea of creating a macro here.
This is great, thanks!

I'd suggest writing the example in a way that's portable, i.e. that will work both with libc++ versions which do and do not have this feature, as well as other standard libraries.

In order to do that, I guess we'll need some kind of feature test macro. I don't think checking the libc++ version number is a good solution as it's not fine grained enough. I'm guessing libc++ has some kind of mechanism for this already similar to the standard feature test macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145628