-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[scudo] Refactor allocator config to support optional flags #81805
[scudo] Refactor allocator config to support optional flags #81805
Conversation
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (ChiaHungDuan) ChangesInstead of explicitly disabling a feature by declaring the variable and set it to false, this change supports the optional flags. I.e., you can skip certain flags if you are not using it. This optional feature supports both forms,
On the other hand, to access the flags will be through one of the wrappers, BaseConfig/PrimaryConfig/SecondaryConfig/CacheConfig (CacheConfig is embedded in SecondaryConfig). These wrappers have the getters to access the value and the type. When adding a new feature, we need to add it to In addition, also remove the need of Patch is 45.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81805.diff 13 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
index 60092005cc33bb..6fb4e88de3155f 100644
--- a/compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -58,6 +58,7 @@ endif()
set(SCUDO_HEADERS
allocator_common.h
allocator_config.h
+ allocator_config_wrapper.h
atomic_helpers.h
bytemap.h
checksum.h
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.def b/compiler-rt/lib/scudo/standalone/allocator_config.def
new file mode 100644
index 00000000000000..ddde71d2463f78
--- /dev/null
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.def
@@ -0,0 +1,123 @@
+//===-- allocator_config.def ------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines all the flags and types supported in Scudo.
+
+#ifndef BASE_REQUIRED_TEMPLATE_TYPE
+#define BASE_REQUIRED_TEMPLATE_TYPE(...)
+#endif
+#ifndef BASE_OPTIONAL
+#define BASE_OPTIONAL(...)
+#endif
+#ifndef PRIMARY_REQUIRED_TYPE
+#define PRIMARY_REQUIRED_TYPE(...)
+#endif
+#ifndef PRIMARY_REQUIRED
+#define PRIMARY_REQUIRED(...)
+#endif
+#ifndef PRIMARY_OPTIONAL
+#define PRIMARY_OPTIONAL(...)
+#endif
+#ifndef PRIMARY_OPTIONAL_TYPE
+#define PRIMARY_OPTIONAL_TYPE(...)
+#endif
+#ifndef SECONDARY_REQUIRED_TEMPLATE_TYPE
+#define SECONDARY_REQUIRED_TEMPLATE_TYPE(...)
+#endif
+#ifndef SECONDARY_CACHE_OPTIONAL
+#define SECONDARY_CACHE_OPTIONAL(...)
+#endif
+
+// BASE_REQUIRED_TEMPLATE_TYPE(NAME)
+//
+// Thread-Specific Data Registry used, shared or exclusive.
+BASE_REQUIRED_TEMPLATE_TYPE(TSDRegistryT)
+
+// Defines the type of Primary allocator to use.
+BASE_REQUIRED_TEMPLATE_TYPE(PrimaryT)
+
+// Defines the type of Secondary allocator to use.
+BASE_REQUIRED_TEMPLATE_TYPE(SecondaryT)
+
+// BASE_OPTIONAL(TYPE, NAME, DEFAULT)
+//
+// Indicates possible support for Memory Tagging.
+BASE_OPTIONAL(const bool, MaySupportMemoryTagging, false)
+
+// PRIMARY_REQUIRED_TYPE(NAME)
+//
+// SizeClassMap to use with the Primary.
+PRIMARY_REQUIRED_TYPE(SizeClassMap)
+
+// Defines the type and scale of a compact pointer. A compact pointer can
+// be understood as the offset of a pointer within the region it belongs
+// to, in increments of a power-of-2 scale. See `CompactPtrScale` also.
+PRIMARY_REQUIRED_TYPE(CompactPtrT)
+
+// PRIMARY_REQUIRED(TYPE, NAME)
+//
+// The scale of a compact pointer. E.g., Ptr = Base + (CompactPtr << Scale).
+PRIMARY_REQUIRED(const uptr, CompactPtrScale)
+
+// Log2 of the size of a size class region, as used by the Primary.
+PRIMARY_REQUIRED(const uptr, RegionSizeLog)
+
+// Log2 of the size of block group, as used by the Primary. Each group
+// contains a range of memory addresses, blocks in the range will belong
+// to the same group. In general, single region may have 1 or 2MB group
+// size. Multiple regions will have the group size equal to the region
+// size because the region size is usually smaller than 1 MB.
+// Smaller value gives fine-grained control of memory usage but the
+// trade-off is that it may take longer time of deallocation.
+PRIMARY_REQUIRED(const uptr, GroupSizeLog)
+
+// Call map for user memory with at least this size. Only used with primary64.
+PRIMARY_REQUIRED(const uptr, MapSizeIncrement)
+
+// Defines the minimal & maximal release interval that can be set.
+PRIMARY_REQUIRED(const s32, MinReleaseToOsIntervalMs)
+PRIMARY_REQUIRED(const s32, MaxReleaseToOsIntervalMs)
+
+// PRIMARY_OPTIONAL(TYPE, NAME, DEFAULT)
+//
+// Indicates support for offsetting the start of a region by a random number of
+// pages. Only used with primary64.
+PRIMARY_OPTIONAL(const bool, EnableRandomOffset, false)
+
+// PRIMARY_OPTIONAL_TYPE(NAME, DEFAULT)
+//
+// Use condition variable to shorten the waiting time of refillment of
+// freelist. Note that this depends on the implementation of condition
+// variable on each platform and the performance may vary so that it doesn not
+// guarantee a performance benefit.
+PRIMARY_OPTIONAL_TYPE(ConditionVariableT, ConditionVariableDummy)
+
+// SECONDARY_REQUIRED_TEMPLATE_TYPE(NAME)
+//
+// Defines the type of Secondary Cache to use.
+SECONDARY_REQUIRED_TEMPLATE_TYPE(CacheT)
+
+// SECONDARY_CACHE_OPTIONAL(TYPE, NAME, DEFAULT)
+//
+// Defines the type of cache used by the Secondary. Some additional
+// configuration entries can be necessary depending on the Cache.
+SECONDARY_CACHE_OPTIONAL(const u32, EntriesArraySize, 0)
+SECONDARY_CACHE_OPTIONAL(const u32, QuarantineSize, 0)
+SECONDARY_CACHE_OPTIONAL(const u32, DefaultMaxEntriesCount, 0)
+SECONDARY_CACHE_OPTIONAL(const u32, DefaultMaxEntrySize, 0)
+SECONDARY_CACHE_OPTIONAL(const s32, MinReleaseToOsIntervalMs, INT32_MIN)
+SECONDARY_CACHE_OPTIONAL(const s32, MaxReleaseToOsIntervalMs, INT32_MAX)
+
+#undef SECONDARY_CACHE_OPTIONAL
+#undef SECONDARY_REQUIRED_TEMPLATE_TYPE
+#undef PRIMARY_OPTIONAL_TYPE
+#undef PRIMARY_OPTIONAL
+#undef PRIMARY_REQUIRED
+#undef PRIMARY_REQUIRED_TYPE
+#undef BASE_OPTIONAL
+#undef BASE_REQUIRED_TEMPLATE_TYPE
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h
index 3c6aa3acb0e45b..1e0cf1015ba67e 100644
--- a/compiler-rt/lib/scudo/standalone/allocator_config.h
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.h
@@ -38,80 +38,10 @@
namespace scudo {
-// The combined allocator uses a structure as a template argument that
-// specifies the configuration options for the various subcomponents of the
-// allocator.
-//
-// struct ExampleConfig {
-// // Indicates possible support for Memory Tagging.
-// static const bool MaySupportMemoryTagging = false;
-//
-// // Thread-Specific Data Registry used, shared or exclusive.
-// template <class A> using TSDRegistryT = TSDRegistrySharedT<A, 8U, 4U>;
-//
-// struct Primary {
-// // SizeClassMap to use with the Primary.
-// using SizeClassMap = DefaultSizeClassMap;
-//
-// // Log2 of the size of a size class region, as used by the Primary.
-// static const uptr RegionSizeLog = 30U;
-//
-// // Log2 of the size of block group, as used by the Primary. Each group
-// // contains a range of memory addresses, blocks in the range will belong
-// // to the same group. In general, single region may have 1 or 2MB group
-// // size. Multiple regions will have the group size equal to the region
-// // size because the region size is usually smaller than 1 MB.
-// // Smaller value gives fine-grained control of memory usage but the
-// // trade-off is that it may take longer time of deallocation.
-// static const uptr GroupSizeLog = 20U;
-//
-// // Defines the type and scale of a compact pointer. A compact pointer can
-// // be understood as the offset of a pointer within the region it belongs
-// // to, in increments of a power-of-2 scale.
-// // eg: Ptr = Base + (CompactPtr << Scale).
-// typedef u32 CompactPtrT;
-// static const uptr CompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG;
-//
-// // Indicates support for offsetting the start of a region by
-// // a random number of pages. Only used with primary64.
-// static const bool EnableRandomOffset = true;
-//
-// // Call map for user memory with at least this size. Only used with
-// // primary64.
-// static const uptr MapSizeIncrement = 1UL << 18;
-//
-// // Defines the minimal & maximal release interval that can be set.
-// static const s32 MinReleaseToOsIntervalMs = INT32_MIN;
-// static const s32 MaxReleaseToOsIntervalMs = INT32_MAX;
-//
-// // Use condition variable to shorten the waiting time of refillment of
-// // freelist. Note that this depends on the implementation of condition
-// // variable on each platform and the performance may vary so that it
-// // doesn't guarantee a performance benefit.
-// // Note that both variables have to be defined to enable it.
-// static const bool UseConditionVariable = true;
-// using ConditionVariableT = ConditionVariableLinux;
-// };
-// // Defines the type of Primary allocator to use.
-// template <typename Config> using PrimaryT = SizeClassAllocator64<Config>;
-//
-// // Defines the type of cache used by the Secondary. Some additional
-// // configuration entries can be necessary depending on the Cache.
-// struct Secondary {
-// struct Cache {
-// static const u32 EntriesArraySize = 32U;
-// static const u32 QuarantineSize = 0U;
-// static const u32 DefaultMaxEntriesCount = 32U;
-// static const uptr DefaultMaxEntrySize = 1UL << 19;
-// static const s32 MinReleaseToOsIntervalMs = INT32_MIN;
-// static const s32 MaxReleaseToOsIntervalMs = INT32_MAX;
-// };
-// // Defines the type of Secondary Cache to use.
-// template <typename Config> using CacheT = MapAllocatorCache<Config>;
-// };
-// // Defines the type of Secondary allocator to use.
-// template <typename Config> using SecondaryT = MapAllocator<Config>;
-// };
+// Scudo uses a structure as a template argument that specifies the
+// configuration options for the various subcomponents of the allocator. See the
+// following configs as examples and check `allocator_config.def` for all the
+// available options.
#ifndef SCUDO_USE_CUSTOM_CONFIG
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config_wrapper.h b/compiler-rt/lib/scudo/standalone/allocator_config_wrapper.h
new file mode 100644
index 00000000000000..c946531dc4e02f
--- /dev/null
+++ b/compiler-rt/lib/scudo/standalone/allocator_config_wrapper.h
@@ -0,0 +1,134 @@
+//===-- allocator_config_wrapper.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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SCUDO_ALLOCATOR_CONFIG_WRAPPER_H_
+#define SCUDO_ALLOCATOR_CONFIG_WRAPPER_H_
+
+#include "condition_variable.h"
+#include "internal_defs.h"
+#include "secondary.h"
+
+namespace {
+
+template <typename T> struct removeConst {
+ using type = T;
+};
+template <typename T> struct removeConst<const T> {
+ using type = T;
+};
+
+} // namespace
+
+namespace scudo {
+
+#define OPTIONAL_TEMPLATE(TYPE, NAME, DEFAULT) \
+ template <typename Config, typename = TYPE> struct NAME##State { \
+ static constexpr removeConst<TYPE>::type getValue() { return DEFAULT; } \
+ }; \
+ template <typename Config> \
+ struct NAME##State<Config, decltype(Config::NAME)> { \
+ static constexpr removeConst<TYPE>::type getValue() { \
+ return Config::NAME; \
+ } \
+ };
+
+#define OPTIONAL_TYPE_TEMPLATE(NAME, DEFAULT) \
+ template <typename Config, typename Void = Config> struct NAME##Type { \
+ static constexpr bool enabled() { return false; } \
+ using NAME = DEFAULT; \
+ }; \
+ template <typename Config> \
+ struct NAME##Type<Config, typename Config::NAME> { \
+ static constexpr bool enabled() { return true; } \
+ using NAME = typename Config::NAME; \
+ };
+
+template <typename AllocatorConfig> struct BaseConfig {
+#define BASE_REQUIRED_TEMPLATE_TYPE(NAME) \
+ template <typename T> using NAME = typename AllocatorConfig::template NAME<T>;
+
+#define BASE_OPTIONAL(TYPE, NAME, DEFAULT) \
+ OPTIONAL_TEMPLATE(TYPE, NAME, DEFAULT) \
+ static constexpr removeConst<TYPE>::type get##NAME() { \
+ return NAME##State<AllocatorConfig>::getValue(); \
+ }
+
+#include "allocator_config.def"
+}; // BaseConfig
+
+template <typename AllocatorConfig> struct PrimaryConfig {
+ // TODO: Pass this flag through template argument to remove this hard-coded
+ // function.
+ static constexpr bool getMaySupportMemoryTagging() {
+ return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
+ }
+
+#define PRIMARY_REQUIRED_TYPE(NAME) \
+ using NAME = typename AllocatorConfig::Primary::NAME;
+
+#define PRIMARY_REQUIRED(TYPE, NAME) \
+ static constexpr removeConst<TYPE>::type get##NAME() { \
+ return AllocatorConfig::Primary::NAME; \
+ }
+
+#define PRIMARY_OPTIONAL(TYPE, NAME, DEFAULT) \
+ OPTIONAL_TEMPLATE(TYPE, NAME, DEFAULT) \
+ static constexpr removeConst<TYPE>::type get##NAME() { \
+ return NAME##State<typename AllocatorConfig::Primary>::getValue(); \
+ }
+
+#define PRIMARY_OPTIONAL_TYPE(NAME, DEFAULT) \
+ OPTIONAL_TYPE_TEMPLATE(NAME, DEFAULT) \
+ static constexpr bool has##NAME() { \
+ return NAME##Type<typename AllocatorConfig::Primary>::enabled(); \
+ } \
+ using NAME = typename NAME##Type<typename AllocatorConfig::Primary>::NAME;
+
+#include "allocator_config.def"
+
+}; // PrimaryConfig
+
+template <typename AllocatorConfig> struct SecondaryConfig {
+ // TODO: Pass this flag through template argument to remove this hard-coded
+ // function.
+ static constexpr bool getMaySupportMemoryTagging() {
+ return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
+ }
+
+#define SECONDARY_REQUIRED_TEMPLATE_TYPE(NAME) \
+ template <typename T> \
+ using NAME = typename AllocatorConfig::Secondary::template NAME<T>;
+#include "allocator_config.def"
+
+ struct CacheConfig {
+ // TODO: Pass this flag through template argument to remove this hard-coded
+ // function.
+ static constexpr bool getMaySupportMemoryTagging() {
+ return BaseConfig<AllocatorConfig>::getMaySupportMemoryTagging();
+ }
+
+#define SECONDARY_CACHE_OPTIONAL(TYPE, NAME, DEFAULT) \
+ template <typename Config, typename = TYPE> struct NAME##State { \
+ static constexpr removeConst<TYPE>::type getValue() { return DEFAULT; } \
+ }; \
+ template <typename Config> \
+ struct NAME##State<Config, decltype(Config::Cache)> { \
+ static constexpr removeConst<TYPE>::type getValue() { \
+ return Config::Cache::NAME; \
+ } \
+ }; \
+ static constexpr removeConst<TYPE>::type get##NAME() { \
+ return NAME##State<typename AllocatorConfig::Secondary>::getValue(); \
+ }
+#include "allocator_config.def"
+ };
+}; // SecondaryConfig
+
+} // namespace scudo
+
+#endif // SCUDO_ALLOCATOR_CONFIG_WRAPPER_H_
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 80774073522a72..3d4023a484073c 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -9,6 +9,7 @@
#ifndef SCUDO_COMBINED_H_
#define SCUDO_COMBINED_H_
+#include "allocator_config_wrapper.h"
#include "chunk.h"
#include "common.h"
#include "flags.h"
@@ -46,11 +47,14 @@ namespace scudo {
template <class Config, void (*PostInitCallback)(void) = EmptyCallback>
class Allocator {
public:
- using PrimaryT = typename Config::template PrimaryT<Config>;
- using SecondaryT = typename Config::template SecondaryT<Config>;
+ using AllocatorConfig = BaseConfig<Config>;
+ using PrimaryT =
+ typename AllocatorConfig::template PrimaryT<PrimaryConfig<Config>>;
+ using SecondaryT =
+ typename AllocatorConfig::template SecondaryT<SecondaryConfig<Config>>;
using CacheT = typename PrimaryT::CacheT;
typedef Allocator<Config, PostInitCallback> ThisT;
- typedef typename Config::template TSDRegistryT<ThisT> TSDRegistryT;
+ typedef typename AllocatorConfig::template TSDRegistryT<ThisT> TSDRegistryT;
void callPostInitCallback() {
pthread_once(&PostInitNonce, PostInitCallback);
@@ -71,7 +75,7 @@ class Allocator {
Header.State = Chunk::State::Available;
Chunk::storeHeader(Allocator.Cookie, Ptr, &Header);
- if (allocatorSupportsMemoryTagging<Config>())
+ if (allocatorSupportsMemoryTagging<AllocatorConfig>())
Ptr = untagPointer(Ptr);
void *BlockBegin = Allocator::getBlockBegin(Ptr, &Header);
Cache.deallocate(Header.ClassId, BlockBegin);
@@ -98,7 +102,8 @@ class Allocator {
// Reset tag to 0 as this chunk may have been previously used for a tagged
// user allocation.
- if (UNLIKELY(useMemoryTagging<Config>(Allocator.Primary.Options.load())))
+ if (UNLIKELY(useMemoryTagging<AllocatorConfig>(
+ Allocator.Primary.Options.load())))
storeTags(reinterpret_cast<uptr>(Ptr),
reinterpret_cast<uptr>(Ptr) + sizeof(QuarantineBatch));
@@ -158,7 +163,7 @@ class Allocator {
Primary.Options.set(OptionBit::DeallocTypeMismatch);
if (getFlags()->delete_size_mismatch)
Primary.Options.set(OptionBit::DeleteSizeMismatch);
- if (allocatorSupportsMemoryTagging<Config>() &&
+ if (allocatorSupportsMemoryTagging<AllocatorConfig>() &&
systemSupportsMemoryTagging())
Primary.Options.set(OptionBit::UseMemoryTagging);
@@ -261,7 +266,7 @@ class Allocator {
void drainCaches() { TSDRegistry.drainCaches(this); }
ALWAYS_INLINE void *getHeaderTaggedPointer(void *Ptr) {
- if (!allocatorSupportsMemoryTagging<Config>())
+ if (!allocatorSupportsMemoryTagging<AllocatorConfig>())
return Ptr;
auto UntaggedPtr = untagPointer(Ptr);
if (UntaggedPtr != Ptr)
@@ -273,7 +278,7 @@ class Allocator {
}
ALWAYS_INLINE uptr addHeaderTag(uptr Ptr) {
- if (!allocatorSupportsMemoryTagging<Config>())
+ if (!allocatorSupportsMemoryTagging<AllocatorConfig>())
return Ptr;
return addFixedTag(Ptr, 2);
}
@@ -410,7 +415,7 @@ class Allocator {
//
// When memory tagging is enabled, zeroing the contents is done as part of
// setting the tag.
- if (UNLIKELY(useMemoryTagging<Config>(Options))) {
+ if (UNLIKELY(useMemoryTagging<AllocatorConfig>(Options))) {
uptr PrevUserPtr;
Chunk::UnpackedHeader Header;
const uptr BlockSize = PrimaryT::getSizeByClassId(ClassId);
@@ -492,7 +497,7 @@ class Allocator {
} else {
Block = addHeaderTag(Block);
Ptr = addHeaderTag(Ptr);
- if (UNLIKELY(useMemoryTagging<Config>(Options))) {
+ if (UNLIKELY(useMemoryTagging<AllocatorConfig>(Options))) {
storeTags(reinterpret_cast<uptr>(Block), reinterpret_cast<uptr>(Ptr));
storeSecondaryAllocationStackMaybe(Options, Ptr, Size);
}
@@ -652,7 +657,7 @@ class Allocator {
(reinterpret_cast<uptr>(OldTaggedPtr) + NewSize)) &
Chunk::SizeOrUnusedBytesMask;
Chunk::storeHeader(Cookie, OldPtr, &Header);
- i...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There seems to be an update in the style. The clang-format on my machine reports this style as an error. I'll fix this later after rebase |
2a26214
to
cf2b0c3
Compare
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.
Is it worth removing the MaySupportMemoryTagging = false for the FuchsiaConfig?
// Log2 of the size of a size class region, as used by the Primary. | ||
PRIMARY_REQUIRED(const uptr, RegionSizeLog) | ||
|
||
// Log2 of the size of block group, as used by the Primary. Each group |
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.
size of block group -> size of a block group
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.
All the comment has been revised
// size. Multiple regions will have the group size equal to the region | ||
// size because the region size is usually smaller than 1 MB. | ||
// Smaller value gives fine-grained control of memory usage but the | ||
// trade-off is that it may take longer time of deallocation. |
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.
I know you are carrying the comment from the old code, but it's a bit confusing.
Would it be better to be explicit about the relationship between GroupSizeLog and RegionSizeLog? I don't know if you need to mention the size explicitly, but maybe say something about how groups much be smaller or the same size as the RegionSizeLog. Also, if there is a minimum for the GroupSizeLog value, that should also be mentioned.
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.
You're right. I revised the comment here. Let me know if it's more clear
// | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This file defines all the flags and types supported in Scudo. |
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.
It would be good mention these as explicitly configuration flags.
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.
Done
My preference is to let each project manage their configuration (like removing unused flags). In addition, the configs in upstream are supposed to be deprecated later, we can review and clean them up at that moment |
c28125d
to
623e19c
Compare
// Log2 of the size of a size class region, as used by the Primary. | ||
PRIMARY_REQUIRED(const uptr, RegionSizeLog) | ||
|
||
// Conceptually, a region will be divided into groups based on the address range |
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.
I think:
rang -> range.
It looks like that's supposed to be the end of the sentence.
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.
Done
PRIMARY_REQUIRED(const uptr, RegionSizeLog) | ||
|
||
// Conceptually, a region will be divided into groups based on the address range | ||
// Each allocation consumes blocks in the same group until the exhaustion then |
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.
group until the exhaustion then -> group until exhaustion then
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.
Done
// | ||
// Use condition variable to shorten the waiting time of refillment of | ||
// freelist. Note that this depends on the implementation of condition | ||
// variable on each platform and the performance may vary so that it doesn not |
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.
doesn -> does
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.
Done
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.
LGTM.
Instead of explicitly disabling a feature by declaring the variable and set it to false, this change supports the optional flags. I.e., you can skip certain flags if you are not using it. This optional feature supports both forms, 1. Value: A parameter for a feature. E.g., EnableRandomOffset 2. Type: A C++ type implementing a feature. E.g., ConditionVariableT On the other hand, to access the flags will be through one of the wrappers, BaseConfig/PrimaryConfig/SecondaryConfig/CacheConfig (CacheConfig is embedded in SecondaryConfig). These wrappers have the getters to access the value and the type. When adding a new feature, we need to add it to `allocator_config.def` and mark the new variable with either *_REQUIRED_* or *_OPTIONAL_* macro so that the accessor will be generated properly. In addition, also remove the need of `UseConditionVariable` to flip on/off of condition variable. Now we only need to define the type of condition variable.
1983fd4
to
60687e7
Compare
Rebase only |
Instead of explicitly disabling a feature by declaring the variable and set it to false, this change supports the optional flags. I.e., you can skip certain flags if you are not using it.
This optional feature supports both forms,
On the other hand, to access the flags will be through one of the wrappers, BaseConfig/PrimaryConfig/SecondaryConfig/CacheConfig (CacheConfig is embedded in SecondaryConfig). These wrappers have the getters to access the value and the type. When adding a new feature, we need to add it to
allocator_config.def
and mark the new variable with either REQUIRED or OPTIONAL macro so that the accessor will be generated properly.In addition, also remove the need of
UseConditionVariable
to flip on/off of condition variable. Now we only need to define the type of condition variable.