strip vs elf section groups

[Mark Wielaard <mjw at redhat dot com>]

Hi,

strip had a couple of issues with elf section groups. binutils gas was
recently fixed so that section group signature symbols are actually real
symbols with names instead of naming the section after the signature and
using the section symbol (which doesn't have a name, so couldn't
technically be used as signature symbol). There was an assert in strip
that fell over the new/correct signature symbol usage. When not all
sections of a section group were removed strip still tried to remove the
section group itself. And when moving a section group to the debug file
it didn't retain the type and original section list (which gets
rewritten for the stripped file). All issues were actually detected by
elflint, so writing a test was easy. The following fixes these issues.

2011-05-18  Mark Wielaard  <mjw@redhat.com>

    * strip.c (handle_elf): Make sure all sections of a removed group
    section are removed too. Don't discard SHT_GROUP sections, copy
    section table before it gets modified. Section group signature
    symbols don't have to be retained.

Patch attached. The git branch mjw/strip-group contains the same plus an
extra testcase for the above issues, which fails before and succeeds
after this patch. Please let me know if I missed anything important in
this fix before I push it to master.

There is still an issue with unstrip which depends on comparing section
names for unallocated sections. New binutils gas names all group
sections ".group" and unstrip would need to compare the signature symbol
names in case of a SHT_GROUP section. I have not fixed that yet.

Cheers,

Mark

index 99374eb..625277a 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -726,6 +726,22 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 		   || shdr_info[cnt].shdr.sh_type == SHT_RELA)
 		  && shdr_info[shdr_info[cnt].shdr.sh_info].idx != 0)
 		shdr_info[cnt].idx = 1;
+
+	      /* If a group section is marked as being removed make
+		 sure all the sections it contains are being remove, too.  */
+	      if (shdr_info[cnt].shdr.sh_type == SHT_GROUP)
+		{
+		  Elf32_Word *grpref;
+		  grpref = (Elf32_Word *) shdr_info[cnt].data->d_buf;
+		  for (size_t in = 1;
+		       in < shdr_info[cnt].data->d_size / sizeof (Elf32_Word);
+		       ++in)
+		    if (shdr_info[grpref[in]].idx != 0)
+		      {
+			shdr_info[cnt].idx = 1;
+			break;
+		      }
+		}
 	    }
 
 	  if (shdr_info[cnt].idx == 1)
@@ -883,6 +899,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 	  bool discard_section = (shdr_info[cnt].idx > 0
 				  && shdr_info[cnt].debug_data == NULL
 				  && shdr_info[cnt].shdr.sh_type != SHT_NOTE
+				  && shdr_info[cnt].shdr.sh_type != SHT_GROUP
 				  && cnt != ehdr->e_shstrndx);
 
 	  /* Set the section header in the new file.  */
@@ -912,7 +929,8 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 	  *debugdata = *shdr_info[cnt].data;
 	  if (discard_section)
 	    debugdata->d_buf = NULL;
-	  else if (shdr_info[cnt].debug_data != NULL)
+	  else if (shdr_info[cnt].debug_data != NULL
+		   || shdr_info[cnt].shdr.sh_type == SHT_GROUP)
 	    {
 	      /* Copy the original data before it gets modified.  */
 	      shdr_info[cnt].debug_data = debugdata;
@@ -1261,9 +1279,15 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
 		      }
 		    else if (debug_fname == NULL
 			     || shdr_info[cnt].debug_data == NULL)
-		      /* This is a section symbol for a section which has
-			 been removed.  */
-		      assert (GELF_ST_TYPE (sym->st_info) == STT_SECTION);
+		      /* This is a section or group signature symbol
+			 for a section which has been removed.  */
+		      {
+			size_t sidx = (sym->st_shndx != SHN_XINDEX
+					? sym->st_shndx : xshndx);
+			assert (GELF_ST_TYPE (sym->st_info) == STT_SECTION
+				|| (shdr_info[sidx].shdr.sh_type == SHT_GROUP
+				    && shdr_info[sidx].shdr.sh_info == inner));
+		      }
 		  }
 
 		if (destidx != inner)