From 53da38ca47d984a1df9a8a8cd7df72d02a1be677 Mon Sep 17 00:00:00 2001 From: Ben Lubar Date: Fri, 21 Feb 2020 17:31:22 -0600 Subject: [PATCH] add additional pointer, string, and vector sanity checks update structures and scripts --- library/xml | 2 +- plugins/devel/check-structures-sanity.cpp | 74 ++++++++++++++++++++--- scripts | 2 +- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/library/xml b/library/xml index c3835a548..6dcfd4210 160000 --- a/library/xml +++ b/library/xml @@ -1 +1 @@ -Subproject commit c3835a548baade596e422666872ec80134c024e1 +Subproject commit 6dcfd4210cbffb71127f8dd8eef170e149990d40 diff --git a/plugins/devel/check-structures-sanity.cpp b/plugins/devel/check-structures-sanity.cpp index d95c9b28c..0905ebea0 100644 --- a/plugins/devel/check-structures-sanity.cpp +++ b/plugins/devel/check-structures-sanity.cpp @@ -88,6 +88,7 @@ public: private: bool ok; + bool address_in_runtime_data(void *); bool check_access(const ToCheck &, void *, type_identity *); bool check_access(const ToCheck &, void *, type_identity *, size_t); bool check_vtable(const ToCheck &, void *, type_identity *); @@ -140,7 +141,7 @@ static command_result command(color_ostream & out, std::vector & pa if (parameters.empty()) { ToCheck global; - global.path.push_back("df::global::"); + global.path.push_back("df.global."); global.ptr = nullptr; global.identity = &df::global::_identity; @@ -240,6 +241,23 @@ bool Checker::check() #define PTR_ADD(base, offset) (reinterpret_cast(reinterpret_cast((base)) + static_cast((offset)))) +bool Checker::address_in_runtime_data(void *ptr) +{ + for (auto & range : mapped) + { + if (!range.isInRange(ptr)) + { + continue; + } + + // TODO: figure out how to differentiate statically-allocated pages from malloc'd data pages + UNEXPECTED; + return false; + } + + return false; +} + bool Checker::check_access(const ToCheck & item, void *base, type_identity *identity) { return check_access(item, base, identity, identity ? identity->byte_size() : 0); @@ -610,6 +628,7 @@ void Checker::check_global(const ToCheck & globals) for (auto field = identity->getFields(); field->mode != struct_field_info::END; field++) { ToCheck item(globals, field->name, nullptr, field->type); + item.path.push_back(""); // tell check_struct that this is a pointer auto base = reinterpret_cast(field->offset); if (!check_access(item, base, df::identity_traits::get())) @@ -669,7 +688,7 @@ void Checker::check_stl_string(const ToCheck & item) { size_t length; size_t capacity; - size_t refcount; + int32_t refcount; } *ptr; }; #endif @@ -690,7 +709,7 @@ void Checker::check_stl_string(const ToCheck & item) if (!check_access(item, string->ptr, item.identity, 1)) { // nullptr is NOT okay here - FAIL("invalid string pointer"); + FAIL("invalid string pointer " << stl_sprintf("%p", string->ptr)); return; } if (!check_access(item, string->ptr - 1, item.identity, sizeof(*string->ptr))) @@ -706,7 +725,7 @@ void Checker::check_stl_string(const ToCheck & item) { FAIL("string length is negative (" << length << ")"); } - if (capacity < 0) + else if (capacity < 0) { FAIL("string capacity is negative (" << capacity << ")"); } @@ -715,22 +734,48 @@ void Checker::check_stl_string(const ToCheck & item) FAIL("string capacity (" << capacity << ") is less than length (" << length << ")"); } +#ifndef WIN32 + const std::string empty_string; + auto empty_string_data = reinterpret_cast(&empty_string); + if (sizes && string->ptr != empty_string_data->ptr) + { + uint32_t tag = *reinterpret_cast(PTR_ADD(string->ptr - 1, -8)); + if (tag == 0xdfdf4ac8) + { + size_t allocated_size = *reinterpret_cast(PTR_ADD(string->ptr - 1, -16)); + size_t expected_size = sizeof(*string->ptr) + capacity + 1; + + if (allocated_size != expected_size) + { + FAIL("allocated string data size (" << allocated_size << ") does not match expected size (" << expected_size << ")"); + } + } + else if (address_in_runtime_data(string->ptr)) + { + UNEXPECTED; + } + } +#endif + check_access(item, start, item.identity, capacity); } void Checker::check_pointer(const ToCheck & item) { - if (!check_access(item, item.ptr, item.identity)) + if (!seen_addr.insert(item.ptr).second) { return; } - if (!seen_addr.insert(item.ptr).second) + auto base = *reinterpret_cast(item.ptr); + auto base_int = uintptr_t(base); + if (base_int != UNINIT_PTR && base_int % alignof(void *) != 0) { - return; + FAIL("unaligned pointer " << stl_sprintf("%p", base)); } - queue.push_back(ToCheck(item, "", *reinterpret_cast(item.ptr), static_cast(item.identity)->getTarget())); + auto target_identity = static_cast(item.identity)->getTarget(); + queue.push_back(ToCheck(item, "", base, target_identity)); } void Checker::check_bitfield(const ToCheck & item) @@ -942,11 +987,15 @@ void Checker::check_vector(const ToCheck & item, type_identity *item_identity, b local_ok = false; } - if (local_ok && check_access(item, reinterpret_cast(vector.start), item.identity, length) && item_identity) + if (local_ok && check_access(item, reinterpret_cast(vector.start), item.identity, capacity) && item_identity) { auto ienum = static_cast(static_cast(item.identity)->getIndexEnumType()); queue_static_array(item, reinterpret_cast(vector.start), item_identity, ulength / item_size, pointer, ienum); } + else if (local_ok && capacity && !vector.start) + { + FAIL("vector has null pointer but capacity " << (ucapacity / item_size)); + } } void Checker::check_deque(const ToCheck & item, type_identity *item_identity) @@ -1013,7 +1062,8 @@ void Checker::check_struct(const ToCheck & item) { bool is_pointer = item.path.back().empty(); bool is_virtual = !item.path.back().empty() && item.path.back().at(0) == '<'; - if (sizes && uintptr_t(item.ptr) % 32 == 16 && (is_pointer || is_virtual)) + bool is_virtual_pointer = is_virtual && item.path.size() >= 2 && item.path.at(item.path.size() - 2).empty(); + if (sizes && uintptr_t(item.ptr) % 32 == 16 && (is_pointer || is_virtual_pointer)) { uint32_t tag = *reinterpret_cast(PTR_ADD(item.ptr, -8)); if (tag == 0xdfdf4ac8) @@ -1026,6 +1076,10 @@ void Checker::check_struct(const ToCheck & item) FAIL("allocated structure size (" << allocated_size << ") does not match expected size (" << expected_size << ")"); } } + else if (address_in_runtime_data(item.ptr)) + { + UNEXPECTED; + } } for (auto identity = static_cast(item.identity); identity; identity = identity->getParent()) diff --git a/scripts b/scripts index 2c403e3f2..fdf22fe47 160000 --- a/scripts +++ b/scripts @@ -1 +1 @@ -Subproject commit 2c403e3f2cb3cc145a249dc509e64e3946a7b611 +Subproject commit fdf22fe4736c2d98801d0c34369493bbb78c0fc3