[sanitizer_common] Move container overflow declarations into new file #168882
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
DanBlackwell
force-pushed
the
common_interface_dunder
branch
2 times, most recently
from
547968f to
15c7907
Compare
DanBlackwell
force-pushed
the
common_interface_dunder
branch
from
8b30ef3 to
4867e25
Compare
🐧 Linux x64 Test Results
|
Member
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Dan Blackwell (DanBlackwell) ChangesCurrently the ASan container overflow function declarations live in the common_interface_defs.h file. In order to allow libcxx to directly use these declarations, rather than forward declaring them, we need to prefix all identifiers with '__'. To minimize the impact of that we can break the effected functions out into a separate file, and alter only these. This patch does the above. It follows on from: #163468. Patch is 26.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/168882.diff 3 Files Affected:
diff --git a/compiler-rt/include/CMakeLists.txt b/compiler-rt/include/CMakeLists.txt
index 242d62b9b447b..746a328b05510 100644
--- a/compiler-rt/include/CMakeLists.txt
+++ b/compiler-rt/include/CMakeLists.txt
@@ -3,6 +3,7 @@ if (COMPILER_RT_BUILD_SANITIZERS)
sanitizer/allocator_interface.h
sanitizer/asan_interface.h
sanitizer/common_interface_defs.h
+ sanitizer/container_overflow_defs.h
sanitizer/coverage_interface.h
sanitizer/dfsan_interface.h
sanitizer/hwasan_interface.h
diff --git a/compiler-rt/include/sanitizer/common_interface_defs.h b/compiler-rt/include/sanitizer/common_interface_defs.h
index 51104093758cd..2e1f6265a66a8 100644
--- a/compiler-rt/include/sanitizer/common_interface_defs.h
+++ b/compiler-rt/include/sanitizer/common_interface_defs.h
@@ -110,250 +110,11 @@ void SANITIZER_CDECL __sanitizer_unaligned_store64(void *p, uint64_t x);
// simultaneously.
int SANITIZER_CDECL __sanitizer_acquire_crash_state();
-/// Annotates the current state of a contiguous container, such as
-/// <c>std::vector</c>, <c>std::string</c>, or similar.
-///
-/// A contiguous container is a container that keeps all of its elements
-/// in a contiguous region of memory. The container owns the region of memory
-/// <c>[beg, end)</c>; the memory <c>[beg, mid)</c> is used to store the
-/// current elements, and the memory <c>[mid, end)</c> is reserved for future
-/// elements (<c>beg <= mid <= end</c>). For example, in
-/// <c>std::vector<> v</c>:
-///
-/// \code
-/// beg = &v[0];
-/// end = beg + v.capacity() * sizeof(v[0]);
-/// mid = beg + v.size() * sizeof(v[0]);
-/// \endcode
-///
-/// This annotation tells the Sanitizer tool about the current state of the
-/// container so that the tool can report errors when memory from
-/// <c>[mid, end)</c> is accessed. Insert this annotation into methods like
-/// <c>push_back()</c> or <c>pop_back()</c>. Supply the old and new values of
-/// <c>mid</c>(<c><i>old_mid</i></c> and <c><i>new_mid</i></c>). In the initial
-/// state <c>mid == end</c>, so that should be the final state when the
-/// container is destroyed or when the container reallocates the storage.
-///
-/// For ASan, <c><i>beg</i></c> no longer needs to be 8-aligned,
-/// first and last granule may be shared with other objects
-/// and therefore the function can be used for any allocator.
-///
-/// The following example shows how to use the function:
-///
-/// \code
-/// int32_t x[3]; // 12 bytes
-/// char *beg = (char*)&x[0];
-/// char *end = beg + 12;
-/// __sanitizer_annotate_contiguous_container(beg, end, beg, end);
-/// \endcode
-///
-/// \note Use this function with caution and do not use for anything other
-/// than vector-like classes.
-/// \note Unaligned <c><i>beg</i></c> or <c><i>end</i></c> may miss bugs in
-/// these granules.
-///
-/// \param beg Beginning of memory region.
-/// \param end End of memory region.
-/// \param old_mid Old middle of memory region.
-/// \param new_mid New middle of memory region.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline void SANITIZER_CDECL
-__sanitizer_annotate_contiguous_container(const void *beg, const void *end,
- const void *old_mid,
- const void *new_mid) {}
-#else
-void SANITIZER_CDECL __sanitizer_annotate_contiguous_container(
- const void *beg, const void *end, const void *old_mid, const void *new_mid);
-#endif
-
-/// Similar to <c>__sanitizer_annotate_contiguous_container</c>.
-///
-/// Annotates the current state of a contiguous container memory,
-/// such as <c>std::deque</c>'s single chunk, when the boundries are moved.
-///
-/// A contiguous chunk is a chunk that keeps all of its elements
-/// in a contiguous region of memory. The container owns the region of memory
-/// <c>[storage_beg, storage_end)</c>; the memory <c>[container_beg,
-/// container_end)</c> is used to store the current elements, and the memory
-/// <c>[storage_beg, container_beg), [container_end, storage_end)</c> is
-/// reserved for future elements (<c>storage_beg <= container_beg <=
-/// container_end <= storage_end</c>). For example, in <c> std::deque </c>:
-/// - chunk with a frist deques element will have container_beg equal to address
-/// of the first element.
-/// - in every next chunk with elements, true is <c> container_beg ==
-/// storage_beg </c>.
-///
-/// Argument requirements:
-/// During unpoisoning memory of empty container (before first element is
-/// added):
-/// - old_container_beg_p == old_container_end_p
-/// During poisoning after last element was removed:
-/// - new_container_beg_p == new_container_end_p
-/// \param storage_beg Beginning of memory region.
-/// \param storage_end End of memory region.
-/// \param old_container_beg Old beginning of used region.
-/// \param old_container_end End of used region.
-/// \param new_container_beg New beginning of used region.
-/// \param new_container_end New end of used region.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline void
- SANITIZER_CDECL __sanitizer_annotate_double_ended_contiguous_container(
- const void *storage_beg, const void *storage_end,
- const void *old_container_beg, const void *old_container_end,
- const void *new_container_beg, const void *new_container_end) {}
-#else
-void SANITIZER_CDECL __sanitizer_annotate_double_ended_contiguous_container(
- const void *storage_beg, const void *storage_end,
- const void *old_container_beg, const void *old_container_end,
- const void *new_container_beg, const void *new_container_end);
-#endif
-
-/// Copies memory annotations from a source storage region to a destination
-/// storage region. After the operation, the destination region has the same
-/// memory annotations as the source region, as long as sanitizer limitations
-/// allow it (more bytes may be unpoisoned than in the source region, resulting
-/// in more false negatives, but never false positives). If the source and
-/// destination regions overlap, only the minimal required changes are made to
-/// preserve the correct annotations. Old storage bytes that are not in the new
-/// storage should have the same annotations, as long as sanitizer limitations
-/// allow it.
-///
-/// This function is primarily designed to be used when moving trivially
-/// relocatable objects that may have poisoned memory, making direct copying
-/// problematic under sanitizer. However, this function does not move memory
-/// content itself, only annotations.
-///
-/// A contiguous container is a container that keeps all of its elements in a
-/// contiguous region of memory. The container owns the region of memory
-/// <c>[src_begin, src_end)</c> and <c>[dst_begin, dst_end)</c>. The memory
-/// within these regions may be alternately poisoned and non-poisoned, with
-/// possibly smaller poisoned and unpoisoned regions.
-///
-/// If this function fully poisons a granule, it is marked as "container
-/// overflow".
-///
-/// Argument requirements: The destination container must have the same size as
-/// the source container, which is inferred from the beginning and end of the
-/// source region. Addresses may be granule-unaligned, but this may affect
-/// performance.
-///
-/// \param src_begin Begin of the source container region.
-/// \param src_end End of the source container region.
-/// \param dst_begin Begin of the destination container region.
-/// \param dst_end End of the destination container region.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline void SANITIZER_CDECL
-__sanitizer_copy_contiguous_container_annotations(const void *src_begin,
- const void *src_end,
- const void *dst_begin,
- const void *dst_end) {}
-#else
-void SANITIZER_CDECL __sanitizer_copy_contiguous_container_annotations(
- const void *src_begin, const void *src_end, const void *dst_begin,
- const void *dst_end);
-#endif
-
-/// Returns true if the contiguous container <c>[beg, end)</c> is properly
-/// poisoned.
-///
-/// Proper poisoning could occur, for example, with
-/// <c>__sanitizer_annotate_contiguous_container</c>), that is, if
-/// <c>[beg, mid)</c> is addressable and <c>[mid, end)</c> is unaddressable.
-/// Full verification requires O (<c>end - beg</c>) time; this function tries
-/// to avoid such complexity by touching only parts of the container around
-/// <c><i>beg</i></c>, <c><i>mid</i></c>, and <c><i>end</i></c>.
-///
-/// \param beg Beginning of memory region.
-/// \param mid Middle of memory region.
-/// \param end Old end of memory region.
-///
-/// \returns True if the contiguous container <c>[beg, end)</c> is properly
-/// poisoned.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline int
- SANITIZER_CDECL __sanitizer_verify_contiguous_container(const void *beg,
- const void *mid,
- const void *end) {}
-#else
-int SANITIZER_CDECL __sanitizer_verify_contiguous_container(const void *beg,
- const void *mid,
- const void *end);
-#endif
-
-/// Returns true if the double ended contiguous
-/// container <c>[storage_beg, storage_end)</c> is properly poisoned.
-///
-/// Proper poisoning could occur, for example, with
-/// <c>__sanitizer_annotate_double_ended_contiguous_container</c>), that is, if
-/// <c>[storage_beg, container_beg)</c> is not addressable, <c>[container_beg,
-/// container_end)</c> is addressable and <c>[container_end, end)</c> is
-/// unaddressable. Full verification requires O (<c>storage_end -
-/// storage_beg</c>) time; this function tries to avoid such complexity by
-/// touching only parts of the container around <c><i>storage_beg</i></c>,
-/// <c><i>container_beg</i></c>, <c><i>container_end</i></c>, and
-/// <c><i>storage_end</i></c>.
-///
-/// \param storage_beg Beginning of memory region.
-/// \param container_beg Beginning of used region.
-/// \param container_end End of used region.
-/// \param storage_end End of memory region.
-///
-/// \returns True if the double-ended contiguous container <c>[storage_beg,
-/// container_beg, container_end, end)</c> is properly poisoned - only
-/// [container_beg; container_end) is addressable.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline int SANITIZER_CDECL
-__sanitizer_verify_double_ended_contiguous_container(const void *storage_beg,
- const void *container_beg,
- const void *container_end,
- const void *storage_end) {}
-#else
-int SANITIZER_CDECL __sanitizer_verify_double_ended_contiguous_container(
- const void *storage_beg, const void *container_beg,
- const void *container_end, const void *storage_end);
-#endif
-
-/// Similar to <c>__sanitizer_verify_contiguous_container()</c> but also
-/// returns the address of the first improperly poisoned byte.
-///
-/// Returns NULL if the area is poisoned properly.
-///
-/// \param beg Beginning of memory region.
-/// \param mid Middle of memory region.
-/// \param end Old end of memory region.
-///
-/// \returns The bad address or NULL.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline const void *SANITIZER_CDECL
-__sanitizer_contiguous_container_find_bad_address(const void *beg,
- const void *mid,
- const void *end) {}
-#else
-const void *SANITIZER_CDECL __sanitizer_contiguous_container_find_bad_address(
- const void *beg, const void *mid, const void *end);
-#endif
-
-/// returns the address of the first improperly poisoned byte.
-///
-/// Returns NULL if the area is poisoned properly.
-///
-/// \param storage_beg Beginning of memory region.
-/// \param container_beg Beginning of used region.
-/// \param container_end End of used region.
-/// \param storage_end End of memory region.
-///
-/// \returns The bad address or NULL.
-#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
-__attribute__((__internal_linkage__)) inline const void *SANITIZER_CDECL
-__sanitizer_double_ended_contiguous_container_find_bad_address(
- const void *storage_beg, const void *container_beg,
- const void *container_end, const void *storage_end) {}
-#else
-const void *SANITIZER_CDECL
-__sanitizer_double_ended_contiguous_container_find_bad_address(
- const void *storage_beg, const void *container_beg,
- const void *container_end, const void *storage_end);
-#endif
+// The container overflow function declarations used to be inline here, but in
+// order to allow libcxx to directly use the header they are now in a separate
+// file. This file is included here to avoid breaking anyone reliant on
+// the definitions appearing in the current file.
+#include "container_overflow_defs.h"
/// Prints the stack trace leading to this call (useful for calling from the
/// debugger).
diff --git a/compiler-rt/include/sanitizer/container_overflow_defs.h b/compiler-rt/include/sanitizer/container_overflow_defs.h
new file mode 100644
index 0000000000000..fe1d0e9e98f30
--- /dev/null
+++ b/compiler-rt/include/sanitizer/container_overflow_defs.h
@@ -0,0 +1,275 @@
+//===-- sanitizer/container_overflow_defs.h ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Public sanitizer interface defs for container overflow checks.
+//===----------------------------------------------------------------------===//
+
+#ifndef SANITIZER_CONTAINER_OVERFLOW_DEFS_H
+#define SANITIZER_CONTAINER_OVERFLOW_DEFS_H
+
+// Windows allows a user to set their default calling convention, but we always
+// use __cdecl
+#ifdef _WIN32
+#define SANITIZER_CDECL __cdecl
+#else
+#define SANITIZER_CDECL
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/// Annotates the current state of a contiguous container, such as
+/// <c>std::vector</c>, <c>std::string</c>, or similar.
+///
+/// A contiguous container is a container that keeps all of its elements
+/// in a contiguous region of memory. The container owns the region of memory
+/// <c>[beg, end)</c>; the memory <c>[beg, mid)</c> is used to store the
+/// current elements, and the memory <c>[mid, end)</c> is reserved for future
+/// elements (<c>beg <= mid <= end</c>). For example, in
+/// <c>std::vector<> v</c>:
+///
+/// \code
+/// beg = &v[0];
+/// end = beg + v.capacity() * sizeof(v[0]);
+/// mid = beg + v.size() * sizeof(v[0]);
+/// \endcode
+///
+/// This annotation tells the Sanitizer tool about the current state of the
+/// container so that the tool can report errors when memory from
+/// <c>[mid, end)</c> is accessed. Insert this annotation into methods like
+/// <c>push_back()</c> or <c>pop_back()</c>. Supply the old and new values of
+/// <c>mid</c>(<c><i>old_mid</i></c> and <c><i>new_mid</i></c>). In the initial
+/// state <c>mid == end</c>, so that should be the final state when the
+/// container is destroyed or when the container reallocates the storage.
+///
+/// For ASan, <c><i>beg</i></c> no longer needs to be 8-aligned,
+/// first and last granule may be shared with other objects
+/// and therefore the function can be used for any allocator.
+///
+/// The following example shows how to use the function:
+///
+/// \code
+/// int32_t x[3]; // 12 bytes
+/// char *beg = (char*)&x[0];
+/// char *end = beg + 12;
+/// __sanitizer_annotate_contiguous_container(beg, end, beg, end);
+/// \endcode
+///
+/// \note Use this function with caution and do not use for anything other
+/// than vector-like classes.
+/// \note Unaligned <c><i>beg</i></c> or <c><i>end</i></c> may miss bugs in
+/// these granules.
+///
+/// \param __beg Beginning of memory region.
+/// \param __end End of memory region.
+/// \param __old_mid Old middle of memory region.
+/// \param __new_mid New middle of memory region.
+#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+__attribute__((__internal_linkage__)) inline void SANITIZER_CDECL
+__sanitizer_annotate_contiguous_container(const void *__beg, const void *__end,
+ const void *__old_mid,
+ const void *__new_mid) {}
+#else
+void SANITIZER_CDECL __sanitizer_annotate_contiguous_container(
+ const void *__beg, const void *__end, const void *__old_mid,
+ const void *__new_mid);
+#endif
+
+/// Similar to <c>__sanitizer_annotate_contiguous_container</c>.
+///
+/// Annotates the current state of a contiguous container memory,
+/// such as <c>std::deque</c>'s single chunk, when the boundries are moved.
+///
+/// A contiguous chunk is a chunk that keeps all of its elements
+/// in a contiguous region of memory. The container owns the region of memory
+/// <c>[storage_beg, storage_end)</c>; the memory <c>[container_beg,
+/// container_end)</c> is used to store the current elements, and the memory
+/// <c>[storage_beg, container_beg), [container_end, storage_end)</c> is
+/// reserved for future elements (<c>storage_beg <= container_beg <=
+/// container_end <= storage_end</c>). For example, in <c> std::deque </c>:
+/// - chunk with a frist deques element will have container_beg equal to address
+/// of the first element.
+/// - in every next chunk with elements, true is <c> container_beg ==
+/// storage_beg </c>.
+///
+/// Argument requirements:
+/// During unpoisoning memory of empty container (before first element is
+/// added):
+/// - old_container_beg_p == old_container_end_p
+/// During poisoning after last element was removed:
+/// - new_container_beg_p == new_container_end_p
+/// \param __storage_beg Beginning of memory region.
+/// \param __storage_end End of memory region.
+/// \param __old_container_beg Old beginning of used region.
+/// \param __old_container_end End of used region.
+/// \param __new_container_beg New beginning of used region.
+/// \param __new_container_end New end of used region.
+#ifdef __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+__attribute__((__internal_linkage__)) inline void
+ SANITIZER_CDECL __sanitizer_annotate_double_ended_contiguous_container(
+ const void *__storage_beg, const void *__storage_end,
+ const void *__old_container_beg, const void *__old_container_end,
+ const void *__new_container_beg, const void *__new_container_end) {}
+#else
+void SANITIZER_CDECL __sanitizer_annotate_double_ended_contiguous_container(
+ const void *__storage_beg, const void *__storage_end,
+ const void *__old_container_beg, const void *__old_container_end,
+ const void *__new_container_beg, const void *__new_container_end);
+#endif
+
+/// Copies memory annotations from a source storage region to a destination
+/// storage region. After the operation, the destination region has the same
+/// memory annotations as the source region, as long as sanitizer limitations
+/// allow it (more bytes may be unpoisoned than in the source region, resulting
+/// in more false negatives, but never false positives). If the source and
+/// destination regions overlap, only the minimal required changes are made to
+/// preserve the correct annotations. Old storage bytes that are not in the new
+/// storage should have the same annotations, as long as sanitizer limitations
+/// allow it.
+///
+/// This function is primarily designed to be used when moving trivially
+/// relocatable objects that may have poisoned memory, making direct copying
+/// problematic under sanitizer. However, this function does not move memory
+/// content itself, only annotations.
+///
+/// A contiguous container is a container that keeps all of its elements in a
+/// contiguous region of memory. The container ...
[truncated]
|
ldionne
reviewed
Nov 20, 2025
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
This makes sense to me, but I think it might be worth having a more general discussion about whether the compiler-rt headers are intended to be considered "system headers" from this point on, which means that you'll need this level of scrutiny on everything. This will also mean that you need to be careful about what other headers you include.
All in all, I'm mostly neutral about this. I think it makes sense for other user code to include this, but for libc++ I'm still a bit on the fence since it makes us vulnerable to changes in the sanitizers.
Comment on lines
+12
to
+13
| #ifndef SANITIZER_CONTAINER_OVERFLOW_DEFS_H | ||
| #define SANITIZER_CONTAINER_OVERFLOW_DEFS_H |
Member
There was a problem hiding this comment.
Suggested change
| #ifndef SANITIZER_CONTAINER_OVERFLOW_DEFS_H | |
| #define SANITIZER_CONTAINER_OVERFLOW_DEFS_H | |
| #ifndef _SANITIZER_CONTAINER_OVERFLOW_DEFS_H | |
| #define _SANITIZER_CONTAINER_OVERFLOW_DEFS_H |
| // Windows allows a user to set their default calling convention, but we always | ||
| // use __cdecl | ||
| #ifdef _WIN32 | ||
| #define SANITIZER_CDECL __cdecl |
Member
There was a problem hiding this comment.
Suggested change
| #define SANITIZER_CDECL __cdecl | |
| #define _SANITIZER_CDECL __cdecl |
This basically needs to be done throughout, for all identifiers. The preprocessor does not differentiate between different "kinds" of tokens.
Contributor
Author
|
#168955 this is a much nicer solution, I'd far rather go with it than open this can of worms. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the ASan container overflow function declarations live in the common_interface_defs.h file. In order to allow libcxx to directly use these declarations, rather than forward declaring them, we need to prefix all identifiers with '__'. To minimize the impact of that we can break the effected functions out into a separate file, and alter only these.
This patch does the above. It follows on from: #163468.