short string annotations cleaning traits #110941
Draft
+441
−0
Conversation
This file contains 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
This PR adds a `__sanitizer_move_contiguous_container_annotations` function, which moves annotations from one memory area to another.
Old area is unpoisoned at the end. New area is annotated in the same way as the old region at the beginning (within limitations of ASan).
```cpp
void __sanitizer_move_contiguous_container_annotations(
const void *old_storage_beg_p, const void *old_storage_end_p,
const void *new_storage_beg_p, const void *new_storage_end_p) {
```
This function aims to help with short string annotations and similar container annotations.
Right now we change trait types of `std::basic_string` when compiling with ASan.
https://github.com/llvm/llvm-project/blob/87f3407856e61a73798af4e41b28bc33b5bf4ce6/libcxx/include/string#L738-L751
The goal is to not change `__trivially_relocatable` when compiling with ASan.
If this function is accpeted and upstreamed, the next step is creating function like `__memcpy_with_asan` moving memory with ASan.
And then using this function instead of `__builtin__memcpy` while moving trivially relocatable objects.
NOTICE: I did not test it yet, so it's probably not compiling, but I don't expect big changes. PR is WIP until I test it.
---
I'm thinking if there is a good way to address fact that in a container the new buffer is usually bigger than the previous one.
We may add two more arguments to the functions to address it (the beginning and the end of the whole buffer.
Another potential change is removing `new_storage_end_p` as it's redundant, because we require the same size.
Implemented correct container annotations copying function (non-overlapping containers only) Refactored the initial PoC to a fully functional implementation for copying container annotations without moving/copying the memory content itself. This function currently supports cases where the source and destination containers do not overlap. Handling overlapping containers would add complexity. The function is designed to work irrespective of whether the buffers are granule-aligned or the distance between them is granule-aligned. However, such scenarios may have an impact on performance. A Test case has been included to verify the correctness of the implementation. Removed unpoisoning oryginal buffer at the end, users can do it by themselves if they want to.
Added helper functions, removed unnecessary branches, simplified code.
Adds support for overlapping containers. Adds test case for that.
- VPrintf level changed to 3. - unique_ptr used in the test example.
For safety. Fix code formatting.
…rLastGranuleAnnotation
You can test this locally with the following command:git-clang-format --diff b0fbfbb6f33f20ef6b72ab003740f8f96efedd7e 017632a6c2d14b7fbf95758ae618d7b3b74963b5 --extensions inc,cpp,h -- compiler-rt/test/asan/TestCases/copy_container_annotations.cpp compiler-rt/include/sanitizer/common_interface_defs.h compiler-rt/lib/asan/asan_errors.cpp compiler-rt/lib/asan/asan_errors.h compiler-rt/lib/asan/asan_poisoning.cpp compiler-rt/lib/asan/asan_report.cpp compiler-rt/lib/asan/asan_report.h compiler-rt/lib/sanitizer_common/sanitizer_common_interface.inc compiler-rt/lib/sanitizer_common/sanitizer_interface_internal.hView the diff from clang-format here.diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index 0fd3a1428c..762670632f 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -728,9 +728,9 @@ void __sanitizer_copy_contiguous_container_annotations(const void *src_beg_p,
bool copy_in_reversed_order = src_beg < dst_beg && dst_beg <= src_end_up;
if (src_beg % granularity != dst_beg % granularity ||
RoundDownTo(dst_end - 1, granularity) <= dst_beg) {
- if (copy_in_reversed_order)
+ if (copy_in_reversed_order)
SlowReversedCopyContainerAnnotations(src_beg, src_end, dst_beg, dst_end);
- else
+ else
SlowCopyContainerAnnotations(src_beg, src_end, dst_beg, dst_end);
return;
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Review #91702