From 8c43e33e35b9906a15a3054226dc9591720a52b5 Mon Sep 17 00:00:00 2001 From: iamamyth Date: Wed, 19 Feb 2025 11:20:53 -0800 Subject: [PATCH] logging: Fix easylogging++ init with blank config The upstream version of el::base::TypedConfigurations::unsafeGetConfigByRef accesses uninitialized memory if a key doesn't exist. Commit b2c59af84de8d35c1eee38878053206a62756968 patched the library to throw in this case, avoiding the invalid access, but the more suitable pattern, both logically, and as evidenced by the behavior of unsafeGetConfigByVal, would be to return a const reference to a default-initialized value with static storage duration. --- external/easylogging++/easylogging++.h | 7 ++++--- tests/unit_tests/logging.cpp | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/external/easylogging++/easylogging++.h b/external/easylogging++/easylogging++.h index b983a796c..7a72b67f5 100644 --- a/external/easylogging++/easylogging++.h +++ b/external/easylogging++/easylogging++.h @@ -1993,7 +1993,7 @@ class TypedConfigurations : public base::threading::ThreadSafe { } template - inline Conf_T& getConfigByRef(Level level, std::unordered_map* confMap, const char* confName) { + inline const Conf_T& getConfigByRef(Level level, std::unordered_map* confMap, const char* confName) { base::threading::ScopedLock scopedLock(lock()); return unsafeGetConfigByRef(level, confMap, confName); // This is not unsafe anymore - mutex locked in scope } @@ -2016,8 +2016,9 @@ class TypedConfigurations : public base::threading::ThreadSafe { } template - Conf_T& unsafeGetConfigByRef(Level level, std::unordered_map* confMap, const char* confName) { + const Conf_T& unsafeGetConfigByRef(Level level, std::unordered_map* confMap, const char* confName) { ELPP_UNUSED(confName); + static const Conf_T empty; typename std::unordered_map::iterator it = confMap->find(level); if (it == confMap->end()) { try { @@ -2026,7 +2027,7 @@ class TypedConfigurations : public base::threading::ThreadSafe { ELPP_INTERNAL_ERROR("Unable to get configuration [" << confName << "] for level [" << LevelHelper::convertToString(level) << "]" << std::endl << "Please ensure you have properly configured logger.", false); - throw; // The exception has to be rethrown, to abort a branch leading to UB. + return empty; } } return it->second; diff --git a/tests/unit_tests/logging.cpp b/tests/unit_tests/logging.cpp index b13919831..599e5ee7a 100644 --- a/tests/unit_tests/logging.cpp +++ b/tests/unit_tests/logging.cpp @@ -209,9 +209,10 @@ TEST(logging, operator_equals_segfault) log2 = log1; } -TEST(logging, empty_configurations_throws) +TEST(logging, empty_configuration) { el::Logger log1("id1", nullptr); const el::Configurations cfg; - EXPECT_ANY_THROW(log1.configure(cfg)); + EXPECT_NO_THROW(log1.configure(cfg)); + EXPECT_EQ(log1.typedConfigurations()->filename(el::Level::Info), ""); }