From 88d9534453250e22a5393555b517f36f2943184a Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Wed, 12 Jun 2019 13:54:14 +0100 Subject: [PATCH] CR Feedback + clangformat --- src/mem/alloc.h | 1 - src/mem/metaslab.h | 73 ++++++++++++++++++++++++++-------------------- src/mem/slab.h | 2 +- 3 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index af9f741..ce6c494 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -817,7 +817,6 @@ namespace snmalloc { Superslab* super = Superslab::get(p); - #ifndef NDEBUG if (p->target_id() != super->get_allocator()->id()) error("Detected memory corruption. Potential use-after-free"); diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index 3c2ff08..2d74747 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -92,27 +92,28 @@ namespace snmalloc } /// Value used to check for corruptions in a block - static constexpr size_t POISON = + static constexpr size_t POISON = static_cast(bits::is64() ? 0xDEADBEEFDEAD0000 : 0xDEAD0000); /// Store next pointer in a block. In Debug using magic value to detect some /// simple corruptions. static void store_next(void* p, uint16_t head) - { + { #ifdef NDEBUG *static_cast(p) = head; #else - *static_cast(p) = head ^ POISON; + *static_cast(p) = + head ^ POISON ^ (static_cast(head) << (bits::BITS - 16)); #endif } /// Accessor function for the next pointer in a block. - /// In Debug checks for simple corruptions. + /// In Debug checks for simple corruptions. static uint16_t follow_next(void* node) - { + { size_t next = *static_cast(node); #ifndef NDEBUG - if ((next ^ POISON) > 0xFFFF) + if (((next ^ POISON) ^ (next << (bits::BITS - 16))) > 0xFFFF) error("Detected memory corruption. Use-after-free."); #endif return static_cast(next); @@ -129,6 +130,38 @@ namespace snmalloc return ((head_start - slab_start) % size) == 0; } + /** + * Check bump-free-list-segment for cycles + * + * Using + * https://en.wikipedia.org/wiki/Cycle_detection#Floyd's_Tortoise_and_Hare + * We don't expect a cycle, so worst case is only followed by a crash, so + * slow doesn't mater. + **/ + void debug_slab_acyclic_free_list(Slab* slab) + { +#ifndef NDEBUG + uint16_t curr = head; + uint16_t curr_slow = head; + bool both = false; + while ((curr & 1) != 1) + { + curr = follow_next(pointer_offset(slab, curr)); + if (both) + { + curr_slow = follow_next(pointer_offset(slab, curr_slow)); + } + + if (curr == curr_slow) + { + error("Free list contains a cycle, typically indicates double free."); + } + + both = !both; + } +#endif + } + void debug_slab_invariant(bool is_short, Slab* slab) { #if !defined(NDEBUG) && !defined(SNMALLOC_CHEAP_CHECKS) @@ -145,36 +178,14 @@ namespace snmalloc // 'link' value is not important if full. return; } + // Block is not full assert(SLAB_SIZE > accounted_for); - // Walk bump-free-list-segment checking for cycles. - // Using - // https://en.wikipedia.org/wiki/Cycle_detection#Floyd's_Tortoise_and_Hare - // We don't expect a cycle, so worst case is only followed by a crash, so - // slow doesn't mater. - uint16_t curr = head; - uint16_t curr_slow = head; - bool both = false; - while ((curr & 1) != 1) - { - curr = follow_next(pointer_offset(slab, curr)); - if (both) - { - curr_slow = follow_next(pointer_offset(slab, curr_slow)); - } - - if (curr == curr_slow) - { - error("Free list contains a cycle, typically indicates double free."); - } - - both = !both; - } - + debug_slab_acyclic_free_list(slab); // Walk bump-free-list-segment accounting for unused space - curr = head; + uint16_t curr = head; while ((curr & 1) != 1) { // Check we are looking at a correctly aligned block diff --git a/src/mem/slab.h b/src/mem/slab.h index cdc2e99..67ff01b 100644 --- a/src/mem/slab.h +++ b/src/mem/slab.h @@ -100,7 +100,7 @@ namespace snmalloc Metaslab& meta = super->get_meta(this); bool was_full = meta.is_full(); - + #ifndef NDEBUG if (meta.is_unused()) error("Detected potential double free.");