misc: Rework the logging functions.

Removed the "verbose" switch which wasn't used.

Replaced the "get(LogLevel)" function with a get for each level. The
parameter was always constant, so we can just call the right function
at the right time.

Made the "exit" behavior of panic/fatal a part of the logging
implementation so that it can be overridden, and corrected a comment
which said that both fatal and panic called ::abort().

Got rid of the printEpilogue function by reworking the print() methods.
The subclasses of Logger can now override a "log" function which takes
a composed message, letting the Logger class centralize how the message
is put together and leaving the actual output mechanism to the
subclass.

Unfortunately there wasn't a way to tell gcc that the panic/fatal
macros wouldn't return, so there needed to be an exit_helper wrapper
function which calls the actual logger exit function. That can be
marked as noreturn, unlike the virtual exit function. If the exit
function does return, the wrapper will call ::abort(), placating gcc
and ensuring that even if exit isn't implemented properly, exit_helper
will still not return. That also provides a handy default
implementation.

Change-Id: I66d0cebd59f1127db980f3b565dbdf60687d8862
Reviewed-on: https://gem5-review.googlesource.com/6263
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>

diff --git a/src/base/logging.cc b/src/base/logging.cc
index cec9c8360daee0e2fbd045827cba14293b0d2188..adc3deb90ed4cc08ac07426f28dc399b9000cebd 100644 (file)
--- a/src/base/logging.cc
+++ b/src/base/logging.cc
@@ -43,80 +43,55 @@
 
 #include "base/logging.hh"
 
-#include <array>
+#include <sstream>
 
 #include "base/hostinfo.hh"
-#include "base/output.hh"
-#include "base/trace.hh"
-#include "base/types.hh"
-#include "sim/core.hh"
 
-void
-Logger::setLevel(LogLevel ll)
-{
-    for (int i = 0; i < NUM_LOG_LEVELS; ++i)
-        get(LogLevel(i)).enabled = (i <= ll);
-}
+namespace {
 
-static void
-newline_if_needed(std::ostream &stream, const char *format)
+class NormalLogger : public Logger
 {
-    const size_t format_len(strlen(format));
-
-    switch (format_len ? format[format_len - 1] : '\0') {
-      case '\n':
-      case '\r':
-        break;
-      default:
-        stream << std::endl;
-    }
-}
+  public:
+    using Logger::Logger;
 
-Logger::Logger(std::ostream &_stream, const char *_prefix)
-    : enabled(true), verbose(false), stream(_stream), prefix(_prefix)
-{
-}
+  protected:
+    void log(const Loc &loc, std::string s) override { std::cerr << s; }
+};
 
-void
-Logger::printEpilogue(const char *func, const char *file, int line,
-                        const char *format)
+class ExitLogger : public NormalLogger
 {
-    newline_if_needed(stream, format);
+  public:
+    using NormalLogger::NormalLogger;
 
-    if (verbose) {
-        ccprintf(stream, " @ tick %d\n[%s:%s, line %d]\n",
-                 curTick(), func, file, line);
+  protected:
+    void
+    log(const Loc &loc, std::string s) override
+    {
+        std::stringstream ss;
+        ccprintf(ss, "Memory Usage: %ld KBytes\n", memUsage());
+        NormalLogger::log(loc, s + ss.str());
     }
-}
+};
 
-class ExitLogger : public Logger
+class FatalLogger : public ExitLogger
 {
   public:
-    using Logger::Logger;
+    using ExitLogger::ExitLogger;
 
-    void printEpilogue(const char *func, const char *file, int line,
-                       const char *format) override;
+  protected:
+    void exit() override { ::exit(1); }
 };
 
-void
-ExitLogger::printEpilogue(const char *func, const char *file, int line,
-                            const char *format)
-{
-    Logger::printEpilogue(func, file, line, format);
-
-    ccprintf(stream, "Memory Usage: %ld KBytes\n", memUsage());
-}
+ExitLogger panicLogger("panic: ");
+FatalLogger fatalLogger("fatal: ");
+NormalLogger warnLogger("warn: ");
+NormalLogger infoLogger("info: ");
+NormalLogger hackLogger("hack: ");
 
-Logger &
-Logger::get(LogLevel ll)
-{
-    static std::array<Logger *, NUM_LOG_LEVELS> loggers{{
-        new ExitLogger(std::cerr, "panic"),
-        new ExitLogger(std::cerr, "fatal"),
-        new Logger(std::cerr, "warn"),
-        new Logger(std::cerr, "info"),
-        new Logger(std::cerr, "hack"),
-    }};
+} // anonymous namespace
 
-    return *loggers[ll];
-}
+Logger &Logger::getPanic() { return panicLogger; }
+Logger &Logger::getFatal() { return fatalLogger; }
+Logger &Logger::getWarn() { return warnLogger; }
+Logger &Logger::getInfo() { return infoLogger; }
+Logger &Logger::getHack() { return hackLogger; }

diff --git a/src/base/logging.hh b/src/base/logging.hh
index b0a9b0c1bde1a07a02091bd96d3192f31cf4943a..1c504d22e517451f890531cf62bc635a9dcc8345 100644 (file)
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -46,113 +46,133 @@
 #define __BASE_LOGGING_HH__
 
 #include <cassert>
-#include <cstdlib>
-#include <iostream>
+#include <sstream>
 #include <utility>
 
 #include "base/compiler.hh"
 #include "base/cprintf.hh"
 
-#if defined(__SUNPRO_CC)
-#define __FUNCTION__ "how to fix me?"
-#endif
-
 class Logger
 {
   public:
+
+    // Get a Logger for the specified type of message.
+    static Logger &getPanic();
+    static Logger &getFatal();
+    static Logger &getWarn();
+    static Logger &getInfo();
+    static Logger &getHack();
+
     enum LogLevel {
-        PANIC = 0,
-        FATAL,
-        WARN,
-        INFO,
-        HACK,
+        PANIC, FATAL, WARN, INFO, HACK,
         NUM_LOG_LEVELS,
     };
 
-    /**
-     * Set the active log level.
-     *
-     * All levels that are lower or equal to the selected log level
-     * will be activated.
-     *
-     * @param ll Maximum log level to print
-     */
-    static void setLevel(LogLevel ll);
+    static void
+    setLevel(LogLevel ll)
+    {
+        getPanic().enabled = (ll >= PANIC);
+        getFatal().enabled = (ll >= FATAL);
+        getWarn().enabled = (ll >= WARN);
+        getInfo().enabled = (ll >= INFO);
+        getHack().enabled = (ll >= HACK);
+    }
+
+    struct Loc
+    {
+        Loc(const char *file, int line) : file(file), line(line) {}
+        const char *file;
+        int line;
+    };
 
-    /**
-     * Get a Logger corresponding to a specific log level
-     *
-     * @param ll Log level to access
-     * @return Reference to the requested logger
-     */
-    static Logger &get(LogLevel ll);
+    Logger(const char *prefix) : enabled(true), prefix(prefix)
+    {
+        assert(prefix);
+    }
 
-  public:
-    Logger(std::ostream &stream, const char *prefix);
     virtual ~Logger() {};
 
-    template<typename ...Args> void
-    print(const char *func, const char *file, int line,
-          const char *format, const Args &...args)
+    void
+    print(const Loc &loc, const std::string &str)
     {
+        std::stringstream ss;
+        ss << prefix << str;
+        if (str.length() && str.back() != '\n' && str.back() != '\r')
+            ss << std::endl;
         if (!enabled)
             return;
+        log(loc, ss.str());
+    }
 
-        if (prefix)
-            stream << prefix << ": ";
-        ccprintf(stream, format, args...);
-
-        printEpilogue(func, file, line, format);
+    template<typename ...Args> void
+    print(const Loc &loc, const char *format, const Args &...args)
+    {
+        std::stringstream ss;
+        ccprintf(ss, format, args...);
+        print(loc, ss.str());
     }
 
     template<typename ...Args> void
-    print(const char *func, const char *file, int line,
-          const std::string &format, const Args &...args)
+    print(const Loc &loc, const std::string &format, const Args &...args)
     {
-        print(func, file, line, format.c_str(), args...);
+        print(loc, format.c_str(), args...);
     }
 
-  protected:
-    virtual void printEpilogue(const char *func, const char *file, int line,
-                               const char *format);
+    // This helper is necessary since noreturn isn't inherited by virtual
+    // functions, and gcc will get mad if a function calls panic and then
+    // doesn't return.
+    void exit_helper() M5_ATTR_NORETURN { exit(); ::abort(); }
 
-  public:
+  protected:
     bool enabled;
-    bool verbose;
 
-  protected:
-    std::ostream &stream;
+    virtual void log(const Loc &loc, std::string s) = 0;
+    virtual void exit() { /* Fall through to the abort in exit_helper. */ }
+
     const char *prefix;
 };
 
-#define exit_message(logger, code, ...)                                 \
-    do {                                                                \
-        logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__);    \
-        if (code < 0)                                                   \
-            ::abort();                                                  \
-        else                                                            \
-            ::exit(code);                                               \
+
+#define base_message(logger, ...) \
+    logger.print(::Logger::Loc(__FILE__, __LINE__), __VA_ARGS__)
+
+/*
+ * Only print the message the first time this expression is
+ * encountered.  i.e.  This doesn't check the string itself and
+ * prevent duplicate strings, this prevents the statement from
+ * happening more than once. So, even if the arguments change and that
+ * would have resulted in a different message thoes messages would be
+ * supressed.
+ */
+#define base_message_once(...) do {                     \
+        static bool once = false;                       \
+        if (!once) {                                    \
+            base_message(__VA_ARGS__);                  \
+            once = true;                                \
+        }                                               \
+    } while (0)
+
+#define exit_message(logger, ...)                       \
+    do {                                                \
+        base_message(logger, __VA_ARGS__);              \
+        logger.exit_helper();                           \
     } while (0)
 
-//
-// This implements a cprintf based panic() function.  panic() should
-// be called when something happens that should never ever happen
-// regardless of what the user does (i.e., an acutal m5 bug).  panic()
-// calls abort which can dump core or enter the debugger.
-//
-//
-#define panic(...) exit_message(::Logger::get(::Logger::PANIC), -1, \
-                                __VA_ARGS__)
+/**
+ * This implements a cprintf based panic() function.  panic() should
+ * be called when something happens that should never ever happen
+ * regardless of what the user does (i.e., an acutal m5 bug).  panic()
+ * might call abort which can dump core or enter the debugger.
+ */
+#define panic(...) exit_message(::Logger::getPanic(), __VA_ARGS__)
 
-//
-// This implements a cprintf based fatal() function.  fatal() should
-// be called when the simulation cannot continue due to some condition
-// that is the user's fault (bad configuration, invalid arguments,
-// etc.) and not a simulator bug.  fatal() calls  abort() like
-// panic() does.
-//
-#define fatal(...) exit_message(::Logger::get(::Logger::FATAL), 1, \
-                                __VA_ARGS__)
+/**
+ * This implements a cprintf based fatal() function.  fatal() should
+ * be called when the simulation cannot continue due to some condition
+ * that is the user's fault (bad configuration, invalid arguments,
+ * etc.) and not a simulator bug.  fatal() might call exit, unlike panic().
+ */
+#define fatal(...) exit_message(::Logger::getFatal(), __VA_ARGS__)
 
 /**
  * Conditional panic macro that checks the supplied condition and only panics
@@ -189,37 +209,13 @@ class Logger
     } while (0)
 
 
-#define base_message(logger, ...)                                       \
-    logger.print(__FUNCTION__, __FILE__, __LINE__, __VA_ARGS__)
-
-// Only print the message the first time this expression is
-// encountered.  i.e.  This doesn't check the string itself and
-// prevent duplicate strings, this prevents the statement from
-// happening more than once. So, even if the arguments change and that
-// would have resulted in a different message thoes messages would be
-// supressed.
-#define base_message_once(...) do {                     \
-        static bool once = false;                       \
-        if (!once) {                                    \
-            base_message(__VA_ARGS__);                  \
-            once = true;                                \
-        }                                               \
-    } while (0)
-
-
-#define warn(...) \
-    base_message(::Logger::get(::Logger::WARN), __VA_ARGS__)
-#define inform(...) \
-    base_message(::Logger::get(::Logger::INFO), __VA_ARGS__)
-#define hack(...) \
-    base_message(::Logger::get(::Logger::HACK), __VA_ARGS__)
+#define warn(...) base_message(::Logger::getWarn(), __VA_ARGS__)
+#define inform(...) base_message(::Logger::getInfo(), __VA_ARGS__)
+#define hack(...) base_message(::Logger::getHack(), __VA_ARGS__)
 
-#define warn_once(...) \
-    base_message_once(::Logger::get(::Logger::WARN), __VA_ARGS__)
-#define inform_once(...) \
-    base_message_once(::Logger::get(::Logger::INFO), __VA_ARGS__)
-#define hack_once(...) \
-    base_message_once(::Logger::get(::Logger::HACK), __VA_ARGS__)
+#define warn_once(...) base_message_once(::Logger::getWarn(), __VA_ARGS__)
+#define inform_once(...) base_message_once(::Logger::getInfo(), __VA_ARGS__)
+#define hack_once(...) base_message_once(::Logger::getHack(), __VA_ARGS__)
 
 /**
  * Conditional warning macro that checks the supplied condition and