[Lldb-commits] [lldb] f78fcd6 - [lldb/Test] Rewrite ReproducerInstrumentationTest
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 10 16:50:51 PDT 2020
Author: Jonas Devlieghere
Date: 2020-04-10T16:50:44-07:00
New Revision: f78fcd6906a032bf5feb0e60c9e8991bf31e990e
URL: https://github.com/llvm/llvm-project/commit/f78fcd6906a032bf5feb0e60c9e8991bf31e990e
DIFF: https://github.com/llvm/llvm-project/commit/f78fcd6906a032bf5feb0e60c9e8991bf31e990e.diff
LOG: [lldb/Test] Rewrite ReproducerInstrumentationTest
The instrumentation unit tests' current implementation uses global
variables to track constructor calls for the instrumented classes during
replay. This is suboptimal because it indirectly relies on how the
reproducer instrumentation is implemented. I found out when adding
support for passive replay and the test broke because we made an extra
(temporary) copy of the instrumented objects.
Additionally, the old approach wasn't very self-explanatory. It took me
a bit of time to understand why we were expecting the number of objects
in the test.
This patch rewrites the test and uses the index-to-object-mapping to
verify the objects created during replay. You can now specify the
expected objects, in order, and whether they should be valid or not. I
find that it makes the tests much easier to understand. More
importantly, this approach is resilient to implementation detail changes
in the instrumentation.
Added:
Modified:
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/Utility/ReproducerInstrumentation.cpp
lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 2246267b62f0..1eae930da5ce 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -255,6 +255,9 @@ class IndexToObject {
const_cast<typename std::remove_const<T>::type *>(&object)));
}
+ /// Get all objects sorted by their index.
+ std::vector<void *> GetAllObjects() const;
+
private:
/// Helper method that does the actual lookup. The void* result is later cast
/// by the caller.
@@ -345,6 +348,10 @@ class Deserializer {
(void)result;
}
+ std::vector<void *> GetAllObjects() const {
+ return m_index_to_object.GetAllObjects();
+ }
+
private:
template <typename T> T Read(ValueTag) {
assert(HasData(sizeof(T)));
@@ -512,6 +519,9 @@ class Registry {
/// Replay functions from a buffer.
bool Replay(llvm::StringRef buffer);
+ /// Replay functions from a deserializer.
+ bool Replay(Deserializer &deserializer);
+
/// Returns the ID for a given function address.
unsigned GetID(uintptr_t addr);
diff --git a/lldb/source/Utility/ReproducerInstrumentation.cpp b/lldb/source/Utility/ReproducerInstrumentation.cpp
index 3bf81286bbe9..ec66da9973ae 100644
--- a/lldb/source/Utility/ReproducerInstrumentation.cpp
+++ b/lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -24,6 +24,25 @@ void IndexToObject::AddObjectForIndexImpl(unsigned idx, void *object) {
m_mapping[idx] = object;
}
+std::vector<void *> IndexToObject::GetAllObjects() const {
+ std::vector<std::pair<unsigned, void *>> pairs;
+ for (auto &e : m_mapping) {
+ pairs.emplace_back(e.first, e.second);
+ }
+
+ // Sort based on index.
+ std::sort(pairs.begin(), pairs.end(),
+ [](auto &lhs, auto &rhs) { return lhs.first < rhs.first; });
+
+ std::vector<void *> objects;
+ objects.reserve(pairs.size());
+ for (auto &p : pairs) {
+ objects.push_back(p.second);
+ }
+
+ return objects;
+}
+
template <> const uint8_t *Deserializer::Deserialize<const uint8_t *>() {
return Deserialize<uint8_t *>();
}
@@ -74,6 +93,11 @@ bool Registry::Replay(const FileSpec &file) {
}
bool Registry::Replay(llvm::StringRef buffer) {
+ Deserializer deserializer(buffer);
+ return Replay(deserializer);
+}
+
+bool Registry::Replay(Deserializer &deserializer) {
#ifndef LLDB_REPRO_INSTR_TRACE
Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_API);
#endif
@@ -82,7 +106,6 @@ bool Registry::Replay(llvm::StringRef buffer) {
// during an interactive session.
setvbuf(stdout, nullptr, _IONBF, 0);
- Deserializer deserializer(buffer);
while (deserializer.HasData(1)) {
unsigned id = deserializer.Deserialize<unsigned>();
diff --git a/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp b/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
index 490d188c83b7..d4b41d55967f 100644
--- a/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ b/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -54,9 +54,22 @@ static llvm::Optional<Serializer> g_serializer;
static llvm::Optional<TestingRegistry> g_registry;
#define LLDB_GET_INSTRUMENTATION_DATA() \
- g_serializer ? InstrumentationData(*g_serializer, *g_registry) : InstrumentationData()
+ g_serializer ? InstrumentationData(*g_serializer, *g_registry) \
+ : InstrumentationData()
-class InstrumentedFoo {
+enum class Class {
+ Foo,
+ Bar,
+};
+
+class Instrumented {
+public:
+ virtual ~Instrumented() = default;
+ virtual void Validate() = 0;
+ virtual bool IsA(Class c) = 0;
+};
+
+class InstrumentedFoo : public Instrumented {
public:
InstrumentedFoo() = default;
/// Instrumented methods.
@@ -70,8 +83,9 @@ class InstrumentedFoo {
int D(const char *d) const;
static void E(double e);
static int F();
- void Validate();
+ void Validate() override;
//// }
+ virtual bool IsA(Class c) override { return c == Class::Foo; }
private:
int m_a = 0;
@@ -83,7 +97,7 @@ class InstrumentedFoo {
mutable int m_called = 0;
};
-class InstrumentedBar {
+class InstrumentedBar : public Instrumented {
public:
/// Instrumented methods.
/// {
@@ -93,8 +107,9 @@ class InstrumentedBar {
InstrumentedFoo *GetInstrumentedFooPtr();
void SetInstrumentedFoo(InstrumentedFoo *foo);
void SetInstrumentedFoo(InstrumentedFoo &foo);
- void Validate();
+ void Validate() override;
/// }
+ virtual bool IsA(Class c) override { return c == Class::Bar; }
private:
bool m_get_instrumend_foo_called = false;
@@ -105,45 +120,47 @@ class InstrumentedBar {
double InstrumentedFoo::g_e = 0;
bool InstrumentedFoo::g_f = false;
-static std::vector<InstrumentedFoo *> g_foos;
-static std::vector<InstrumentedBar *> g_bars;
-
void ClearObjects() {
g_registry.reset();
g_serializer.reset();
- g_foos.clear();
- g_bars.clear();
}
-void ValidateObjects(size_t expected_foos, size_t expected_bars) {
- EXPECT_EQ(expected_foos, g_foos.size());
- EXPECT_EQ(expected_bars, g_bars.size());
-
- for (auto *foo : g_foos) {
- foo->Validate();
- }
+struct Validator {
+ enum Validation { valid, invalid };
+ Validator(Class clazz, Validation validation)
+ : clazz(clazz), validation(validation) {}
+ Class clazz;
+ Validation validation;
+};
- for (auto *bar : g_bars) {
- bar->Validate();
+void ValidateObjects(std::vector<void *> objects,
+ std::vector<Validator> validators) {
+ ASSERT_EQ(validators.size(), objects.size());
+ for (size_t i = 0; i < validators.size(); ++i) {
+ Validator &validator = validators[i];
+ Instrumented *instrumented = static_cast<Instrumented *>(objects[i]);
+ EXPECT_TRUE(instrumented->IsA(validator.clazz));
+ switch (validator.validation) {
+ case Validator::valid:
+ instrumented->Validate();
+ break;
+ case Validator::invalid:
+ break;
+ }
}
}
InstrumentedFoo::InstrumentedFoo(int i) {
LLDB_RECORD_CONSTRUCTOR(InstrumentedFoo, (int), i);
- g_foos.push_back(this);
}
InstrumentedFoo::InstrumentedFoo(const InstrumentedFoo &foo) {
LLDB_RECORD_CONSTRUCTOR(InstrumentedFoo, (const InstrumentedFoo &), foo);
- g_foos.erase(std::remove(g_foos.begin(), g_foos.end(), &foo));
- g_foos.push_back(this);
}
InstrumentedFoo &InstrumentedFoo::operator=(const InstrumentedFoo &foo) {
LLDB_RECORD_METHOD(InstrumentedFoo &,
InstrumentedFoo, operator=,(const InstrumentedFoo &), foo);
- g_foos.erase(std::remove(g_foos.begin(), g_foos.end(), &foo));
- g_foos.push_back(this);
return *this;
}
@@ -195,7 +212,6 @@ void InstrumentedFoo::Validate() {
InstrumentedBar::InstrumentedBar() {
LLDB_RECORD_CONSTRUCTOR_NO_ARGS(InstrumentedBar);
- g_bars.push_back(this);
}
InstrumentedFoo InstrumentedBar::GetInstrumentedFoo() {
@@ -242,7 +258,7 @@ void InstrumentedBar::Validate() {
}
TestingRegistry::TestingRegistry() {
- Registry& R = *this;
+ Registry &R = *this;
LLDB_REGISTER_CONSTRUCTOR(InstrumentedFoo, (int i));
LLDB_REGISTER_CONSTRUCTOR(InstrumentedFoo, (const InstrumentedFoo &));
@@ -525,9 +541,11 @@ TEST(RecordReplayTest, InstrumentedFoo) {
ClearObjects();
TestingRegistry registry;
- registry.Replay(os.str());
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
- ValidateObjects(1, 0);
+ ValidateObjects(deserializer.GetAllObjects(),
+ {{Class::Foo, Validator::valid}});
}
TEST(RecordReplayTest, InstrumentedFooSameThis) {
@@ -563,9 +581,11 @@ TEST(RecordReplayTest, InstrumentedFooSameThis) {
ClearObjects();
TestingRegistry registry;
- registry.Replay(os.str());
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
- ValidateObjects(2, 0);
+ ValidateObjects(deserializer.GetAllObjects(),
+ {{Class::Foo, Validator::valid}});
}
TEST(RecordReplayTest, InstrumentedBar) {
@@ -577,10 +597,6 @@ TEST(RecordReplayTest, InstrumentedBar) {
{
InstrumentedBar bar;
InstrumentedFoo foo = bar.GetInstrumentedFoo();
-#if 0
- InstrumentedFoo& foo_ref = bar.GetInstrumentedFooRef();
- InstrumentedFoo* foo_ptr = bar.GetInstrumentedFooPtr();
-#endif
int b = 200;
float c = 300.3f;
@@ -602,9 +618,16 @@ TEST(RecordReplayTest, InstrumentedBar) {
ClearObjects();
TestingRegistry registry;
- registry.Replay(os.str());
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
- ValidateObjects(1, 1);
+ ValidateObjects(
+ deserializer.GetAllObjects(),
+ {
+ {Class::Bar, Validator::valid}, // bar
+ {Class::Foo, Validator::invalid}, // bar.GetInstrumentedFoo()
+ {Class::Foo, Validator::valid}, // foo
+ });
}
TEST(RecordReplayTest, InstrumentedBarRef) {
@@ -637,9 +660,12 @@ TEST(RecordReplayTest, InstrumentedBarRef) {
ClearObjects();
TestingRegistry registry;
- registry.Replay(os.str());
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
- ValidateObjects(1, 1);
+ ValidateObjects(
+ deserializer.GetAllObjects(),
+ {{Class::Bar, Validator::valid}, {Class::Foo, Validator::valid}});
}
TEST(RecordReplayTest, InstrumentedBarPtr) {
@@ -672,7 +698,10 @@ TEST(RecordReplayTest, InstrumentedBarPtr) {
ClearObjects();
TestingRegistry registry;
- registry.Replay(os.str());
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
- ValidateObjects(1, 1);
+ ValidateObjects(
+ deserializer.GetAllObjects(),
+ {{Class::Bar, Validator::valid}, {Class::Foo, Validator::valid}});
}
More information about the lldb-commits
mailing list