From 2e266396ee14f3c00252863704716b0e4b0fc3f0 Mon Sep 17 00:00:00 2001 From: Ted Mielczarek Date: Thu, 14 Apr 2016 10:32:20 -0400 Subject: [PATCH] Fix DWARF handling of inlined functions in namespaces Currently an inlined function in a namespace in DWARF will be given a name comprised of just `namespace::`. This is due to a logic error in ComputeQualifiedName, where it doesn't handle an empty `unqualified_name` properly. We apparently have a fair number of these in our Mac builds, an example of the DWARF that's being mishandled looks like: 0x117eda40: TAG_namespace [5] * AT_name( "js" ) AT_decl_file( "../../dist/include/js/Utility.h" ) AT_decl_line( 35 ) 0x11808500: TAG_subprogram [251] * AT_low_pc( 0x0000000002f12110 ) AT_high_pc( 0x0000000002f1216b ) AT_APPLE_omit_frame_ptr( 0x01 ) AT_frame_base( rsp ) AT_abstract_origin( {0x0000000011800a4f}"_ZN2js40TraceManuallyBarrieredGenericPointerEdgeEP8JSTracerPPNS_2gc4CellEPKc" ) AT_MIPS_linkage_name( "_ZN2js40TraceManuallyBarrieredGenericPointerEdgeEP8JSTracerPPNS_2gc4CellEPKc" ) AT_name( "TraceManuallyBarrieredGenericPointerEdge" ) AT_decl_file( "/builds/slave/rel-m-rel-m64_bld-000000000000/build/js/src/gc/Marking.cpp" ) AT_decl_line( 547 ) AT_external( 0x01 ) AT_APPLE_optimized( 0x01 ) AT_inline( DW_INL_inlined ) This turned a few instances of this in the file I was testing on into ``, which seems to just be a symptom of the "DW_AT_abstract_origin comes later in the file" issue. (Which is probably also worth fixing given that it occurs some 29k times when dumping symbols from Firefox's XUL binary, but it's a separate issue.) R=mark@chromium.org BUG= Review URL: https://codereview.chromium.org/1887033002 . --- src/common/dwarf_cu_to_module.cc | 18 +++++++++--------- src/common/dwarf_cu_to_module_unittest.cc | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index c52dffb3..99a7bf30 100644 --- a/src/common/dwarf_cu_to_module.cc +++ b/src/common/dwarf_cu_to_module.cc @@ -384,17 +384,17 @@ string DwarfCUToModule::GenericDIEHandler::ComputeQualifiedName() { qualified_name = &specification_->qualified_name; } - const string *unqualified_name; + const string *unqualified_name = NULL; const string *enclosing_name; if (!qualified_name) { - // Find our unqualified name. If the DIE has its own DW_AT_name - // attribute, then use that; otherwise, check our specification. - if (name_attribute_.empty() && specification_) - unqualified_name = &specification_->unqualified_name; - else + // Find the unqualified name. If the DIE has its own DW_AT_name + // attribute, then use that; otherwise, check the specification. + if (!name_attribute_.empty()) unqualified_name = &name_attribute_; + else if (specification_) + unqualified_name = &specification_->unqualified_name; - // Find the name of our enclosing context. If we have a + // Find the name of the enclosing context. If this DIE has a // specification, it's the specification's enclosing context that // counts; otherwise, use this DIE's context. if (specification_) @@ -408,7 +408,7 @@ string DwarfCUToModule::GenericDIEHandler::ComputeQualifiedName() { string return_value; if (qualified_name) { return_value = *qualified_name; - } else { + } else if (unqualified_name && enclosing_name) { // Combine the enclosing name and unqualified name to produce our // own fully-qualified name. return_value = cu_context_->language->MakeQualifiedName(*enclosing_name, @@ -417,7 +417,7 @@ string DwarfCUToModule::GenericDIEHandler::ComputeQualifiedName() { // If this DIE was marked as a declaration, record its names in the // specification table. - if (declaration_) { + if (declaration_ && qualified_name || (unqualified_name && enclosing_name)) { Specification spec; if (qualified_name) { spec.qualified_name = *qualified_name; diff --git a/src/common/dwarf_cu_to_module_unittest.cc b/src/common/dwarf_cu_to_module_unittest.cc index 0ae73e81..619e90a2 100644 --- a/src/common/dwarf_cu_to_module_unittest.cc +++ b/src/common/dwarf_cu_to_module_unittest.cc @@ -1323,6 +1323,29 @@ TEST_F(Specifications, InlineFunction) { 0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL); } +// An inline function in a namespace should correctly derive its +// name from its abstract origin, and not just the namespace name. +TEST_F(Specifications, InlineFunctionInNamespace) { + PushLine(0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL, "line-file", 75173118); + + StartCU(); + DIEHandler* space_handler + = StartNamedDIE(&root_handler_, dwarf2reader::DW_TAG_namespace, + "Namespace"); + ASSERT_TRUE(space_handler != NULL); + AbstractInstanceDIE(space_handler, 0x1e8dac5d507ed7abULL, + dwarf2reader::DW_INL_inlined, 0LL, "func-name"); + DefineInlineInstanceDIE(space_handler, "", 0x1e8dac5d507ed7abULL, + 0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL); + space_handler->Finish(); + delete space_handler; + root_handler_.Finish(); + + TestFunctionCount(1); + TestFunction(0, "Namespace::func-name", + 0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL); +} + // Check name construction for a long chain containing each combination of: // - struct, union, class, namespace // - direct and definition