Skip to content

Commit

Permalink
Add error handling for common failure case seen processing debug symbols
Browse files Browse the repository at this point in the history
Propagates error from ProcessDIEs to the top level where it can be
handled.

Change-Id: Ib6ba171ff642a898c75233a3f70995837c6c0552
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5564423
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
  • Loading branch information
tyrelrussell authored and Joshua Peraza committed May 23, 2024
1 parent 417f5db commit 9dd7d34
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
20 changes: 17 additions & 3 deletions src/common/dwarf/dwarf2reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,11 @@ uint64_t CompilationUnit::Start() {
}

// Now that we have our abbreviations, start processing DIE's.
ProcessDIEs();
if (!ProcessDIEs()) {
// If ProcessDIEs fails return 0, ourlength must be non-zero
// as it is equal to header_.length + (12 or 4)
return 0;
}

// If this is a skeleton compilation unit generated with split DWARF,
// and the client needs the full debug info, we need to find the full
Expand Down Expand Up @@ -922,7 +926,7 @@ const uint8_t* CompilationUnit::ProcessDIE(uint64_t dieoffset,
return start;
}

void CompilationUnit::ProcessDIEs() {
bool CompilationUnit::ProcessDIEs() {
const uint8_t* dieptr = after_header_;
size_t len;

Expand Down Expand Up @@ -953,13 +957,22 @@ void CompilationUnit::ProcessDIEs() {
if (abbrev_num == 0) {
if (die_stack.size() == 0)
// If it is padding, then we are done with the compilation unit's DIEs.
return;
return true;
const uint64_t offset = die_stack.top();
die_stack.pop();
handler_->EndDIE(offset);
continue;
}

// Abbrev > abbrev_.size() indicates a corruption in the dwarf file
// We attempt to recover
if (abbrev_num > abbrevs_->size()) {
fprintf(stderr, "An invalid abbrev was referenced %d / %d. Stopped "
"procesing following DIEs in this CU.", abbrev_num,
abbrevs_->size());
return false;
}

const Abbrev& abbrev = abbrevs_->at(static_cast<size_t>(abbrev_num));
const enum DwarfTag tag = abbrev.tag;
if (!handler_->StartDIE(absolute_offset, tag)) {
Expand Down Expand Up @@ -989,6 +1002,7 @@ void CompilationUnit::ProcessDIEs() {
handler_->EndDIE(absolute_offset);
}
}
return true;
}

// Check for a valid ELF file and return the Address size.
Expand Down
2 changes: 1 addition & 1 deletion src/common/dwarf/dwarf2reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ class CompilationUnit {
}

// Processes all DIEs for this compilation unit
void ProcessDIEs();
bool ProcessDIEs();

// Skips the die with attributes specified in ABBREV starting at
// START, and return the new place to position the stream to.
Expand Down
21 changes: 15 additions & 6 deletions src/common/linux/dump_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,11 @@ bool LoadDwarf(const string& dwarf_filename,
&byte_reader,
&die_dispatcher);
// Process the entire compilation unit; get the offset of the next.
offset += reader.Start();
uint64_t result = reader.Start();
if (result == 0) {
return false;
}
offset += result;
// Start to process split dwarf file.
if (reader.ShouldProcessSplitDwarf()) {
StartProcessSplitDwarf(&reader, module, endianness, handle_inter_cu_refs,
Expand Down Expand Up @@ -878,6 +882,7 @@ bool LoadSymbols(const string& obj_file,
const char* names_end = names + section_names->sh_size;
bool found_debug_info_section = false;
bool found_usable_info = false;
bool usable_info_parsed = false;

if ((options.symbol_data & SYMBOLS_AND_FILES) ||
(options.symbol_data & INLINES)) {
Expand All @@ -893,8 +898,10 @@ bool LoadSymbols(const string& obj_file,
found_debug_info_section = true;
found_usable_info = true;
info->LoadedSection(".stab");
if (!LoadStabs<ElfClass>(elf_header, stab_section, stabstr_section,
big_endian, module)) {
bool result = LoadStabs<ElfClass>(elf_header, stab_section, stabstr_section,
big_endian, module);
usable_info_parsed = usable_info_parsed || result;
if (!result) {
fprintf(stderr, "%s: \".stab\" section found, but failed to load"
" STABS debugging information\n", obj_file.c_str());
}
Expand Down Expand Up @@ -981,9 +988,11 @@ bool LoadSymbols(const string& obj_file,
found_debug_info_section = true;
found_usable_info = true;
info->LoadedSection(".debug_info");
if (!LoadDwarf<ElfClass>(obj_file, elf_header, big_endian,
bool result = LoadDwarf<ElfClass>(obj_file, elf_header, big_endian,
options.handle_inter_cu_refs,
options.symbol_data & INLINES, module)) {
options.symbol_data & INLINES, module);
usable_info_parsed = usable_info_parsed || result;
if (!result){
fprintf(stderr, "%s: \".debug_info\" section found, but failed to load "
"DWARF debugging information\n", obj_file.c_str());
}
Expand Down Expand Up @@ -1088,7 +1097,7 @@ bool LoadSymbols(const string& obj_file,
return false;
}

return true;
return usable_info_parsed;
}

// Return the breakpad symbol file identifier for the architecture of
Expand Down

0 comments on commit 9dd7d34

Please sign in to comment.