The work was done in a8ae392718313e64e2d7bc092b00f51476c074b2 which
implements parts of
N4258: Cleaning-up noexcept in the Library
However that's paper Summary of Proposed Changes contains
No change in vector<bool>
This patch updates the status and as a few NFC cleanups. The fixed
version is based on the release page and might be off.
Updates
- LWG3778 vector<bool> missing exception specifications
~~NB: This PR depends on #78876. Ignore the first commit when reviewing,
and don't merge it until #78876 is resolved. When/if #78876 lands, I'll
clean this up.~~
This partially restores parity with the old, since removed debug build.
We now can re-enable a bunch of the disabled tests. Some things of note:
- `bounded_iter`'s converting constructor has never worked. It needs a
friend declaration to access the other `bound_iter` instantiation's
private fields.
- The old debug iterators also checked that callers did not try to
compare iterators from different objects. `bounded_iter` does not
currently do this, so I've left those disabled. However, I think we
probably should add those. See
https://github.com/llvm/llvm-project/issues/78771#issuecomment-1902999181
- The `std::vector` iterators are bounded up to capacity, not size. This
makes for a weaker safety check. This is because the STL promises not to
invalidate iterators when appending up to the capacity. Since we cannot
retroactively update all the iterators on `push_back()`, I've instead
sized it to the capacity. This is not as good, but at least will stop
the iterator from going off the end of the buffer.
There was also no test for this, so I've added one in the `std`
directory.
- `std::string` has two ambiguities to deal with. First, I opted not to
size it against the capacity. https://eel.is/c++draft/string.require#4
says iterators are invalidated on an non-const operation. Second,
whether the iterator can reach the NUL terminator. The previous debug
tests and the special-case in https://eel.is/c++draft/string.access#2
suggest no. If either of these causes widespread problems, I figure we
can revisit.
- `resize_and_overwrite.pass.cpp` assumed `std::string`'s iterator
supported `s.begin().base()`, but I see no promise of this in the
standard. GCC also doesn't support this. I fixed the test to use
`std::to_address`.
- `alignof.compile.pass.cpp`'s pointer isn't enough of a real pointer.
(It needs to satisfy `NullablePointer`, `LegacyRandomAccessIterator`,
and `LegacyContiguousIterator`.) `__bounded_iter` seems to instantiate
enough to notice. I've added a few more bits to satisfy it.
Fixes#78805
This assures shrink_to_fit does not increase the allocated size.
Partly addresses https://github.com/llvm/llvm-project/issues/95161
---------
Co-authored-by: Mital Ashok <mital.vaja@googlemail.com>
There is code duplication in all containers that static_assert the
allocator matches the allocator requirements in the spec. This check can
be moved into a more centralised place.
As time went by, a few files have become mis-formatted w.r.t.
clang-format. This was made worse by the fact that formatting was not
being enforced in extensionless headers. This commit simply brings all
of libcxx/include in-line with clang-format again.
We might have to do this from time to time as we update our clang-format
version, but frankly this is really low effort now that we've formatted
everything once.
This changes the `is_swappable` implementation to use variable templates
first and basing the class templates on that. This avoids instantiating
them when the `_v` versions are used, which are generally less resource
intensive.
These were required a long time ago due to `static_assert` not actually
being available in C++03. Now `static_assert` is simply mapped to
`_Static_assert` in C++03, making the additional parens unnecessary.
This is a follow-up to https://github.com/llvm/llvm-project/pull/80282.
The transitive includes of `<locale>` in `<vector>` were all guarded by
the availability macro -- the new include should also be guarded,
otherwise any users who compile with localization disabled will start
getting errors trying to include `<vector>`.
In essence, this header has always been related to configuration of
the library but we didn't want to put it inside <__config> due to
complexity reasons. Now that we have sub-headers in <__config>, we
can move <__availability> to it and stop including it everywhere since
we already obtain the required macros via <__config>.
Placing physically next to each other remaining constructors filling
vector with the same value
will make code better, since they all have nearly identical
implementation, which needs to be kept in sync.
Co-authored-by: Mark de Wever <koraq@xs4all.nl>
These headers have become very small by using compiler builtins, often
containing only two declarations. This merges these headers, since
there doesn't seem to be much of a benefit keeping them separate.
Specifically, `is_{,_nothrow,_trivially}{assignable,constructible}` are
kept and the `copy`, `move` and `default` versions of these type traits
are moved in to the respective headers.
Originally, we used __libcpp_verbose_abort to handle assertion failures.
That function was declared from all public headers. Since we don't use
that mechanism anymore, we don't need to declare __libcpp_verbose_abort
from all public headers, and we can clean up a lot of unnecessary
includes.
This patch also moves the definition of the various assertion categories
to the <__assert> header, since we now rely on regular IWYU for these
assertion macros.
rdar://105510916
This patch introduces a new trait to represent whether a type is
trivially
relocatable, and uses that trait to optimize the growth of a std::vector
of trivially relocatable objects.
```
--------------------------------------------------
Benchmark old new
--------------------------------------------------
bm_grow<int> 1354 ns 1301 ns
bm_grow<std::string> 5584 ns 3370 ns
bm_grow<std::unique_ptr<int>> 3506 ns 1994 ns
bm_grow<std::deque<int>> 27114 ns 27209 ns
```
This also changes to order of moving and destroying the objects when
growing the vector. This should not affect our conformance.
This commit simplifies ASan helper functions in `std::vector` by
removing arguments which can be calculated later.
Short term it improves readability of helper functions in `std::vector`.
Long term it aims to help with a bigger refactor of container
annotations.
This commit implements conditional compilation for ASan helper code.
As convey to me by @EricWF, string benchmarks with UBSan have been
experiencing significant performance hit after the commit with ASan
string annotations. This is likely due to the fact that no-op ASan code
is not optimized out with UBSan. To address this issue, this commit
conditionalizes the inclusion of ASan helper function bodies using
`#ifdef` directives. This approach allows us to selectively include only
the ASan code when it's actually required, thereby enhancing
optimizations and improving performance.
While issue was noticed in string benchmarks, I expect same overhead
(just less noticeable) in other containers, therefore `std::vector` and
`std::deque` have same changes.
To see impact of that change run `string.libcxx.out` with UBSan and
`--benchmark_filter=BM_StringAssign` or
`--benchmark_filter=BM_StringConstruct`.
This patch runs clang-format on all of libcxx/include and libcxx/src, in
accordance with the RFC discussed at [1]. Follow-up patches will format
the benchmarks, the test suite and remaining parts of the code. I'm
splitting this one into its own patch so the diff is a bit easier to
review.
This patch was generated with:
find libcxx/include libcxx/src -type f \
| grep -v 'module.modulemap.in' \
| grep -v 'CMakeLists.txt' \
| grep -v 'README.txt' \
| grep -v 'libcxx.imp' \
| grep -v '__config_site.in' \
| xargs clang-format -i
A Git merge driver is available in libcxx/utils/clang-format-merge-driver.sh
to help resolve merge and rebase issues across these formatting changes.
[1]: https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all
It's not that I have much love for C++03, but we should ensure that it
works. Some recent changes broke this configuration because slightly
older Clang versions don't support attribute syntax in C++03 mode.
This commit refactors the ASan annotation functions in libc++ to reduce
unnecessary code duplication. Additionally it adds a small optimization.
- Eliminates two redundant function versions by utilizing the
`[[maybe_unused]]` attribute and guarding function bodies with `#ifndef
_LIBCPP_HAS_NO_ASAN`.
- Introduces an additional guard to an auxiliary function, allowing the
removal of a no-ops function body. This approach avoids relying on the
optimizer for code elimination.
Fixes#73043
This is in preparation for clang-formatting the whole code base. These
annotations are required either to avoid clang-format bugs or because
the manually formatted code is significantly more readable than the
clang-formatted alternative. All in all, it seems like very few
annotations are required, which means that clang-format is doing a very
good job in most cases.
Credits: this change is based on analysis and a proof of concept by
gerbens@google.com.
Before, the compiler loses track of end as 'this' and other references
possibly escape beyond the compiler's scope. This can be see in the
generated assembly:
16.28 │200c80: mov %r15d,(%rax)
60.87 │200c83: add $0x4,%rax
│200c87: mov %rax,-0x38(%rbp)
0.03 │200c8b: → jmpq 200d4e
...
...
1.69 │200d4e: cmp %r15d,%r12d
│200d51: → je 200c40
16.34 │200d57: inc %r15d
0.05 │200d5a: mov -0x38(%rbp),%rax
3.27 │200d5e: mov -0x30(%rbp),%r13
1.47 │200d62: cmp %r13,%rax
│200d65: → jne 200c80
We fix this by always explicitly storing the loaded local and pointer
back at the end of push back. This generates some slight source 'noise',
but creates nice and compact fast path code, i.e.:
32.64 │200760: mov %r14d,(%r12)
9.97 │200764: add $0x4,%r12
6.97 │200768: mov %r12,-0x38(%rbp)
32.17 │20076c: add $0x1,%r14d
2.36 │200770: cmp %r14d,%ebx
│200773: → je 200730
8.98 │200775: mov -0x30(%rbp),%r13
6.75 │200779: cmp %r13,%r12
│20077c: → jne 200760
Now there is a single store for the push_back value (as before), and a
single store for the end without a reload (dependency).
For fully local vectors, (i.e., not referenced elsewhere), the capacity
load and store inside the loop could also be removed, but this requires
more substantial refactoring inside vector.
Differential Revision: https://reviews.llvm.org/D80588
This brings most of the enable_ifs in libc++ to the same style. It also has the nice side-effect of reducing the size of names of these symbols, since the arguments don't get mangled anymore.
Reviewed By: #libc, Mordante
Spies: Mordante, libcxx-commits
Differential Revision: https://reviews.llvm.org/D157748
This brings most of the enable_ifs in libc++ to the same style. It also has the nice side-effect of reducing the size of names of these symbols, since the depedent return type is shorter.
Reviewed By: #libc, ldionne
Spies: ldionne, libcxx-commits
Differential Revision: https://reviews.llvm.org/D157736
ASan capabilities were extended, but some comments were not updated and describe old behavior. This commit updates outdated comments, which I found.
Mentioned changes are:
- All allocators in containers (`std::vector` and `std::deque`; D146815 D136765) are supported, but it's possible to turn off annotations for a specific allocator (D145628).
- Buffers don't have to be aligned (D132522).
Reviewed By: #libc, philnik
Differential Revision: https://reviews.llvm.org/D156155
Replace most uses of `_LIBCPP_ASSERT` with
`_LIBCPP_ASSERT_UNCATEGORIZED`.
This is done as a prerequisite to introducing hardened mode to libc++.
The idea is to make enabling assertions an opt-in with (somewhat)
fine-grained controls over which categories of assertions are enabled.
The vast majority of assertions are currently uncategorized; the new
macro will allow turning on `_LIBCPP_ASSERT` (the underlying mechanism
for all kinds of assertions) without enabling all the uncategorized
assertions (in the future; this patch preserves the current behavior).
Differential Revision: https://reviews.llvm.org/D153816
This fixes rdar://110330781, which asked for the feature-test macro
for std::pmr to take into account the deployment target. It doesn't
fix https://llvm.org/PR62212, though, because the availability markup
itself must be disabled until some Clang bugs have been fixed.
This is pretty vexing, however at least everything should work once
those Clang bugs have been fixed. In the meantime, this patch at least
adds the required markup (as disabled) and ensures that the feature-test
macro for std::pmr is aware of the deployment target requirement.
Differential Revision: https://reviews.llvm.org/D135813
Implements part of P1614R2 "The Mothership has Landed"
Depends on D150188
Reviewed By: Mordante, #libc
Differential Revision: https://reviews.llvm.org/D132268
The code has been quite ready for a while now and there are no more ABI
breaking papers. So this is a good time to mark the feature as stable.
Reviewed By: #libc, ldionne
Differential Revision: https://reviews.llvm.org/D150802
We plan to add concepts for checking that iterators actually provide what they claim to. This is to avoid people thinking that these type traits actually check the iterator requirements in more detail.
Reviewed By: ldionne, #libc
Spies: Mordante, libcxx-commits, wenlei
Differential Revision: https://reviews.llvm.org/D150801
- add the `from_range_t` constructors and the related deduction guides;
- add the `insert_range`/`assign_range`/etc. member functions.
(Note: this patch is split from https://reviews.llvm.org/D142335)
Differential Revision: https://reviews.llvm.org/D149826
Makes sure the formatter for the vector<bool>::reference is enabled
when only the header <vector> is included. Before this change it
required <vector> and <format> to be included. This violated the
requirements in the Standard.
Fixes: https://llvm.org/PR61314
Reviewed By: #libc, ldionne
Differential Revision: https://reviews.llvm.org/D149543
This revision is a part of a series of patches extending
AddressSanitizer C++ container overflow detection
capabilities by adding annotations, similar to those existing
in std::vector, to std::string and std::deque collections.
These changes allow ASan to detect cases when the instrumented
program accesses memory which is internally allocated by
the collection but is still not in-use (accesses before or
after the stored elements for std::deque, or between the size and
capacity bounds for std::string).
The motivation for the research and those changes was a bug,
found by Trail of Bits, in a real code where an out-of-bounds read
could happen as two strings were compared via a std::equals function
that took iter1_begin, iter1_end, iter2_begin iterators
(with a custom comparison function).
When object iter1 was longer than iter2, read out-of-bounds on iter2
could happen. Container sanitization would detect it.
In revision D132522, support for non-aligned memory buffers (sharing
first/last granule with other objects) was added, therefore the
check for standard allocator is not necessary anymore.
This patch removes the check in std::vector annotation member
function (__annotate_contiguous_container) to support
different allocators.
Additionally, this revision fixes unpoisoning in std::vector.
It guarantees that __alloc_traits::deallocate may access returned memory.
Originally suggested in D144155 revision.
If you have any questions, please email:
- advenam.tacet@trailofbits.com
- disconnect3d@trailofbits.com
Reviewed By: #libc, #sanitizers, philnik, vitalybuka, ldionne
Spies: mikhail.ramalho, manojgupta, ldionne, AntonBikineev, ayzhao, hans, EricWF, philnik, #sanitizers, libcxx-commits
Differential Revision: https://reviews.llvm.org/D136765
We already have a clang-tidy check for making sure that `_LIBCPP_HIDE_FROM_ABI` is on free functions. This patch extends this to class members. The places where we don't check for `_LIBCPP_HIDE_FROM_ABI` are classes for which we have an instantiation in the library.
Reviewed By: ldionne, Mordante, #libc
Spies: jplehr, mikhail.ramalho, sstefan1, libcxx-commits, krytarowski, miyuki, smeenai
Differential Revision: https://reviews.llvm.org/D142332
