-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[scudo] Add static vector functionality. #98986
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
Conversation
…e option handler. Initially, the scudo allocator would return an error if the user attempted to set the cache capacity (i.e. the number of possible entries in the cache) above the maximum cache capacity. Now the allocator will resort to using the maximum cache capacity in this event. An error will still be returned if the user attempts to set the number of entries to a negative value.
The scudo vector implementation maintains static local data before switching to dynamically allocated data as the array size grows. Users of the vector can now specify the size of the static local data through the vector template (the default size is currently set to 10 * sizeof(T) where T is the vector element type).
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Joshua Baehring (JoshuaMBa) ChangesThe scudo vector implementation maintains static local data before Full diff: https://github.com/llvm/llvm-project/pull/98986.diff 1 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/vector.h b/compiler-rt/lib/scudo/standalone/vector.h
index ca10cc281d770..d972ef068bee5 100644
--- a/compiler-rt/lib/scudo/standalone/vector.h
+++ b/compiler-rt/lib/scudo/standalone/vector.h
@@ -9,6 +9,7 @@
#ifndef SCUDO_VECTOR_H_
#define SCUDO_VECTOR_H_
+#include "common.h"
#include "mem_map.h"
#include <string.h>
@@ -21,7 +22,7 @@ namespace scudo {
// implementation supports only POD types.
//
// NOTE: This class is not meant to be used directly, use Vector<T> instead.
-template <typename T> class VectorNoCtor {
+template <typename T, size_t StaticCapacity> class VectorNoCtor {
public:
T &operator[](uptr I) {
DCHECK_LT(I, Size);
@@ -116,18 +117,19 @@ template <typename T> class VectorNoCtor {
uptr CapacityBytes = 0;
uptr Size = 0;
- T LocalData[256 / sizeof(T)] = {};
+ T LocalData[StaticCapacity] = {};
MemMapT ExternalBuffer;
};
-template <typename T> class Vector : public VectorNoCtor<T> {
+template <typename T, size_t StaticCapacity = 10>
+class Vector : public VectorNoCtor<T, Max<size_t>(10, StaticCapacity)> {
public:
- constexpr Vector() { VectorNoCtor<T>::init(); }
+ constexpr Vector() { VectorNoCtor<T, StaticCapacity>::init(); }
explicit Vector(uptr Count) {
- VectorNoCtor<T>::init(Count);
+ VectorNoCtor<T, StaticCapacity>::init(Count);
this->resize(Count);
}
- ~Vector() { VectorNoCtor<T>::destroy(); }
+ ~Vector() { VectorNoCtor<T, StaticCapacity>::destroy(); }
// Disallow copies and moves.
Vector(const Vector &) = delete;
Vector &operator=(const Vector &) = delete;
@@ -138,3 +140,4 @@ template <typename T> class Vector : public VectorNoCtor<T> {
} // namespace scudo
#endif // SCUDO_VECTOR_H_
+
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally, I though we might want to allow a zero length capacity. But thinking about it a bit more, I don't think we should since that would likely be a bug. If we ever decide to allow it, we can change the static assert.
Chia-hung, make sure the usage of bytes instead of items is okay with you.
Left a comment about the default size. About the zero length, I told Jushua that he can either do default to 1 if it's 0 (like what llvm::SmallVector does) or static_assert that. Based on your suggestion, I think it's better to just disallow it. |
4e72ed8
to
d8f1007
Compare
Summary: The scudo vector implementation maintains static local data before switching to dynamically allocated data as the array size grows. Users of the vector must now specify the size of the static local data through the vector template (the default size has been removed). If 0 is specified for the size of the static local data, an assertion will be triggered. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250929
The scudo vector implementation maintains static local data before
switching to dynamically allocated data as the array size grows.
Users of the vector must now specify the size of the static local data
through the vector template (the default size has been removed).
If 0 is specified for the size of the static local data, an assertion will
be triggered.