Convert _log to function to prevent optional argument evaluation

_log, being a macro, evaluated the variadic arguments only when the
log-level was allowing the print to happen. As a result, statements like
`log_debug("%d", something_with_side_effects())` didn't always lead to
evaluation of `something_with_side_effects()`, which could be expected,
as `log_*` uses the function-like naming convention.

Signed-off-by: Michał Kowalczyk <[email protected]>

LibOS/shim/include/shim_internal.h

@@ -46,11 +46,9 @@ void debug_puts(const char* str);
 void debug_putch(int ch);
 void debug_vprintf(const char* fmt, va_list ap) __attribute__((format(printf, 1, 0)));
 
-#define _log(level, fmt...)                          \
-    do {                                             \
-        if ((level) <= g_log_level)                  \
-            debug_printf(fmt);                       \
-    } while (0)
+// TODO(mkow): We should make it cross-object-inlinable, ideally by enabling LTO, less ideally by
+// pasting it here and making `inline`, but our current linker scripts prevent both.
+void _log(int level, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
 
 #define log_error(fmt...)    _log(PAL_LOG_ERROR, fmt)
 #define log_warning(fmt...)  _log(PAL_LOG_WARNING, fmt)

LibOS/shim/src/utils/printf.c

@@ -142,3 +142,12 @@ void debug_setprefix(shim_tcb_t* tcb) {
 
     buf->start = buf->end;
 }
+
+void _log(int level, const char* fmt, ...) {
+    if (level <= g_log_level) {
+        va_list ap;
+        va_start(ap, fmt);
+        debug_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}

Pal/include/pal/pal.h

@@ -146,7 +146,7 @@ typedef struct PAL_CONTROL_ {
     PAL_STR executable;          /*!< initial executable name. TODO: remove from PAL */
     PAL_HANDLE parent_process;   /*!< handle of parent process */
     PAL_HANDLE first_thread;     /*!< handle of first thread */
-    PAL_NUM log_level;           /*!< what log messsages to enable */
+    int log_level;               /*!< what log messages to enable */
 
     /*
      * Memory layout

Pal/include/pal/pal_debug.h

@@ -11,7 +11,6 @@
 #include "pal.h"
 
 int pal_printf(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
-int pal_fdprintf(int fd, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
 void warn(const char* format, ...);
 
 void DkDebugMapAdd(PAL_STR uri, PAL_PTR start_addr);

Pal/src/db_rtld.c

@@ -23,6 +23,11 @@ struct link_map* g_exec_map = NULL;
 
 struct link_map* lookup_symbol(const char* undef_name, ElfW(Sym)** ref);
 
+/* err - positive value of error code */
+static inline void print_error(const char* msg, int err) {
+    printf("%s (%s)\n", msg, pal_strerror(err));
+}
+
 /* This macro is used as a callback from the ELF_DYNAMIC_RELOCATE code.  */
 static struct link_map* resolve_map(const char** strtab, ElfW(Sym)** ref) {
     if (ELFW(ST_BIND)((*ref)->st_info) != STB_LOCAL) {

Pal/src/host/Linux-SGX/sgx_graphene.c

@@ -9,6 +9,7 @@
 #include "pal_debug.h"
 #include "pal_error.h"
 #include "sgx_internal.h"
+#include "sgx_log.h"
 
 #define PRINTBUF_SIZE 256
 
@@ -41,17 +42,6 @@ static int vfdprintf(int fd, const char* fmt, va_list ap) {
     return b.cnt;
 }
 
-int pal_fdprintf(int fd, const char* fmt, ...) {
-    va_list ap;
-    int cnt;
-
-    va_start(ap, fmt);
-    cnt = vfdprintf(fd, fmt, ap);
-    va_end(ap);
-
-    return cnt;
-}
-
 int pal_printf(const char* fmt, ...) {
     va_list ap;
     int cnt;
@@ -62,3 +52,12 @@ int pal_printf(const char* fmt, ...) {
 
     return cnt;
 }
+
+void _urts_log(int level, const char* fmt, ...) {
+    if (level <= g_urts_log_level) {
+        va_list ap;
+        va_start(ap, fmt);
+        vfdprintf(g_urts_log_fd, fmt, ap);
+        va_end(ap);
+    }
+}

Pal/src/host/Linux-SGX/sgx_log.h

@@ -20,11 +20,9 @@ extern int g_urts_log_fd;
 int urts_log_init(const char* path);
 int urts_log_printf(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
 
-#define _urts_log(level, fmt...)                          \
-    do {                                                 \
-        if ((level) <= g_urts_log_level)                  \
-            pal_fdprintf(g_urts_log_fd, fmt);             \
-    }  while(0)
+// TODO(mkow): We should make it cross-object-inlinable, ideally by enabling LTO, less ideally by
+// pasting it here and making `inline`, but our current linker scripts prevent both.
+void _urts_log(int level, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
 
 #define urts_log_error(fmt...)    _urts_log(PAL_LOG_ERROR, fmt)
 #define urts_log_warning(fmt...)  _urts_log(PAL_LOG_WARNING, fmt)

Pal/src/pal_internal.h

@@ -304,23 +304,14 @@ ssize_t _DkDebugLog(const void* buf, size_t size);
 void _DkPrintConsole(const void* buf, int size);
 int printf(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
 int vprintf(const char* fmt, va_list ap) __attribute__((format(printf, 1, 0)));
-int log_printf(const char* fmt, ...) __attribute__((format(printf, 1, 2)));
-int log_vprintf(const char* fmt, va_list ap) __attribute__((format(printf, 1, 0)));
 
-/* err - positive value of error code */
-static inline void print_error(const char* msg, int err) {
-    printf("%s (%s)\n", msg, pal_strerror(err));
-}
+// TODO(mkow): We should make it cross-object-inlinable, ideally by enabling LTO, less ideally by
+// pasting it here and making `inline`, but our current linker scripts prevent both.
+void _log(int level, const char* fmt, ...) __attribute__((format(printf, 2, 3)));
 
 #define PAL_LOG_DEFAULT_LEVEL  PAL_LOG_ERROR
 #define PAL_LOG_DEFAULT_FD     2
 
-#define _log(level, fmt...)                          \
-    do {                                             \
-        if ((level) <= g_pal_control.log_level)      \
-            log_printf(fmt);                         \
-    }  while(0)
-
 #define log_error(fmt...)    _log(PAL_LOG_ERROR, fmt)
 #define log_warning(fmt...)  _log(PAL_LOG_WARNING, fmt)
 #define log_debug(fmt...)    _log(PAL_LOG_DEBUG, fmt)

Pal/src/printf.c

@@ -4,8 +4,6 @@
 #include "api.h"
 #include "pal_internal.h"
 
-#ifndef NO_INTERNAL_PRINTF
-
 // Collect up to PRINTBUF_SIZE characters into a buffer
 // and perform ONE system call to print all of them,
 // in order to make the lines output to the console atomic
@@ -33,6 +31,7 @@ static int fputch(void* f, int ch, void* putdat) {
     return 0;
 }
 
+__attribute__((format(printf, 1, 0)))
 int vprintf(const char* fmt, va_list ap) {
     struct printbuf b;
 
@@ -44,7 +43,7 @@ int vprintf(const char* fmt, va_list ap) {
     return b.cnt;
 }
 
-int log_vprintf(const char* fmt, va_list ap) {
+static int log_vprintf(const char* fmt, va_list ap) {
     struct printbuf b;
 
     b.idx = 0;
@@ -67,14 +66,11 @@ int printf(const char* fmt, ...) {
 }
 EXTERN_ALIAS(printf);
 
-int log_printf(const char* fmt, ...) {
-    va_list ap;
-    int cnt;
-
-    va_start(ap, fmt);
-    cnt = log_vprintf(fmt, ap);
-    va_end(ap);
-
-    return cnt;
+void _log(int level, const char* fmt, ...) {
+    if (level <= g_pal_control.log_level) {
+        va_list ap;
+        va_start(ap, fmt);
+        log_vprintf(fmt, ap);
+        va_end(ap);
+    }
 }
-#endif