From 9ada6b493e33fe43057b03897d57f2ad931d219f Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Fri, 24 Jun 2011 10:13:59 -0500 Subject: Improve assert handling and options. - Allow a custom assert trace as an optional parameter. - Call a trace function instead of printk for most cases. - Provide a critical library assert (for syslibs, trace, etc.) - Provide a kassert function for kernel code. Change-Id: If24d57d0832a587258503b3fd0046c21da3712b5 Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/159 Tested-by: Jenkins Server Reviewed-by: Andrew J. Geissler Reviewed-by: CAMVAN T. NGUYEN --- src/include/assert.h | 150 ++++++++++++++++++++++++++++++++++++++++++++--- src/include/builtins.h | 12 +++- src/kernel/futexmgr.C | 2 +- src/lib/assert.C | 53 +++++++++++++++-- src/lib/sync.C | 2 +- src/lib/syscall_mmio.C | 2 +- src/sys/init/init_main.C | 2 +- src/usr/trace/assert.C | 42 +++++++++++++ src/usr/trace/makefile | 2 +- 9 files changed, 247 insertions(+), 20 deletions(-) create mode 100644 src/usr/trace/assert.C (limited to 'src') diff --git a/src/include/assert.h b/src/include/assert.h index 13888bee8..a8f3fb664 100644 --- a/src/include/assert.h +++ b/src/include/assert.h @@ -1,27 +1,159 @@ -#include - +/** @file assert.h + * @brief Define the interfaces for the standard 'assert' macros. + * + * There are four different assert types provided now: + * Standard assert behavior: + * assert(foo) + * + * Standard assert behavior with a custom trace message: + * assert(foo, "This is a trace %d", 1234) + * + * Critical assert, which should only be used by system libraries or + * code paths which cannot use trace or error logging: + * crit_assert(foo) + * + * Kernel assert: + * kassert(foo) + * + * Most code should use the standard asserts. Kernel code must use kassert + * exclusively. Usage of the critical assert should be limited to system + * library code (/src/lib, /src/sys) or init service, trace and error log. + */ #ifndef _ASSERT_H #define _ASSERT_H +#include + +#ifdef __HOSTBOOT_MODULE // Only allow traced assert in module code. +#include +namespace TRACE { extern trace_desc_t* g_assertTraceBuffer; }; +#endif + #ifdef __cplusplus extern "C" { #endif +/** @enum AssertBehavior + * @brief Types of assert behavior used by the internal __assert function. + */ +enum AssertBehavior +{ + /** Standard assert, custom trace already done. */ + ASSERT_TRACE_DONE, + /** Standard assert, no custom trace. */ + ASSERT_TRACE_NOTDONE, + /** Critical / System library assert. */ + ASSERT_CRITICAL, + /** Kernel-level assert. */ + ASSERT_KERNEL, +}; + +/** @fn __assert + * @brief Internal function utilized by assert macros to commonize handling. + * + * @param[in] i_assertb - Internal enumeration used by macros to communicate + * desired behavior. + * @param[in] i_line - Line number at which the assert macro was called. + * + * Current Behaviors: + * User-space application - A trace is created, either a custom one + * provided by the caller or a common one + * determined by the trace callback hook, and + * the asserting task is ended. + * + * Critical library - A printk is performed, similar in structure to the + * traces, and the user task is ended. + * + * Kernel - A printk is performed and a while(1) loop is entered to cease + * user-space dispatching. + */ NO_RETURN -void __assert(bool expr, const char *exprStr, const char *file, int line); +void __assert(AssertBehavior i_assertb, int i_line); + +#ifdef __HOSTBOOT_MODULE // Only allow traced assert in module code. -#define assert(expr) \ +// Macro tricks to determine if there is a custom string. +#define __ASSERT_HAS_TRACE_(_1, _2, ...) _2 +#define __ASSERT_HAS_TRACE(...) __ASSERT_HAS_TRACE_(0, ##__VA_ARGS__, 0) + +/** + * @brief Macro to do the custom trace if one is provided. + * + * This results in larger code size of the caller to call the trace routines + * but may provide additional debug information. + * + * The "code" here will be compiled down to nothing or a single trace call by + * the optimizer. Search for "Constant Folding" for compiler background. + */ +#define __ASSERT_DO_TRACE(expr, ...) { \ + int __assert_unused_var = 0; \ + __assert_unused_var += (__ASSERT_HAS_TRACE(__VA_ARGS__) ? \ + TRACFCOMP(TRACE::g_assertTraceBuffer, \ + "Assertion [ " #expr " ] failed; " __VA_ARGS__),1 \ + : 0); \ + } + +/** + * @brief Standard assert macro. + * + * Verfies condition, calls custom trace if provided, and calls internal + * __assert function for remainder of common assert behavior. + */ +#define assert(expr,...) \ {\ - if (!(expr))\ + if (unlikely(!(expr)))\ {\ - __assert((expr), #expr, __FILE__, __LINE__);\ + __ASSERT_DO_TRACE(expr, __VA_ARGS__); \ + __assert((__ASSERT_HAS_TRACE(__VA_ARGS__) ? \ + ASSERT_TRACE_DONE : ASSERT_TRACE_NOTDONE),\ + __LINE__);\ }\ -}\ +} + +#else // Only allow kernel assert in non-module code. -#ifdef NDEBUG +/** + * @brief Kernel assert macro. + * + * Verifies condition and calls __assert function for common behavior. + */ +#define kassert(expr) \ +{\ + if (unlikely(!(expr)))\ + {\ + __assert(ASSERT_KERNEL, __LINE__);\ + }\ +} + +#endif // ifdef __HOSTBOOT_MODULE + +// Allow critical assert anywhere. +/** + * @brief Critical assert macro. + * + * Verifies condition and calls __assert function for common behavior. + */ +#define crit_assert(expr) \ +{\ + if (unlikely(!(expr)))\ + {\ + __assert(ASSERT_CRITICAL, __LINE__);\ + }\ +} + + + +#ifdef NDEBUG // Empty macro definitions for when NDEBUG is defined. +#ifdef MODULE #undef assert -#define assert(expr) { } +#define assert(expr,...) { } +#else +#undef kassert +#define kassert(expr) { } +#endif +#undef crit_assert +#define crit_assert(expr,...) { } #endif #ifdef __cplusplus diff --git a/src/include/builtins.h b/src/include/builtins.h index 07b185697..54b7dc768 100644 --- a/src/include/builtins.h +++ b/src/include/builtins.h @@ -44,6 +44,16 @@ extern "C" */ #define PACKED __attribute__((packed)) +/** + * Compiler hint for branch conditions. "condition is likely to be true" + */ +#define likely(expr) __builtin_expect((expr),1) + +/** + * Compiler hint for branch conditions. "condition is likely to be false" + */ +#define unlikely(expr) __builtin_expect((expr),0) + /** * Get the value of the link register * @@ -52,7 +62,7 @@ extern "C" ALWAYS_INLINE static inline void *linkRegister() { - return __builtin_return_address(1); + return __builtin_return_address(0); } /** diff --git a/src/kernel/futexmgr.C b/src/kernel/futexmgr.C index aaae95b53..6271471aa 100644 --- a/src/kernel/futexmgr.C +++ b/src/kernel/futexmgr.C @@ -77,7 +77,7 @@ uint64_t FutexManager::_wake(uint64_t * i_addr, uint64_t i_count) // This means we had a waiter in the queue, but that waiter had // a Null task assigned to it. This should NEVER happen - assert(wait_task != NULL); + kassert(wait_task != NULL); wait_task->cpu->scheduler->addTask(wait_task); ++started; diff --git a/src/lib/assert.C b/src/lib/assert.C index 953068d4a..c8b454a2f 100644 --- a/src/lib/assert.C +++ b/src/lib/assert.C @@ -1,10 +1,53 @@ +/** @file assert.C + * @brief Common handling functions for assert paths. + */ + +#include #include +#include +#include +#include + +/** Hook location for trace module to set up when loaded. */ +namespace TRACE { void (*traceCallback)(void*, size_t) = NULL; }; -extern "C" void __assert(bool expr, const char *exprStr, - const char *file, int line) +extern "C" void __assert(AssertBehavior i_assertb, int i_line) { - // TODO - printk("Assert in %s (%d): %s\n", file, line, exprStr); - while (true); + switch (i_assertb) + { + case ASSERT_TRACE_DONE: // Custom trace was provided. + task_end(); + break; + + case ASSERT_TRACE_NOTDONE: // Do a normal trace. + if (NULL != TRACE::traceCallback) + { + (*TRACE::traceCallback)(linkRegister(), i_line); + } + else + { + printk("Assertion failed @%p on line %d.\n", + linkRegister(), i_line); + } + task_end(); + break; + + case ASSERT_CRITICAL: // Critical task, trace not available. + printk("Assertion failed @%p on line %d.\n", + linkRegister(), i_line); + task_end(); + break; + + case ASSERT_KERNEL: // Kernel assert called. + printk("Assertion failed @%p on line %d.\n", + linkRegister(), i_line); + break; + } + + // Loop forever if we make it here. Should only happen in kernel code. + while (true) + { + setThreadPriorityLow(); + } } diff --git a/src/lib/sync.C b/src/lib/sync.C index 093c7f41e..788ecb1c4 100644 --- a/src/lib/sync.C +++ b/src/lib/sync.C @@ -33,7 +33,7 @@ void barrier_init (barrier_t * o_barrier, uint64_t i_count) void barrier_destroy (barrier_t * i_barrier) { - assert(i_barrier->iv_missing == i_barrier->iv_count); + crit_assert(i_barrier->iv_missing == i_barrier->iv_count); return; } diff --git a/src/lib/syscall_mmio.C b/src/lib/syscall_mmio.C index 83a4f8f82..754666a35 100644 --- a/src/lib/syscall_mmio.C +++ b/src/lib/syscall_mmio.C @@ -33,7 +33,7 @@ mutex_t * mmio_xscom_mutex() asm volatile("mr %0, 13" : "=r"(task)); // Ensure task is pinned. - assert(task->affinity_pinned); + crit_assert(task->affinity_pinned); // Return mutex from cpu structure. return &task->cpu->xscom_mutex; diff --git a/src/sys/init/init_main.C b/src/sys/init/init_main.C index 5a7cf16ae..6292500e3 100644 --- a/src/sys/init/init_main.C +++ b/src/sys/init/init_main.C @@ -33,7 +33,7 @@ void init_main(void* unused) { printk( "ERROR: init_main: failed to launch initservice: %d\n", tidrc ); - assert( 0 ); // stop here. + crit_assert( 0 ); // stop here. } // should never reach this point... diff --git a/src/usr/trace/assert.C b/src/usr/trace/assert.C new file mode 100644 index 000000000..49724c96e --- /dev/null +++ b/src/usr/trace/assert.C @@ -0,0 +1,42 @@ +/** @file assert.C + * Hooks for dealing with trace in assert statements. + * + * This hook function is registered with the system-library assert functions + * when the trace library loads, so that if an application asserts after that + * point in time a message will get added to a common ASSERT trace buffer. + */ +#include +#include + +namespace TRACE +{ + /** Unique trace buffer for assert statements. */ + trace_desc_t* g_assertTraceBuffer; + TRAC_INIT(&g_assertTraceBuffer, "ASSERT", 4096); + + /** @fn assertTrace + * @brief Hook to perform a trace on an assert failure. + * + * @param[in] i_linkRegister - Address from which 'assert' was called. + * @param[in] i_lineNumber - Line number in the file which 'assert'ed. + */ + void assertTrace(void* i_linkRegister, size_t i_lineNumber) + { + TRACFCOMP(g_assertTraceBuffer, + "Assertion failed @%p on line %d.", + i_linkRegister, i_lineNumber); + } + + /** Location to assign the assertTrace hook for __assert function to use. */ + extern void(*traceCallback)(void*, size_t); + + /** @class __init_trace_callback + * @brief Class which registers the assertTrace function statically. + */ + class __init_trace_callback + { + public: + __init_trace_callback() { traceCallback = &assertTrace; }; + }; + __init_trace_callback __init_trace_callback_instance; +}; diff --git a/src/usr/trace/makefile b/src/usr/trace/makefile index fa4b14324..38ecb9e20 100644 --- a/src/usr/trace/makefile +++ b/src/usr/trace/makefile @@ -1,7 +1,7 @@ ROOTPATH = ../../.. MODULE = trace -OBJS = trace.o tracebuffer.o +OBJS = trace.o tracebuffer.o assert.o SUBDIRS = test.d -- cgit v1.2.1