yuzu-emu/breakpad
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix dump_syms memory leak

Browse source 
It moves InlineOriginMap to module.h. Let Module keeps the global InlineOriginMap to easily get all referenced InlineOrigin when emitting. And release allocated memory inside its destructor.

Verified that the symbol file with inline records for chrome is the same before and after this change.

Change-Id: I7541aa05d3d2df0b9d52d670cab58241baecf20d
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3171638
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Zequan Wu 2021-09-24 13:27:13 -07:00 committed by Joshua Peraza
parent 1147c2fcf0
commit 1816ae7f35
3 changed files with 116 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

79
src/common/dwarf_cu_to_module.cc
View file

@ -100,71 +100,6 @@ struct AbstractOrigin {
typedef map<uint64_t, AbstractOrigin> AbstractOriginByOffset;
using InlineOriginByOffset = map<uint64_t, Module::InlineOrigin*>;
class InlineOriginMap {
public:
Module::InlineOrigin* GetOrCreateInlineOrigin(uint64_t offset,
const string& name) {
uint64_t specification_offset = references_[offset];
if (inline_origins_.find(specification_offset) != inline_origins_.end()) {
if (inline_origins_[specification_offset]->name == "<name omitted>") {
inline_origins_[specification_offset]->name = name;
}
return inline_origins_[specification_offset];
}
inline_origins_[specification_offset] = new Module::InlineOrigin(name);
return inline_origins_[specification_offset];
}
// offset is the offset of a DW_TAG_subprogram. specification_offset is the
// value of its DW_AT_specification or equals to offset if DW_AT_specification
// doesn't exist in that DIE.
void SetReference(uint64_t offset, uint64_t specification_offset) {
// If we haven't seen this doesn't exist in reference map, always add it.
if (references_.find(offset) == references_.end()) {
references_[offset] = specification_offset;
return;
}
// If offset equals specification_offset and offset exists in references_,
// there is no need to update the references_ map. This early return is
// necessary because the call to erase in following if will remove the entry
// of specification_offset in inline_origins_.
// If specification_offset equals to references_[offset], it might be
// duplicate debug info.
if (offset == specification_offset ||
specification_offset == references_[offset])
return;
// Fix up mapping in inline_origins_.
auto remove = inline_origins_.find(references_[offset]);
if (remove != inline_origins_.end()) {
inline_origins_[specification_offset] = remove->second;
inline_origins_.erase(remove);
}
references_[offset] = specification_offset;
}
void AssignFilesToInlineOrigins(vector<uint64_t>& inline_origin_offsets,
Module::File* file) {
for (uint64_t offset : inline_origin_offsets)
if (references_.find(offset) != references_.end()) {
auto origin = inline_origins_.find(references_[offset]);
if (origin != inline_origins_.end())
origin->second->file = file;
}
}
private:
// A map from a DW_TAG_subprogram's offset to the DW_TAG_subprogram.
InlineOriginByOffset inline_origins_;
// A map from a DW_TAG_subprogram's offset to the offset of its specification
// or abstract origin subprogram. The set of values in this map should always
// be the same set of keys in inline_origins_.
map<uint64_t, uint64_t> references_;
};
// Data global to the DWARF-bearing file that is private to the
// DWARF-to-Module process.
struct DwarfCUToModule::FilePrivate {
@ -197,8 +132,6 @@ struct DwarfCUToModule::FilePrivate {
// Keep a list of forward references from DW_AT_abstract_origin and
// DW_AT_specification attributes so names can be fixed up.
std::map<uint64_t, Module::Function*> forward_ref_die_to_func;
InlineOriginMap inline_origin_map;
};
DwarfCUToModule::FileContext::FileContext(const string& filename,
@ -751,10 +684,10 @@ void DwarfCUToModule::InlineHandler::Finish() {
// Every DW_TAG_inlined_subroutine should have a DW_AT_abstract_origin.
assert(specification_offset_ != 0);
cu_context_->file_context->file_private_->inline_origin_map.SetReference(
cu_context_->file_context->module_->inline_origin_map.SetReference(
specification_offset_, specification_offset_);
Module::InlineOrigin* origin =
cu_context_->file_context->file_private_->inline_origin_map
cu_context_->file_context->module_->inline_origin_map
.GetOrCreateInlineOrigin(specification_offset_, name_);
unique_ptr<Module::Inline> in(
new Module::Inline(origin, ranges, call_site_line_, inline_nest_level_,
@ -956,9 +889,9 @@ void DwarfCUToModule::FuncHandler::Finish() {
(!empty_range || inline_ || decl_file_data_ != UINT64_MAX)) {
uint64_t offset =
specification_offset_ != 0 ? specification_offset_ : offset_;
cu_context_->file_context->file_private_->inline_origin_map.SetReference(
offset_, offset);
cu_context_->file_context->file_private_->inline_origin_map
cu_context_->file_context->module_->inline_origin_map.SetReference(offset_,
offset);
cu_context_->file_context->module_->inline_origin_map
.GetOrCreateInlineOrigin(offset_,
name_.empty() ? "<name omitted>" : name_);
if (decl_file_data_ != UINT64_MAX)
@ -1546,7 +1479,7 @@ void DwarfCUToModule::AssignLinesToFunctions() {
void DwarfCUToModule::AssignFilesToInlines() {
for (auto iter : files_) {
cu_context_->file_context->file_private_->inline_origin_map
cu_context_->file_context->module_->inline_origin_map
.AssignFilesToInlineOrigins(cu_context_->inline_origins[iter.first],
iter.second);
}

84
src/common/module.cc
View file

@ -49,6 +49,64 @@ using std::dec;
using std::hex;
using std::unique_ptr;
Module::InlineOrigin* Module::InlineOriginMap::GetOrCreateInlineOrigin(
uint64_t offset,
const string& name) {
uint64_t specification_offset = references_[offset];
// Find the root offset.
auto iter = references_.find(specification_offset);
while (iter != references_.end() &&
specification_offset != references_[specification_offset]) {
specification_offset = references_[specification_offset];
iter = references_.find(specification_offset);
}
if (inline_origins_.find(specification_offset) != inline_origins_.end()) {
if (inline_origins_[specification_offset]->name == "<name omitted>") {
inline_origins_[specification_offset]->name = name;
}
return inline_origins_[specification_offset];
}
inline_origins_[specification_offset] = new Module::InlineOrigin(name);
return inline_origins_[specification_offset];
}
void Module::InlineOriginMap::SetReference(uint64_t offset,
uint64_t specification_offset) {
// If we haven't seen this doesn't exist in reference map, always add it.
if (references_.find(offset) == references_.end()) {
references_[offset] = specification_offset;
return;
}
// If offset equals specification_offset and offset exists in
// references_, there is no need to update the references_ map.
// This early return is necessary because the call to erase in following if
// will remove the entry of specification_offset in inline_origins_. If
// specification_offset equals to references_[offset], it might be
// duplicate debug info.
if (offset == specification_offset ||
specification_offset == references_[offset])
return;
// Fix up mapping in inline_origins_.
auto remove = inline_origins_.find(references_[offset]);
if (remove != inline_origins_.end()) {
inline_origins_[specification_offset] = std::move(remove->second);
inline_origins_.erase(remove);
}
references_[offset] = specification_offset;
}
void Module::InlineOriginMap::AssignFilesToInlineOrigins(
vector<uint64_t>& inline_origin_offsets,
Module::File* file) {
for (uint64_t offset : inline_origin_offsets)
if (references_.find(offset) != references_.end()) {
auto origin = inline_origins_.find(references_[offset]);
if (origin != inline_origins_.end())
origin->second->file = file;
}
}
Module::Module(const string& name, const string& os,
const string& architecture, const string& id,
const string& code_id /* = "" */) :
@ -200,7 +258,8 @@ void Module::GetStackFrameEntries(vector<StackFrameEntry*>* vec) const {
*vec = stack_frame_entries_;
}
void Module::AssignSourceIds() {
void Module::AssignSourceIds(
set<InlineOrigin*, InlineOriginCompare>& inline_origins) {
// First, give every source file an id of -1.
for (FileByNameMap::iterator file_it = files_.begin();
file_it != files_.end(); ++file_it) {
@ -218,7 +277,7 @@ void Module::AssignSourceIds() {
}
// Also mark all files cited by inline functions by setting each one's source
// id to zero.
for (InlineOrigin* origin : inline_origins_)
for (InlineOrigin* origin : inline_origins)
// There are some artificial inline functions which don't belong to
// any file. Those will have file id -1.
if (origin->file)
@ -245,20 +304,21 @@ static void InlineDFS(
}
}
void Module::CreateInlineOrigins() {
void Module::CreateInlineOrigins(
set<InlineOrigin*, InlineOriginCompare>& inline_origins) {
// Only add origins that have file and deduplicate origins with same name and
// file id by doing a DFS.
auto addInlineOrigins = [&](unique_ptr<Inline> &in) {
auto it = inline_origins_.find(in->origin);
if (it == inline_origins_.end())
inline_origins_.insert(in->origin);
auto addInlineOrigins = [&](unique_ptr<Inline>& in) {
auto it = inline_origins.find(in->origin);
if (it == inline_origins.end())
inline_origins.insert(in->origin);
else
in->origin = *it;
};
for (Function* func : functions_)
InlineDFS(func->inlines, addInlineOrigins);
int next_id = 0;
for (InlineOrigin* origin: inline_origins_) {
for (InlineOrigin* origin : inline_origins) {
origin->id = next_id++;
}
}
@ -303,8 +363,10 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) {
}
if (symbol_data & SYMBOLS_AND_FILES) {
CreateInlineOrigins();
AssignSourceIds();
// Get all referenced inline origins.
set<InlineOrigin*, InlineOriginCompare> inline_origins;
CreateInlineOrigins(inline_origins);
AssignSourceIds(inline_origins);
// Write out files.
for (FileByNameMap::iterator file_it = files_.begin();
@ -317,7 +379,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) {
}
}
// Write out inline origins.
for (InlineOrigin* origin : inline_origins_) {
for (InlineOrigin* origin : inline_origins) {
stream << "INLINE_ORIGIN " << origin->id << " " << origin->getFileID()
<< " " << origin->name << "\n";
if (!stream.good())

44
src/common/module.h
View file

@ -129,7 +129,8 @@ class Module {
};
struct InlineOrigin {
InlineOrigin(const string& name): id(-1), name(name), file(NULL) {}
explicit InlineOrigin(const string& name)
: id(-1), name(name), file(nullptr) {}
// A unique id for each InlineOrigin object. INLINE records use the id to
// refer to its INLINE_ORIGIN record.
@ -169,6 +170,38 @@ class Module {
vector<std::unique_ptr<Inline>> child_inlines;
};
typedef map<uint64_t, InlineOrigin*> InlineOriginByOffset;
class InlineOriginMap {
public:
// Add INLINE ORIGIN to the module. Return a pointer to origin .
InlineOrigin* GetOrCreateInlineOrigin(uint64_t offset, const string& name);
// offset is the offset of a DW_TAG_subprogram. specification_offset is the
// value of its DW_AT_specification or equals to offset if
// DW_AT_specification doesn't exist in that DIE.
void SetReference(uint64_t offset, uint64_t specification_offset);
void AssignFilesToInlineOrigins(
const vector<uint64_t>& inline_origin_offsets,
File* file);
~InlineOriginMap() {
for (const auto& iter : inline_origins_) {
delete iter.second;
}
}
private:
// A map from a DW_TAG_subprogram's offset to the DW_TAG_subprogram.
InlineOriginByOffset inline_origins_;
// A map from a DW_TAG_subprogram's offset to the offset of its
// specification or abstract origin subprogram. The set of values in this
// map should always be the same set of keys in inline_origins_.
map<uint64_t, uint64_t> references_;
};
InlineOriginMap inline_origin_map;
// A source line.
struct Line {
// For sorting by address. (Not style-guide compliant, but it's
@ -328,11 +361,12 @@ class Module {
// Set the source id numbers for all other files --- unused by the
// source line data --- to -1. We do this before writing out the
// symbol file, at which point we omit any unused files.
void AssignSourceIds();
void AssignSourceIds(set<InlineOrigin*, InlineOriginCompare>& inline_origins);
// This function should be called before AssignSourceIds() to get the set of
// valid InlineOrigins*.
void CreateInlineOrigins();
void CreateInlineOrigins(
set<InlineOrigin*, InlineOriginCompare>& inline_origins);
// Call AssignSourceIds, and write this module to STREAM in the
// breakpad symbol format. Return true if all goes well, or false if
@ -393,9 +427,6 @@ class Module {
// A set containing Function structures, sorted by address.
typedef set<Function*, FunctionCompare> FunctionSet;
// A set containing Function structures, sorted by address.
typedef set<InlineOrigin*, InlineOriginCompare> InlineOriginSet;
// A set containing Extern structures, sorted by address.
typedef set<Extern*, ExternCompare> ExternSet;
@ -404,7 +435,6 @@ class Module {
// point to.
FileByNameMap files_; // This module's source files.
FunctionSet functions_; // This module's functions.
InlineOriginSet inline_origins_; // This module's inline origins.
// The module owns all the call frame info entries that have been
// added to it.