Skip to content

[scudo] allocation_ring_buffer_size <= 0 disables buffer #71791

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

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Nov 9, 2023

Prevent a null pointer exception for allocation_ring_buffer_size < 0.

Prevent a null pointer exception for allocation_ring_buffer_size < 0.
@llvmbot
Copy link
Member

llvmbot commented Nov 9, 2023

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

Author: Florian Mayer (fmayer)

Changes

Prevent a null pointer exception for allocation_ring_buffer_size < 0.


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

2 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/combined.h (+1-1)
  • (modified) compiler-rt/lib/scudo/standalone/flags.inc (+2-1)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index b1700e5ecef7f5b..cb48c8c1e3a1e77 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -886,7 +886,7 @@ class Allocator {
 
   void setTrackAllocationStacks(bool Track) {
     initThreadMaybe();
-    if (getFlags()->allocation_ring_buffer_size == 0) {
+    if (getFlags()->allocation_ring_buffer_size <= 0) {
       DCHECK(!Primary.Options.load().get(OptionBit::TrackAllocationStacks));
       return;
     }
diff --git a/compiler-rt/lib/scudo/standalone/flags.inc b/compiler-rt/lib/scudo/standalone/flags.inc
index 60aeb1f1df570ac..f5a2bab5057ae13 100644
--- a/compiler-rt/lib/scudo/standalone/flags.inc
+++ b/compiler-rt/lib/scudo/standalone/flags.inc
@@ -47,4 +47,5 @@ SCUDO_FLAG(int, release_to_os_interval_ms, SCUDO_ANDROID ? INT32_MIN : 5000,
            "memory to the OS. Negative values disable the feature.")
 
 SCUDO_FLAG(int, allocation_ring_buffer_size, 32768,
-           "Entries to keep in the allocation ring buffer for scudo.")
+           "Entries to keep in the allocation ring buffer for scudo. "
+           "Values less or equal to zero disable the buffer.")

@fmayer fmayer changed the title allocation_ring_buffer_size <= 0 disables buffer [scudo] allocation_ring_buffer_size <= 0 disables buffer Nov 9, 2023
Copy link
Collaborator

@hctim hctim left a comment

Choose a reason for hiding this comment

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

compiler-rt/lib/scudo/standalone/combined.h:1494

Also this (to avoid mapping it as well)?

1492   void mapAndInitializeRingBuffer() {
1493     u32 AllocationRingBufferSize =
1494         static_cast<u32>(getFlags()->allocation_ring_buffer_size);
////     Hoist this check up, and make it be |getFlags()->allocation_ring_buffer_size < 1| to avoid the
////     type punning?
1495     if (AllocationRingBufferSize < 1)
1496       return;
1497     MemMapT MemMap;
1498     MemMap.map(
1499         /*Addr=*/0U,
1500         roundUp(ringBufferSizeInBytes(AllocationRingBufferSize),

@fmayer
Copy link
Contributor Author

fmayer commented Nov 9, 2023

compiler-rt/lib/scudo/standalone/combined.h:1494

Also this (to avoid mapping it as well)?

1492   void mapAndInitializeRingBuffer() {
1493     u32 AllocationRingBufferSize =
1494         static_cast<u32>(getFlags()->allocation_ring_buffer_size);
////     Hoist this check up, and make it be |getFlags()->allocation_ring_buffer_size < 1| to avoid the
////     type punning?
1495     if (AllocationRingBufferSize < 1)
1496       return;
1497     MemMapT MemMap;
1498     MemMap.map(
1499         /*Addr=*/0U,
1500         roundUp(ringBufferSizeInBytes(AllocationRingBufferSize),

ah, yes. thanks

Copy link
Collaborator

@hctim hctim left a comment

Choose a reason for hiding this comment

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

LGTM

@fmayer
Copy link
Contributor Author

fmayer commented Nov 14, 2023

PTAL

@fmayer fmayer merged commit a66dc46 into llvm:main Nov 14, 2023
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
Prevent a null pointer exception for allocation_ring_buffer_size < 0.
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.

5 participants