Merge trunk r875 to the chrome_15 branch.
Fix some shadow variables, including one in file_id.cc that causes all files to
generate the same hash. Add a test to make sure this doesn't happen again.
Review URL: http://breakpad.appspot.com/316002
Original patch by Lei Zhang <thestig@chromium.org>
Review URL: http://breakpad.appspot.com/320001
Cr-Mirrored-From: https://chromium.googlesource.com/breakpad/breakpad
Cr-Mirrored-Commit: 3a642b92dc35b93107080ff791353882d878c82e
diff --git a/client/linux/minidump_writer/minidump_writer.cc b/client/linux/minidump_writer/minidump_writer.cc
index 77ff7ea..b860e43 100644
--- a/client/linux/minidump_writer/minidump_writer.cc
+++ b/client/linux/minidump_writer/minidump_writer.cc
@@ -671,8 +671,8 @@
// don't bother trying to write it.
bool ip_is_mapped = false;
MDMemoryDescriptor ip_memory_d;
- for (unsigned i = 0; i < dumper_.mappings().size(); ++i) {
- const MappingInfo& mapping = *dumper_.mappings()[i];
+ for (unsigned j = 0; j < dumper_.mappings().size(); ++j) {
+ const MappingInfo& mapping = *dumper_.mappings()[j];
if (ip >= mapping.start_addr &&
ip < mapping.start_addr + mapping.size) {
ip_is_mapped = true;
diff --git a/common/linux/file_id.cc b/common/linux/file_id.cc
index acd448a..13f4f3a 100644
--- a/common/linux/file_id.cc
+++ b/common/linux/file_id.cc
@@ -107,10 +107,10 @@
const Shdr* section = NULL;
for (int i = 0; i < elf_header->e_shnum; ++i) {
if (sections[i].sh_type == section_type) {
- const char* section_name = (char*)(elf_base +
- string_section->sh_offset +
- sections[i].sh_name);
- if (!my_strncmp(section_name, section_name, name_len)) {
+ const char* current_section_name = (char*)(elf_base +
+ string_section->sh_offset +
+ sections[i].sh_name);
+ if (!my_strncmp(current_section_name, section_name, name_len)) {
section = §ions[i];
break;
}
@@ -166,8 +166,7 @@
template<typename ElfClass>
static bool ElfClassBuildIDNoteIdentifier(const void *section,
- uint8_t identifier[kMDGUIDSize])
-{
+ uint8_t identifier[kMDGUIDSize]) {
typedef typename ElfClass::Nhdr Nhdr;
const Nhdr* note_header = reinterpret_cast<const Nhdr*>(section);
@@ -190,8 +189,7 @@
// Attempt to locate a .note.gnu.build-id section in an ELF binary
// and copy as many bytes of it as will fit into |identifier|.
static bool FindElfBuildIDNote(const void *elf_mapped_base,
- uint8_t identifier[kMDGUIDSize])
-{
+ uint8_t identifier[kMDGUIDSize]) {
void* note_section;
int note_size, elfclass;
if (!FindElfSection(elf_mapped_base, ".note.gnu.build-id", SHT_NOTE,
@@ -213,7 +211,6 @@
// a simple hash by XORing the first page worth of bytes into |identifier|.
static bool HashElfTextSection(const void *elf_mapped_base,
uint8_t identifier[kMDGUIDSize]) {
-
void* text_section;
int text_size;
if (!FindElfSection(elf_mapped_base, ".text", SHT_PROGBITS,
@@ -235,8 +232,7 @@
// static
bool FileID::ElfFileIdentifierFromMappedFile(void* base,
- uint8_t identifier[kMDGUIDSize])
-{
+ uint8_t identifier[kMDGUIDSize]) {
// Look for a build id note first.
if (FindElfBuildIDNote(base, identifier))
return true;
diff --git a/common/linux/file_id_unittest.cc b/common/linux/file_id_unittest.cc
index 2420898..7479a96 100644
--- a/common/linux/file_id_unittest.cc
+++ b/common/linux/file_id_unittest.cc
@@ -43,6 +43,18 @@
using google_breakpad::test_assembler::kLittleEndian;
using google_breakpad::test_assembler::Section;
+namespace {
+
+// Simply calling Section::Append(size, byte) produces a uninteresting pattern
+// that tends to get hashed to 0000...0000. This populates the section with
+// data to produce better hashes.
+void PopulateSection(Section* section, int size, int prime_number) {
+ for (int i = 0; i < size; i++)
+ section->Append(1, (i % prime_number) % 256);
+}
+
+} // namespace
+
TEST(FileIDStripTest, StripSelf) {
// Calculate the File ID of this binary using
// FileID::ElfFileIdentifier, then make a copy of this binary,
@@ -181,3 +193,92 @@
sizeof(identifier_string));
EXPECT_STREQ(expected_identifier_string, identifier_string);
}
+
+// Test to make sure two files with different text sections produce
+// different hashes when not using a build id.
+TEST_F(FileIDTest, UniqueHashes32) {
+ char identifier_string_1[] =
+ "00000000-0000-0000-0000-000000000000";
+ char identifier_string_2[] =
+ "00000000-0000-0000-0000-000000000000";
+ uint8_t identifier_1[sizeof(MDGUID)];
+ uint8_t identifier_2[sizeof(MDGUID)];
+
+ {
+ ELF elf1(EM_386, ELFCLASS32, kLittleEndian);
+ Section foo_1(kLittleEndian);
+ PopulateSection(&foo_1, 32, 5);
+ elf1.AddSection(".foo", foo_1, SHT_PROGBITS);
+ Section text_1(kLittleEndian);
+ PopulateSection(&text_1, 4096, 17);
+ elf1.AddSection(".text", text_1, SHT_PROGBITS);
+ elf1.Finish();
+ GetElfContents(elf1);
+ }
+
+ EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_1));
+ FileID::ConvertIdentifierToString(identifier_1, identifier_string_1,
+ sizeof(identifier_string_1));
+
+ {
+ ELF elf2(EM_386, ELFCLASS32, kLittleEndian);
+ Section text_2(kLittleEndian);
+ Section foo_2(kLittleEndian);
+ PopulateSection(&foo_2, 32, 5);
+ elf2.AddSection(".foo", foo_2, SHT_PROGBITS);
+ PopulateSection(&text_2, 4096, 31);
+ elf2.AddSection(".text", text_2, SHT_PROGBITS);
+ elf2.Finish();
+ GetElfContents(elf2);
+ }
+
+ EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_2));
+ FileID::ConvertIdentifierToString(identifier_2, identifier_string_2,
+ sizeof(identifier_string_2));
+
+ EXPECT_STRNE(identifier_string_1, identifier_string_2);
+}
+
+// Same as UniqueHashes32, for x86-64.
+TEST_F(FileIDTest, UniqueHashes64) {
+ char identifier_string_1[] =
+ "00000000-0000-0000-0000-000000000000";
+ char identifier_string_2[] =
+ "00000000-0000-0000-0000-000000000000";
+ uint8_t identifier_1[sizeof(MDGUID)];
+ uint8_t identifier_2[sizeof(MDGUID)];
+
+ {
+ ELF elf1(EM_X86_64, ELFCLASS64, kLittleEndian);
+ Section foo_1(kLittleEndian);
+ PopulateSection(&foo_1, 32, 5);
+ elf1.AddSection(".foo", foo_1, SHT_PROGBITS);
+ Section text_1(kLittleEndian);
+ PopulateSection(&text_1, 4096, 17);
+ elf1.AddSection(".text", text_1, SHT_PROGBITS);
+ elf1.Finish();
+ GetElfContents(elf1);
+ }
+
+ EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_1));
+ FileID::ConvertIdentifierToString(identifier_1, identifier_string_1,
+ sizeof(identifier_string_1));
+
+ {
+ ELF elf2(EM_X86_64, ELFCLASS64, kLittleEndian);
+ Section text_2(kLittleEndian);
+ Section foo_2(kLittleEndian);
+ PopulateSection(&foo_2, 32, 5);
+ elf2.AddSection(".foo", foo_2, SHT_PROGBITS);
+ PopulateSection(&text_2, 4096, 31);
+ elf2.AddSection(".text", text_2, SHT_PROGBITS);
+ elf2.Finish();
+ GetElfContents(elf2);
+ }
+
+ EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_2));
+ FileID::ConvertIdentifierToString(identifier_2, identifier_string_2,
+ sizeof(identifier_string_2));
+
+ EXPECT_STRNE(identifier_string_1, identifier_string_2);
+}
diff --git a/common/stabs_reader.cc b/common/stabs_reader.cc
index 7dd2eec..4897361 100644
--- a/common/stabs_reader.cc
+++ b/common/stabs_reader.cc
@@ -130,7 +130,7 @@
// There may be an N_SO entry whose name ends with a slash,
// indicating the directory in which the compilation occurred.
// The build directory defaults to NULL.
- const char *build_directory = NULL;
+ const char *build_directory = NULL;
{
const char *name = SymbolString();
if (name[0] && name[strlen(name) - 1] == '/') {
@@ -138,7 +138,7 @@
++iterator_;
}
}
-
+
// We expect to see an N_SO entry with a filename next, indicating
// the start of the compilation unit.
{
@@ -212,7 +212,7 @@
queued_lines_.clear();
return true;
-}
+}
bool StabsReader::ProcessFunction() {
assert(!iterator_->at_end && iterator_->type == N_FUN);
@@ -237,7 +237,7 @@
return false;
}
queued_lines_.clear();
-
+
while (!iterator_->at_end) {
if (iterator_->type == N_SO || iterator_->type == N_FUN)
break;
@@ -266,8 +266,8 @@
if (!iterator_->at_end) {
assert(iterator_->type == N_SO || iterator_->type == N_FUN);
if (iterator_->type == N_FUN) {
- const char *name = SymbolString();
- if (name[0] == '\0') {
+ const char *symbol_name = SymbolString();
+ if (symbol_name[0] == '\0') {
// An N_FUN entry with no name is a terminator for this function;
// its value is the function's size.
ending_address = function_address + iterator_->value;
diff --git a/processor/source_line_resolver_base.cc b/processor/source_line_resolver_base.cc
index b8362dc..49088c8 100644
--- a/processor/source_line_resolver_base.cc
+++ b/processor/source_line_resolver_base.cc
@@ -88,7 +88,7 @@
int error_code = stat(map_file.c_str(), &buf);
if (error_code == -1) {
string error_string;
- int error_code = ErrnoString(&error_string);
+ error_code = ErrnoString(&error_string);
BPLOG(ERROR) << "Could not open " << map_file <<
", error " << error_code << ": " << error_string;
return false;
@@ -110,7 +110,7 @@
FILE *f = fopen(map_file.c_str(), "rt");
if (!f) {
string error_string;
- int error_code = ErrnoString(&error_string);
+ error_code = ErrnoString(&error_string);
BPLOG(ERROR) << "Could not open " << map_file <<
", error " << error_code << ": " << error_string;
delete [] (*symbol_data);
@@ -126,7 +126,7 @@
if (items_read != file_size) {
string error_string;
- int error_code = ErrnoString(&error_string);
+ error_code = ErrnoString(&error_string);
BPLOG(ERROR) << "Could not slurp " << map_file <<
", error " << error_code << ": " << error_string;
delete [] (*symbol_data);
@@ -240,11 +240,11 @@
if (!code_module)
return;
- ModuleMap::iterator iter = modules_->find(code_module->code_file());
- if (iter != modules_->end()) {
- Module *symbol_module = iter->second;
+ ModuleMap::iterator mod_iter = modules_->find(code_module->code_file());
+ if (mod_iter != modules_->end()) {
+ Module *symbol_module = mod_iter->second;
delete symbol_module;
- modules_->erase(iter);
+ modules_->erase(mod_iter);
}
if (ShouldDeleteMemoryBufferAfterLoadModule()) {
diff --git a/tools/linux/md2core/minidump-2-core.cc b/tools/linux/md2core/minidump-2-core.cc
index f08da76..efec22e 100644
--- a/tools/linux/md2core/minidump-2-core.cc
+++ b/tools/linux/md2core/minidump-2-core.cc
@@ -442,7 +442,7 @@
#else
#error "This code has not been ported to your platform yet"
#endif
- if (!strstr(full_file.GetString(sysinfo->csd_version_rva).c_str(), "Linux")){
+ if (!strstr(full_file.GetString(sysinfo->csd_version_rva).c_str(), "Linux")) {
fprintf(stderr, "This minidump was not generated by Linux.\n");
_exit(1);
}
@@ -654,9 +654,9 @@
len = range.length() > args_len ? args_len : range.length();
memcpy(crashinfo->prps.pr_psargs, cmdline, len);
- for (unsigned i = 0; i < len; ++i) {
- if (crashinfo->prps.pr_psargs[i] == 0)
- crashinfo->prps.pr_psargs[i] = ' ';
+ for (unsigned j = 0; j < len; ++j) {
+ if (crashinfo->prps.pr_psargs[j] == 0)
+ crashinfo->prps.pr_psargs[j] = ' ';
}
break;
}
