From 9a79e531ef6443998486e0dcac51f5ecdf029448 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Wed, 2 Nov 2011 16:02:07 +0000 Subject: [PATCH] bonefish+mmlr: * Introduce TracingMetaData::IsInBuffer() to validate that a certain memory range is within the valid tracing buffer limits. * Use that when validating in tracing_is_entry_valid() before trying to access the entry, resolving a TODO. * Validate the candidate time against the handed in time (if specified) as an additional check. * Tiny unrelated text cleanup. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@43116 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/tracing.h | 3 ++- src/system/kernel/debug/tracing.cpp | 40 ++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/headers/private/kernel/tracing.h b/headers/private/kernel/tracing.h index d1a91fc8a7..acad6bc636 100644 --- a/headers/private/kernel/tracing.h +++ b/headers/private/kernel/tracing.h @@ -265,7 +265,8 @@ TraceOutput::Print(const char* format,...) int dump_tracing(int argc, char** argv, WrapperTraceFilter* wrapperFilter); -bool tracing_is_entry_valid(TraceEntry* entry, bigtime_t entryTime); +bool tracing_is_entry_valid(AbstractTraceEntry* entry, + bigtime_t entryTime = -1); #endif // __cplusplus diff --git a/src/system/kernel/debug/tracing.cpp b/src/system/kernel/debug/tracing.cpp index a7c424d6fe..72cbb959aa 100644 --- a/src/system/kernel/debug/tracing.cpp +++ b/src/system/kernel/debug/tracing.cpp @@ -100,6 +100,8 @@ public: trace_entry* AllocateEntry(size_t size, uint16 flags); + bool IsInBuffer(void* address, size_t size); + private: bool _FreeFirstEntry(); bool _MakeSpace(size_t needed); @@ -285,6 +287,25 @@ TracingMetaData::AllocateEntry(size_t size, uint16 flags) } +bool +TracingMetaData::IsInBuffer(void* address, size_t size) +{ + if (fEntries == 0) + return false; + + addr_t start = (addr_t)address; + addr_t end = start + size; + + if (start < (addr_t)fBuffer || end > (addr_t)(fBuffer + kBufferSize)) + return false; + + if (fFirstEntry > fAfterLastEntry) + return start >= (addr_t)fFirstEntry || end <= (addr_t)fAfterLastEntry; + + return start >= (addr_t)fFirstEntry && end <= (addr_t)fAfterLastEntry; +} + + bool TracingMetaData::_FreeFirstEntry() { @@ -495,8 +516,8 @@ bool TracingMetaData::_InitPreviousTracingData() { // TODO: ATM re-attaching the previous tracing buffer doesn't work very - // well. The entries should checked more thoroughly for validity -- e.g. the - // pointers to the entries' vtable pointers could be invalid, which can + // well. The entries should be checked more thoroughly for validity -- e.g. + // the pointers to the entries' vtable pointers could be invalid, which can // make the "traced" command quite unusable. The validity of the entries // could be checked in a safe environment (i.e. with a fault handler) with // typeid() and call of a virtual function. @@ -1696,18 +1717,25 @@ dump_tracing(int argc, char** argv, WrapperTraceFilter* wrapperFilter) bool -tracing_is_entry_valid(TraceEntry* candidate, bigtime_t entryTime) +tracing_is_entry_valid(AbstractTraceEntry* candidate, bigtime_t entryTime) { #if ENABLE_TRACING + if (!sTracingMetaData->IsInBuffer(candidate, sizeof(*candidate))) + return false; + + if (entryTime < 0) + return true; + TraceEntryIterator iterator; while (TraceEntry* entry = iterator.Next()) { AbstractTraceEntry* abstract = dynamic_cast(entry); if (abstract == NULL) continue; - // TODO: This could be better by additionally checking if the - // candidate entry address falls within the valid entry range. - return abstract == candidate || abstract->Time() < entryTime; + if (abstract != candidate && abstract->Time() > entryTime) + return false; + + return candidate->Time() == entryTime; } #endif