[libcxx-commits] [PATCH] D132769: [2b/3][ASan][libcxx] std::basic_string annotations
Tacet via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Feb 25 16:05:42 PST 2023
AdvenamTacet marked an inline comment as done.
AdvenamTacet added inline comments.
================
Comment at: libcxx/include/string:1853
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_new(size_type __current_size) const _NOEXCEPT {
+ if (!__libcpp_is_constant_evaluated() && (__annotate_short_string_check() || __is_long()))
+ __annotate_contiguous_container(data(), data() + capacity() + 1, data() + capacity() + 1, data() + __current_size + 1);
----------------
philnik wrote:
> Isn't this redundant? `__annotate_short_string_check()` already checks this.
But for long strings it also has to be checked, as every string is long during constant evaluation.
```
bool __is_long() const _NOEXCEPT {
if (__libcpp_is_constant_evaluated())
return true;
```
It has to be checked here and we cannot wait until the check in `__annotate_contiguous_container`, because during constant evaluation, next line would rise `cannot perform pointer arithmetic on null pointer`.
I don't want to remove it from `__annotate_short_string_check()`, because then returned values won't be correct.
And I don't want to remove it from `__annotate_contiguous_container` to make sure it can be simply called.
It also has no impact on performance.
Possibly it may be changed to `__annotate_short_string_check() || (!__libcpp_is_constant_evaluated() && __is_long())`, but I have a strong preference for current version, as that would suggest possibility of poisoning short string during constant evaluation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132769/new/
https://reviews.llvm.org/D132769
More information about the libcxx-commits
mailing list