Revert D19444082: Get the correct file name for inline functions.

Differential Revision:
D19444082

Original commit changeset: 5a5524046090

fbshipit-source-id: ecb9b9048af9c00c951cb6e540433b5a3cecaf71

folly/experimental/symbolizer/Dwarf.cpp

@@ -78,8 +78,7 @@ struct Attribute {
 
 struct CodeLocation {
   Die die;
-  folly::Optional<uint64_t> file;
-  folly::Optional<uint64_t> line;
+  uint64_t line;
 };
 
 } // namespace detail
@@ -538,13 +537,10 @@ bool Dwarf::findLocation(
            inlineLocIter != inlineLocsRange.end();
            inlineLocIter++, inlineFrameIter++) {
         inlineFrameIter->location.line = callerLine;
-        if (!inlineLocIter->line) {
-          break;
-        }
-        callerLine = inlineLocIter->line.value();
-
+        callerLine = inlineLocIter->line;
         folly::Optional<folly::StringPiece> linkageName;
         folly::Optional<folly::StringPiece> name;
+        folly::Optional<uint64_t> file;
         forEachAttribute(
             cu, inlineLocIter->die, [&](const detail::Attribute& attr) {
               switch (attr.spec.name) {
@@ -555,18 +551,12 @@ bool Dwarf::findLocation(
                   name = boost::get<folly::StringPiece>(attr.attrValue);
                   break;
                 case DW_AT_decl_file:
-                  // If the inline function is declared and defined in the same
-                  // file, the declaration subprogram may not have attribute
-                  // DW_AT_decl_file, then we need to find decl_file in the
-                  // definition subprogram.
-                  if (!inlineLocIter->file) {
-                    inlineLocIter->file = boost::get<uint64_t>(attr.attrValue);
-                  }
+                  file = boost::get<uint64_t>(attr.attrValue);
                   break;
               }
-              return !inlineLocIter->file || !name || !linkageName;
+              return !file || !name || !linkageName;
             });
-        if (!(name || linkageName) || !inlineLocIter->file) {
+        if (!(name || linkageName) || !file) {
           break;
         }
 
@@ -577,8 +567,7 @@ bool Dwarf::findLocation(
         inlineFrameIter->location.hasFileAndLine = true;
         inlineFrameIter->location.name =
             linkageName ? linkageName.value() : name.value();
-        inlineFrameIter->location.file =
-            lineVM.getFullFileName(inlineLocIter->file.value());
+        inlineFrameIter->location.file = lineVM.getFullFileName(file.value());
       }
       locationInfo.line = callerLine;
     }
@@ -661,9 +650,17 @@ detail::Die Dwarf::findDefinitionDie(
     const detail::CompilationUnit& cu,
     const detail::Die& die) const {
   // Find the real definition instead of declaration.
-  // DW_AT_specification: Incomplete, non-defining, or separate declaration
-  // corresponding to a declaration
-  auto offset = getAttribute<uint64_t>(cu, die, DW_AT_specification);
+  folly::Optional<uint64_t> offset;
+  forEachAttribute(cu, die, [&](const detail::Attribute& attr) {
+    // Incomplete, non-defining, or separate declaration corresponding to
+    // a declaration
+    if (attr.spec.name == DW_AT_specification) {
+      offset = boost::get<uint64_t>(attr.attrValue);
+      return false;
+    }
+    return true;
+  });
+
   if (!offset) {
     return die;
   }
@@ -718,22 +715,6 @@ size_t Dwarf::forEachAttribute(
   return values.data() - debugInfo_.data();
 }
 
-template <class T>
-folly::Optional<T> Dwarf::getAttribute(
-    const detail::CompilationUnit& cu,
-    const detail::Die& die,
-    uint64_t attrName) const {
-  folly::Optional<T> result;
-  forEachAttribute(cu, die, [&](const detail::Attribute& attr) {
-    if (attr.spec.name == attrName) {
-      result = boost::get<T>(attr.attrValue);
-      return false;
-    }
-    return true;
-  });
-  return result;
-}
-
 void Dwarf::findSubProgramDieForAddress(
     const detail::CompilationUnit& cu,
     const detail::Die& die,
@@ -793,7 +774,7 @@ void Dwarf::findInlinedSubroutineDieForAddress(
     bool isHighPcAddr = false;
     uint64_t origin = 0;
     uint64_t originRefType = 0;
-    folly::Optional<uint64_t> callLine = 0;
+    uint64_t callLine = 0;
     forEachAttribute(cu, childDie, [&](const detail::Attribute& attr) {
       switch (attr.spec.name) {
         // Here DW_AT_ranges is not supported since it requires looking up
@@ -822,30 +803,22 @@ void Dwarf::findInlinedSubroutineDieForAddress(
       // Address doesn't match. Keep searching other children.
       return true;
     }
-
     isrLoc[0].line = callLine;
-    auto setLocationInfo = [&](uint64_t dieOffset) {
-      auto declDie = getDieAtOffset(cu, dieOffset);
-      // If the inline function is declared and defined in different file, then
-      // DW_AT_decl_file attribute of the declaration may contain the definition
-      // file name.
-      isrLoc[0].file = getAttribute<uint64_t>(cu, declDie, DW_AT_decl_file);
-      // Jump to the actual function definition instead of declaration for name
-      // and line info.
-      isrLoc[0].die = findDefinitionDie(cu, declDie);
-      isrLoc.advance(1);
-    };
     if (originRefType == DW_FORM_ref1 || originRefType == DW_FORM_ref2 ||
         originRefType == DW_FORM_ref4 || originRefType == DW_FORM_ref8 ||
         originRefType == DW_FORM_ref_udata) {
-      setLocationInfo(cu.offset + origin);
+      // Jump to the actual function definition instead of declaration for name
+      // and line info.
+      isrLoc[0].die =
+          findDefinitionDie(cu, getDieAtOffset(cu, cu.offset + origin));
+      isrLoc.advance(1);
       findInlinedSubroutineDieForAddress(cu, childDie, address, isrLoc);
     } else if (originRefType == DW_FORM_ref_addr) {
-      setLocationInfo(origin);
       auto srcu = findCompilationUnit(debugInfo_, origin);
+      isrLoc[0].die = findDefinitionDie(cu, getDieAtOffset(cu, origin));
+      isrLoc.advance(1);
       findInlinedSubroutineDieForAddress(srcu, childDie, address, isrLoc);
     }
-
     return false;
   });
 }

folly/experimental/symbolizer/Dwarf.h

@@ -183,12 +183,6 @@ class Dwarf {
       const detail::Die& die,
       folly::FunctionRef<bool(const detail::Attribute& die)> f) const;
 
-  template <class T>
-  folly::Optional<T> getAttribute(
-      const detail::CompilationUnit& cu,
-      const detail::Die& die,
-      uint64_t attrName) const;
-
   const ElfFile* elf_;
   const folly::StringPiece debugInfo_; // .debug_info
   const folly::StringPiece debugAbbrev_; // .debug_abbrev

folly/experimental/symbolizer/test/SymbolizerTest.cpp

@@ -23,7 +23,6 @@
 #include <folly/Range.h>
 #include <folly/String.h>
 #include <folly/experimental/symbolizer/SymbolizedFrame.h>
-#include <folly/experimental/symbolizer/test/SymbolizerTestUtils.h>
 #include <folly/portability/GTest.h>
 
 namespace folly {
@@ -50,6 +49,17 @@ TEST(Symbolizer, Single) {
   EXPECT_EQ("SymbolizerTest.cpp", basename.str());
 }
 
+void* framesToFill{nullptr};
+
+template <size_t kNumFrames = 100>
+int comparator(const void* ap, const void* bp) {
+  getStackTrace(*static_cast<FrameArray<kNumFrames>*>(framesToFill));
+
+  int a = *static_cast<const int*>(ap);
+  int b = *static_cast<const int*>(bp);
+  return a < b ? -1 : a > b ? 1 : 0;
+}
+
 // Test stack frames...
 FOLLY_NOINLINE void bar();
 
@@ -123,6 +133,21 @@ TEST(SymbolizerTest, SymbolCache) {
 
 namespace {
 
+size_t kQsortCallLineNo = 0;
+size_t kFooCallByStandaloneBarLineNo = 0;
+size_t kFooCallByClassBarLineNo = 0;
+size_t kFooCallByClassStaticBarLineNo = 0;
+
+template <size_t kNumFrames = 100>
+FOLLY_ALWAYS_INLINE void inlineFoo(FrameArray<kNumFrames>& frames) {
+  framesToFill = &frames;
+  std::array<int, 2> a = {1, 2};
+  // Use qsort, which is in a different library
+  kQsortCallLineNo = __LINE__ + 1;
+  qsort(a.data(), 2, sizeof(int), comparator<kNumFrames>);
+  framesToFill = nullptr;
+}
+
 template <size_t kNumFrames = 100>
 FOLLY_ALWAYS_INLINE void inlineBar(FrameArray<kNumFrames>& frames) {
   kFooCallByStandaloneBarLineNo = __LINE__ + 1;
@@ -132,19 +157,20 @@ FOLLY_ALWAYS_INLINE void inlineBar(FrameArray<kNumFrames>& frames) {
 void verifyStackTrace(
     const FrameArray<100>& frames,
     const std::string& barName,
-    size_t barLine,
-    const std::string& barFile) {
+    size_t barLine) {
   EXPECT_TRUE(frames.frameCount == 14 || frames.frameCount == 15);
   EXPECT_EQ(
-      "void folly::symbolizer::test::inlineFoo<100ul>("
+      "void folly::symbolizer::test::(anonymous namespace)::inlineFoo<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",
       std::string(folly::demangle(frames.frames[5].name)));
   EXPECT_EQ(
-      "folly/experimental/symbolizer/test/SymbolizerTestUtils-inl.h",
+      "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
       std::string(frames.frames[5].location.file.toString()));
   EXPECT_EQ(kQsortCallLineNo, frames.frames[5].location.line);
   EXPECT_EQ(barName, std::string(folly::demangle(frames.frames[6].name)));
-  EXPECT_EQ(barFile, std::string(frames.frames[6].location.file.toString()));
+  EXPECT_EQ(
+      "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
+      std::string(frames.frames[6].location.file.toString()));
   EXPECT_EQ(barLine, frames.frames[6].location.line);
 }
 
@@ -190,12 +216,11 @@ TEST(SymbolizerTest, InlineFunctionBasic) {
   // clang-format off
   // Expected full stack trace with @mode/dev. The last frame is missing in opt
   // mode.
-  //  Frame: _ZN5folly10symbolizer13getStackTraceEPmm
   //  Frame: _ZN5folly10symbolizer13getStackTraceILm100EEEbRNS0_10FrameArrayIXT_EEE
   //  Frame: _ZN5folly10symbolizer4test10comparatorILm100EEEiPKvS4_
   //  Frame: msort_with_tmp.part.0
   //  Frame: __GI___qsort_r
-  //  Frame: _ZN5folly10symbolizer4test9inlineFooILm100EEEvRNS0_10FrameArrayIXT_EEE
+  //  Frame: _ZN5folly10symbolizer4test12_GLOBAL__N_19inlineFooILm100EEEvRNS0_10FrameArrayIXT_EEE
   //  Frame: _ZN5folly10symbolizer4test12_GLOBAL__N_19inlineBarILm100EEEvRNS0_10FrameArrayIXT_EEE
   //  Frame: _ZN5folly10symbolizer4test39SymbolizerTest_InlineFunctionBasic_Test8TestBodyEv
   //  Frame: _ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc
@@ -203,13 +228,14 @@ TEST(SymbolizerTest, InlineFunctionBasic) {
   //  Frame: _ZN7testing8TestInfo3RunEv
   //  Frame: _ZN7testing8TestCase3RunEv
   //  Frame: _ZN7testing8internal12UnitTestImpl11RunAllTestsEv
+  //  Frame: _ZN7testing8UnitTest3RunEv
+  //  Frame: _Z13RUN_ALL_TESTSv
   // clang-format on
   verifyStackTrace(
       frames,
       "void folly::symbolizer::test::(anonymous namespace)::inlineBar<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",
-      kFooCallByStandaloneBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTest.cpp");
+      kFooCallByStandaloneBarLineNo);
 
   FrameArray<100> frames2;
   inlineBar<100>(frames2);
@@ -230,8 +256,7 @@ TEST(SymbolizerTest, InlineClassMemberFunction) {
       frames,
       "folly::symbolizer::test::ClassWithInlineFunctions::inlineBar("
       "folly::symbolizer::FrameArray<100ul>&) const",
-      kFooCallByClassBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTest.cpp");
+      kFooCallByClassBarLineNo);
 }
 
 TEST(SymbolizerTest, StaticInlineClassMemberFunction) {
@@ -245,39 +270,7 @@ TEST(SymbolizerTest, StaticInlineClassMemberFunction) {
       frames,
       "folly::symbolizer::test::ClassWithInlineFunctions::staticInlineBar("
       "folly::symbolizer::FrameArray<100ul>&)",
-      kFooCallByClassStaticBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTest.cpp");
-}
-
-TEST(SymbolizerTest, InlineClassMemberFunctionInDifferentFile) {
-  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL_WITH_INLINE, 0);
-
-  FrameArray<100> frames;
-  InlineFunctionsWrapper obj;
-  obj.inlineBar(frames);
-  symbolizer.symbolize(frames);
-
-  verifyStackTrace(
-      frames,
-      "folly::symbolizer::test::InlineFunctionsWrapper::inlineBar("
-      "folly::symbolizer::FrameArray<100ul>&) const",
-      kFooCallByClassInDifferentFileBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTestUtils-inl.h");
-}
-
-TEST(SymbolizerTest, StaticInlineClassMemberFunctionInDifferentFile) {
-  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL_WITH_INLINE, 0);
-
-  FrameArray<100> frames;
-  InlineFunctionsWrapper::staticInlineBar(frames);
-  symbolizer.symbolize(frames);
-
-  verifyStackTrace(
-      frames,
-      "folly::symbolizer::test::InlineFunctionsWrapper::staticInlineBar("
-      "folly::symbolizer::FrameArray<100ul>&)",
-      kFooCallByClassInDifferentFileStaticBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTestUtils-inl.h");
+      kFooCallByClassStaticBarLineNo);
 }
 
 // No inline frames should be filled because of no extra frames.
@@ -306,8 +299,7 @@ TEST(SymbolizerTest, InlineFunctionWithCache) {
       frames,
       "void folly::symbolizer::test::(anonymous namespace)::inlineBar<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",
-      kFooCallByStandaloneBarLineNo,
-      "folly/experimental/symbolizer/test/SymbolizerTest.cpp");
+      kFooCallByStandaloneBarLineNo);
 
   FrameArray<100> frames2;
   inlineBar<100>(frames2);

folly/experimental/symbolizer/test/SymbolizerTestUtils-inl.h

-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <folly/experimental/symbolizer/test/SymbolizerTestUtils.h>
-
-namespace folly {
-namespace symbolizer {
-namespace test {
-
-/*
- * Put the inline functions definition in a separate -inl.h file to cover test
- * cases that define and declare inline functions in different files.
- */
-
-template <size_t kNumFrames>
-FOLLY_ALWAYS_INLINE void inlineFoo(FrameArray<kNumFrames>& frames) {
-  framesToFill = &frames;
-  std::array<int, 2> a = {1, 2};
-  // Use qsort, which is in a different library
-  kQsortCallLineNo = __LINE__ + 1;
-  qsort(a.data(), 2, sizeof(int), comparator<kNumFrames>);
-  framesToFill = nullptr;
-}
-
-FOLLY_ALWAYS_INLINE void InlineFunctionsWrapper::inlineBar(
-    FrameArray<100>& frames) const {
-  kFooCallByClassInDifferentFileBarLineNo = __LINE__ + 1;
-  inlineFoo(frames);
-}
-
-/* static */ FOLLY_ALWAYS_INLINE void InlineFunctionsWrapper::staticInlineBar(
-    FrameArray<100>& frames) {
-  kFooCallByClassInDifferentFileStaticBarLineNo = __LINE__ + 1;
-  inlineFoo(frames);
-}
-
-} // namespace test
-} // namespace symbolizer
-} // namespace folly

folly/experimental/symbolizer/test/SymbolizerTestUtils.h

-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <folly/experimental/symbolizer/SymbolizedFrame.h>
-
-namespace folly {
-namespace symbolizer {
-namespace test {
-
-void* framesToFill{nullptr};
-
-template <size_t kNumFrames = 100>
-int comparator(const void* ap, const void* bp) {
-  getStackTrace(*static_cast<FrameArray<kNumFrames>*>(framesToFill));
-
-  int a = *static_cast<const int*>(ap);
-  int b = *static_cast<const int*>(bp);
-  return a < b ? -1 : a > b ? 1 : 0;
-}
-
-size_t kQsortCallLineNo = 0;
-size_t kFooCallByStandaloneBarLineNo = 0;
-size_t kFooCallByClassBarLineNo = 0;
-size_t kFooCallByClassStaticBarLineNo = 0;
-size_t kFooCallByClassInDifferentFileBarLineNo = 0;
-size_t kFooCallByClassInDifferentFileStaticBarLineNo = 0;
-
-template <size_t kNumFrames = 100>
-FOLLY_ALWAYS_INLINE void inlineFoo(FrameArray<kNumFrames>& frames);
-
-class InlineFunctionsWrapper {
- public:
-  FOLLY_ALWAYS_INLINE void inlineBar(FrameArray<100>& frames) const;
-
-  FOLLY_ALWAYS_INLINE static void staticInlineBar(FrameArray<100>& frames);
-
-  // Dummy non-inline function.
-  size_t dummy() const {
-    return dummy_;
-  }
-
-  size_t dummy_ = 0;
-};
-
-} // namespace test
-} // namespace symbolizer
-} // namespace folly
-
-#include <folly/experimental/symbolizer/test/SymbolizerTestUtils-inl.h>