xserver: Branch 'master' - 19 commits

Keith Packard keithp at kemper.freedesktop.org
Mon Jul 2 22:38:42 PDT 2012


 dix/getevents.c                      |    4 
 dix/touch.c                          |    5 -
 hw/xfree86/common/xf86Init.c         |   14 +-
 hw/xfree86/common/xf86Module.h       |    2 
 hw/xfree86/common/xf86Xinput.h       |    6 -
 hw/xfree86/loader/loader.c           |    2 
 hw/xfree86/loader/loadmod.c          |    5 -
 hw/xfree86/os-support/shared/sigio.c |    4 
 include/globals.h                    |    3 
 include/misc.h                       |   10 +-
 include/os.h                         |   13 ++
 mi/mieq.c                            |   19 +--
 os/backtrace.c                       |   49 +++++----
 os/log.c                             |  175 ++++++++++++++++++++++++++++++++++-
 os/osinit.c                          |    8 -
 os/utils.c                           |   46 +++++++++
 test/.gitignore                      |    1 
 test/Makefile.am                     |    3 
 test/signal-logging.c                |  115 +++++++++++++++++++++++
 19 files changed, 423 insertions(+), 61 deletions(-)

New commits:
commit 24525d96a3b9dba67eb75042500b2f208a2cc246
Merge: 4cd91bd... 35e3d22...
Author: Keith Packard <keithp at keithp.com>
Date:   Mon Jul 2 22:35:39 2012 -0700

    Merge branch 'sigsafe-logging-varargs'
    
    This merge includes a minor fixup for '%p' arguments; must cast to
    uintptr_t instead of uint64_t as we use -Werror=pointer-to-int-cast
    which complains when doing a cast (even explicitly) from a pointer
    to an integer of different size.

commit 35e3d229150395a222a0f53318daf5dbeb8f6eb6
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Thu May 24 14:04:42 2012 +1000

    Bump to ABI_XINPUT_VERSION 18
    
    The input ABI hasn't changed, but input drivers need something to hook on if
    they want to log from within signal handlers and the input ABI is the
    simplest way of doing so.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index bf56acd..7671cea 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -83,7 +83,7 @@ typedef enum {
  */
 #define ABI_ANSIC_VERSION	SET_ABI_VERSION(0, 4)
 #define ABI_VIDEODRV_VERSION	SET_ABI_VERSION(13, 0)
-#define ABI_XINPUT_VERSION	SET_ABI_VERSION(17, 0)
+#define ABI_XINPUT_VERSION	SET_ABI_VERSION(18, 0)
 #define ABI_EXTENSION_VERSION	SET_ABI_VERSION(6, 0)
 #define ABI_FONT_VERSION	SET_ABI_VERSION(0, 6)
 
commit 541934168dbeb17059542bb5a1da8eba7995fa05
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Mon May 28 10:10:30 2012 +1000

    xfree86: constify InputDriverPtr->driverName and default_options
    
    Already treated as const anyway by all drivers.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Chase Douglas <chase.douglas at canonical.com>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/common/xf86Xinput.h b/hw/xfree86/common/xf86Xinput.h
index 6ccccf1..35c38a5 100644
--- a/hw/xfree86/common/xf86Xinput.h
+++ b/hw/xfree86/common/xf86Xinput.h
@@ -68,14 +68,14 @@
 /* This holds the input driver entry and module information. */
 typedef struct _InputDriverRec {
     int driverVersion;
-    char *driverName;
+    const char *driverName;
     void (*Identify) (int flags);
     int (*PreInit) (struct _InputDriverRec * drv,
                     struct _InputInfoRec * pInfo, int flags);
     void (*UnInit) (struct _InputDriverRec * drv,
                     struct _InputInfoRec * pInfo, int flags);
     pointer module;
-    char **default_options;
+    const char **default_options;
 } InputDriverRec, *InputDriverPtr;
 
 /* This is to input devices what the ScrnInfoRec is to screens. */
commit c66089d2206bafc01307a8327ff6089edcb4ed2d
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Mon May 28 09:57:33 2012 +1000

    xfree86: constify InputInfoPtr->type_name
    
    This corresponds to XListInputDevice(3)'s "type" field (after being
    converted to an Atom). Input drivers use the XI_KEYBOARD and similar
    defines, even Wacom which falls out of the common defines uses constant
    strings here. The use-case for having this non-const is small.
    
    Input ABI break technically, since we never freed this information anyway it
    is not a noticable change.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Chase Douglas <chase.douglas at canonical.com>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/common/xf86Xinput.h b/hw/xfree86/common/xf86Xinput.h
index 1d4363a..6ccccf1 100644
--- a/hw/xfree86/common/xf86Xinput.h
+++ b/hw/xfree86/common/xf86Xinput.h
@@ -98,7 +98,7 @@ typedef struct _InputInfoRec {
     int fd;
     DeviceIntPtr dev;
     pointer private;
-    char *type_name;
+    const char *type_name;
     InputDriverPtr drv;
     pointer module;
     XF86OptionPtr options;
commit 505c8a2b2cae0318db1148417ec850d54b38f7df
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Apr 9 09:41:38 2012 -0700

    Log in OsVendorFatalError() in a signal safe manner
    
    The function can be called from a fatal signal handler.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index ca6efd4..84c8669 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -1058,16 +1058,16 @@ void
 OsVendorFatalError(const char *f, va_list args)
 {
 #ifdef VENDORSUPPORT
-    ErrorF("\nPlease refer to your Operating System Vendor support pages\n"
-           "at %s for support on this crash.\n", VENDORSUPPORT);
+    ErrorFSigSafe("\nPlease refer to your Operating System Vendor support "
+                 "pages\nat %s for support on this crash.\n", VENDORSUPPORT);
 #else
-    ErrorF("\nPlease consult the " XVENDORNAME " support \n"
-           "\t at " __VENDORDWEBSUPPORT__ "\n for help. \n");
+    ErrorFSigSafe("\nPlease consult the " XVENDORNAME " support \n\t at "
+                 __VENDORDWEBSUPPORT__ "\n for help. \n");
 #endif
     if (xf86LogFile && xf86LogFileWasOpened)
-        ErrorF("Please also check the log file at \"%s\" for additional "
-               "information.\n", xf86LogFile);
-    ErrorF("\n");
+        ErrorFSigSafe("Please also check the log file at \"%s\" for additional "
+                     "information.\n", xf86LogFile);
+    ErrorFSigSafe("\n");
 }
 
 int
commit d51aebdbf99a9f240f7c318a70ba40e61cd43049
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Apr 9 08:30:50 2012 -0700

    Log in LoaderUnload() in a signal safe manner
    
    The function may be called from a fatal signal handler.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/loader/loader.c b/hw/xfree86/loader/loader.c
index b72b8b8..edaefb8 100644
--- a/hw/xfree86/loader/loader.c
+++ b/hw/xfree86/loader/loader.c
@@ -163,7 +163,7 @@ LoaderSymbol(const char *name)
 void
 LoaderUnload(const char *name, void *handle)
 {
-    xf86Msg(X_INFO, "Unloading %s\n", name);
+    LogMessageVerbSigSafe(X_INFO, 1, "Unloading %s\n", name);
     if (handle)
         dlclose(handle);
 }
commit c3e1168778ec20beeac9979dc57e36400c70dd63
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Apr 9 08:28:17 2012 -0700

    Log in UnloadModuleOrDriver() in a signal safe manner
    
    The function may be called from a fatal signal handler.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 706b9b3..72020a5 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -1093,9 +1093,10 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
         return;
 
     if (mod->parent)
-        xf86MsgVerb(X_INFO, 3, "UnloadSubModule: \"%s\"\n", mod->name);
+        LogMessageVerbSigSafe(X_INFO, 3, "UnloadSubModule: \"%s\"\n",
+                              mod->name);
     else
-        xf86MsgVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name);
+        LogMessageVerbSigSafe(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name);
 
     if (mod->TearDownData != ModuleDuplicated) {
         if ((mod->TearDownProc) && (mod->TearDownData))
commit 89e3ac07aca1def155299aff6f7a57ccafb68fd7
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Apr 9 08:23:32 2012 -0700

    Log safely in fatal signal handler
    
    While we probably don't need to be signal safe here since we will never
    return to the normal context, the logging signal context check will
    cause unsafe logging to be unhandled. Using signal safe logging here
    resolves the issue.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/os/osinit.c b/os/osinit.c
index e2a2208..6cc0401 100644
--- a/os/osinit.c
+++ b/os/osinit.c
@@ -113,7 +113,7 @@ OsSigHandler(int signo)
     const char *dlerr = dlerror();
 
     if (dlerr) {
-        LogMessage(X_ERROR, "Dynamic loader error: %s\n", dlerr);
+        LogMessageVerbSigSafe(X_ERROR, 1, "Dynamic loader error: %s\n", dlerr);
     }
 #endif                          /* RTLD_DI_SETSIGNAL */
 
@@ -129,8 +129,8 @@ OsSigHandler(int signo)
 
 #ifdef SA_SIGINFO
     if (sip->si_code == SI_USER) {
-        ErrorF("Recieved signal %d sent by process %ld, uid %ld\n",
-               signo, (long) sip->si_pid, (long) sip->si_uid);
+        ErrorFSigSafe("Recieved signal %u sent by process %u, uid %u\n", signo,
+                     sip->si_pid, sip->si_uid);
     }
     else {
         switch (signo) {
@@ -138,7 +138,7 @@ OsSigHandler(int signo)
         case SIGBUS:
         case SIGILL:
         case SIGFPE:
-            ErrorF("%s at address %p\n", strsignal(signo), sip->si_addr);
+            ErrorFSigSafe("%s at address %p\n", strsignal(signo), sip->si_addr);
         }
     }
 #endif
commit 6fd5add005d0660b591d808583d1a6c6a85f1277
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 16:17:41 2012 -0700

    Log mieq enqueue overflow in a signal safe manner
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/mi/mieq.c b/mi/mieq.c
index e117a8d..b2c7769 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -276,23 +276,22 @@ mieqEnqueue(DeviceIntPtr pDev, InternalEvent *e)
          */
         miEventQueue.dropped++;
         if (miEventQueue.dropped == 1) {
-            ErrorF
-                ("[mi] EQ overflowing.  Additional events will be discarded until existing events are processed.\n");
+            ErrorFSigSafe("[mi] EQ overflowing.  Additional events will be "
+                         "discarded until existing events are processed.\n");
             xorg_backtrace();
-            ErrorF
-                ("[mi] These backtraces from mieqEnqueue may point to a culprit higher up the stack.\n");
-            ErrorF("[mi] mieq is *NOT* the cause.  It is a victim.\n");
+            ErrorFSigSafe("[mi] These backtraces from mieqEnqueue may point to "
+                         "a culprit higher up the stack.\n");
+            ErrorFSigSafe("[mi] mieq is *NOT* the cause.  It is a victim.\n");
         }
         else if (miEventQueue.dropped % QUEUE_DROP_BACKTRACE_FREQUENCY == 0 &&
                  miEventQueue.dropped / QUEUE_DROP_BACKTRACE_FREQUENCY <=
                  QUEUE_DROP_BACKTRACE_MAX) {
-            ErrorF
-                ("[mi] EQ overflow continuing.  %lu events have been dropped.\n",
-                 miEventQueue.dropped);
+            ErrorFSigSafe("[mi] EQ overflow continuing.  %u events have been "
+                         "dropped.\n", miEventQueue.dropped);
             if (miEventQueue.dropped / QUEUE_DROP_BACKTRACE_FREQUENCY ==
                 QUEUE_DROP_BACKTRACE_MAX) {
-                ErrorF
-                    ("[mi] No further overflow reports will be reported until the clog is cleared.\n");
+                ErrorFSigSafe("[mi] No further overflow reports will be "
+                             "reported until the clog is cleared.\n");
             }
             xorg_backtrace();
         }
commit 7f4a69b628a6246855054a0b94d6d6dd14e8842c
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 13 16:01:38 2012 -0700

    Log messages in TouchBeginDDXTouch() in a signal-safe manner
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/touch.c b/dix/touch.c
index aa17faf..a01f152 100644
--- a/dix/touch.c
+++ b/dix/touch.c
@@ -198,8 +198,9 @@ TouchBeginDDXTouch(DeviceIntPtr dev, uint32_t ddx_id)
     /* If we get here, then we've run out of touches and we need to drop the
      * event (we're inside the SIGIO handler here) schedule a WorkProc to
      * grow the queue for us for next time. */
-    ErrorF("%s: not enough space for touch events (max %d touchpoints). "
-           "Dropping this event.\n", dev->name, dev->last.num_touches);
+    ErrorFSigSafe("%s: not enough space for touch events (max %u touchpoints). "
+                  "Dropping this event.\n", dev->name, dev->last.num_touches);
+
     if (!BitIsOn(resize_waiting, dev->id)) {
         SetBit(resize_waiting, dev->id);
         QueueWorkProc(TouchResizeQueue, serverClient, NULL);
commit f752226e40890643df213a62f0c96e6a0243e754
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 10:21:14 2012 -0700

    Log messages in GetTouchEvents() in a signal safe manner
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/getevents.c b/dix/getevents.c
index baa26c4..3fa3a1e 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -1839,8 +1839,8 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
         touchpoint.ti =
             TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin));
         if (!touchpoint.ti) {
-            ErrorF("[dix] %s: unable to %s touch point %x\n", dev->name,
-                   type == XI_TouchBegin ? "begin" : "find", ddx_touchid);
+            ErrorFSigSafe("[dix] %s: unable to %s touch point %u\n", dev->name,
+                          type == XI_TouchBegin ? "begin" : "find", ddx_touchid);
             return 0;
         }
         client_id = touchpoint.ti->client_id;
commit 82d1c6b310eaa5095eed9ee4ea958261a46a78e1
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 08:32:28 2012 -0700

    Warn when attempting to log in a signal unsafe manner from signal context
    
    Also, print out the offending message format. This will hopefully help
    developers track down unsafe logging.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/os/log.c b/os/log.c
index 47ba348..25da9f6 100644
--- a/os/log.c
+++ b/os/log.c
@@ -463,6 +463,16 @@ LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
     Bool newline;
     size_t len = 0;
 
+    if (inSignalContext) {
+        BUG_WARN_MSG(inSignalContext,
+                     "Warning: attempting to log data in a signal unsafe "
+                     "manner while in signal context. Please update to check "
+                     "inSignalContext and/or use LogMessageVerbSigSafe() or "
+                     "ErrorFSigSafe(). The offending log format message is:\n"
+                     "%s\n", format);
+        return;
+    }
+
     type_str = LogMessageTypeVerbString(type, verb);
     if (!type_str)
         return;
@@ -552,6 +562,16 @@ LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
     Bool newline;
     size_t len = 0;
 
+    if (inSignalContext) {
+        BUG_WARN_MSG(inSignalContext,
+                     "Warning: attempting to log data in a signal unsafe "
+                     "manner while in signal context. Please update to check "
+                     "inSignalContext and/or use LogMessageVerbSigSafe(). The "
+                     "offending header and log message formats are:\n%s %s\n",
+                     hdr_format, msg_format);
+        return;
+    }
+
     type_str = LogMessageTypeVerbString(type, verb);
     if (!type_str)
         return;
commit 512bec06be6c79ca263da9de8f40430b8095b57b
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Apr 16 09:47:42 2012 -0700

    Make BUG_WARN* signal safe
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/include/misc.h b/include/misc.h
index 6ae020a..aa62f6a 100644
--- a/include/misc.h
+++ b/include/misc.h
@@ -371,10 +371,10 @@ extern _X_EXPORT unsigned long serverGeneration;
 /* Don't use this directly, use BUG_WARN or BUG_WARN_MSG instead */
 #define __BUG_WARN_MSG(cond, with_msg, ...)                                \
           do { if (cond) {                                                \
-              ErrorF("BUG: triggered 'if (" #cond ")'\n");                \
-              ErrorF("BUG: %s:%d in %s()\n",                              \
-                      __FILE__, __LINE__, __func__);                      \
-              if (with_msg) ErrorF(__VA_ARGS__);                          \
+              ErrorFSigSafe("BUG: triggered 'if (" #cond ")'\n");          \
+              ErrorFSigSafe("BUG: %s:%u in %s()\n",                        \
+                           __FILE__, __LINE__, __func__);                 \
+              if (with_msg) ErrorFSigSafe(__VA_ARGS__);                    \
               xorg_backtrace();                                           \
           } } while(0)
 
commit 0fa5217836cf7fd3872fccc9f3ff9ff32426c25b
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 16:09:05 2012 -0700

    Print backtrace in a signal-safe manner
    
    Backtraces are often printed in signal context, such as when a segfault
    occurs.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    
    os: print offset as unsigned int, not long unsigned int
    
    pnprintf() takes unsigned int for %u
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/os/backtrace.c b/os/backtrace.c
index 81348f4..daac60c 100644
--- a/os/backtrace.c
+++ b/os/backtrace.c
@@ -45,29 +45,37 @@ xorg_backtrace(void)
     int size, i;
     Dl_info info;
 
-    ErrorF("\n");
-    ErrorF("Backtrace:\n");
+    ErrorFSigSafe("\n");
+    ErrorFSigSafe("Backtrace:\n");
     size = backtrace(array, 64);
     for (i = 0; i < size; i++) {
         int rc = dladdr(array[i], &info);
 
         if (rc == 0) {
-            ErrorF("%d: ?? [%p]\n", i, array[i]);
+            ErrorFSigSafe("%u: ?? [%p]\n", i, array[i]);
             continue;
         }
         mod = (info.dli_fname && *info.dli_fname) ? info.dli_fname : "(vdso)";
         if (info.dli_saddr)
-            ErrorF("%d: %s (%s+0x%lx) [%p]\n", i, mod,
-                   info.dli_sname,
-                   (long unsigned int) ((char *) array[i] -
-                                        (char *) info.dli_saddr), array[i]);
+            ErrorFSigSafe(
+                "%u: %s (%s+0x%x) [%p]\n",
+                i,
+                mod,
+                info.dli_sname,
+                (unsigned int)((char *) array[i] -
+                               (char *) info.dli_saddr),
+                array[i]);
         else
-            ErrorF("%d: %s (%p+0x%lx) [%p]\n", i, mod,
-                   info.dli_fbase,
-                   (long unsigned int) ((char *) array[i] -
-                                        (char *) info.dli_fbase), array[i]);
+            ErrorFSigSafe(
+                "%u: %s (%p+0x%x) [%p]\n",
+                i,
+                mod,
+                info.dli_fbase,
+                (unsigned int)((char *) array[i] -
+                               (char *) info.dli_fbase),
+                array[i]);
     }
-    ErrorF("\n");
+    ErrorFSigSafe("\n");
 }
 
 #else                           /* not glibc or glibc < 2.1 */
@@ -105,7 +113,7 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg)
             strcpy(signame, "unknown");
         }
 
-        ErrorF("** Signal %d (%s)\n", signo, signame);
+        ErrorFSigSafe("** Signal %u (%s)\n", signo, signame);
     }
 
     snprintf(header, sizeof(header), "%d: 0x%lx", depth, pc);
@@ -123,7 +131,8 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg)
             symname = "<section start>";
             offset = pc - (uintptr_t) dlinfo.dli_fbase;
         }
-        ErrorF("%s: %s:%s+0x%lx\n", header, dlinfo.dli_fname, symname, offset);
+        ErrorFSigSafe("%s: %s:%s+0x%x\n", header, dlinfo.dli_fname, symname,
+                     offset);
 
     }
     else {
@@ -131,7 +140,7 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg)
          * probably poke elfloader here, but haven't written that code yet,
          * so we just print the pc.
          */
-        ErrorF("%s\n", header);
+        ErrorFSigSafe("%s\n", header);
     }
 
     return 0;
@@ -183,7 +192,7 @@ xorg_backtrace_pstack(void)
 
             if (bytesread > 0) {
                 btline[bytesread] = 0;
-                ErrorF("%s", btline);
+                ErrorFSigSafe("%s", btline);
             }
             else if ((bytesread < 0) || ((errno != EINTR) && (errno != EAGAIN)))
                 done = 1;
@@ -203,8 +212,8 @@ void
 xorg_backtrace(void)
 {
 
-    ErrorF("\n");
-    ErrorF("Backtrace:\n");
+    ErrorFSigSafe("\n");
+    ErrorFSigSafe("Backtrace:\n");
 
 #ifdef HAVE_PSTACK
 /* First try fork/exec of pstack - otherwise fall back to walkcontext
@@ -221,9 +230,9 @@ xorg_backtrace(void)
             walkcontext(&u, xorg_backtrace_frame, &depth);
         else
 #endif
-            ErrorF("Failed to get backtrace info: %s\n", strerror(errno));
+            ErrorFSigSafe("Failed to get backtrace info: %s\n", strerror(errno));
     }
-    ErrorF("\n");
+    ErrorFSigSafe("\n");
 }
 
 #else
commit ac20815d5235e7a8e7b331365aabf5a489fc5e34
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Tue Jun 5 15:39:41 2012 +1000

    Add ErrorFSigSafe() alternative to ErrorF()
    
    ErrorF() is not signal safe. Use ErrorSigSafe() whenever an error
    message may be logged in signal context.
    
    [whot: edited to "ErrorFSigSafe"]
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/include/os.h b/include/os.h
index b3f9088..e93c48a 100644
--- a/include/os.h
+++ b/include/os.h
@@ -656,6 +656,12 @@ extern _X_EXPORT void
 ErrorF(const char *f, ...)
 _X_ATTRIBUTE_PRINTF(1, 2);
 extern _X_EXPORT void
+VErrorFSigSafe(const char *f, va_list args)
+_X_ATTRIBUTE_PRINTF(1, 0);
+extern _X_EXPORT void
+ErrorFSigSafe(const char *f, ...)
+_X_ATTRIBUTE_PRINTF(1, 2);
+extern _X_EXPORT void
 LogPrintMarkers(void);
 
 extern _X_EXPORT void
diff --git a/os/log.c b/os/log.c
index f1a6e3b..47ba348 100644
--- a/os/log.c
+++ b/os/log.c
@@ -780,6 +780,22 @@ ErrorF(const char *f, ...)
 }
 
 void
+VErrorFSigSafe(const char *f, va_list args)
+{
+    LogVMessageVerbSigSafe(X_ERROR, -1, f, args);
+}
+
+void
+ErrorFSigSafe(const char *f, ...)
+{
+    va_list args;
+
+    va_start(args, f);
+    VErrorFSigSafe(f, args);
+    va_end(args);
+}
+
+void
 LogPrintMarkers(void)
 {
     /* Show what the message marker symbols mean. */
commit 164b38c72fe9c69d13ea4f9c46d4ccc46566d826
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 08:28:40 2012 -0700

    Add LogMessageVerbSigSafe() for logging messages while in signal context
    
    [whot: edited to use varargs, squashed commit below]
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    
    os: fix vararg length calculation
    
    Make %u and %x sizeof(unsigned int), %p sizeof(void*). This is printf
    behaviour and we can't guarantee that void* is uint64_t anyway.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Chase Douglas <chase.douglas at canonical.com>

diff --git a/include/os.h b/include/os.h
index 276eb52..b3f9088 100644
--- a/include/os.h
+++ b/include/os.h
@@ -49,6 +49,7 @@ SOFTWARE.
 
 #include "misc.h"
 #include <stdarg.h>
+#include <stdint.h>
 #include <string.h>
 
 #define SCREEN_SAVER_ON   0
@@ -604,6 +605,12 @@ _X_ATTRIBUTE_PRINTF(3, 4);
 extern _X_EXPORT void
 LogMessage(MessageType type, const char *format, ...)
 _X_ATTRIBUTE_PRINTF(2, 3);
+extern _X_EXPORT void
+LogMessageVerbSigSafe(MessageType type, int verb, const char *format, ...)
+_X_ATTRIBUTE_PRINTF(3, 4);
+extern _X_EXPORT void
+LogVMessageVerbSigSafe(MessageType type, int verb, const char *format, va_list args)
+_X_ATTRIBUTE_PRINTF(3, 0);
 
 extern _X_EXPORT void
 LogVHdrMessageVerb(MessageType type, int verb,
diff --git a/os/log.c b/os/log.c
index 5394847..f1a6e3b 100644
--- a/os/log.c
+++ b/os/log.c
@@ -172,6 +172,14 @@ asm(".desc ___crashreporter_info__, 0x10");
 #define X_NONE_STRING			""
 #endif
 
+static size_t
+strlen_sigsafe(const char *s)
+{
+    size_t len;
+    for (len = 0; s[len]; len++);
+    return len;
+}
+
 /*
  * LogInit is called to start logging to a file.  It is also called (with
  * NULL arguments) when logging to a file is not wanted.  It must always be
@@ -271,16 +279,97 @@ LogSetParameter(LogParameter param, int value)
     }
 }
 
-/* This function does the actual log message writes. */
+static int
+pnprintf(char *string, size_t size, const char *f, va_list args)
+{
+    int f_idx = 0;
+    int s_idx = 0;
+    int f_len = strlen_sigsafe(f);
+    char *string_arg;
+    char number[21];
+    int p_len;
+    int i;
+    uint64_t ui;
+
+    for (; f_idx < f_len && s_idx < size - 1; f_idx++) {
+        if (f[f_idx] != '%') {
+            string[s_idx++] = f[f_idx];
+            continue;
+        }
+
+        switch (f[++f_idx]) {
+        case 's':
+            string_arg = va_arg(args, char*);
+            p_len = strlen_sigsafe(string_arg);
+
+            for (i = 0; i < p_len && s_idx < size - 1; i++)
+                string[s_idx++] = string_arg[i];
+            break;
+
+        case 'u':
+            ui = va_arg(args, unsigned);
+            FormatUInt64(ui, number);
+            p_len = strlen_sigsafe(number);
+
+            for (i = 0; i < p_len && s_idx < size - 1; i++)
+                string[s_idx++] = number[i];
+            break;
+
+        case 'p':
+            string[s_idx++] = '0';
+            if (s_idx < size - 1)
+                string[s_idx++] = 'x';
+            ui = (uintptr_t)va_arg(args, void*);
+            FormatUInt64Hex(ui, number);
+            p_len = strlen_sigsafe(number);
+
+            for (i = 0; i < p_len && s_idx < size - 1; i++)
+                string[s_idx++] = number[i];
+            break;
+
+        case 'x':
+            ui = va_arg(args, unsigned);
+            FormatUInt64Hex(ui, number);
+            p_len = strlen_sigsafe(number);
+
+            for (i = 0; i < p_len && s_idx < size - 1; i++)
+                string[s_idx++] = number[i];
+            break;
+
+        default:
+            va_arg(args, char*);
+            string[s_idx++] = '%';
+            if (s_idx < size - 1)
+                string[s_idx++] = f[f_idx];
+            break;
+        }
+    }
+
+    string[s_idx] = '\0';
+
+    return s_idx;
+}
+
+/* This function does the actual log message writes. It must be signal safe.
+ * When attempting to call non-signal-safe functions, guard them with a check
+ * of the inSignalContext global variable. */
 static void
 LogSWrite(int verb, const char *buf, size_t len, Bool end_line)
 {
     static Bool newline = TRUE;
 
     if (verb < 0 || logVerbosity >= verb)
-        fwrite(buf, len, 1, stderr);
+        write(2, buf, len);
+
     if (verb < 0 || logFileVerbosity >= verb) {
-        if (logFile) {
+        if (inSignalContext && logFileFd >= 0) {
+            write(logFileFd, buf, len);
+#ifdef WIN32
+            if (logFlush && logSync)
+                fsync(logFileFd);
+#endif
+        }
+        else if (!inSignalContext && logFile) {
             if (newline)
                 fprintf(logFile, "[%10.3f] ", GetTimeInMillis() / 1000.0);
             newline = end_line;
@@ -293,7 +382,7 @@ LogSWrite(int verb, const char *buf, size_t len, Bool end_line)
 #endif
             }
         }
-        else if (needBuffer) {
+        else if (!inSignalContext && needBuffer) {
             if (len > bufferUnused) {
                 bufferSize += 1024;
                 bufferUnused += 1024;
@@ -415,6 +504,44 @@ LogMessage(MessageType type, const char *format, ...)
     va_end(ap);
 }
 
+/* Log a message using only signal safe functions. */
+void
+LogMessageVerbSigSafe(MessageType type, int verb, const char *format, ...)
+{
+    va_list ap;
+    va_start(ap, format);
+    LogVMessageVerbSigSafe(type, verb, format, ap);
+    va_end(ap);
+}
+
+void
+LogVMessageVerbSigSafe(MessageType type, int verb, const char *format, va_list args)
+{
+    const char *type_str;
+    char buf[1024];
+    int len;
+    Bool newline;
+
+    type_str = LogMessageTypeVerbString(type, verb);
+    if (!type_str)
+        return;
+
+    /* if type_str is not "", prepend it and ' ', to message */
+    if (type_str[0] != '\0') {
+        LogSWrite(verb, type_str, strlen_sigsafe(type_str), FALSE);
+        LogSWrite(verb, " ", 1, FALSE);
+    }
+
+    len = pnprintf(buf, sizeof(buf), format, args);
+
+    /* Force '\n' at end of truncated line */
+    if (sizeof(buf) - len == 1)
+        buf[len - 1] = '\n';
+
+    newline = (buf[len - 1] == '\n');
+    LogSWrite(verb, buf, len, newline);
+}
+
 void
 LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
                    va_list msg_args, const char *hdr_format, va_list hdr_args)
commit 704b847abfd29e9adde27127a15a963414f8bcf4
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 10:13:45 2012 -0700

    Add FormatUInt64{,Hex}() for formatting numbers in a signal safe manner
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/include/misc.h b/include/misc.h
index fea74b8..6ae020a 100644
--- a/include/misc.h
+++ b/include/misc.h
@@ -229,6 +229,8 @@ pad_to_int32(const int bytes)
 }
 
 extern char **xstrtokenize(const char *str, const char *separators);
+extern void FormatUInt64(uint64_t num, char *string);
+extern void FormatUInt64Hex(uint64_t num, char *string);
 
 /**
  * Compare the two version numbers comprising of major.minor.
diff --git a/os/utils.c b/os/utils.c
index 2f07548..998c3ed 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -1793,3 +1793,47 @@ xstrtokenize(const char *str, const char *separators)
     free(list);
     return NULL;
 }
+
+/* Format a number into a string in a signal safe manner. The string should be
+ * at least 21 characters in order to handle all uint64_t values. */
+void
+FormatUInt64(uint64_t num, char *string)
+{
+    uint64_t divisor;
+    int len;
+    int i;
+
+    for (len = 1, divisor = 10;
+         len < 20 && num / divisor;
+         len++, divisor *= 10);
+
+    for (i = len, divisor = 1; i > 0; i--, divisor *= 10)
+        string[i - 1] = '0' + ((num / divisor) % 10);
+
+    string[len] = '\0';
+}
+
+/* Format a number into a hexadecimal string in a signal safe manner. The string
+ * should be at least 17 characters in order to handle all uint64_t values. */
+void
+FormatUInt64Hex(uint64_t num, char *string)
+{
+    uint64_t divisor;
+    int len;
+    int i;
+
+    for (len = 1, divisor = 0x10;
+         len < 16 && num / divisor;
+         len++, divisor *= 0x10);
+
+    for (i = len, divisor = 1; i > 0; i--, divisor *= 0x10) {
+        int val = (num / divisor) % 0x10;
+
+        if (val < 10)
+            string[i - 1] = '0' + val;
+        else
+            string[i - 1] = 'a' + val - 10;
+    }
+
+    string[len] = '\0';
+}
diff --git a/test/.gitignore b/test/.gitignore
index 5d4fdfa..27eb254 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -7,3 +7,4 @@ touch
 xfree86
 xkb
 xtest
+signal-logging
diff --git a/test/Makefile.am b/test/Makefile.am
index b2a53aa..c049b26 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -5,7 +5,7 @@ if XORG
 # Tests that require at least some DDX functions in order to fully link
 # For now, requires xf86 ddx, could be adjusted to use another
 SUBDIRS += xi2
-noinst_PROGRAMS += xkb input xtest misc fixes xfree86  hashtabletest
+noinst_PROGRAMS += xkb input xtest misc fixes xfree86 hashtabletest signal-logging
 endif
 check_LTLIBRARIES = libxservertest.la
 
@@ -36,6 +36,7 @@ misc_LDADD=$(TEST_LDADD)
 fixes_LDADD=$(TEST_LDADD)
 xfree86_LDADD=$(TEST_LDADD)
 touch_LDADD=$(TEST_LDADD)
+signal_logging_LDADD=$(TEST_LDADD)
 hashtabletest_LDADD=$(TEST_LDADD) $(top_srcdir)/Xext/hashtable.c
 
 libxservertest_la_LIBADD = $(XSERVER_LIBS)
diff --git a/test/signal-logging.c b/test/signal-logging.c
new file mode 100644
index 0000000..8aab0dd
--- /dev/null
+++ b/test/signal-logging.c
@@ -0,0 +1,115 @@
+/**
+ * Copyright © 2012 Canonical, Ltd.
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a
+ *  copy of this software and associated documentation files (the "Software"),
+ *  to deal in the Software without restriction, including without limitation
+ *  the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ *  and/or sell copies of the Software, and to permit persons to whom the
+ *  Software is furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice (including the next
+ *  paragraph) shall be included in all copies or substantial portions of the
+ *  Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ *  THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ *  DEALINGS IN THE SOFTWARE.
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include <dix-config.h>
+#endif
+
+#include <stdint.h>
+#include "assert.h"
+#include "misc.h"
+
+struct number_format_test {
+    uint64_t number;
+    char string[21];
+    char hex_string[17];
+};
+
+static Bool
+check_number_format_test(const struct number_format_test *test)
+{
+    char string[21];
+
+    FormatUInt64(test->number, string);
+    if(strncmp(string, test->string, 21) != 0) {
+        fprintf(stderr, "Failed to convert %ju to decimal string (%s vs %s)\n",
+                test->number, test->string, string);
+        return FALSE;
+    }
+    FormatUInt64Hex(test->number, string);
+    if(strncmp(string, test->hex_string, 17) != 0) {
+        fprintf(stderr,
+                "Failed to convert %ju to hexadecimal string (%s vs %s)\n",
+                test->number, test->hex_string, string);
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+static Bool
+number_formatting(void)
+{
+    int i;
+    struct number_format_test tests[] = {
+        { /* Zero */
+            0,
+            "0",
+            "0",
+        },
+        { /* Single digit number */
+            5,
+            "5",
+            "5",
+        },
+        { /* Two digit decimal number */
+            12,
+            "12",
+            "c",
+        },
+        { /* Two digit hex number */
+            37,
+            "37",
+            "25",
+        },
+        { /* Large < 32 bit number */
+            0xC90B2,
+            "823474",
+            "c90b2",
+        },
+        { /* Large > 32 bit number */
+            0x15D027BF211B37A,
+            "98237498237498234",
+            "15d027bf211b37a",
+        },
+        { /* Maximum 64-bit number */
+            0xFFFFFFFFFFFFFFFF,
+            "18446744073709551615",
+            "ffffffffffffffff",
+        },
+    };
+
+    for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
+        if (!check_number_format_test(tests + i))
+            return FALSE;
+
+    return TRUE;
+}
+
+int
+main(int argc, char **argv)
+{
+    int ok = number_formatting();
+
+    return ok ? 0 : 1;
+}
commit bc85c81687a24aea738094ff11f4448fb3b3afbb
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 08:03:09 2012 -0700

    Save log file file descriptor for signal context logging
    
    None of the FILE based functions are signal safe, including fileno(), so
    we need to save the file descriptor for when we are in signal context.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/os/log.c b/os/log.c
index 2c13c1a..5394847 100644
--- a/os/log.c
+++ b/os/log.c
@@ -108,6 +108,7 @@ void (*OsVendorVErrorFProc) (const char *, va_list args) = NULL;
 #endif
 
 static FILE *logFile = NULL;
+static int logFileFd = -1;
 static Bool logFlush = FALSE;
 static Bool logSync = FALSE;
 static int logVerbosity = DEFAULT_LOG_VERBOSITY;
@@ -211,6 +212,8 @@ LogInit(const char *fname, const char *backup)
             FatalError("Cannot open log file \"%s\"\n", logFileName);
         setvbuf(logFile, NULL, _IONBF, 0);
 
+        logFileFd = fileno(logFile);
+
         /* Flush saved log information. */
         if (saveBuffer && bufferSize > 0) {
             fwrite(saveBuffer, bufferPos, 1, logFile);
@@ -243,6 +246,7 @@ LogClose(enum ExitCode error)
                (error == EXIT_NO_ERROR) ? "successfully" : "with error", error);
         fclose(logFile);
         logFile = NULL;
+        logFileFd = -1;
     }
 }
 
commit d3725549f0276487fba1d419094209d18e86669f
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Fri Apr 6 07:43:57 2012 -0700

    Add global variable inSignalContext
    
    This will be used for checking for proper logging when in signal
    context.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/os-support/shared/sigio.c b/hw/xfree86/os-support/shared/sigio.c
index 12ae8a4..231d6c0 100644
--- a/hw/xfree86/os-support/shared/sigio.c
+++ b/hw/xfree86/os-support/shared/sigio.c
@@ -99,6 +99,8 @@ xf86SIGIO(int sig)
     int save_errno = errno;     /* do not clobber the global errno */
     int r;
 
+    inSignalContext = TRUE;
+
     ready = xf86SigIOMask;
     to.tv_sec = 0;
     to.tv_usec = 0;
@@ -114,6 +116,8 @@ xf86SIGIO(int sig)
     }
     /* restore global errno */
     errno = save_errno;
+
+    inSignalContext = FALSE;
 }
 
 static int
diff --git a/include/globals.h b/include/globals.h
index 8076955..6d3f3d7 100644
--- a/include/globals.h
+++ b/include/globals.h
@@ -2,6 +2,8 @@
 #ifndef _XSERV_GLOBAL_H_
 #define _XSERV_GLOBAL_H_
 
+#include <signal.h>
+
 #include "window.h"             /* for WindowPtr */
 
 /* Global X server variables that are visible to mi, dix, os, and ddx */
@@ -23,6 +25,7 @@ extern _X_EXPORT int GrabInProgress;
 extern _X_EXPORT Bool noTestExtensions;
 extern _X_EXPORT char *SeatId;
 extern _X_EXPORT char *ConnectionInfo;
+extern _X_EXPORT sig_atomic_t inSignalContext;
 
 #ifdef DPMSExtension
 extern _X_EXPORT CARD32 DPMSStandbyTime;
diff --git a/os/utils.c b/os/utils.c
index 3a1ef93..2f07548 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -204,6 +204,8 @@ int auditTrailLevel = 1;
 
 char *SeatId = NULL;
 
+sig_atomic_t inSignalContext = FALSE;
+
 #if defined(SVR4) || defined(__linux__) || defined(CSRG_BASED)
 #define HAS_SAVED_IDS_AND_SETEUID
 #endif


More information about the xorg-commit mailing list