https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/213100
>From 94cd2213e228e93621acb139dc12786968b33584 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 30 Jul 2026 19:43:04 +0100 Subject: [PATCH 1/2] [lldb] Don't assume a category dump order in TimeTest --- lldb/unittests/Utility/TimerTest.cpp | 71 +++++++++++++++++++--------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/lldb/unittests/Utility/TimerTest.cpp b/lldb/unittests/Utility/TimerTest.cpp index b371ebff4fe77..22c5c5c6920a8 100644 --- a/lldb/unittests/Utility/TimerTest.cpp +++ b/lldb/unittests/Utility/TimerTest.cpp @@ -6,13 +6,44 @@ // //===----------------------------------------------------------------------===// -#include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" +#include "lldb/Utility/StreamString.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "gtest/gtest.h" +#include <optional> +#include <string> #include <thread> using namespace lldb_private; +namespace { +struct CategoryStats { + double seconds; + double total; + double child; + int count; +}; + +/// Finds the line describing \p category in a DumpCategoryTimes() dump and +/// parses its statistics. +std::optional<CategoryStats> ParseCategory(llvm::StringRef dump, + llvm::StringRef category) { + const std::string suffix = " for " + category.str(); + for (llvm::StringRef line : llvm::split(dump, '\n')) { + if (!line.rtrim("\r").ends_with(suffix)) + continue; + CategoryStats stats; + if (sscanf(line.str().c_str(), + "%lf sec (total: %lfs; child: %lfs; count: %d)", &stats.seconds, + &stats.total, &stats.child, &stats.count) == 4) + return stats; + return std::nullopt; + } + return std::nullopt; +} +} // namespace + TEST(TimerTest, CategoryTimes) { Timer::ResetCategoryTimes(); { @@ -60,16 +91,14 @@ TEST(TimerTest, CategoryTimes2) { } StreamString ss; Timer::DumpCategoryTimes(ss); - double seconds1, seconds2; - ASSERT_EQ(2, sscanf(ss.GetData(), - "%lf sec (total: %*fs; child: %*fs; count: %*d) for " - "CAT1%*[\n ]%lf sec for CAT2", - &seconds1, &seconds2)) - << "String: " << ss.GetData(); - EXPECT_LT(0.01, seconds1); - EXPECT_GT(1, seconds1); - EXPECT_LT(0.001, seconds2); - EXPECT_GT(0.1, seconds2); + std::optional<CategoryStats> cat1 = ParseCategory(ss.GetString(), "CAT1"); + std::optional<CategoryStats> cat2 = ParseCategory(ss.GetString(), "CAT2"); + ASSERT_TRUE(cat1.has_value()) << "String: " << ss.GetData(); + ASSERT_TRUE(cat2.has_value()) << "String: " << ss.GetData(); + EXPECT_LT(0.01, cat1->seconds); + EXPECT_GT(1, cat1->seconds); + EXPECT_LT(0.001, cat2->seconds); + EXPECT_GT(0.1, cat2->seconds); } TEST(TimerTest, CategoryTimesStats) { @@ -93,16 +122,12 @@ TEST(TimerTest, CategoryTimesStats) { // 0.026772798 sec (total: 0.027s; child: 0.000s; count: 2) for CAT2 StreamString ss; Timer::DumpCategoryTimes(ss); - double seconds1, total1, child1, seconds2; - int count1, count2; - ASSERT_EQ( - 6, sscanf(ss.GetData(), - "%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n\r ]" - "%lf sec (total: %*fs; child: %*fs; count: %d) for CAT2", - &seconds1, &total1, &child1, &count1, &seconds2, &count2)) - << "String: " << ss.GetData(); - EXPECT_NEAR(total1 - child1, seconds1, 0.002); - EXPECT_EQ(1, count1); - EXPECT_NEAR(child1, seconds2, 0.002); - EXPECT_EQ(2, count2); + std::optional<CategoryStats> cat1 = ParseCategory(ss.GetString(), "CAT1"); + std::optional<CategoryStats> cat2 = ParseCategory(ss.GetString(), "CAT2"); + ASSERT_TRUE(cat1.has_value()) << "String: " << ss.GetData(); + ASSERT_TRUE(cat2.has_value()) << "String: " << ss.GetData(); + EXPECT_NEAR(cat1->total - cat1->child, cat1->seconds, 0.002); + EXPECT_EQ(1, cat1->count); + EXPECT_NEAR(cat1->child, cat2->seconds, 0.002); + EXPECT_EQ(2, cat2->count); } >From dbc3484c023720ad5097586aeb828f2623a942b9 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 12 Aug 2026 17:44:01 +0100 Subject: [PATCH 2/2] [lldb] Parse the timer dump with a regex instead of sscanf Match the whole line, including the category name, so a malformed line can't be silently accepted, and use getAsDouble/getAsInteger instead of sscanf's untyped varargs. --- lldb/unittests/Utility/TimerTest.cpp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lldb/unittests/Utility/TimerTest.cpp b/lldb/unittests/Utility/TimerTest.cpp index 22c5c5c6920a8..4b97cd14026d8 100644 --- a/lldb/unittests/Utility/TimerTest.cpp +++ b/lldb/unittests/Utility/TimerTest.cpp @@ -8,11 +8,12 @@ #include "lldb/Utility/Timer.h" #include "lldb/Utility/StreamString.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" #include "gtest/gtest.h" #include <optional> -#include <string> #include <thread> using namespace lldb_private; @@ -26,19 +27,25 @@ struct CategoryStats { }; /// Finds the line describing \p category in a DumpCategoryTimes() dump and -/// parses its statistics. +/// parses its statistics. A line looks like: +/// 0.105202764 sec (total: 0.132s; child: 0.027s; count: 1) for CAT1 std::optional<CategoryStats> ParseCategory(llvm::StringRef dump, llvm::StringRef category) { - const std::string suffix = " for " + category.str(); + llvm::Regex line_pattern(R"(^([0-9.]+) sec \(total: ([0-9.]+)s; )" + R"(child: ([0-9.]+)s; count: ([0-9]+)\) for (.+)$)"); for (llvm::StringRef line : llvm::split(dump, '\n')) { - if (!line.rtrim("\r").ends_with(suffix)) + llvm::SmallVector<llvm::StringRef, 6> matches; + if (!line_pattern.match(line.trim(), &matches)) + continue; + if (matches[5] != category) continue; CategoryStats stats; - if (sscanf(line.str().c_str(), - "%lf sec (total: %lfs; child: %lfs; count: %d)", &stats.seconds, - &stats.total, &stats.child, &stats.count) == 4) - return stats; - return std::nullopt; + if (matches[1].getAsDouble(stats.seconds) || + matches[2].getAsDouble(stats.total) || + matches[3].getAsDouble(stats.child) || + matches[4].getAsInteger(10, stats.count)) + return std::nullopt; + return stats; } return std::nullopt; } @@ -117,9 +124,6 @@ TEST(TimerTest, CategoryTimesStats) { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } } - // Example output: - // 0.105202764 sec (total: 0.132s; child: 0.027s; count: 1) for CAT1 - // 0.026772798 sec (total: 0.027s; child: 0.000s; count: 2) for CAT2 StreamString ss; Timer::DumpCategoryTimes(ss); std::optional<CategoryStats> cat1 = ParseCategory(ss.GetString(), "CAT1"); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
