Page MenuHomePhabricator

Check pointers for NULL in src/lib/eina/eina_debug.c and eina_debug_bt.c
Closed, ResolvedPublic

Description

The code of escwyp (synthesizer with Elementary gui) was crashing with segmentation faults since the commit 5f268ec26a76b130a78ab182981dde372951f075 "eina debug layer rewriting".

It turned out that following changes to

src/lib/eina/eina_debug.c
src/lib/eina/eina_debug_bt.c

fixed the crashes. Would it be possible/sane to merge these NULL checks or something similar
into the code? Or is it useful to investigate why the values of 'session', 'source' etc. are NULL in the first place?

Running 'make check' with these patches applied showed all tests passed.
The patches were created against 1adb73cef82c45f2cc8766f43ffb88288e7e8a65

diff --git a/src/lib/eina/eina_debug.c b/src/lib/eina/eina_debug.c
index 9dd1a092d5..706a257b3c 100644
--- a/src/lib/eina/eina_debug.c
+++ b/src/lib/eina/eina_debug.c
@@ -662,6 +662,12 @@ eina_debug_opcodes_register(Eina_Debug_Session *session, const Eina_Debug_Opcode
 EAPI Eina_Bool
 eina_debug_dispatch(Eina_Debug_Session *session, void *buffer)
 {
+   if (!session || !buffer)
+     {
+        e_debug("Empty session = %x or buffer = %x", session, buffer);
+        return EINA_FALSE;
+     }
+
    Eina_Debug_Packet_Header *hdr = buffer;
    Eina_List *itr;
    int opcode = hdr->opcode;
diff --git a/src/lib/eina/eina_debug_bt.c b/src/lib/eina/eina_debug_bt.c
index 21b067c395..952a19d00e 100644
--- a/src/lib/eina/eina_debug_bt.c
+++ b/src/lib/eina/eina_debug_bt.c
@@ -170,7 +170,10 @@ found:
    // we have consumed (it's cumulative so subtracing deltas can give
    // you an average amount of cpu time consumed between now and the
    // previous time we looked) and also a full backtrace
-   _bt_cpu[slot] = sched_getcpu();
+   if (_bt_cpu)
+     {
+       _bt_cpu[slot] = sched_getcpu();
+     }
 # ifdef HAVE_PTHREAD_GETCPUCLOCKID
    /* Try pthread_getcpuclockid() first */
    pthread_getcpuclockid(self, &cid);
@@ -182,7 +185,10 @@ found:
 #  error Cannot determine the clock id for clock_gettime()
 # endif
    clock_gettime(cid, &(_bt_ts[slot]));
-   _bt_buf_len[slot] = _eina_debug_unwind_bt(_bt_buf[slot], EINA_MAX_BT);
+   if (_bt_buf_len)
+     {
+       _bt_buf_len[slot] = _eina_debug_unwind_bt(_bt_buf[slot], EINA_MAX_BT);
+     }
 #endif /* HAVE_CLOCK_GETTIME && HAVE_SCHED_GETCPU */
    // now wake up the monitor to let them know we are done collecting our
    // backtrace info
escwyp created this task.Jun 17 2018, 12:47 PM
raster added a subscriber: raster.Jun 17 2018, 7:26 PM

this is very odd. why is this the only app that has these issues. it's during init and the code does nothing special before init... so why here? this is very very very odd. ....

wait! found it. the cmake files compile with -pg - profiling. this is sending SIGPROF ... and thus a signal handler is being called to PROFILE a thread and get a bt... but it isn't set up/initted. SIGPROF was not the best choice here. so switching to SIGRT + x would avoid this path. so your patches above are checks within a code path that should not have even been triggered, but -pg made it trigger.

zmike reopened this task as Open.Jun 18 2018, 9:22 AM