commit 52b33b2a4b9f374255817afecc6e7d215fa80f28
author Mark Mentovai <mark@chromium.org> Fri Mar 06 10:40:39 2020 -0500
committer Mark Mentovai <mark@chromium.org> Fri Mar 06 17:08:57 2020 +0000
tree 342ab495274f375a6a2845be50d598e8c3bde71d
parent c7522272ffafa9b162f135aaee5d02a8895fcb0b

dump_syms: Reintroduce warnings inadvertently removed by 47cd498384fd

Previously, dump_syms produced warnings whenever a DW_AT_specification
or DW_AT_abstract_origin attribute was a forward reference. 47cd498384fd
allowed those attributes to carry forward references, removing the
warnings altogether. It was not correct to remove the warnings entirely.
References that do not point to valid DIEs should still produce
warnings, whether a back reference or a forward reference.

This reintroduces those warnings as appropriate.

Bug: google-breakpad:813
Test: dumper_unittest SimpleCU.UnknownAbstractOrigin,Specifications.BadOffset
Change-Id: Ie7222c7a1886bab31423f27e2fbcce93e69625b3
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2090103
Reviewed-by: Mike Frysinger <vapier@chromium.org>

default.xml

diff --git a/default.xml b/default.xml
index ec1cd04..8e8c6cd 100644
--- a/default.xml
+++ b/default.xml
@@ -32,7 +32,7 @@
 
   <project path='src/src/third_party/lss'
            name='linux-syscall-support/'
-           revision='8048ece6c16c91acfe0d36d1d3cc0890ab6e945c'
+           revision='f70e2f1641e280e777edfdad7f73a2cfa38139c7'
            remote='chromium' />
 
   <project path='src/src/third_party/protobuf/protobuf'

src/common/dwarf_cu_to_module.cc

diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc
index 1d8cbcb..56399c3 100644
--- a/src/common/dwarf_cu_to_module.cc
+++ b/src/common/dwarf_cu_to_module.cc
@@ -350,9 +350,10 @@
       SpecificationByOffset::iterator spec = specifications->find(data);
       if (spec != specifications->end()) {
         specification_ = &spec->second;
-      } else {
-        // The DW_AT_specification is a forward reference.
+      } else if (data > offset_) {
         forward_ref_die_offset_ = data;
+      } else {
+        cu_context_->reporter->UnknownSpecification(offset_, data);
       }
       break;
     }
@@ -544,8 +545,10 @@
       AbstractOriginByOffset::const_iterator origin = origins.find(data);
       if (origin != origins.end()) {
         abstract_origin_ = &(origin->second);
-      } else {
+      } else if (data > offset_) {
         forward_ref_die_offset_ = data;
+      } else {
+        cu_context_->reporter->UnknownAbstractOrigin(offset_, data);
       }
       break;
     }
@@ -581,9 +584,9 @@
   // to be processed, and fix up the name of the appropriate Module::Function.
   // "name_" will have already been fixed up in EndAttributes().
   if (!name_.empty()) {
-    auto Iter = cu_context_->forward_ref_die_to_func.find(offset_);
-    if (Iter != cu_context_->forward_ref_die_to_func.end())
-      Iter->second->name = name_;
+    auto iter = cu_context_->forward_ref_die_to_func.find(offset_);
+    if (iter != cu_context_->forward_ref_die_to_func.end())
+      iter->second->name = name_;
   }
 
   if (!ranges_) {
@@ -638,10 +641,17 @@
       // If the function address is zero this is a sign that this function
       // description is just empty debug data and should just be discarded.
       cu_context_->functions.push_back(func.release());
-      if (forward_ref_die_offset_ != 0)
-        cu_context_->forward_ref_die_to_func[forward_ref_die_offset_] =
-            cu_context_->functions.back();
-     }
+      if (forward_ref_die_offset_ != 0) {
+        auto iter =
+            cu_context_->forward_ref_die_to_func.find(forward_ref_die_offset_);
+        if (iter == cu_context_->forward_ref_die_to_func.end()) {
+          cu_context_->reporter->UnknownSpecification(offset_,
+                                                      forward_ref_die_offset_);
+        } else {
+          iter->second = cu_context_->functions.back();
+        }
+      }
+    }
   } else if (inline_) {
     AbstractOrigin origin(name_);
     cu_context_->file_context->file_private_->origins[offset_] = origin;
@@ -695,8 +705,8 @@
                                                             uint64 target) {
   CUHeading();
   fprintf(stderr, "%s: the DIE at offset 0x%llx has a DW_AT_specification"
-          " attribute referring to the die at offset 0x%llx, which either"
-          " was not marked as a declaration, or comes later in the file\n",
+          " attribute referring to the DIE at offset 0x%llx, which was not"
+          " marked as a declaration\n",
           filename_.c_str(), offset, target);
 }
 
@@ -704,8 +714,8 @@
                                                              uint64 target) {
   CUHeading();
   fprintf(stderr, "%s: the DIE at offset 0x%llx has a DW_AT_abstract_origin"
-          " attribute referring to the die at offset 0x%llx, which either"
-          " was not marked as an inline, or comes later in the file\n",
+          " attribute referring to the DIE at offset 0x%llx, which was not"
+          " marked as an inline\n",
           filename_.c_str(), offset, target);
 }