[Patch mach-o/bfd] improve build command routine robustness.

Iain Sandoe developer@sandoe-acoustics.co.uk
Mon Jan 9 11:01:00 GMT 2012


Although I do not see the reported build warning with 4.6 or 4.7 as  
the host compiler, I can see where it could arise.

Additionally, when looking harder at this, it seems that almost any  
permutation of commands is valid - the following patch improves  
robustness - and should fix the reported warning.

OK?
Iain

bfd:

	* mach-o.c (bfd_mach_o_build_commands): Make the building of each  
command
	type independent.

  bfd/mach-o.c |  129 ++++++++++++++++++++++++++++++++++ 
+----------------------
  1 files changed, 79 insertions(+), 50 deletions(-)

diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 07ca65a..a1f6596 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -2149,15 +2149,19 @@ bfd_mach_o_build_dysymtab_command (bfd *abfd,
     TODO: Other file formats, rebuilding symtab/dysymtab commands for  
strip
     and copy functionality.  */

+#define MACHO_SEG_CMD_PRESENT	0x0001
+#define MACHO_SYM_CMD_PRESENT	0x0002
+#define MACHO_DYSYM_CMD_PRESENT	0x0004
+
  bfd_boolean
  bfd_mach_o_build_commands (bfd *abfd)
  {
    bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (abfd);
-  unsigned int wide = mach_o_wide_p (&mdata->header);
-  bfd_mach_o_segment_command *seg;
-  bfd_mach_o_load_command *cmd;
-  bfd_mach_o_load_command *symtab_cmd;
-  unsigned symcind;
+  unsigned wide = mach_o_wide_p (&mdata->header);
+  unsigned present = 0;
+  unsigned symind = 0;
+  unsigned dysymind = 0;
+  unsigned long base_offset = 0;

    /* Return now if commands are already present.  */
    if (mdata->header.ncmds)
@@ -2187,31 +2191,49 @@ bfd_mach_o_build_commands (bfd *abfd)
    if (!bfd_mach_o_mangle_symbols (abfd))
      return FALSE;

-  /* It's valid to have a file with only absolute symbols...  */
+  /* Very simple command set (only really applicable to MH_OBJECTs):
+     All the commands are optional - present only when there is  
suitable data.
+     (i.e. it is valid to have an empty file)
+
+	a command (segment) to contain all the sections,
+	command for the symbol table,
+	a command for the dysymtab.
+
+     ??? maybe we should assert that this is an MH_OBJECT?  */
+
    if (mdata->nsects > 0)
      {
        mdata->header.ncmds = 1;
-      symcind = 1;
+      symind = 1;
+      dysymind = 1;
+      present |= MACHO_SEG_CMD_PRESENT;
      }
-  else
-    symcind = 0;

-  /* It's OK to have a file with only section statements.  */
    if (bfd_get_symcount (abfd) > 0)
-    mdata->header.ncmds += 1;
+    {
+      mdata->header.ncmds += 1;
+      dysymind += 1;
+      present |= MACHO_SYM_CMD_PRESENT;
+    }

-  /* Very simple version (only really applicable to MH_OBJECTs):
-	a command (segment) to contain all the sections,
-	a command for the symbol table
-	a n (optional) command for the dysymtab.
+  /* FIXME:
+     This is a rather crude test for whether we should build a  
dysymtab.  */
+  if (bfd_mach_o_should_emit_dysymtab ()
+      && bfd_get_symcount (abfd))
+    {
+      mdata->header.ncmds += 1;
+      present |= MACHO_DYSYM_CMD_PRESENT;
+    }

-     ??? maybe we should assert that this is an MH_OBJECT?  */
+  if (wide)
+    base_offset = BFD_MACH_O_HEADER_64_SIZE;
+  else
+    base_offset = BFD_MACH_O_HEADER_SIZE;

-  if (bfd_mach_o_should_emit_dysymtab ()
-      && bfd_get_symcount (abfd) > 0)
-    mdata->header.ncmds += 1;
+  /* Well, we must have a header, at least.  */
+  mdata->filelen = base_offset;

-  /* A bit weird, but looks like no content;
+  /* A bit unusual, but no content is valid;
       as -n empty.s -o empty.o  */
    if (mdata->header.ncmds == 0)
      return TRUE;
@@ -2221,10 +2243,10 @@ bfd_mach_o_build_commands (bfd *abfd)
    if (mdata->commands == NULL)
      return FALSE;

-  if (mdata->nsects > 0)
+  if (present & MACHO_SEG_CMD_PRESENT)
      {
-      cmd = &mdata->commands[0];
-      seg = &cmd->command.segment;
+      bfd_mach_o_load_command *cmd = &mdata->commands[0];
+      bfd_mach_o_segment_command *seg = &cmd->command.segment;

        /* Count the segctions in the special blank segment used for  
MH_OBJECT.  */
        seg->nsects = bfd_mach_o_count_sections_for_seg (NULL, mdata);
@@ -2232,50 +2254,57 @@ bfd_mach_o_build_commands (bfd *abfd)
  	return FALSE;

        /* Init segment command.  */
+      cmd->offset = base_offset;
        if (wide)
  	{
  	  cmd->type = BFD_MACH_O_LC_SEGMENT_64;
-	  cmd->offset = BFD_MACH_O_HEADER_64_SIZE;
  	  cmd->len = BFD_MACH_O_LC_SEGMENT_64_SIZE
  			+ BFD_MACH_O_SECTION_64_SIZE * seg->nsects;
  	}
        else
  	{
  	  cmd->type = BFD_MACH_O_LC_SEGMENT;
-	  cmd->offset = BFD_MACH_O_HEADER_SIZE;
  	  cmd->len = BFD_MACH_O_LC_SEGMENT_SIZE
  			+ BFD_MACH_O_SECTION_SIZE * seg->nsects;
  	}
+
        cmd->type_required = FALSE;
        mdata->header.sizeofcmds = cmd->len;
-      mdata->filelen = cmd->offset + cmd->len;
+      mdata->filelen += cmd->len;
      }

-  if (bfd_get_symcount (abfd) > 0)
+  if (present & MACHO_SYM_CMD_PRESENT)
      {
        /* Init symtab command.  */
-      symtab_cmd = &mdata->commands[symcind];
-
-      symtab_cmd->type = BFD_MACH_O_LC_SYMTAB;
-      if (symcind > 0)
-        symtab_cmd->offset = mdata->commands[0].offset
-			     + mdata->commands[0].len;
-      else
-        symtab_cmd->offset = 0;
-      symtab_cmd->len = 6 * 4;
-      symtab_cmd->type_required = FALSE;
+      bfd_mach_o_load_command *cmd = &mdata->commands[symind];

-      mdata->header.sizeofcmds += symtab_cmd->len;
-      mdata->filelen += symtab_cmd->len;
+      cmd->type = BFD_MACH_O_LC_SYMTAB;
+      cmd->offset = base_offset;
+      if (present & MACHO_SEG_CMD_PRESENT)
+        cmd->offset += mdata->commands[0].len;
+
+      cmd->len = 6 * 4;
+      cmd->type_required = FALSE;
+      mdata->header.sizeofcmds += cmd->len;
+      mdata->filelen += cmd->len;
      }

-  /* If required, setup symtab command.  */
-  if (bfd_mach_o_should_emit_dysymtab ()
-      && bfd_get_symcount (abfd) > 0)
+  /* If required, setup symtab command, see comment above about the  
quality
+     of this test.  */
+  if (present & MACHO_DYSYM_CMD_PRESENT)
      {
-      cmd = &mdata->commands[symcind+1];
+      bfd_mach_o_load_command *cmd = &mdata->commands[dysymind];
+
        cmd->type = BFD_MACH_O_LC_DYSYMTAB;
-      cmd->offset = symtab_cmd->offset + symtab_cmd->len;
+      if (present & MACHO_SYM_CMD_PRESENT)
+        cmd->offset = mdata->commands[symind].offset
+		    + mdata->commands[symind].len;
+      else if (present & MACHO_SEG_CMD_PRESENT)
+        cmd->offset = mdata->commands[0].offset
+		    + mdata->commands[0].len;
+      else
+	cmd->offset = base_offset;
+
        cmd->type_required = FALSE;
        cmd->len = 18 * 4 + BFD_MACH_O_LC_SIZE;

@@ -2285,18 +2314,18 @@ bfd_mach_o_build_commands (bfd *abfd)

    /* So, now we have sized the commands and the filelen set to that.
       Now we can build the segment command and set the section file  
offsets.  */
-  if (mdata->nsects > 0
-      && ! bfd_mach_o_build_seg_command (NULL, mdata, seg))
+  if ((present & MACHO_SEG_CMD_PRESENT)
+      && ! bfd_mach_o_build_seg_command (NULL, mdata,
+					 &mdata->commands[0].command.segment))
      return FALSE;

    /* If we're doing a dysymtab, cmd points to its load command.  */
-  if (bfd_mach_o_should_emit_dysymtab ()
-      && bfd_get_symcount (abfd) > 0
+  if ((present & MACHO_DYSYM_CMD_PRESENT)
        && ! bfd_mach_o_build_dysymtab_command (abfd, mdata,
-					      &mdata->commands[symcind+1]))
+					      &mdata->commands[dysymind]))
      return FALSE;

-  /* The symtab command is filled in when the symtab is written.  */
+  /* The symtab command is filled when the symtab is written.  */
    return TRUE;
  }




More information about the Binutils mailing list