This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] another discarded DWARF issue


I'm sorry to report that I've run into another problem with DWARF debug info for discarded sections. I wholeheartedly agree with others who have expressed the opinion that the linker should remove DWARF entries for discarded sections, instead of piling on more hacks to keep it limping along.

But.... I'm not going to be able to fix it properly, at least not now, so here's another hack.

The problem is the Xtensa port's link-time relaxations that change the offsets in the code and the overall size of the sections. There are actually 2 separate issues that interact.

* The comparison of the kept section size in _bfd_elf_check_kept_section does not account for changes in the section size due to linker relaxation. You may start with a bunch of identical linkonce sections, but only the one that is kept will be relaxed. If the kept section changes size, the comparison will fail (or worse, match when it shouldn't -- see below).

* Assuming that the section size comparison works and a DWARF reference to a discarded section is replaced by a reference to the corresponding kept section, the Xtensa relaxation code needs to anticipate that and adjust the offsets.

The most serious effect of these problems was reported to me in the context of a large C++ application for which I do not have the source code. For whatever reason, the various instances of a particular linkonce section have slightly different sizes in different object files. In at least one case, that size difference must exactly match the change in size due to relaxation of the kept section. Then, the section size check in _bfd_elf_check_kept_sections matches, even though the sections are different, and the references to discarded code are changed to the kept code. That happens after the Xtensa relaxation is finished and the offsets never get adjusted to account for the relaxation. This is more serious than it may sound because of Xtensa's odd-sized instructions. If the debug line offsets are wrong and you insert a breakpoint, that breakpoint might be in the middle of some other instruction and will cause a crash when it is hit.

Anyway, here is a patch to work around these problems. It changes _bfd_elf_check_kept_section to compare versus the original, unrelaxed size of the kept section. I'm assuming the ELF section header is the right place to get the original size, since sec->rawsize is not set for all relaxed sections. Let me know if there's a better way. The elf32-xtensa.c change is a bit hacky because _bfd_elf_check_kept_section is not exported from elflink.c, but that doesn't bother me -- I'm still optimistic that this will all get fixed properly someday.

I tested this with an xtensa-elf build with the testcase for bug2342. I also tried several variations of the code to better exercise the Xtensa relaxation, and I manually inspected the resulting DWARF info. I ran all the binutils testsuites for both xtensa-elf and i686-pc-linux-gnu builds, with no new failures.

Is this OK? Any better ideas?

2007-10-05 Bob Wilson <bob.wilson@acm.org>

	* elf32-xtensa.c (relax_section): Check for a reference to a discarded
	DWARF section and anticipate its replacement with the kept section.
	* elflink.c (_bfd_elf_check_kept_section): Compare against the kept
	section size from its ELF header.
Index: elf32-xtensa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-xtensa.c,v
retrieving revision 1.99
diff -u -p -r1.99 elf32-xtensa.c
--- elf32-xtensa.c	5 Oct 2007 19:05:35 -0000	1.99
+++ elf32-xtensa.c	5 Oct 2007 20:27:19 -0000
@@ -8150,8 +8150,53 @@ relax_section (bfd *abfd, asection *sec,
 	     we may need to change the relocation's target offset.  */
 
 	  target_sec = r_reloc_get_section (&r_rel);
-	  target_relax_info = get_xtensa_relax_info (target_sec);
 
+	  /* For a reference to a discarded section from a DWARF section,
+	     i.e., where action_discarded is PRETEND, the symbol will
+	     eventually be modified to refer to the kept section (at least if
+	     the kept and discarded sections are the same size).  Anticipate
+	     that here and adjust things accordingly.  */
+	  if (sec->sec_info_type != ELF_INFO_TYPE_STABS
+	      && ! elf_xtensa_ignore_discarded_relocs (sec)
+	      && elf_xtensa_action_discarded (sec) == PRETEND
+	      && target_sec != NULL
+	      && elf_discarded_section (target_sec))
+	    {
+	      /* It would be natural to call _bfd_elf_check_kept_section
+		 here, but it's not exported from elflink.c.  It's also a
+		 fairly expensive check.  Adjusting the relocations to the
+		 discarded section is fairly harmless; it will only adjust
+		 some addends and difference values.  If it turns out that
+		 _bfd_elf_check_kept_section fails later, it won't matter,
+		 so just compare the section names to find the right group
+		 member.  */
+	      asection *kept = target_sec->kept_section;
+	      if (kept != NULL)
+		{
+		  if ((kept->flags & SEC_GROUP) != 0)
+		    {
+		      asection *first = elf_next_in_group (kept);
+		      asection *s = first;
+
+		      kept = NULL;
+		      while (s != NULL)
+			{
+			  if (strcmp (s->name, target_sec->name) == 0)
+			    {
+			      kept = s;
+			      break;
+			    }
+			  s = elf_next_in_group (s);
+			  if (s == first)
+			    break;
+			}
+		    }
+		}
+	      if (kept)
+		target_sec = kept;
+	    }
+
+	  target_relax_info = get_xtensa_relax_info (target_sec);
 	  if (target_relax_info
 	      && (target_relax_info->is_relaxable_literal_section
 		  || target_relax_info->is_relaxable_asm_section))
Index: elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.281
diff -u -p -r1.281 elflink.c
--- elflink.c	30 Sep 2007 13:43:23 -0000	1.281
+++ elflink.c	5 Oct 2007 20:27:21 -0000
@@ -8787,7 +8787,9 @@ _bfd_elf_check_kept_section (asection *s
     {
       if ((kept->flags & SEC_GROUP) != 0)
 	kept = match_group_member (sec, kept, info);
-      if (kept != NULL && sec->size != kept->size)
+      /* The kept section may have been relaxed; compare its original size.  */
+      if (kept != NULL &&
+	  sec->size != elf_section_data (kept)->this_hdr.sh_size)
 	kept = NULL;
       sec->kept_section = kept;
     }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]