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 <andrewg@us.ibm.com>
Reviewed-by: CAMVAN T. NGUYEN <ctnguyen@us.ibm.com>

10 files changed, 249 insertions, 21 deletions

diff --git a/config.mk b/config.mk
index be75fbade..8fd7c4f55 100644
--- a/config.mk
+++ b/config.mk
@@ -7,7 +7,8 @@ OBJDIR = ${ROOTPATH}/obj/modules/${MODULE}
 BEAMDIR = ${ROOTPATH}/obj/beam/${MODULE}
 GENDIR = ${ROOTPATH}/obj/genfiles/
 IMGDIR = ${ROOTPATH}/img
-EXTRACOMMONFLAGS += -fPIC -Bsymbolic -Bsymbolic-functions
+EXTRACOMMONFLAGS += -fPIC -Bsymbolic -Bsymbolic-functions 
+CUSTOMFLAGS += -D__HOSTBOOT_MODULE=${MODULE}
 LIBS += $(addsuffix .so, $(addprefix lib, ${MODULE}))
 MODULE_INIT = ${ROOTPATH}/obj/core/module_init.o
 EXTRAINCDIR += ${ROOTPATH}/src/include/usr ${GENDIR}
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 <builtins.h>
-
+/** @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 <builtins.h>
+
+#ifdef __HOSTBOOT_MODULE // Only allow traced assert in module code.
+#include <trace/interface.H>
+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
@@ -45,6 +45,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
  *
  * @return 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 <builtins.h>
 #include <kernel/console.H>
+#include <assert.h>
+#include <sys/task.h>
+#include <arch/ppc.H>
+
+/** 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 <assert.h>
+#include <trace/interface.H>
+
+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