Skip to content

[libc++] Add assumption for align of begin and end pointers of vector. #108961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 16, 2025
Merged

[libc++] Add assumption for align of begin and end pointers of vector. #108961

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
94c2ede
[libc++] Add assumption for align of begin and end pointers of vector.
fhahn Sep 17, 2024
3daa6c5
Address review comments
ldionne Jan 13, 2025
ddb387b
Handle C++03
ldionne Jan 13, 2025
a2cd12c
Add missing include
ldionne Jan 13, 2025
a9fb6aa
Avoid instantiating vector::begin() when declaring flat_map
ldionne Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions libcxx/include/__vector/vector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,13 +341,17 @@ class _LIBCPP_TEMPLATE_VIS vector {
//
// Iterators
//
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT { return __make_iter(this->__begin_); }
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT {
return __make_iter(__add_alignment_assumption(this->__begin_));
}
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT {
return __make_iter(this->__begin_);
return __make_iter(__add_alignment_assumption(this->__begin_));
}
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT {
return __make_iter(__add_alignment_assumption(this->__end_));
}
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT { return __make_iter(this->__end_); }
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT {
return __make_iter(this->__end_);
return __make_iter(__add_alignment_assumption(this->__end_));
}

_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI reverse_iterator rbegin() _NOEXCEPT {
Expand Down Expand Up @@ -775,6 +779,15 @@ class _LIBCPP_TEMPLATE_VIS vector {
}

_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __move_assign_alloc(vector&, false_type) _NOEXCEPT {}

static _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer __add_alignment_assumption(pointer __p) _NOEXCEPT {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to #118837 (review) I think this one needs _LIBCPP_NO_CFI since we're potentially static_cast'ing uninitialized memory (for the end pointer).

(Apologies for the late notice, we're a bit behind on libc++.)

Copy link
Member

@ldionne ldionne Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I'd be curious to see how this failure can be reproduced. Is it just a matter of adding a -fsanitize=cfi job to our pre-commit CI?

CF #124837

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also #124839 for adding NO_CFI

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried to reproduce it on the libc++ tests directly, but it should be possible. I think we're currently only using a subset of the cfi checks: -fsanitize=cfi-vcall -fsanitize=cfi-derived-cast -fsanitize=cfi-unrelated-cast. The annoying thing is that cfi requires (thin) lto.

if constexpr (is_pointer<pointer>::value) {
if (!__libcpp_is_constant_evaluated()) {
return static_cast<pointer>(__builtin_assume_aligned(__p, alignof(decltype(*__p))));
}
}
return __p;
}
};

#if _LIBCPP_STD_VER >= 17
Expand Down
Loading