dwarflint versus linkage_name attributes

[Mark Wielaard <mjw at redhat dot com>]

Hi,

I was cleaning up the expected-at.cc table for dwarflint, in particular
for DW_AT_linkage_name, which is the dwarf4 variant of
DW_AT_MIPS_linkage_name. I removed some XXXs that I think are not valid
anymore (in particlar DW_TAG_member shouldn't have a linkage_name
attached, which was fixed in gcc some time ago). And I added a new check
to see if there are DIEs which have a linkage_name, but aren't marked
external. Patch attached and now on dwarf branch.

But I am seeing lots of warnings with this. So either gcc is not
following the dwarf spec, has some bugs, or I am misinterpreting things.
The warnings are of two variants:

- DW_TAG_variables, which do have a linkage_name, can be found in symtab
(not checked by the code, just my manual spot checking), but don't have
DW_AT_external set. I think this is just a gcc bug.

- DW_TAG_union_type and DW_TAG_structure_type with DW_AT_linkage_name
set (but not DW_AT_external, which on itself makes sense). These are
unexpected because the dwarf spec doesn't explain why or how these would
be necessary.

The linkage_names strings can be pretty large, so it would be nice if we
could get rid of them when not necessary. But I might have
misinterpreted how they are actually used. So, does any of the above
make sense?

Cheers,

Mark

>From d2c0eb5359352b687a737bb19ca6dbce7fe5aa31 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Mon, 4 Apr 2011 12:42:21 +0200
Subject: [PATCH] dwarflint: linkage_name attributes checking.

Dwarf4 introduces DW_AT_linkage_name, which is like DW_AT_MIPS_linkage_name
already used by gcc for earlier versions. Mark DW_TAG_common_block,
DW_TAG_constant, DW_TAG_entry_point, DW_TAG_subprogram and DW_TAG_variable
as optionally having at_linkage_name (either the old or new variant) in
expected-at.cc. Add new check_linkage_external_die.cc to find DIEs with
linkage_name set, but not marked DW_AT_external.
---
 dwarflint/Makefile.am                   |    1 +
 dwarflint/check_linkage_external_die.cc |   79 +++++++++++++++++++++++++++++++
 dwarflint/expected-at.cc                |   20 +++++---
 3 files changed, 93 insertions(+), 7 deletions(-)
 create mode 100644 dwarflint/check_linkage_external_die.cc

diff --git a/dwarflint/Makefile.am b/dwarflint/Makefile.am
index b4164dd..d7def5d 100644
--- a/dwarflint/Makefile.am
+++ b/dwarflint/Makefile.am
@@ -85,6 +85,7 @@ dwarflint_SOURCES = \
 	check_nodebug.cc \
 	check_range_out_of_scope.cc \
 	check_self_referential_die.cc \
+	check_linkage_external_die.cc \
 	locstats.cc \
 	lowlevel_checks.cc lowlevel_checks.hh \
 	\
diff --git a/dwarflint/check_linkage_external_die.cc b/dwarflint/check_linkage_external_die.cc
new file mode 100644
index 0000000..e18d0e9
--- /dev/null
+++ b/dwarflint/check_linkage_external_die.cc
@@ -0,0 +1,79 @@
+/* Check that every die that has a linkage_name is also external.
+   Copyright (C) 2011 Red Hat, Inc.
+   This file is part of Red Hat elfutils.
+
+   Red Hat elfutils is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by the
+   Free Software Foundation; version 2 of the License.
+
+   Red Hat elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with Red Hat elfutils; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301 USA.
+
+   Red Hat elfutils is an included package of the Open Invention Network.
+   An included package of the Open Invention Network is a package for which
+   Open Invention Network licensees cross-license their patents.  No patent
+   license is granted, either expressly or impliedly, by designation as an
+   included package.  Should you wish to participate in the Open Invention
+   Network licensing program, please visit www.openinventionnetwork.com
+   <http://www.openinventionnetwork.com>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "highlevel_check.hh"
+#include "../src/dwarfstrings.h"
+#include "all-dies-it.hh"
+#include "pri.hh"
+#include "messages.hh"
+#include <map>
+
+using elfutils::dwarf;
+
+namespace
+{
+  class check_linkage_external_die
+    : public highlevel_check<check_linkage_external_die>
+  {
+  public:
+    static checkdescriptor const *descriptor ()
+    {
+      static checkdescriptor cd
+	(checkdescriptor::create ("check_linkage_external_die")
+	 .inherit<highlevel_check<check_linkage_external_die> > ()
+	 .description (
+"Check that each DIE that has a linkage_name also has an external attribute.\n"
+		       ));
+      return &cd;
+    }
+
+    explicit check_linkage_external_die (checkstack &stack, dwarflint &lint)
+      : highlevel_check<check_linkage_external_die> (stack, lint)
+    {
+      for (all_dies_iterator<dwarf> it = all_dies_iterator<dwarf> (dw);
+	   it != all_dies_iterator<dwarf> (); ++it)
+	{
+	  dwarf::debug_info_entry const &die = *it;
+	  dwarf::debug_info_entry::attributes_type attrs = die.attributes ();
+	  if ((attrs.find (DW_AT_linkage_name) != attrs.end ()
+	       || attrs.find (DW_AT_MIPS_linkage_name) != attrs.end ())
+	      && attrs.find (DW_AT_external) == attrs.end ())
+	    {
+	      wr_message (to_where (die),
+			  mc_impact_3 | mc_acc_suboptimal | mc_die_other)
+		<< elfutils::dwarf::tags::name (die.tag ())
+		<< " has linkage_name attribute, but no external attribute."
+		<< std::endl;
+	    }
+	}
+    }
+  };
+
+  reg<check_linkage_external_die> reg;
+}
diff --git a/dwarflint/expected-at.cc b/dwarflint/expected-at.cc
index 7a433c7..72a2104 100644
--- a/dwarflint/expected-at.cc
+++ b/dwarflint/expected-at.cc
@@ -1,5 +1,5 @@
 /* Pedantic checking of DWARF files.
-   Copyright (C) 2009 Red Hat, Inc.
+   Copyright (C) 2009, 2011 Red Hat, Inc.
    This file is part of Red Hat elfutils.
    Written by Petr Machata <pmachata@redhat.com>, 2009.
 
@@ -35,6 +35,10 @@ expected_at_map::expected_at_map ()
   at_set_decl.insert (DW_AT_decl_file);
   at_set_decl.insert (DW_AT_decl_line);
 
+  std::set <int> at_linkage_name;
+  at_linkage_name.insert (DW_AT_MIPS_linkage_name);
+  at_linkage_name.insert (DW_AT_linkage_name);
+
   m_map [DW_TAG_access_declaration]
     .optional (at_set_decl)
     .optional (DW_AT_accessibility)
@@ -120,6 +124,7 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_segment)
     .optional (DW_AT_sibling)
     .optional (DW_AT_visibility)
+    .optional (at_linkage_name)
     ;
 
   m_map [DW_TAG_common_inclusion]
@@ -175,6 +180,7 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_start_scope)
     .optional (DW_AT_type)
     .optional (DW_AT_visibility)
+    .optional (at_linkage_name)
     ;
 
   m_map [DW_TAG_dwarf_procedure]
@@ -195,6 +201,7 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_GNU_all_tail_call_sites)
     .optional (DW_AT_GNU_all_call_sites)
     .optional (DW_AT_GNU_all_source_call_sites)
+    .optional (at_linkage_name)
     ;
 
   m_map [DW_TAG_enumeration_type]
@@ -341,6 +348,9 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_sibling)
     ;
 
+  // At one time gcc did emit at_linkage_name for members, but that
+  // has been corrected:
+  // http://gcc.gnu.org/ml/gcc-patches/2010-06/msg01713.html
   m_map [DW_TAG_member]
     .optional (at_set_decl)
     .optional (DW_AT_accessibility)
@@ -355,10 +365,6 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_sibling)
     .optional (DW_AT_type)
     .optional (DW_AT_visibility)
-    .optional (DW_AT_MIPS_linkage_name) // xxx
-    .optional (DW_AT_external) // xxx
-    .optional (DW_AT_const_value) // xxx
-    .optional (DW_AT_artificial) // xxx
     ;
 
   m_map [DW_TAG_module]
@@ -572,7 +578,7 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_visibility)
     .optional (DW_AT_virtuality)
     .optional (DW_AT_vtable_elem_location)
-    .optional (DW_AT_MIPS_linkage_name) // XXX added to reflect reality
+    .optional (at_linkage_name)
     .optional (DW_AT_containing_type) // XXX added to reflect reality
     .optional (DW_AT_GNU_all_tail_call_sites)
     .optional (DW_AT_GNU_all_call_sites)
@@ -717,7 +723,7 @@ expected_at_map::expected_at_map ()
     .optional (DW_AT_start_scope)
     .optional (DW_AT_type)
     .optional (DW_AT_visibility)
-    .optional (DW_AT_MIPS_linkage_name) // XXX added to reflect reality
+    .optional (at_linkage_name)
     .optional (DW_AT_artificial) // XXX added to reflect reality
     ;
 
-- 
1.7.4