aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/common/module.cc21
-rw-r--r--src/common/module.h25
-rw-r--r--src/common/module_unittest.cc69
-rw-r--r--src/common/stabs_to_module.cc19
-rw-r--r--src/common/stabs_to_module_unittest.cc34
5 files changed, 134 insertions, 34 deletions
diff --git a/src/common/module.cc b/src/common/module.cc
index f7f5788e..01f4985f 100644
--- a/src/common/module.cc
+++ b/src/common/module.cc
@@ -49,7 +49,7 @@ Module::Module(const string &name, const string &os,
Module::~Module() {
for (FileByNameMap::iterator it = files_.begin(); it != files_.end(); it++)
delete it->second;
- for (vector<Function *>::iterator it = functions_.begin();
+ for (set<Function *>::iterator it = functions_.begin();
it != functions_.end(); it++)
delete *it;
for (vector<StackFrameEntry *>::iterator it = stack_frame_entries_.begin();
@@ -62,12 +62,17 @@ void Module::SetLoadAddress(Address address) {
}
void Module::AddFunction(Function *function) {
- functions_.push_back(function);
+ std::pair<set<Function *>::iterator,bool> ret = functions_.insert(function);
+ if (!ret.second) {
+ // Free the duplicate we failed to insert because we own it.
+ delete function;
+ }
}
void Module::AddFunctions(vector<Function *>::iterator begin,
vector<Function *>::iterator end) {
- functions_.insert(functions_.end(), begin, end);
+ for (vector<Function *>::iterator it = begin; it != end; it++)
+ AddFunction(*it);
}
void Module::AddStackFrameEntry(StackFrameEntry *stack_frame_entry) {
@@ -130,7 +135,7 @@ void Module::AssignSourceIds() {
// Next, mark all files actually cited by our functions' line number
// info, by setting each one's source id to zero.
- for (vector<Function *>::const_iterator func_it = functions_.begin();
+ for (set<Function *>::const_iterator func_it = functions_.begin();
func_it != functions_.end(); func_it++) {
Function *func = *func_it;
for (vector<Line>::iterator line_it = func->lines.begin();
@@ -145,13 +150,13 @@ void Module::AssignSourceIds() {
int next_source_id = 0;
for (FileByNameMap::iterator file_it = files_.begin();
file_it != files_.end(); file_it++)
- if (! file_it->second->source_id)
+ if (!file_it->second->source_id)
file_it->second->source_id = next_source_id++;
}
bool Module::ReportError() {
fprintf(stderr, "error writing symbol file: %s\n",
- strerror (errno));
+ strerror(errno));
return false;
}
@@ -187,7 +192,7 @@ bool Module::Write(FILE *stream) {
}
// Write out functions and their lines.
- for (vector<Function *>::const_iterator func_it = functions_.begin();
+ for (set<Function *>::const_iterator func_it = functions_.begin();
func_it != functions_.end(); func_it++) {
Function *func = *func_it;
if (0 > fprintf(stream, "FUNC %llx %llx %llx %s\n",
@@ -217,7 +222,7 @@ bool Module::Write(FILE *stream) {
|| !WriteRuleMap(entry->initial_rules, stream)
|| 0 > putc('\n', stream))
return ReportError();
-
+
// Write out this entry's delta rules as 'STACK CFI' records.
for (RuleChangeMap::const_iterator delta_it = entry->rule_changes.begin();
delta_it != entry->rule_changes.end(); delta_it++) {
diff --git a/src/common/module.h b/src/common/module.h
index 64707f3f..18351319 100644
--- a/src/common/module.h
+++ b/src/common/module.h
@@ -41,6 +41,7 @@
#include <stdio.h>
#include <map>
+#include <set>
#include <string>
#include <vector>
@@ -48,6 +49,7 @@
namespace google_breakpad {
+using std::set;
using std::string;
using std::vector;
using std::map;
@@ -90,7 +92,7 @@ class Module {
// The function's name.
string name;
-
+
// The start address and length of the function's code.
Address address, size;
@@ -124,7 +126,7 @@ class Module {
// A map from addresses to RuleMaps, representing changes that take
// effect at given addresses.
typedef map<Address, RuleMap> RuleChangeMap;
-
+
// A range of 'STACK CFI' stack walking information. An instance of
// this structure corresponds to a 'STACK CFI INIT' record and the
// subsequent 'STACK CFI' records that fall within its range.
@@ -143,10 +145,19 @@ class Module {
// including the address you're interested in.
RuleChangeMap rule_changes;
};
-
+
+ struct FunctionCompare {
+ bool operator() (const Function *lhs,
+ const Function *rhs) const {
+ if (lhs->address == rhs->address)
+ return lhs->name < rhs->name;
+ return lhs->address < rhs->address;
+ }
+ };
+
// Create a new module with the given name, operating system,
// architecture, and ID string.
- Module(const string &name, const string &os, const string &architecture,
+ Module(const string &name, const string &os, const string &architecture,
const string &id);
~Module();
@@ -176,7 +187,7 @@ class Module {
vector<Function *>::iterator end);
// 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);
@@ -262,8 +273,8 @@ class Module {
// The module owns all the files and functions that have been added
// to it; destroying the module frees the Files and Functions these
// point to.
- FileByNameMap files_; // This module's source files.
- vector<Function *> functions_; // This module's functions.
+ FileByNameMap files_; // This module's source files.
+ set<Function *, FunctionCompare> functions_; // This module's functions.
// The module owns all the call frame info entries that have been
// added to it.
diff --git a/src/common/module_unittest.cc b/src/common/module_unittest.cc
index 18c8ad61..7ba1c17a 100644
--- a/src/common/module_unittest.cc
+++ b/src/common/module_unittest.cc
@@ -91,6 +91,19 @@ void checked_fclose(FILE *stream) {
}
}
+Module::Function *generate_duplicate_function(const string &name) {
+ const Module::Address DUP_ADDRESS = 0xd35402aac7a7ad5cLL;
+ const Module::Address DUP_SIZE = 0x200b26e605f99071LL;
+ const Module::Address DUP_PARAMETER_SIZE = 0xf14ac4fed48c4a99LL;
+
+ Module::Function *function = new(Module::Function);
+ function->name = name;
+ function->address = DUP_ADDRESS;
+ function->size = DUP_SIZE;
+ function->parameter_size = DUP_PARAMETER_SIZE;
+ return function;
+}
+
#define MODULE_NAME "name with spaces"
#define MODULE_OS "os-name"
#define MODULE_ARCH "architecture"
@@ -222,7 +235,7 @@ TEST(Write, OmitUnusedFiles) {
m.AddFunction(function);
m.AssignSourceIds();
-
+
vector<Module::File *> vec;
m.GetFiles(&vec);
EXPECT_EQ((size_t) 3, vec.size());
@@ -280,10 +293,10 @@ TEST(Construct, AddFunctions) {
string contents = checked_read(f);
checked_fclose(f);
EXPECT_STREQ("MODULE os-name architecture id-string name with spaces\n"
- "FUNC d35024aa7ca7da5c 200b26e605f99071 f14ac4fed48c4a99"
- " _without_form\n"
"FUNC 2987743d0b35b13f b369db048deb3010 938e556cb5a79988"
- " _and_void\n",
+ " _and_void\n"
+ "FUNC d35024aa7ca7da5c 200b26e605f99071 f14ac4fed48c4a99"
+ " _without_form\n",
contents.c_str());
// Check that m.GetFunctions returns the functions we expect.
@@ -303,7 +316,7 @@ TEST(Construct, AddFrames) {
entry1->address = 0xddb5f41285aa7757ULL;
entry1->size = 0x1486493370dc5073ULL;
m.AddStackFrameEntry(entry1);
-
+
// Second STACK CFI entry, with initial rules but no deltas.
Module::StackFrameEntry *entry2 = new Module::StackFrameEntry();
entry2->address = 0x8064f3af5e067e38ULL;
@@ -396,3 +409,49 @@ TEST(Construct, UniqueFiles) {
EXPECT_EQ(file1, m.FindExistingFile("foo"));
EXPECT_TRUE(m.FindExistingFile("baz") == NULL);
}
+
+TEST(Construct, DuplicateFunctions) {
+ FILE *f = checked_tmpfile();
+ Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID);
+
+ // Two functions.
+ Module::Function *function1 = generate_duplicate_function("_without_form");
+ Module::Function *function2 = generate_duplicate_function("_without_form");
+
+ m.AddFunction(function1);
+ m.AddFunction(function2);
+
+ m.Write(f);
+ checked_fflush(f);
+ rewind(f);
+ string contents = checked_read(f);
+ checked_fclose(f);
+ EXPECT_STREQ("MODULE os-name architecture id-string name with spaces\n"
+ "FUNC d35402aac7a7ad5c 200b26e605f99071 f14ac4fed48c4a99"
+ " _without_form\n",
+ contents.c_str());
+}
+
+TEST(Construct, FunctionsWithSameAddress) {
+ FILE *f = checked_tmpfile();
+ Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID);
+
+ // Two functions.
+ Module::Function *function1 = generate_duplicate_function("_without_form");
+ Module::Function *function2 = generate_duplicate_function("_and_void");
+
+ m.AddFunction(function1);
+ m.AddFunction(function2);
+
+ m.Write(f);
+ checked_fflush(f);
+ rewind(f);
+ string contents = checked_read(f);
+ checked_fclose(f);
+ EXPECT_STREQ("MODULE os-name architecture id-string name with spaces\n"
+ "FUNC d35402aac7a7ad5c 200b26e605f99071 f14ac4fed48c4a99"
+ " _and_void\n"
+ "FUNC d35402aac7a7ad5c 200b26e605f99071 f14ac4fed48c4a99"
+ " _without_form\n",
+ contents.c_str());
+}
diff --git a/src/common/stabs_to_module.cc b/src/common/stabs_to_module.cc
index fbe4c02f..62fcd39e 100644
--- a/src/common/stabs_to_module.cc
+++ b/src/common/stabs_to_module.cc
@@ -58,7 +58,7 @@ static string Demangle(const string &mangled) {
StabsToModule::~StabsToModule() {
// Free any functions we've accumulated but not added to the module.
- for (vector<Module::Function *>::iterator func_it = functions_.begin();
+ for (vector<Module::Function *>::const_iterator func_it = functions_.begin();
func_it != functions_.end(); func_it++)
delete *func_it;
// Free any function that we're currently within.
@@ -104,16 +104,8 @@ bool StabsToModule::EndFunction(uint64_t address) {
assert(current_function_);
// Functions in this compilation unit should have address bigger
// than the compilation unit's starting address. There may be a lot
- // of duplicated entries for functions in the STABS data; only one
- // entry can meet this requirement.
- //
- // (I don't really understand the above comment; just bringing it along
- // from the previous code, and leaving the behavior unchanged. GCC marks
- // the end of each function with an N_FUN entry with no name, whose value
- // is the size of the function; perhaps this test was concerned with
- // skipping those. Now StabsReader interprets them properly. If you know
- // the whole story, please patch this comment. --jimb)
- //
+ // of duplicated entries for functions in the STABS data. We will
+ // count on the Module to remove the duplicates.
if (current_function_->address >= comp_unit_base_address_)
functions_.push_back(current_function_);
else
@@ -153,12 +145,13 @@ void StabsToModule::Finalize() {
// Sort all functions by address, just for neatness.
sort(functions_.begin(), functions_.end(),
Module::Function::CompareByAddress);
- for (vector<Module::Function *>::iterator func_it = functions_.begin();
+
+ for (vector<Module::Function *>::const_iterator func_it = functions_.begin();
func_it != functions_.end();
func_it++) {
Module::Function *f = *func_it;
// Compute the function f's size.
- vector<Module::Address>::iterator boundary
+ vector<Module::Address>::const_iterator boundary
= std::upper_bound(boundaries_.begin(), boundaries_.end(), f->address);
if (boundary != boundaries_.end())
f->size = *boundary - f->address;
diff --git a/src/common/stabs_to_module_unittest.cc b/src/common/stabs_to_module_unittest.cc
index 4248b3c0..2c432a3e 100644
--- a/src/common/stabs_to_module_unittest.cc
+++ b/src/common/stabs_to_module_unittest.cc
@@ -74,6 +74,38 @@ TEST(StabsToModule, SimpleCU) {
EXPECT_EQ(174823314, line->number);
}
+TEST(StabsToModule, DuplicateFunctionNames) {
+ Module m("name", "os", "arch", "id");
+ StabsToModule h(&m);
+
+ // Compilation unit with one function, mangled name.
+ EXPECT_TRUE(h.StartCompilationUnit("compilation-unit", 0xf2cfda36ecf7f46cLL,
+ "build-directory"));
+ EXPECT_TRUE(h.StartFunction("funcfoo",
+ 0xf2cfda36ecf7f46dLL));
+ EXPECT_TRUE(h.EndFunction(0));
+ EXPECT_TRUE(h.StartFunction("funcfoo",
+ 0xf2cfda36ecf7f46dLL));
+ EXPECT_TRUE(h.EndFunction(0));
+ EXPECT_TRUE(h.EndCompilationUnit(0));
+
+ h.Finalize();
+
+ // Now check to see what has been added to the Module.
+ Module::File *file = m.FindExistingFile("compilation-unit");
+ ASSERT_TRUE(file != NULL);
+
+ vector<Module::Function *> functions;
+ m.GetFunctions(&functions, functions.end());
+ ASSERT_EQ(1U, functions.size());
+
+ Module::Function *function = functions[0];
+ EXPECT_EQ(0xf2cfda36ecf7f46dLL, function->address);
+ EXPECT_LT(0U, function->size); // should have used dummy size
+ EXPECT_EQ(0U, function->parameter_size);
+ ASSERT_EQ(0U, function->lines.size());
+}
+
TEST(InferSizes, LineSize) {
Module m("name", "os", "arch", "id");
StabsToModule h(&m);
@@ -88,7 +120,7 @@ TEST(InferSizes, LineSize) {
EXPECT_TRUE(h.EndFunction(0)); // unknown function end address
EXPECT_TRUE(h.EndCompilationUnit(0)); // unknown CU end address
EXPECT_TRUE(h.StartCompilationUnit("compilation-unit-2", 0xb4523963eff94e92LL,
- "build-directory-2")); // next boundary
+ "build-directory-2")); // next boundary
EXPECT_TRUE(h.EndCompilationUnit(0));
h.Finalize();