Skip to content

Commit

Permalink
fix(libmain/common-args): don't overflow verbosity
Browse files Browse the repository at this point in the history
This patch gets rid of UB when verbosity overflows to invalid values (e.g. 8). This is actually triggered
in functional tests. There are too many occurrences to list, but here's one from the UBSAN log:

../src/libstore/gc.cc:610:5: runtime error: load of value 8, which is not a valid value for type 'Verbosity'

Verbosity is now a strong type instead of a plain enum (which is mostly indistinguishable from the underlying type).
The type still for all intents and purposes looks like an enumeration, can be used in switches, provides
static constexpr member types, but also has member functions.

This makes it very unlikely that we'll have such problems in the future, since now `Verbosity` has
a saturating contract for increment/decrement.

Regarding the implementation. This might look like witchcraft, but is completely legal.
I've tested that this code (the `Verbosity` static const member pattern) compiles when
going back to GCC 7.1.0, Clang 5.0.0 and MSVC 19.21.
  • Loading branch information
xokdvium committed Nov 11, 2024
1 parent 76cd80d commit fb75d27
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/libmain/common-args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ MixCommonArgs::MixCommonArgs(const std::string & programName)
.shortName = 'v',
.description = "Increase the logging verbosity level.",
.category = loggingCategory,
.handler = {[]() { verbosity = (Verbosity) (verbosity + 1); }},
.handler = {[]() { verbosity.increment(); }},
});

addFlag({
.longName = "quiet",
.description = "Decrease the logging verbosity level.",
.category = loggingCategory,
.handler = {[]() { verbosity = verbosity > lvlError ? (Verbosity) (verbosity - 1) : lvlError; }},
.handler = {[]() { verbosity.decrement(); }},
});

addFlag({
Expand Down
104 changes: 94 additions & 10 deletions src/libutil/error.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "suggestions.hh"
#include "fmt.hh"

#include <algorithm>
#include <cstring>
#include <list>
#include <memory>
Expand All @@ -29,17 +30,100 @@

namespace nix {

/**
* Strong type representing the logging verbosity level. For all intents and purposes this looks like an enum,
* but with member functions.
*/
class Verbosity
{
public:
using UnderlyingValueType = int;

private:
static constexpr UnderlyingValueType kDefaultValue = 0;

typedef enum {
lvlError = 0,
lvlWarn,
lvlNotice,
lvlInfo,
lvlTalkative,
lvlChatty,
lvlDebug,
lvlVomit
} Verbosity;
static constexpr Verbosity fromRaw(UnderlyingValueType rawVal) noexcept
{
auto level = Verbosity{};
level.raw = rawVal;
return level;
}

static constexpr UnderlyingValueType clampInRange(UnderlyingValueType toClamp) noexcept;

public:
constexpr Verbosity()
: raw(kDefaultValue)
{
}

constexpr Verbosity(UnderlyingValueType verbosityVal)
: raw(clampInRange(verbosityVal))
{
}

constexpr UnderlyingValueType asInt() const noexcept
{
return raw;
}

constexpr operator UnderlyingValueType() const noexcept
{
return raw;
}

/**
* Increase the logging verbosity by n levels. Saturates to the maximum
* allowed verbosity level.
*/
constexpr void increment(unsigned n = 1) noexcept
{
raw = clampInRange(raw + n);
}

/**
* Decrease the logging verbosity by n levels. Saturates to the minimum
* allowed verbosity level.
*/
constexpr void decrement(unsigned n = 1) noexcept
{
raw = clampInRange(raw - n);
}

constexpr std::strong_ordering operator<=>(const Verbosity & rhs) const noexcept = default;

static const Verbosity lvlError;
static const Verbosity lvlWarn;
static const Verbosity lvlNotice;
static const Verbosity lvlInfo;
static const Verbosity lvlTalkative;
static const Verbosity lvlChatty;
static const Verbosity lvlDebug;
static const Verbosity lvlVomit;

private:
UnderlyingValueType raw;
};

#define DEF_VERBOSITY_LEVEL(level_, value_) \
constexpr inline Verbosity Verbosity::level_ = Verbosity::fromRaw(value_); \
inline constexpr auto level_ = Verbosity::level_;

DEF_VERBOSITY_LEVEL(lvlError, kDefaultValue)
DEF_VERBOSITY_LEVEL(lvlWarn, 1)
DEF_VERBOSITY_LEVEL(lvlNotice, 2)
DEF_VERBOSITY_LEVEL(lvlInfo, 3)
DEF_VERBOSITY_LEVEL(lvlTalkative, 4)
DEF_VERBOSITY_LEVEL(lvlChatty, 5)
DEF_VERBOSITY_LEVEL(lvlDebug, 6)
DEF_VERBOSITY_LEVEL(lvlVomit, 7)

#undef DEF_VERBOSITY_LEVEL

inline constexpr Verbosity::UnderlyingValueType Verbosity::clampInRange(UnderlyingValueType toClamp) noexcept
{
return std::clamp(toClamp, lvlError.asInt(), lvlVomit.asInt());
}

/**
* The lines of code surrounding an error.
Expand Down
6 changes: 3 additions & 3 deletions src/libutil/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ struct JSONLogger : Logger {
{
nlohmann::json json;
json["action"] = "msg";
json["level"] = lvl;
json["level"] = lvl.asInt();
json["msg"] = s;
write(json);
}
Expand All @@ -209,7 +209,7 @@ struct JSONLogger : Logger {

nlohmann::json json;
json["action"] = "msg";
json["level"] = ei.level;
json["level"] = ei.level.asInt();
json["msg"] = oss.str();
json["raw_msg"] = ei.msg.str();
to_json(json, ei.pos);
Expand All @@ -235,7 +235,7 @@ struct JSONLogger : Logger {
nlohmann::json json;
json["action"] = "start";
json["id"] = act;
json["level"] = lvl;
json["level"] = lvl.asInt();
json["type"] = type;
json["text"] = s;
json["parent"] = parent;
Expand Down

0 comments on commit fb75d27

Please sign in to comment.