Re: [Patch, Darwin/libiberty] fix target/48108 by adding a section wrapper scheme to the darwin port.

2011-10-18 Thread Iain Sandoe 


On 30 Sep 2011, at 00:33, Iain Sandoe wrote:
I'll re-jig with the typographical changes (sorry that were so  
many ... )


I've addressed your points in :

http://gcc.gnu.org/ml/gcc-patches/2011-09/msg01974.html

specifically (other than typographical issues_

 o we retain the ability to read an old-style file.

   - I wonder if there should be an argument for removing this from  
the trunk version, but retaining in 4.6.

   - or, at the least, can we remove it from 4.8?

 o I dropped the ability to read all the sections from a file (by  
retaining the requirement to specify a segment name).


  - I will try to submit a separate patch to do this - which would  
essentially give the darwin version of simple-object the
   same functionality as the others (at the moment one can only read  
data from one segment at a time).


 o there's no need to amend the header, since the functionality is  
unchanged.


- retested on i686/x86-64 darwin 9/10 with bootstrap-lto bootstrap- 
debug

 (darwin10 requires the ranlib -c fix to work when XCode = 3.2.6 ).

Once again, apologies for the number of typos in the original.

OK for trunk/4.6?
Iain

libiberty:

PR target/48108
* simple-object-mach-o.c  (GNU_WRAPPER_SECTS, GNU_WRAPPER_INDEX,
GNU_WRAPPER_NAMES): New macros.
(simple_object_mach_o_segment): Handle wrapper scheme.
	(simple_object_mach_o_write_section_header): Allow the segment name  
to be supplied.
	(simple_object_mach_o_write_segment): Handle wrapper scheme.  Ensure  
that the

top-level segment name in the load command is empty.
	(simple_object_mach_o_write_to_file): Determine the number of  
sections during

segment output, use that in writing the header.

gcc:

PR target/48108
* config/darwin.c (top level): Amend comments concerning LTO output.
(lto_section_num): New variable.  (darwin_lto_section_e): New GTY.
(LTO_SECTS_SECTION, LTO_INDEX_SECTION): New.
(LTO_NAMES_SECTION): Rename.
(darwin_asm_named_section): Record LTO section counts and switches
in a vec of darwin_lto_section_e.
(darwin_file_start): Remove unused code.
(darwin_file_end): Put an LTO section termination label.  Handle output
of the wrapped LTO sections, index and names table.

Index: libiberty/simple-object-mach-o.c
===
--- libiberty/simple-object-mach-o.c(revision 180097)
+++ libiberty/simple-object-mach-o.c(working copy)
@@ -1,5 +1,5 @@
 /* simple-object-mach-o.c -- routines to manipulate Mach-O object files.
-   Copyright 2010 Free Software Foundation, Inc.
+   Copyright 2010, 2011 Free Software Foundation, Inc.
Written by Ian Lance Taylor, Google.
 
 This program is free software; you can redistribute it and/or modify it
@@ -174,6 +174,15 @@ struct mach_o_section_64
 
 #define GNU_SECTION_NAMES __section_names
 
+/* A GNU-specific extension to wrap multiple sections using three
+   mach-o sections within a given segment.  The section '__wrapper_sects'
+   is subdivided according to the index '__wrapper_index' and each sub
+   sect is named according to the names supplied in '__wrapper_names'.  */
+
+#define GNU_WRAPPER_SECTS __wrapper_sects
+#define GNU_WRAPPER_INDEX __wrapper_index
+#define GNU_WRAPPER_NAMES __wrapper_names
+
 /* Private data for an simple_object_read.  */
 
 struct simple_object_mach_o_read
@@ -214,8 +223,19 @@ struct simple_object_mach_o_attributes
   unsigned int reserved;
 };
 
-/* See if we have a Mach-O file.  */
+/* See if we have a Mach-O MH_OBJECT file:
 
+   A standard MH_OBJECT (from as) will have three load commands:
+   0 - LC_SEGMENT/LC_SEGMENT64
+   1 - LC_SYMTAB
+   2 - LC_DYSYMTAB
+
+   The LC_SEGMENT/LC_SEGMENT64 will introduce a single anonymous segment
+   containing all the sections.
+
+   Files written by simple-object will have only the segment command
+   (no symbol tables).  */
+
 static void *
 simple_object_mach_o_match (
 unsigned char header[SIMPLE_OBJECT_MATCH_HEADER_LEN],
@@ -356,9 +376,30 @@ simple_object_mach_o_section_info (int is_big_endi
 }
 }
 
-/* Handle a segment in a Mach-O file.  Return 1 if we should continue,
-   0 if the caller should return.  */
+/* Handle a segment in a Mach-O Object file.
 
+   This will callback to the function pfn for each section found the meaning
+   of which depends on gnu extensions to mach-o:
+
+   If we find mach-o sections (with the segment name as specified) which also
+   contain: a 'sects' wrapper, an index, and a  name table, we expand this into
+   as many sections as are specified in the index.  In this case, there will
+   be a callback for each of these.
+
+   We will also allow an extension that permits long names (more than 16
+   characters) to be used with mach-o.  In this case, the section name has
+   a specific format embedding an index into a name table, and the file must
+   contain such name table.
+
+   Return 1 if we

Re: [Patch, Darwin/libiberty] fix target/48108 by adding a section wrapper scheme to the darwin port.

2011-10-18 Thread Mike Stump 
On Oct 18, 2011, at 5:23 AM, Iain Sandoe wrote:
 o we retain the ability to read an old-style file.

   - I wonder if there should be an argument for removing this from the trunk 
 version, but retaining in 4.6.
   - or, at the least, can we remove it from 4.8?

I'd be fine with simply requiring recompiles, since support is so new.  If you 
do up the code, might as well leave it in.

 OK for trunk/4.6?

Ok from my perspective.  Don't recall if others want to review the 
simple-object-mach-o.c change.

Re: [Patch, Darwin/libiberty] fix target/48108 by adding a section wrapper scheme to the darwin port.

2011-09-29 Thread Mike Stump 
On Sep 29, 2011, at 1:27 AM, Iain Sandoe wrote:
 ... so here is a patch that implements a wrapper scheme on top of mach-o 
 (used presently only for LTO - but in the most general sense available 
 elsewhere to mach-o should the problem re-emerge).

 OK for trunk and 4.6 ?

Ok for all my bits.  I don't know if others want me to own the darwin bits in 
libiberty, or if they want to own them...  I'll let them chime in.

Re: [Patch, Darwin/libiberty] fix target/48108 by adding a section wrapper scheme to the darwin port.

2011-09-29 Thread Ian Lance Taylor 
Iain Sandoe develo...@sandoe-acoustics.co.uk writes:


This patch looks basically OK but it seems to have a number of
extraneous changes which make it harder to review.


 libiberty:

   PR target/48108
   * simple-object-mach-o.c (GNU_SECTION_NAMES): Remove.
   (GNU_WRAPPER_SECTS, GNU_WRAPPER_INDEX,
   GNU_WRAPPER_NAMES): New macros.
   (simple_object_mach_o_match): Amend comment and error message.
   Do not require that a segment name is specified.
   (simple_object_mach_o_segment): Handle wrapper scheme, if a
 segment name is
   specified.  (simple_object_mach_o_write_section_header): Allow
 the segment name
   to be supplied. (simple_object_mach_o_write_segment): Handle
 wrapper scheme, if
   a segment name is specified.  Ensure that the top-level
 segment name in the load
   command is empty.   (simple_object_mach_o_write_to_file): Determine the
   number of sections during segment output, use that in writing
 the header.

 gcc:

   PR target/48108
   * config/darwin.c (top level): Amend comments concerning LTO output.
   (lto_section_num): New variable.  (darwin_lto_section_e): New GTY.
   (LTO_SECTS_SECTION, LTO_INDEX_SECTION): New.
   (LTO_NAMES_SECTION): Rename.
   (darwin_asm_named_section): Record LTO section counts and switches
   in a vec of darwin_lto_section_e.
   (darwin_file_start): Remove unused code.
   (darwin_file_end): Put an LTO section termination label.  Handle output
   of the wrapped LTO sections, index and names table.
   
 include:

   PR target/48108
   *simple-object.h: Amend comments for simple read/write to note the
   wrapper scheme for Mach-o.

Space after '*'.


 -#define GNU_SECTION_NAMES __section_names

Are we sure it is OK to drop support for __section_names?  Won't that
mean that any old LTO files will not work with new gcc?  Is that OK?


 @@ -258,18 +274,10 @@ simple_object_mach_o_match (
  }
  #endif
  
 -  /* We require the user to provide a segment name.  This is
 - unfortunate but I don't see any good choices here.  */
 -
 -  if (segment_name == NULL)
 +  /* Although a standard MH_OBJECT has a single anonymous segment, we allow
 + a segment name to be specified - but don't act on it at this stage.  */
 +  if (segment_name  strlen (segment_name)  MACH_O_NAME_LEN)
  {
 -  *errmsg = Mach-O file found but no segment name specified;
 -  *err = 0;
 -  return NULL;
 -}
 -
 -  if (strlen (segment_name)  MACH_O_NAME_LEN)
 -{
*errmsg = Mach-O segment name too long;
*err = 0;
return NULL;

Please write segment_name != NULL , not just segment_name .



 @@ -294,13 +302,14 @@ simple_object_mach_o_match (
filetype = (*fetch_32) (b + offsetof (struct mach_o_header_32, filetype));
if (filetype != MACH_O_MH_OBJECT)
  {
 -  *errmsg = Mach-O file is not object file;
 +  *errmsg = Mach-O file is not an MH_OBJECT file;
*err = 0;
return NULL;
  }

I'm not sure this error message change is an improvement.  This error
message may be seen by end users, not just developers.  I think more end
users will understand object file.  Why do you want to make this
change?


omr = XNEW (struct simple_object_mach_o_read);
 -  omr-segment_name = xstrdup (segment_name);
 +  if (segment_name)
 +omr-segment_name = xstrdup (segment_name);
omr-magic = magic;
omr-is_big_endian = is_big_endian;
omr-cputype = (*fetch_32) (b + offsetof (struct mach_o_header_32, 
 cputype));

Please write if (segment_name != NULL).


 @@ -356,9 +365,25 @@ simple_object_mach_o_section_info (int is_big_endi
  }
  }
  
 -/* Handle a segment in a Mach-O file.  Return 1 if we should continue,
 -   0 if the caller should return.  */
 +/* Handle a segment in a Mach-O Object file.
  
 +   This will callback to the function pfn for each section found the 
 meaning
 +   of which depends on a gnu extension to mach-o:
 +   
 +   When omr-segment_name is specified, we implement a wrapper scheme (which
 +   whilst it will, in principle, work with any segment name, is likely to be
 +   meaningless current for anything except __GNU_LTO).
 +   
 +   If we find mach-o sections (with the segment name as specified) which also
 +   contain: a 'sects' wrapper, an index, and a  name table, we expand this 
 into
 +   as many sections as are specified in the index.  In this case, there will
 +   be a callback for each of these.
 +   
 +   When the omr-segment_name is not specified, then we would call-back for
 +   every mach-o section specified in the file.
 +
 +   Return 1 if we should continue, 0 if the caller should return.  */

Is there a reason that you are changing the code to support not
specifying a segment?  Should that be a separate change?


 @@ -375,12 +400,19 @@ simple_object_mach_o_segment (simple_object_read *
size_t sechdrsize;
size_t segname_offset;
size_t sectname_offset;
 -  unsigned int

Re: [Patch, Darwin/libiberty] fix target/48108 by adding a section wrapper scheme to the darwin port.

2011-09-29 Thread Iain Sandoe 

Hello Ian,

I'll re-jig with the typographical changes (sorry that were so  
many ... )

... and re-post - but I'd like to clear up a couple of points first.

On 30 Sep 2011, at 00:00, Ian Lance Taylor wrote:

-#define GNU_SECTION_NAMES __section_names


Are we sure it is OK to drop support for __section_names?  Won't that
mean that any old LTO files will not work with new gcc?  Is that OK?


I was under the impression that an lto built with an earlier revision  
of gcc would not interwork anyway.


If that is mistaken, then we could re-enable the option to allow both  
(I had that in the previous version, in fact).


+   If we find mach-o sections (with the segment name as specified)  
which also
+   contain: a 'sects' wrapper, an index, and a  name table, we  
expand this into
+   as many sections as are specified in the index.  In this case,  
there will

+   be a callback for each of these.
+
+   When the omr-segment_name is not specified, then we would call- 
back for

+   every mach-o section specified in the file.
+
+   Return 1 if we should continue, 0 if the caller should return.   
*/


Is there a reason that you are changing the code to support not
specifying a segment?  Should that be a separate change?


I'll separate it out.
I think that it could help us build some small tools to assist in  
making GCC output more closely match what the system tool-chain is  
expecting (e.g. stripping off LTO sections).  But that would require  
being able to read all sections as they come and then apply a filter.


Perhaps it's just better to invest the time in BFD, but that does mean  
the user having to build a second set of stuff or a combined tree.


thanks for reviewing and sorry about the number of trivial errors.

Iain