Skip to content
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.

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

[libc++] Add details about string annotations #80912

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

AdvenamTacet
Copy link
Member

This commit adds information that only long strings are annotated, and with all allocators by default.

To read why short string annotations are not turned on yet, read related PR: #79536

This commit adds information that only long strings are annotated,
and with all allocators by default.

To read why short string annotations are not turned on yet, read
related PR: llvm#79536
@AdvenamTacet AdvenamTacet added documentation libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Feb 6, 2024
@AdvenamTacet AdvenamTacet requested a review from a team as a code owner February 6, 2024 23:01
@llvmbot
Copy link

llvmbot commented Feb 6, 2024

@llvm/pr-subscribers-libcxx

Author: Tacet (AdvenamTacet)

Changes

This commit adds information that only long strings are annotated, and with all allocators by default.

To read why short string annotations are not turned on yet, read related PR: #79536


Full diff: https://github.com/llvm/llvm-project/pull/80912.diff

1 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/18.rst (+3-1)
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 80b42ad7f653a..862284ce5494c 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -124,7 +124,9 @@ Improvements and New Features
   ``-DLIBCXX_INSTALL_MODULE_DIR=<path>``. The default location is
   ``${PREFIX}/share/libc++/v1``.
 
-- AddressSanitizer annotations have been added to ``std::basic_string``.
+- AddressSanitizer annotations have been added to ``std::basic_string``
+  external buffers (long strings only).
+  Enabled with all allocators by default.
 
 - The libc++ source code has been formatted with ``clang-format``. This
   `discourse thread <https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all>`_

@AdvenamTacet
Copy link
Member Author

Short string annotations are not merged and we did not figure out why short string annotations are problematic, yet. Therefore I won't suggest backporting them, but it would be good to update release notes to reflect that.

@mordante mordante added this to the LLVM 18.X Release milestone Feb 17, 2024
@mordante mordante self-assigned this Feb 17, 2024
Co-authored-by: Mark de Wever <[email protected]>
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM! Please make sure you backport the fix, if you need assistance let me know.

@tstellar
Copy link
Collaborator

Does this need to be backported to the release/18.x branch?

@AdvenamTacet
Copy link
Member Author

It's an update to a release note, so I think it should be for consistency. I can take care of it by the end of today.

@AdvenamTacet AdvenamTacet merged commit 7661ade into llvm:main Feb 20, 2024
52 checks passed
@AdvenamTacet AdvenamTacet deleted the release-notes-annotations branch February 20, 2024 06:20
AdvenamTacet pushed a commit that referenced this pull request Feb 23, 2024
This commit adds information that only long strings are annotated, and
with all allocators by default.

To read why short string annotations are not turned on yet, read comments in a related
PR: #79536

---------

Co-authored-by: Mark de Wever <[email protected]>
tstellar pushed a commit that referenced this pull request Feb 23, 2024
This commit adds information that only long strings are annotated, and
with all allocators by default.

To read why short string annotations are not turned on yet, read
comments in a related PR:
#79536

Upstreamed in: 7661ade
Upstream PR: #80912

---------

Co-authored-by: Mark de Wever <[email protected]>

Co-authored-by: Mark de Wever <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

6 participants