derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

base: Unit tests miscellaneous patches (#73)

Browse Source 
* base: Fix Memoizer constructor parameter type

* base: switch from new to mk_unq in amo.test.cc

* base: Fix memory management in IniFile

* base: Fix memory management in Trie

* sim: Fix out-of-bounds access in CheckpointIn::setDir

Change-Id: Iac50bbf01b6d7acc458c786da8ac371582a4ce09

---------

Co-authored-by: Gabriel Busnot <gabriel.busnot@arteris.com>
This commit is contained in:
Gabriel Busnot
2023-07-19 17:45:29 +02:00
committed by GitHub
parent 4d9bd7dedf
commit 4c4419296b
6 changed files with 135 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
src/base/amo.test.cc
View File

@@ -64,9 +64,10 @@ TEST(AmoTest, AtomicOpMin)
std::string test_string_smaller = "apple";
std::string test_string_bigger = "cat";
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpMin<int>(10);
TypedAtomicOpFunctor<std::string> *amo_op_string =
new AtomicOpMin<std::string>("base");
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpMin<int>>(10);
std::unique_ptr<TypedAtomicOpFunctor<std::string>> amo_op_string =
std::make_unique<AtomicOpMin<std::string>>("base");
amo_op_int->execute(&test_int_smaller);
amo_op_int->execute(&test_int_bigger);
amo_op_string->execute(&test_string_smaller);
@@ -85,9 +86,10 @@ TEST(AmoTest, AtomicOpMax)
std::string test_string_smaller = "apple";
std::string test_string_bigger = "cat";
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpMax<int>(10);
TypedAtomicOpFunctor<std::string> *amo_op_string =
new AtomicOpMax<std::string>("base");
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpMax<int>>(10);
std::unique_ptr<TypedAtomicOpFunctor<std::string>> amo_op_string =
std::make_unique<AtomicOpMax<std::string>>("base");
amo_op_int->execute(&test_int_smaller);
amo_op_int->execute(&test_int_bigger);
amo_op_string->execute(&test_string_smaller);
@@ -104,8 +106,10 @@ TEST(AmoTest, AtomicOpDec)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpDec<int>();
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpDec<char>();
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpDec<int>>();
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpDec<char>>();
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -118,8 +122,10 @@ TEST(AmoTest, AtomicOpInc)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpInc<int>();
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpInc<char>();
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpInc<int>>();
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpInc<char>>();
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -132,8 +138,10 @@ TEST(AmoTest, AtomicOpSub)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpSub<int>(2);
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpSub<char>('a');
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpSub<int>>(2);
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpSub<char>>('a');
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -146,8 +154,10 @@ TEST(AmoTest, AtomicOpAdd)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpAdd<int>(2);
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpAdd<char>(2);
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpAdd<int>>(2);
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpAdd<char>>(2);
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -160,8 +170,10 @@ TEST(AmoTest, AtomicOpExch)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpExch<int>(2);
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpExch<char>('a');
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpExch<int>>(2);
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpExch<char>>('a');
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -174,8 +186,10 @@ TEST(AmoTest, AtomicOpXor)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpXor<int>(2);
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpXor<char>('a');
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpXor<int>>(2);
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpXor<char>>('a');
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -188,8 +202,10 @@ TEST(AmoTest, AtomicOpOr)
int test_int = 8;
bool test_bool = true;
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpOr<int>(2);
TypedAtomicOpFunctor<bool> *amo_op_bool = new AtomicOpOr<bool>(false);
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpOr<int>>(2);
std::unique_ptr<TypedAtomicOpFunctor<bool>> amo_op_bool =
std::make_unique<AtomicOpOr<bool>>(false);
amo_op_int->execute(&test_int);
amo_op_bool->execute(&test_bool);
@@ -202,8 +218,10 @@ TEST(AmoTest, AtomicOpAnd)
int test_int = 10;
char test_char = 'c';
TypedAtomicOpFunctor<int> *amo_op_int = new AtomicOpAnd<int>(6);
TypedAtomicOpFunctor<char> *amo_op_char = new AtomicOpAnd<char>('a');
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicOpAnd<int>>(6);
std::unique_ptr<TypedAtomicOpFunctor<char>> amo_op_char =
std::make_unique<AtomicOpAnd<char>>('a');
amo_op_int->execute(&test_int);
amo_op_char->execute(&test_char);
@@ -215,8 +233,8 @@ TEST(AmoTest, AtomicGeneric2Op)
{
int test_int = 9;
TypedAtomicOpFunctor<int> *amo_op_int =
new AtomicGeneric2Op<int>(9, multiply2Op);
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicGeneric2Op<int>>(9, multiply2Op);
amo_op_int->execute(&test_int);
EXPECT_EQ(test_int, 81);
@@ -226,8 +244,8 @@ TEST(AmoTest, AtomicGeneric3Op)
{
int test_int = 2;
TypedAtomicOpFunctor<int> *amo_op_int =
new AtomicGeneric3Op<int>(4, 3, multiply3Op);
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicGeneric3Op<int>>(4, 3, multiply3Op);
amo_op_int->execute(&test_int);
EXPECT_EQ(test_int, 24);
@@ -239,8 +257,8 @@ TEST(AmoTest, AtomicGenericPair3Op)
std::array<int, 2> a = {6, 3};
std::array<int, 2> c = {10, 8};
TypedAtomicOpFunctor<int> *amo_op_int =
new AtomicGenericPair3Op<int>(a, c, addSubColumns);
std::unique_ptr<TypedAtomicOpFunctor<int>> amo_op_int =
std::make_unique<AtomicGenericPair3Op<int>>(a, c, addSubColumns);
amo_op_int->execute(&test_int);
EXPECT_EQ(test_int, 10);

105
src/base/inifile.cc
View File

@@ -42,17 +42,6 @@ namespace gem5
IniFile::IniFile()
{}
IniFile::~IniFile()
{
SectionTable::iterator i = table.begin();
SectionTable::iterator end = table.end();
while (i != end) {
delete (*i).second;
++i;
}
}
bool
IniFile::load(const std::string &file)
{
@@ -82,15 +71,15 @@ IniFile::Section::addEntry(const std::string &entryName,
if (ei == table.end()) {
// new entry
table[entryName] = new Entry(value);
table.emplace(entryName, value);
}
else if (append) {
// append new reult to old entry
ei->second->appendValue(value);
ei->second.appendValue(value);
}
else {
// override old entry
ei->second->setValue(value);
ei->second.setValue(value);
}
}
@@ -120,39 +109,42 @@ IniFile::Section::add(const std::string &assignment)
IniFile::Entry *
IniFile::Section::findEntry(const std::string &entryName)
{
return const_cast<IniFile::Entry *>(
std::as_const(*this).findEntry(entryName));
}
const IniFile::Entry *
IniFile::Section::findEntry(const std::string &entryName) const
{
referenced = true;
EntryTable::const_iterator ei = table.find(entryName);
auto ei = table.find(entryName);
return (ei == table.end()) ? NULL : ei->second;
return (ei == table.end()) ? nullptr : &ei->second;
}
IniFile::Section *
IniFile::addSection(const std::string &sectionName)
{
SectionTable::iterator i = table.find(sectionName);
if (i != table.end()) {
return i->second;
}
else {
// new entry
Section *sec = new Section();
table[sectionName] = sec;
return sec;
}
return &table[sectionName];
}
IniFile::Section *
IniFile::findSection(const std::string &sectionName)
{
return const_cast<IniFile::Section*>(
std::as_const(*this).findSection(sectionName));
}
const IniFile::Section *
IniFile::findSection(const std::string &sectionName) const
{
SectionTable::const_iterator i = table.find(sectionName);
auto i = table.find(sectionName);
return (i == table.end()) ? NULL : i->second;
return (i == table.end()) ? nullptr : &i->second;
}
@@ -215,11 +207,11 @@ bool
IniFile::find(const std::string &sectionName, const std::string &entryName,
std::string &value) const
{
Section *section = findSection(sectionName);
auto* section = findSection(sectionName);
if (section == NULL)
return false;
Entry *entry = section->findEntry(entryName);
auto* entry = section->findEntry(entryName);
if (entry == NULL)
return false;
@@ -232,7 +224,7 @@ bool
IniFile::entryExists(const std::string &sectionName,
const std::string &entryName) const
{
Section *section = findSection(sectionName);
auto* section = findSection(sectionName);
if (!section)
return false;
@@ -248,13 +240,13 @@ IniFile::sectionExists(const std::string &sectionName) const
bool
IniFile::Section::printUnreferenced(const std::string &sectionName)
IniFile::Section::printUnreferenced(const std::string &sectionName) const
{
bool unref = false;
bool search_unref_entries = false;
std::vector<std::string> unref_ok_entries;
Entry *entry = findEntry("unref_entries_ok");
auto* entry = findEntry("unref_entries_ok");
if (entry != NULL) {
tokenize(unref_ok_entries, entry->getValue(), ' ');
if (unref_ok_entries.size()) {
@@ -262,10 +254,9 @@ IniFile::Section::printUnreferenced(const std::string &sectionName)
}
}
for (EntryTable::iterator ei = table.begin();
ei != table.end(); ++ei) {
const std::string &entryName = ei->first;
entry = ei->second;
for (auto& ei: table) {
const std::string &entryName = ei.first;
entry = &ei.second;
if (entryName == "unref_section_ok" ||
entryName == "unref_entries_ok")
@@ -294,32 +285,29 @@ IniFile::Section::printUnreferenced(const std::string &sectionName)
void
IniFile::getSectionNames(std::vector<std::string> &list) const
{
for (SectionTable::const_iterator i = table.begin();
i != table.end(); ++i)
{
list.push_back((*i).first);
for (auto& entry: table) {
auto& sectionName = entry.first;
list.push_back(sectionName);
}
}
bool
IniFile::printUnreferenced()
IniFile::printUnreferenced() const
{
bool unref = false;
for (SectionTable::iterator i = table.begin();
i != table.end(); ++i) {
const std::string &sectionName = i->first;
Section *section = i->second;
for (auto& entry: table) {
auto& [sectionName, section] = entry;
if (!section->isReferenced()) {
if (section->findEntry("unref_section_ok") == NULL) {
if (!section.isReferenced()) {
if (section.findEntry("unref_section_ok") == NULL) {
std::cerr << "Section " << sectionName << " not referenced."
<< std::endl;
unref = true;
}
}
else {
if (section->printUnreferenced(sectionName)) {
if (section.printUnreferenced(sectionName)) {
unref = true;
}
}
@@ -330,12 +318,11 @@ IniFile::printUnreferenced()
void
IniFile::Section::dump(const std::string &sectionName)
IniFile::Section::dump(const std::string &sectionName) const
{
for (EntryTable::iterator ei = table.begin();
ei != table.end(); ++ei) {
std::cout << sectionName << ": " << (*ei).first << " => "
<< (*ei).second->getValue() << "\n";
for (auto& ei: table) {
std::cout << sectionName << ": " << ei.first << " => "
<< ei.second.getValue() << "\n";
}
}
@@ -344,7 +331,7 @@ IniFile::dump()
{
for (SectionTable::iterator i = table.begin();
i != table.end(); ++i) {
i->second->dump(i->first);
i->second.dump(i->first);
}
}
@@ -364,9 +351,9 @@ void
IniFile::visitSection(const std::string &sectionName,
IniFile::VisitSectionCallback cb)
{
const auto& section = *table.at(sectionName);
const auto& section = table.at(sectionName);
for (const auto& pair : section) {
cb(pair.first, pair.second->getValue());
cb(pair.first, pair.second.getValue());
}
}

23
src/base/inifile.hh
View File

@@ -72,7 +72,7 @@ class IniFile
}
/// Has this entry been used?
bool isReferenced() { return referenced; }
bool isReferenced() const { return referenced; }
/// Fetch the value.
const std::string &getValue() const;
@@ -94,7 +94,7 @@ class IniFile
class Section
{
/// EntryTable type. Map of strings to Entry object pointers.
typedef std::unordered_map<std::string, Entry *> EntryTable;
typedef std::unordered_map<std::string, Entry> EntryTable;
EntryTable table; ///< Table of entries.
mutable bool referenced; ///< Has this section been used?
@@ -107,7 +107,7 @@ class IniFile
}
/// Has this section been used?
bool isReferenced() { return referenced; }
bool isReferenced() const { return referenced; }
/// Add an entry to the table. If an entry with the same name
/// already exists, the 'append' parameter is checked If true,
@@ -125,24 +125,25 @@ class IniFile
/// Find the entry with the given name.
/// @retval Pointer to the entry object, or NULL if none.
Entry *findEntry(const std::string &entryName) const;
Entry *findEntry(const std::string &entryName);
const Entry *findEntry(const std::string &entryName) const;
/// Print the unreferenced entries in this section to cerr.
/// Messages can be suppressed using "unref_section_ok" and
/// "unref_entries_ok".
/// @param sectionName Name of this section, for use in output message.
/// @retval True if any entries were printed.
bool printUnreferenced(const std::string &sectionName);
bool printUnreferenced(const std::string &sectionName) const;
/// Print the contents of this section to cout (for debugging).
void dump(const std::string &sectionName);
void dump(const std::string &sectionName) const;
EntryTable::const_iterator begin() const;
EntryTable::const_iterator end() const;
};
/// SectionTable type. Map of strings to Section object pointers.
typedef std::unordered_map<std::string, Section *> SectionTable;
typedef std::unordered_map<std::string, Section> SectionTable;
protected:
/// Hash of section names to Section object pointers.
@@ -155,15 +156,13 @@ class IniFile
/// Look up section with the given name.
/// @retval Pointer to section object, or NULL if not found.
Section *findSection(const std::string &sectionName) const;
Section *findSection(const std::string &sectionName);
const Section *findSection(const std::string &sectionName) const;
public:
/// Constructor.
IniFile();
/// Destructor.
~IniFile();
/// Load parameter settings from given istream. This is a helper
/// function for load(string) and loadCPP(), which open a file
/// and then pass it here.
@@ -206,7 +205,7 @@ class IniFile
/// Print unreferenced entries in object. Iteratively calls
/// printUnreferend() on all the constituent sections.
bool printUnreferenced();
bool printUnreferenced() const;
/// Dump contents to cout. For debugging.
void dump();

2
src/base/memoizer.hh
View File

@@ -85,7 +85,7 @@ class Memoizer
using ret_type = Ret;
using args_type = std::tuple<Args...>;
constexpr Memoizer(Ret _func(Args...))
constexpr Memoizer(Ret (*_func)(Args...))
: func(_func)
{
validateMemoizer();

64
src/base/trie.hh
View File

@@ -70,7 +70,7 @@ class Trie
Value *value;
Node *parent;
Node *kids[2];
std::unique_ptr<Node> kids[2];
Node(Key _key, Key _mask, Value *_val) :
key(_key & _mask), mask(_mask), value(_val),
@@ -83,16 +83,8 @@ class Trie
void
clear()
{
if (kids[1]) {
kids[1]->clear();
delete kids[1];
kids[1] = NULL;
}
if (kids[0]) {
kids[0]->clear();
delete kids[0];
kids[0] = NULL;
}
kids[1].reset();
kids[0].reset();
}
void
@@ -188,9 +180,9 @@ class Trie
return node;
if (node->kids[0] && node->kids[0]->matches(key))
node = node->kids[0];
node = node->kids[0].get();
else if (node->kids[1] && node->kids[1]->matches(key))
node = node->kids[1];
node = node->kids[1].get();
else
node = NULL;
}
@@ -225,8 +217,8 @@ class Trie
// Walk past all the nodes this new node will be inserted after. They
// can be ignored for the purposes of this function.
Node *node = &head;
while (goesAfter(&node, node->kids[0], key, new_mask) ||
goesAfter(&node, node->kids[1], key, new_mask))
while (goesAfter(&node, node->kids[0].get(), key, new_mask) ||
goesAfter(&node, node->kids[1].get(), key, new_mask))
{}
assert(node);
@@ -239,14 +231,13 @@ class Trie
}
for (unsigned int i = 0; i < 2; i++) {
Node *&kid = node->kids[i];
Node *new_node;
auto& kid = node->kids[i];
if (!kid) {
// No kid. Add a new one.
new_node = new Node(key, new_mask, val);
auto new_node = std::make_unique<Node>(key, new_mask, val);
new_node->parent = node;
kid = new_node;
return new_node;
kid = std::move(new_node);
return kid.get();
}
// Walk down the leg until something doesn't match or we run out
@@ -266,23 +257,23 @@ class Trie
continue;
// At the point we walked to above, add a new node.
new_node = new Node(key, cur_mask, NULL);
auto new_node = std::make_unique<Node>(key, cur_mask, nullptr);
new_node->parent = node;
kid->parent = new_node;
new_node->kids[0] = kid;
kid = new_node;
kid->parent = new_node.get();
new_node->kids[0] = std::move(kid);
kid = std::move(new_node);
// If we ran out of bits, the value goes right here.
if (cur_mask == new_mask) {
new_node->value = val;
return new_node;
kid->value = val;
return kid.get();
}
// Still more bits to deal with, so add a new node for that path.
new_node = new Node(key, new_mask, val);
new_node->parent = kid;
kid->kids[1] = new_node;
return new_node;
new_node = std::make_unique<Node>(key, new_mask, val);
new_node->parent = kid.get();
kid->kids[1] = std::move(new_node);
return kid->kids[1].get();
}
panic("Reached the end of the Trie insert function!\n");
@@ -332,23 +323,22 @@ class Trie
if (node->kids[0])
node->kids[0]->parent = parent;
// Figure out which kid we are, and update our parent's pointers.
if (parent->kids[0] == node)
parent->kids[0] = node->kids[0];
else if (parent->kids[1] == node)
parent->kids[1] = node->kids[0];
if (parent->kids[0].get() == node)
parent->kids[0] = std::move(node->kids[0]);
else if (parent->kids[1].get() == node)
parent->kids[1] = std::move(node->kids[0]);
else
panic("Trie: Inconsistent parent/kid relationship.\n");
// Make sure if the parent only has one kid, it's kid[0].
if (parent->kids[1] && !parent->kids[0]) {
parent->kids[0] = parent->kids[1];
parent->kids[1] = NULL;
parent->kids[0] = std::move(parent->kids[1]);
parent->kids[1] = nullptr;
}
// If the parent has less than two kids and no cargo and isn't the
// root, delete it too.
if (!parent->kids[1] && !parent->value && parent->parent)
remove(parent);
delete node;
return val;
}

5
src/sim/serialize.cc
View File

@@ -145,8 +145,11 @@ CheckpointIn::setDir(const std::string &name)
// appears to have a format placeholder in it.
currentDirectory = (name.find("%") != std::string::npos) ?
csprintf(name, curTick()) : name;
if (currentDirectory[currentDirectory.size() - 1] != '/')
auto isEmptyPath = currentDirectory.empty();
auto endsWithSlash = !isEmptyPath && currentDirectory.back() == '/';
if (!endsWithSlash) {
currentDirectory += "/";
}
return currentDirectory;
}