diff --git a/src/common/dwarf_cfi_to_module.cc b/src/common/dwarf_cfi_to_module.cc index e7e595e7..7da8507d 100644 --- a/src/common/dwarf_cfi_to_module.cc +++ b/src/common/dwarf_cfi_to_module.cc @@ -33,7 +33,9 @@ // Implementation of google_breakpad::DwarfCFIToModule. // See dwarf_cfi_to_module.h for details. +#include #include +#include #include "common/dwarf_cfi_to_module.h" @@ -151,7 +153,7 @@ bool DwarfCFIToModule::Entry(size_t offset, uint64_t address, uint64_t length, // need to check them here. // Get ready to collect entries. - entry_ = new Module::StackFrameEntry; + entry_ = std::make_unique(); entry_->address = address; entry_->size = length; entry_offset_ = offset; @@ -258,8 +260,7 @@ bool DwarfCFIToModule::ValExpressionRule(uint64_t address, int reg, } bool DwarfCFIToModule::End() { - module_->AddStackFrameEntry(entry_); - entry_ = NULL; + module_->AddStackFrameEntry(std::move(entry_)); return true; } diff --git a/src/common/dwarf_cfi_to_module.h b/src/common/dwarf_cfi_to_module.h index 3b092654..42b618d5 100644 --- a/src/common/dwarf_cfi_to_module.h +++ b/src/common/dwarf_cfi_to_module.h @@ -43,6 +43,7 @@ #include #include +#include #include #include "common/module.h" @@ -131,9 +132,9 @@ class DwarfCFIToModule: public CallFrameInfo::Handler { DwarfCFIToModule(Module* module, const vector& register_names, Reporter* reporter) : module_(module), register_names_(register_names), reporter_(reporter), - entry_(NULL), return_address_(-1), cfa_name_(".cfa"), ra_name_(".ra") { + return_address_(-1), cfa_name_(".cfa"), ra_name_(".ra") { } - virtual ~DwarfCFIToModule() { delete entry_; } + virtual ~DwarfCFIToModule() = default; virtual bool Entry(size_t offset, uint64_t address, uint64_t length, uint8_t version, const string& augmentation, @@ -170,7 +171,7 @@ class DwarfCFIToModule: public CallFrameInfo::Handler { Reporter* reporter_; // The current entry we're constructing. - Module::StackFrameEntry* entry_; + std::unique_ptr entry_; // The section offset of the current frame description entry, for // use in error messages. diff --git a/src/common/linux/elf_symbols_to_module.cc b/src/common/linux/elf_symbols_to_module.cc index 4aee38d6..3c33be99 100644 --- a/src/common/linux/elf_symbols_to_module.cc +++ b/src/common/linux/elf_symbols_to_module.cc @@ -36,6 +36,9 @@ #include #include +#include +#include + #include "common/byte_cursor.h" #include "common/module.h" @@ -156,7 +159,7 @@ bool ELFSymbolsToModule(const uint8_t* symtab_section, while(!iterator->at_end) { if (ELF32_ST_TYPE(iterator->info) == STT_FUNC && iterator->shndx != SHN_UNDEF) { - Module::Extern* ext = new Module::Extern(iterator->value); + auto ext = std::make_unique(iterator->value); ext->name = SymbolString(iterator->name_offset, strings); #if !defined(__ANDROID__) // Android NDK doesn't provide abi::__cxa_demangle. int status = 0; @@ -168,7 +171,7 @@ bool ELFSymbolsToModule(const uint8_t* symtab_section, free(demangled); } #endif - module->AddExtern(ext); + module->AddExtern(std::move(ext)); } ++iterator; } diff --git a/src/common/module.cc b/src/common/module.cc index 2e61693c..75782ab1 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -117,12 +117,6 @@ Module::~Module() { it != functions_.end(); ++it) { delete *it; } - for (vector::iterator it = stack_frame_entries_.begin(); - it != stack_frame_entries_.end(); ++it) { - delete *it; - } - for (ExternSet::iterator it = externs_.begin(); it != externs_.end(); ++it) - delete *it; } void Module::SetLoadAddress(Address address) { @@ -155,12 +149,11 @@ bool Module::AddFunction(Function* function) { } if (it_ext != externs_.end()) { if (enable_multiple_field_) { - Extern* found_ext = *it_ext; + Extern* found_ext = it_ext->get(); // If the PUBLIC is for the same symbol as the FUNC, don't mark multiple. function->is_multiple |= found_ext->name != function->name || found_ext->is_multiple; } - delete *it_ext; externs_.erase(it_ext); } #if _DEBUG @@ -194,27 +187,22 @@ bool Module::AddFunction(Function* function) { return true; } -void Module::AddStackFrameEntry(StackFrameEntry* stack_frame_entry) { +void Module::AddStackFrameEntry(std::unique_ptr stack_frame_entry) { if (!AddressIsInModule(stack_frame_entry->address)) { return; } - stack_frame_entries_.push_back(stack_frame_entry); + stack_frame_entries_.push_back(std::move(stack_frame_entry)); } -void Module::AddExtern(Extern* ext) { +void Module::AddExtern(std::unique_ptr ext) { if (!AddressIsInModule(ext->address)) { return; } - std::pair ret = externs_.insert(ext); - if (!ret.second) { - if (enable_multiple_field_) { - (*ret.first)->is_multiple = true; - } - // Free the duplicate that was not inserted because this Module - // now owns it. - delete ext; + std::pair ret = externs_.emplace(std::move(ext)); + if (!ret.second && enable_multiple_field_) { + (*ret.first)->is_multiple = true; } } @@ -225,7 +213,11 @@ void Module::GetFunctions(vector* vec, void Module::GetExterns(vector* vec, vector::iterator i) { - vec->insert(i, externs_.begin(), externs_.end()); + auto pos = vec->insert(i, externs_.size(), nullptr); + for (const std::unique_ptr& ext : externs_) { + *pos = ext.get(); + ++pos; + } } Module::File* Module::FindFile(const string& name) { @@ -267,7 +259,11 @@ void Module::GetFiles(vector* vec) { } void Module::GetStackFrameEntries(vector* vec) const { - *vec = stack_frame_entries_; + vec->clear(); + vec->reserve(stack_frame_entries_.size()); + for (const auto& ent : stack_frame_entries_) { + vec->push_back(ent.get()); + } } void Module::AssignSourceIds( @@ -443,7 +439,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { // Write out 'PUBLIC' records. for (ExternSet::const_iterator extern_it = externs_.begin(); extern_it != externs_.end(); ++extern_it) { - Extern* ext = *extern_it; + Extern* ext = extern_it->get(); stream << "PUBLIC " << (ext->is_multiple ? "m " : "") << hex << (ext->address - load_address_) << " 0 " << ext->name << dec << "\n"; @@ -452,10 +448,9 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { if (symbol_data & CFI) { // Write out 'STACK CFI INIT' and 'STACK CFI' records. - vector::const_iterator frame_it; - for (frame_it = stack_frame_entries_.begin(); + for (auto frame_it = stack_frame_entries_.begin(); frame_it != stack_frame_entries_.end(); ++frame_it) { - StackFrameEntry* entry = *frame_it; + StackFrameEntry* entry = frame_it->get(); stream << "STACK CFI INIT " << hex << (entry->address - load_address_) << " " << entry->size << " " << dec; diff --git a/src/common/module.h b/src/common/module.h index a8fcc81e..c1fd9f59 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -292,7 +292,18 @@ class Module { }; struct ExternCompare { - bool operator() (const Extern* lhs, const Extern* rhs) const { + // Defining is_transparent allows + // std::set, ExternCompare>::find() to be called + // with an Extern* and have set use the overloads below. + using is_transparent = void; + bool operator() (const std::unique_ptr& lhs, + const std::unique_ptr& rhs) const { + return lhs->address < rhs->address; + } + bool operator() (const Extern* lhs, const std::unique_ptr& rhs) const { + return lhs->address < rhs->address; + } + bool operator() (const std::unique_ptr& lhs, const Extern* rhs) const { return lhs->address < rhs->address; } }; @@ -340,12 +351,12 @@ class Module { // Add STACK_FRAME_ENTRY to the module. // This module owns all StackFrameEntry objects added with this // function: destroying the module destroys them as well. - void AddStackFrameEntry(StackFrameEntry* stack_frame_entry); + void AddStackFrameEntry(std::unique_ptr stack_frame_entry); // Add PUBLIC to the module. // This module owns all Extern objects added with this function: // destroying the module destroys them as well. - void AddExtern(Extern* ext); + void AddExtern(std::unique_ptr ext); // If this module has a file named NAME, return a pointer to it. If // it has none, then create one and return a pointer to the new @@ -465,7 +476,7 @@ class Module { typedef set FunctionSet; // A set containing Extern structures, sorted by address. - typedef set ExternSet; + typedef set, ExternCompare> ExternSet; // The module owns all the files and functions that have been added // to it; destroying the module frees the Files and Functions these @@ -477,7 +488,7 @@ class Module { // The module owns all the call frame info entries that have been // added to it. - vector stack_frame_entries_; + vector> stack_frame_entries_; // The module owns all the externs that have been added to it; // destroying the module frees the Externs these point to. diff --git a/src/common/module_unittest.cc b/src/common/module_unittest.cc index 220add03..39727554 100644 --- a/src/common/module_unittest.cc +++ b/src/common/module_unittest.cc @@ -36,8 +36,10 @@ #include #include +#include #include #include +#include #include "breakpad_googletest_includes.h" #include "common/module.h" @@ -137,7 +139,7 @@ TEST(Module, WriteRelativeLoadAddress) { m.AddFunction(function); // Some stack information. - Module::StackFrameEntry* entry = new Module::StackFrameEntry(); + auto entry = std::make_unique(); entry->address = 0x30f9e5c83323973dULL; entry->size = 0x49fc9ca7c7c13dc2ULL; entry->initial_rules[".cfa"] = "he was a handsome man"; @@ -145,7 +147,7 @@ TEST(Module, WriteRelativeLoadAddress) { entry->rule_changes[0x30f9e5c83323973eULL]["how"] = "do you like your blueeyed boy"; entry->rule_changes[0x30f9e5c83323973eULL]["Mister"] = "Death"; - m.AddStackFrameEntry(entry); + m.AddStackFrameEntry(std::move(entry)); // Set the load address. Doing this after adding all the data to // the module must work fine. @@ -242,7 +244,7 @@ TEST(Module, WriteNoCFI) { m.AddFunction(function); // Some stack information. - Module::StackFrameEntry* entry = new Module::StackFrameEntry(); + auto entry = std::make_unique(); entry->address = 0x30f9e5c83323973dULL; entry->size = 0x49fc9ca7c7c13dc2ULL; entry->initial_rules[".cfa"] = "he was a handsome man"; @@ -250,7 +252,7 @@ TEST(Module, WriteNoCFI) { entry->rule_changes[0x30f9e5c83323973eULL]["how"] = "do you like your blueeyed boy"; entry->rule_changes[0x30f9e5c83323973eULL]["Mister"] = "Death"; - m.AddStackFrameEntry(entry); + m.AddStackFrameEntry(std::move(entry)); // Set the load address. Doing this after adding all the data to // the module must work fine. @@ -321,18 +323,18 @@ TEST(Module, WriteOutOfRangeAddresses) { // Add three stack frames (one lower, one in, and one higher than the allowed // address range). Only the middle frame should be captured. - Module::StackFrameEntry* entry1 = new Module::StackFrameEntry(); + auto entry1 = std::make_unique(); entry1->address = 0x1000ULL; entry1->size = 0x100ULL; - m.AddStackFrameEntry(entry1); - Module::StackFrameEntry* entry2 = new Module::StackFrameEntry(); + m.AddStackFrameEntry(std::move(entry1)); + auto entry2 = std::make_unique(); entry2->address = 0x2000ULL; entry2->size = 0x100ULL; - m.AddStackFrameEntry(entry2); - Module::StackFrameEntry* entry3 = new Module::StackFrameEntry(); + m.AddStackFrameEntry(std::move(entry2)); + auto entry3 = std::make_unique(); entry3->address = 0x3000ULL; entry3->size = 0x100ULL; - m.AddStackFrameEntry(entry3); + m.AddStackFrameEntry(std::move(entry3)); // Add a function outside the allowed range. Module::File* file = m.FindFile("file_name.cc"); @@ -346,9 +348,9 @@ TEST(Module, WriteOutOfRangeAddresses) { m.AddFunction(function); // Add an extern outside the allowed range. - Module::Extern* extern1 = new Module::Extern(0x5000ULL); + auto extern1 = std::make_unique(0x5000ULL); extern1->name = "_xyz"; - m.AddExtern(extern1); + m.AddExtern(std::move(extern1)); m.Write(s, ALL_SYMBOL_DATA); @@ -357,10 +359,7 @@ TEST(Module, WriteOutOfRangeAddresses) { s.str().c_str()); // Cleanup - Prevent Memory Leak errors. - delete (extern1); delete (function); - delete (entry3); - delete (entry1); } TEST(Module, ConstructAddFrames) { @@ -368,22 +367,22 @@ TEST(Module, ConstructAddFrames) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID); // First STACK CFI entry, with no initial rules or deltas. - Module::StackFrameEntry* entry1 = new Module::StackFrameEntry(); + auto entry1 = std::make_unique(); entry1->address = 0xddb5f41285aa7757ULL; entry1->size = 0x1486493370dc5073ULL; - m.AddStackFrameEntry(entry1); + m.AddStackFrameEntry(std::move(entry1)); // Second STACK CFI entry, with initial rules but no deltas. - Module::StackFrameEntry* entry2 = new Module::StackFrameEntry(); + auto entry2 = std::make_unique(); entry2->address = 0x8064f3af5e067e38ULL; entry2->size = 0x0de2a5ee55509407ULL; entry2->initial_rules[".cfa"] = "I think that I shall never see"; entry2->initial_rules["stromboli"] = "a poem lovely as a tree"; entry2->initial_rules["cannoli"] = "a tree whose hungry mouth is prest"; - m.AddStackFrameEntry(entry2); + m.AddStackFrameEntry(std::move(entry2)); // Third STACK CFI entry, with initial rules and deltas. - Module::StackFrameEntry* entry3 = new Module::StackFrameEntry(); + auto entry3 = std::make_unique(); entry3->address = 0x5e8d0db0a7075c6cULL; entry3->size = 0x1c7edb12a7aea229ULL; entry3->initial_rules[".cfa"] = "Whose woods are these"; @@ -395,7 +394,7 @@ TEST(Module, ConstructAddFrames) { "his house is in"; entry3->rule_changes[0x36682fad3763ffffULL][".cfa"] = "I think I know"; - m.AddStackFrameEntry(entry3); + m.AddStackFrameEntry(std::move(entry3)); // Check that Write writes STACK CFI records properly. m.Write(s, ALL_SYMBOL_DATA); @@ -541,13 +540,13 @@ TEST(Module, ConstructExterns) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID); // Two externs. - Module::Extern* extern1 = new Module::Extern(0xffff); + auto extern1 = std::make_unique(0xffff); extern1->name = "_abc"; - Module::Extern* extern2 = new Module::Extern(0xaaaa); + auto extern2 = std::make_unique(0xaaaa); extern2->name = "_xyz"; - m.AddExtern(extern1); - m.AddExtern(extern2); + m.AddExtern(std::move(extern1)); + m.AddExtern(std::move(extern2)); m.Write(s, ALL_SYMBOL_DATA); string contents = s.str(); @@ -566,13 +565,13 @@ TEST(Module, ConstructDuplicateExterns) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID); // Two externs. - Module::Extern* extern1 = new Module::Extern(0xffff); + auto extern1 = std::make_unique(0xffff); extern1->name = "_xyz"; - Module::Extern* extern2 = new Module::Extern(0xffff); + auto extern2 = std::make_unique(0xffff); extern2->name = "_abc"; - m.AddExtern(extern1); - m.AddExtern(extern2); + m.AddExtern(std::move(extern1)); + m.AddExtern(std::move(extern2)); m.Write(s, ALL_SYMBOL_DATA); string contents = s.str(); @@ -589,13 +588,13 @@ TEST(Module, ConstructDuplicateExternsMultiple) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID, "", true); // Two externs. - Module::Extern* extern1 = new Module::Extern(0xffff); + auto extern1 = std::make_unique(0xffff); extern1->name = "_xyz"; - Module::Extern* extern2 = new Module::Extern(0xffff); + auto extern2 = std::make_unique(0xffff); extern2->name = "_abc"; - m.AddExtern(extern1); - m.AddExtern(extern2); + m.AddExtern(std::move(extern1)); + m.AddExtern(std::move(extern2)); m.Write(s, ALL_SYMBOL_DATA); string contents = s.str(); @@ -613,13 +612,13 @@ TEST(Module, ConstructFunctionsAndExternsWithSameAddress) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID); // Two externs. - Module::Extern* extern1 = new Module::Extern(0xabc0); + auto extern1 = std::make_unique(0xabc0); extern1->name = "abc"; - Module::Extern* extern2 = new Module::Extern(0xfff0); + auto extern2 = std::make_unique(0xfff0); extern2->name = "xyz"; - m.AddExtern(extern1); - m.AddExtern(extern2); + m.AddExtern(std::move(extern1)); + m.AddExtern(std::move(extern2)); Module::Function* function = new Module::Function("_xyz", 0xfff0); Module::Range range(0xfff0, 0x10); @@ -644,13 +643,13 @@ TEST(Module, ConstructFunctionsAndExternsWithSameAddressMultiple) { Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID, "", true); // Two externs. - Module::Extern* extern1 = new Module::Extern(0xabc0); + auto extern1 = std::make_unique(0xabc0); extern1->name = "abc"; - Module::Extern* extern2 = new Module::Extern(0xfff0); + auto extern2 = std::make_unique(0xfff0); extern2->name = "xyz"; - m.AddExtern(extern1); - m.AddExtern(extern2); + m.AddExtern(std::move(extern1)); + m.AddExtern(std::move(extern2)); Module::Function* function = new Module::Function("_xyz", 0xfff0); Module::Range range(0xfff0, 0x10); @@ -676,17 +675,17 @@ TEST(Module, ConstructFunctionsAndThumbExternsWithSameAddress) { Module m(MODULE_NAME, MODULE_OS, "arm", MODULE_ID); // Two THUMB externs. - Module::Extern* thumb_extern1 = new Module::Extern(0xabc1); + auto thumb_extern1 = std::make_unique(0xabc1); thumb_extern1->name = "thumb_abc"; - Module::Extern* thumb_extern2 = new Module::Extern(0xfff1); + auto thumb_extern2 = std::make_unique(0xfff1); thumb_extern2->name = "thumb_xyz"; - Module::Extern* arm_extern1 = new Module::Extern(0xcc00); + auto arm_extern1 = std::make_unique(0xcc00); arm_extern1->name = "arm_func"; - m.AddExtern(thumb_extern1); - m.AddExtern(thumb_extern2); - m.AddExtern(arm_extern1); + m.AddExtern(std::move(thumb_extern1)); + m.AddExtern(std::move(thumb_extern2)); + m.AddExtern(std::move(arm_extern1)); // The corresponding function from the DWARF debug data have the actual // address. diff --git a/src/common/stabs_to_module.cc b/src/common/stabs_to_module.cc index 6cdb96a2..3d026c22 100644 --- a/src/common/stabs_to_module.cc +++ b/src/common/stabs_to_module.cc @@ -36,6 +36,8 @@ #include #include +#include +#include #include "common/stabs_to_module.h" #include "common/using_std_string.h" @@ -132,7 +134,7 @@ bool StabsToModule::Line(uint64_t address, const char *name, int number) { } bool StabsToModule::Extern(const string& name, uint64_t address) { - Module::Extern *ext = new Module::Extern(address); + auto ext = std::make_unique(address); // Older libstdc++ demangle implementations can crash on unexpected // input, so be careful about what gets passed in. if (name.compare(0, 3, "__Z") == 0) { @@ -142,7 +144,7 @@ bool StabsToModule::Extern(const string& name, uint64_t address) { } else { ext->name = name; } - module_->AddExtern(ext); + module_->AddExtern(std::move(ext)); return true; } diff --git a/src/tools/mac/dump_syms/dump_syms_tool.cc b/src/tools/mac/dump_syms/dump_syms_tool.cc index d0f29ba7..2e05cbf3 100644 --- a/src/tools/mac/dump_syms/dump_syms_tool.cc +++ b/src/tools/mac/dump_syms/dump_syms_tool.cc @@ -36,6 +36,8 @@ #include #include +#include +#include #include #include "common/mac/dump_syms.h" @@ -108,7 +110,8 @@ static void CopyCFIDataBetweenModules(Module* to_module, // If the entry does not overlap, then it is safe to copy to |to_module|. if (to_it == to_data.end() || (from_entry->address < (*to_it)->address && from_entry_end < (*to_it)->address)) { - to_module->AddStackFrameEntry(new Module::StackFrameEntry(*from_entry)); + to_module->AddStackFrameEntry( + std::make_unique(*from_entry)); } } }