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

[scudo] Change CompactPtrT and CompactPtrScale to optional #90797

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

ChiaHungDuan
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented May 1, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (ChiaHungDuan)

Changes

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

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/allocator_config.def (+8-8)
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.def b/compiler-rt/lib/scudo/standalone/allocator_config.def
index dcd130ac449a0a..ce37b1cfaedccc 100644
--- a/compiler-rt/lib/scudo/standalone/allocator_config.def
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.def
@@ -56,16 +56,8 @@ BASE_OPTIONAL(const bool, MaySupportMemoryTagging, false)
 // 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)
 
@@ -86,6 +78,9 @@ PRIMARY_REQUIRED(const s32, MaxReleaseToOsIntervalMs)
 
 // PRIMARY_OPTIONAL(TYPE, NAME, DEFAULT)
 //
+// The scale of a compact pointer. E.g., Ptr = Base + (CompactPtr << Scale).
+PRIMARY_OPTIONAL(const uptr, CompactPtrScale, SCUDO_MIN_ALIGNMENT_LOG)
+
 // Indicates support for offsetting the start of a region by a random number of
 // pages. This is only used if `EnableContiguousRegions` is enabled.
 PRIMARY_OPTIONAL(const bool, EnableRandomOffset, false)
@@ -104,6 +99,11 @@ PRIMARY_OPTIONAL(const bool, EnableContiguousRegions, true)
 // guarantee a performance benefit.
 PRIMARY_OPTIONAL_TYPE(ConditionVariableT, ConditionVariableDummy)
 
+// 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_OPTIONAL_TYPE(CompactPtrT, uptr)
+
 // SECONDARY_REQUIRED_TEMPLATE_TYPE(NAME)
 //
 // Defines the type of Secondary Cache to use.

@ChiaHungDuan
Copy link
Contributor Author

Add more context, given that primary32 doesn't do pointer compaction, make them optional. This is one of the CLs to deprecate primary32.

Copy link
Contributor

@cferris1000 cferris1000 left a comment

Choose a reason for hiding this comment

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

LGTM.

@ChiaHungDuan ChiaHungDuan force-pushed the primary32-deprecation branch 2 times, most recently from cb66d46 to ccde599 Compare July 8, 2024 21:10
@ChiaHungDuan ChiaHungDuan merged commit b30d667 into llvm:main Jul 8, 2024
4 of 5 checks passed
@@ -104,6 +99,11 @@ PRIMARY_OPTIONAL(const bool, EnableContiguousRegions, true)
// guarantee a performance benefit.
PRIMARY_OPTIONAL_TYPE(ConditionVariableT, ConditionVariableDummy)

// Defines the type and scale of a compact pointer. A compact pointer can
Copy link
Contributor

Choose a reason for hiding this comment

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

Defines ... and scale -> the scale is actually defined by CompactPtrScale

I think it might help here to document how to choose the proper type, for instance

This type is used to efficiently store arrays of pointers to free blocks, and it must be large enough to express offsets within an entire region, i.e. this type's maximum possible value must uphold MaxCompactPtrValue << CompactPtrScale >= 1 << RegionSizeLog. The default type (uptr) trivially satisfies this requirement but, depending on CompactPtrScale and RegionSizeLog, a smaller type could work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants