Fix void* arithmetic bug. Add dassert facility.
authorGodfrey van der Linden <gvdl@google.com>
Thu, 29 Jan 2015 17:17:32 +0000 (09:17 -0800)
committerGodfrey van der Linden <gvdl@google.com>
Thu, 29 Jan 2015 17:17:32 +0000 (09:17 -0800)
In C the behaviour of arthmatic on void * pointers is undefined. GCC has
a language extension that equates it with a uint8_t, but other compilers
don't. Change the tr_buf to a unsigned char to define byte arithmatic.

Split the __get_tr_slot into two adding __get_tr_slot_overwrite for the
version that masks the slot down to a limited range.

Add dassert() for assertions that are only optionally compiled in. Used
at the moment to check index is in range in __get_tr_slot.

Kconfig
kern/include/assert.h
kern/include/trace.h
kern/src/trace.c

diff --git a/Kconfig b/Kconfig
index 605b873..0981d1f 100644 (file)
--- a/Kconfig
+++ b/Kconfig
@@ -123,6 +123,12 @@ config TRACE_LOCKS
 
 endmenu
 
+config DEVELOPMENT_ASSERTIONS
+       bool "dasserts"
+       default n
+       help
+               Turn on dassert() in code, dassert will compile to assert().
+
 config SPINLOCK_DEBUG
        bool "Spinlock debugging"
        default n
index b04306e..2dcb6d7 100644 (file)
@@ -22,4 +22,10 @@ void ( _panic)(const char* NTS, int, const char* NTS, ...)
 // static_assert(x) will generate a compile-time error if 'x' is false.
 #define static_assert(x)       switch (x) case 0: case (x):
 
+#ifdef CONFIG_DEVELOPMENT_ASSERTIONS
+#define dassert(x) assert(x)
+#else
+#define dassert(x)
+#endif /* DEVELOPMENT_ASSERTIONS */
+
 #endif /* !ROS_INC_ASSERT_H */
index 77c8c0a..611eb11 100644 (file)
 #define ROS_INC_TRACE_H
 
 #include <ros/common.h>
+#include <assert.h>
 
 struct trace_ring {
-       void                                            *tr_buf;
+       unsigned char                           *tr_buf;
        size_t                                          tr_buf_sz;
        unsigned int                            tr_event_sz_shift;
        unsigned int                            tr_max;
@@ -67,12 +68,21 @@ void trace_ring_foreach(struct trace_ring *tr, trace_handler_t tr_func,
 /* Inlined funcs, declared above */
 
 /* Helper */
-static inline void *__get_tr_slot(struct trace_ring *tr, unsigned long slot)
+/* Get next trace ring slot with no wrapping */
+static inline void *__get_tr_slot(struct trace_ring *tr, unsigned long ind)
 {
-       /* tr_max is a power of 2, we're ignoring the upper bits of my_slot */
-       slot &= tr->tr_max - 1;
+       dassert(0 <= ind && ind < tr->tr_max);
        /* event sizes are rounded up to the nearest power of 2 (sz_shift) */
-       return tr->tr_buf + (slot << tr->tr_event_sz_shift);
+       return (void *) (tr->tr_buf + (ind << tr->tr_event_sz_shift));
+}
+
+/* Get next trace ring slot with wrapping */
+static inline void *
+__get_tr_slot_overwrite(struct trace_ring *tr, unsigned long slot)
+{
+       /* tr_max is a power of 2, we're ignoring the upper bits of slot */
+       slot &= tr->tr_max - 1;
+       return __get_tr_slot(tr, slot);
 }
 
 static inline void *get_trace_slot(struct trace_ring *tr)
@@ -91,7 +101,7 @@ static inline void *get_trace_slot(struct trace_ring *tr)
 
 static inline void *get_trace_slot_overwrite(struct trace_ring *tr)
 {
-       return __get_tr_slot(tr, __sync_fetch_and_add(&tr->tr_next, 1));
+       return __get_tr_slot_overwrite(tr, __sync_fetch_and_add(&tr->tr_next, 1));
 }
 
 static inline void *get_trace_slot_racy(struct trace_ring *tr)
@@ -105,7 +115,7 @@ static inline void *get_trace_slot_racy(struct trace_ring *tr)
 
 static inline void *get_trace_slot_overwrite_racy(struct trace_ring *tr)
 {
-       return __get_tr_slot(tr, tr->tr_next++);
+       return __get_tr_slot_overwrite(tr, tr->tr_next++);
 }
 
 #endif /* ROS_INC_TRACE_H */
index c1ea617..8725f21 100644 (file)
@@ -35,5 +35,5 @@ void trace_ring_foreach(struct trace_ring *tr, trace_handler_t tr_func,
                         void *data)
 {
        for (int i = 0; i < tr->tr_max; i++)
-               tr_func(tr->tr_buf + (i << tr->tr_event_sz_shift), data);
+               tr_func(__get_tr_slot(tr, i), data);
 }