* [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output
2013-10-29 17:54 [PATCH] implement correct alignment for Mach-O archive entries Nathan Froyd
@ 2013-10-29 17:54 ` Nathan Froyd
2013-11-04 9:17 ` Tristan Gingold
2013-10-29 18:11 ` [PATCH] implement correct alignment for Mach-O archive entries Matt Thomas
1 sibling, 1 reply; 6+ messages in thread
From: Nathan Froyd @ 2013-10-29 17:54 UTC (permalink / raw)
To: binutils
Hi,
Generating static library archives with binutils and comparing them with
archives produced by Apple's tools reveals one significant difference:
those produced by Apple's tools are significantly smaller. binutils
includes many ${SYMBOL_NAME}.eh symbols in the archive table of contents,
whereas Apple's tools do not.
This difference is due to the .eh symbols living in the .eh_frame section,
which is tagged with the Mach-O section flag S_ATTR_NO_TOC. This flag
instructs the archiver to not include symbols from such a section in the
table of contents for an archive.
The patch below changes bfd_mach_o_canonicalize_symtab to respect this flag.
Tested on x86_64-apple-darwin11, no regressions. OK to checkin?
-Nathan
* mach-o.c (bfd_mach_o_canonicalize_symtab): Respect the
BFD_MACH_O_S_ATTR_NO_TOC flag on symbol sections.
---
bfd/mach-o.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 32e48ac..095afeb 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -630,7 +630,7 @@ bfd_mach_o_canonicalize_symtab (bfd *abfd, asymbol **alocation)
bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (abfd);
long nsyms = bfd_mach_o_count_symbols (abfd);
bfd_mach_o_symtab_command *sym = mdata->symtab;
- unsigned long j;
+ unsigned long i, j;
if (nsyms < 0)
return nsyms;
@@ -651,12 +651,23 @@ bfd_mach_o_canonicalize_symtab (bfd *abfd, asymbol **alocation)
BFD_ASSERT (sym->symbols != NULL);
- for (j = 0; j < sym->nsyms; j++)
- alocation[j] = &sym->symbols[j].symbol;
+ for (i = 0, j = 0; j < sym->nsyms; j++)
+ {
+ bfd_mach_o_asymbol *asym = &sym->symbols[j];
+ if (asym->n_sect != 0)
+ {
+ bfd_mach_o_section *sect = mdata->sections[asym->n_sect-1];
+ if (sect->flags & BFD_MACH_O_S_ATTR_NO_TOC)
+ continue;
+ }
- alocation[j] = NULL;
+ alocation[i] = &asym->symbol;
+ i++;
+ }
+
+ alocation[i] = NULL;
- return nsyms;
+ return i;
}
/* Create synthetic symbols for indirect symbols. */
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] implement correct alignment for Mach-O archive entries
@ 2013-10-29 17:54 Nathan Froyd
2013-10-29 17:54 ` [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output Nathan Froyd
2013-10-29 18:11 ` [PATCH] implement correct alignment for Mach-O archive entries Matt Thomas
0 siblings, 2 replies; 6+ messages in thread
From: Nathan Froyd @ 2013-10-29 17:54 UTC (permalink / raw)
To: binutils
Hi,
Static archives for the Mach-O file format have an undocumented requirement:
archive entries must be aligned on four-byte boundaries. The Mach-O file
format reference document:
https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/MachORuntime/Reference/reference.html
has no mention of this. Apple's cctools sources suggest that this requirement
is only for archive entries that require long names (20 characters or a name
that contains spaces). But Apple's linker (at least) appears to enforce this
requirement all the time, regardless of the particular characteristics of the
archive. It is not clear to me whether this has changed over time, or whether
the requirement has always been there and is simply undocumented.
BFD already has code to align archive entries, so increasing that alignment
in certain cases is not too hard. I considered placing this code in mach-o.c
and implementing target jump table methods for them, but it did not seem worth
it. Suggestions on how to not call bfd_get_flavour numerous times in
archive.c welcome. I don't feel too bad about it due to the check for
bfd_target_coff_flavour that is pre-existing.
Tested on x86_64-apple-darwin11, no regressions. These changes also make
substantial static libraries work correctly with the Apple linker.
OK to checkin?
-Nathan
* archive.c (pad_fileptr_for_entry): New function.
(bfd_generic_openr_next_archived_file): Call it.
(do_slurp_bsd_armap, do_slurp_coff_armp): Likewise.
(bfd_write_armap): Likewise. Align entries if we have a
mach-o archive.
(write_mach_o_entry_padding): New function.
(_bfd_write_archive_contents): Call it if we have a mach-o
archive.
---
bfd/archive.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 9 deletions(-)
diff --git a/bfd/archive.c b/bfd/archive.c
index 32b07a7..176fb9c 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -443,6 +443,19 @@ get_extended_arelt_filename (bfd *arch, const char *name, file_ptr *originp)
return bfd_ardata (arch)->extended_names + table_index;
}
+/* Pad PTR appropriately for ABFD. */
+
+static file_ptr
+pad_fileptr_for_entry (bfd *abfd, file_ptr ptr)
+{
+ if (bfd_get_flavour (abfd) == bfd_target_mach_o_flavour)
+ ptr += ptr % 4;
+ else
+ ptr += ptr % 2;
+
+ return ptr;
+}
+
/* This functions reads an arch header and returns an areltdata pointer, or
NULL on error.
@@ -780,7 +793,7 @@ bfd_generic_openr_next_archived_file (bfd *archive, bfd *last_file)
/* Pad to an even boundary...
Note that last_file->origin can be odd in the case of
BSD-4.4-style element with a long odd size. */
- filestart += filestart % 2;
+ filestart = pad_fileptr_for_entry (archive, filestart);
}
return _bfd_get_elt_at_filepos (archive, filestart);
@@ -943,9 +956,7 @@ do_slurp_bsd_armap (bfd *abfd)
set->file_offset = H_GET_32 (abfd, rbase + BSD_SYMDEF_OFFSET_SIZE);
}
- ardata->first_file_filepos = bfd_tell (abfd);
- /* Pad to an even boundary if you have to. */
- ardata->first_file_filepos += (ardata->first_file_filepos) % 2;
+ ardata->first_file_filepos = pad_fileptr_for_entry (abfd, bfd_tell (abfd));
/* FIXME, we should provide some way to free raw_ardata when
we are done using the strings from it. For now, it seems
to be allocated on an objalloc anyway... */
@@ -1048,9 +1059,7 @@ do_slurp_coff_armap (bfd *abfd)
*stringbase = 0;
ardata->symdef_count = nsymz;
- ardata->first_file_filepos = bfd_tell (abfd);
- /* Pad to an even boundary if you have to. */
- ardata->first_file_filepos += (ardata->first_file_filepos) % 2;
+ ardata->first_file_filepos = pad_fileptr_for_entry (abfd, bfd_tell(abfd));
bfd_has_map (abfd) = TRUE;
bfd_release (abfd, raw_armap);
@@ -2099,6 +2108,20 @@ bfd_gnu_truncate_arname (bfd *abfd, const char *pathname, char *arhdr)
(hdr->ar_name)[length] = ar_padchar (abfd);
}
\f
+/* Write any padding necessary for Mach-O archive entries, which must be
+ aligned on 32-bit boundaries, of ABFD. Return true if the write was
+ successful, false otherwise. */
+
+static bfd_boolean
+write_mach_o_entry_padding (bfd *abfd)
+{
+ static const char pad[] = "\0\0\0";
+ file_ptr fpos = bfd_tell (abfd);
+ unsigned int amt = fpos % 4;
+
+ return bfd_bwrite (&pad[0], amt, abfd) == amt;
+}
+
/* The BFD is open for write and has its format set to bfd_archive. */
bfd_boolean
@@ -2170,6 +2193,12 @@ _bfd_write_archive_contents (bfd *arch)
return FALSE;
}
+ if (bfd_get_flavour (arch) == bfd_target_mach_o_flavour)
+ {
+ if (!write_mach_o_entry_padding (arch))
+ return FALSE;
+ }
+
if (elength != 0)
{
struct ar_hdr hdr;
@@ -2225,7 +2254,12 @@ _bfd_write_archive_contents (bfd *arch)
remaining -= amt;
}
- if ((arelt_size (current) % 2) == 1)
+ if (bfd_get_flavour (arch) == bfd_target_mach_o_flavour)
+ {
+ if (!write_mach_o_entry_padding (arch))
+ return FALSE;
+ }
+ else if ((arelt_size (current) % 2) == 1)
{
if (bfd_bwrite (&ARFMAG[1], 1, arch) != 1)
return FALSE;
@@ -2465,6 +2499,10 @@ bsd_write_armap (bfd *arch,
if (bfd_bwrite (temp, sizeof (temp), arch) != sizeof (temp))
return FALSE;
+ /* Mach-O requires 32-bit aligned entries. */
+ if (bfd_get_flavour (arch) == bfd_target_mach_o_flavour)
+ firstreal += firstreal % 4;
+
for (count = 0; count < orl_count; count++)
{
unsigned int offset;
@@ -2478,7 +2516,7 @@ bsd_write_armap (bfd *arch,
firstreal += (ared->parsed_size + ared->extra_size
+ sizeof (struct ar_hdr));
- firstreal += firstreal % 2;
+ firstreal = pad_fileptr_for_entry (arch, firstreal);
current = current->archive_next;
}
while (current != map[count].u.abfd);
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] implement correct alignment for Mach-O archive entries
2013-10-29 17:54 [PATCH] implement correct alignment for Mach-O archive entries Nathan Froyd
2013-10-29 17:54 ` [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output Nathan Froyd
@ 2013-10-29 18:11 ` Matt Thomas
2013-10-29 18:15 ` Nathan Froyd
1 sibling, 1 reply; 6+ messages in thread
From: Matt Thomas @ 2013-10-29 18:11 UTC (permalink / raw)
To: Nathan Froyd; +Cc: binutils
On Oct 29, 2013, at 10:54 AM, Nathan Froyd <froydnj@mozilla.com> wrote:
> + if (bfd_get_flavour (abfd) == bfd_target_mach_o_flavour)
> + ptr += ptr % 4;
> + else
That does not pad to 4.
ptr = (ptr + 3) & ~3;
or
ptr += 4 - (ptr % 4);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] implement correct alignment for Mach-O archive entries
2013-10-29 18:11 ` [PATCH] implement correct alignment for Mach-O archive entries Matt Thomas
@ 2013-10-29 18:15 ` Nathan Froyd
2013-11-04 9:19 ` Tristan Gingold
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Froyd @ 2013-10-29 18:15 UTC (permalink / raw)
To: Matt Thomas; +Cc: binutils
----- Original Message -----
> On Oct 29, 2013, at 10:54 AM, Nathan Froyd <froydnj@mozilla.com> wrote:
>
> > + if (bfd_get_flavour (abfd) == bfd_target_mach_o_flavour)
> > + ptr += ptr % 4;
> > + else
>
> That does not pad to 4.
Ah, it doesn't quite. We probably get lucky in that PTR is always even,
where that formula does work.
> ptr += 4 - (ptr % 4);
This pads unnecessarily when ptr is a multiple of 4 already.
-Nathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output
2013-10-29 17:54 ` [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output Nathan Froyd
@ 2013-11-04 9:17 ` Tristan Gingold
0 siblings, 0 replies; 6+ messages in thread
From: Tristan Gingold @ 2013-11-04 9:17 UTC (permalink / raw)
To: Nathan Froyd; +Cc: binutils
On Oct 29, 2013, at 6:54 PM, Nathan Froyd <froydnj@mozilla.com> wrote:
> Hi,
>
> Generating static library archives with binutils and comparing them with
> archives produced by Apple's tools reveals one significant difference:
> those produced by Apple's tools are significantly smaller. binutils
> includes many ${SYMBOL_NAME}.eh symbols in the archive table of contents,
> whereas Apple's tools do not.
>
> This difference is due to the .eh symbols living in the .eh_frame section,
> which is tagged with the Mach-O section flag S_ATTR_NO_TOC. This flag
> instructs the archiver to not include symbols from such a section in the
> table of contents for an archive.
>
> The patch below changes bfd_mach_o_canonicalize_symtab to respect this flag.
>
> Tested on x86_64-apple-darwin11, no regressions. OK to checkin?
Yes, this is OK.
(Interesting to know that GNU ar is used on Darwin :-)
Tristan.
>
> -Nathan
>
> * mach-o.c (bfd_mach_o_canonicalize_symtab): Respect the
> BFD_MACH_O_S_ATTR_NO_TOC flag on symbol sections.
> ---
> bfd/mach-o.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/bfd/mach-o.c b/bfd/mach-o.c
> index 32e48ac..095afeb 100644
> --- a/bfd/mach-o.c
> +++ b/bfd/mach-o.c
> @@ -630,7 +630,7 @@ bfd_mach_o_canonicalize_symtab (bfd *abfd, asymbol **alocation)
> bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (abfd);
> long nsyms = bfd_mach_o_count_symbols (abfd);
> bfd_mach_o_symtab_command *sym = mdata->symtab;
> - unsigned long j;
> + unsigned long i, j;
>
> if (nsyms < 0)
> return nsyms;
> @@ -651,12 +651,23 @@ bfd_mach_o_canonicalize_symtab (bfd *abfd, asymbol **alocation)
>
> BFD_ASSERT (sym->symbols != NULL);
>
> - for (j = 0; j < sym->nsyms; j++)
> - alocation[j] = &sym->symbols[j].symbol;
> + for (i = 0, j = 0; j < sym->nsyms; j++)
> + {
> + bfd_mach_o_asymbol *asym = &sym->symbols[j];
> + if (asym->n_sect != 0)
> + {
> + bfd_mach_o_section *sect = mdata->sections[asym->n_sect-1];
> + if (sect->flags & BFD_MACH_O_S_ATTR_NO_TOC)
> + continue;
> + }
>
> - alocation[j] = NULL;
> + alocation[i] = &asym->symbol;
> + i++;
> + }
> +
> + alocation[i] = NULL;
>
> - return nsyms;
> + return i;
> }
>
> /* Create synthetic symbols for indirect symbols. */
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] implement correct alignment for Mach-O archive entries
2013-10-29 18:15 ` Nathan Froyd
@ 2013-11-04 9:19 ` Tristan Gingold
0 siblings, 0 replies; 6+ messages in thread
From: Tristan Gingold @ 2013-11-04 9:19 UTC (permalink / raw)
To: Nathan Froyd; +Cc: Matt Thomas, binutils
On Oct 29, 2013, at 7:15 PM, Nathan Froyd <froydnj@mozilla.com> wrote:
> ----- Original Message -----
>> On Oct 29, 2013, at 10:54 AM, Nathan Froyd <froydnj@mozilla.com> wrote:
>>
>>> + if (bfd_get_flavour (abfd) == bfd_target_mach_o_flavour)
>>> + ptr += ptr % 4;
>>> + else
>>
>> That does not pad to 4.
>
> Ah, it doesn't quite. We probably get lucky in that PTR is always even,
> where that formula does work.
This is OK for the darwin part, but you need a global maintainer OK for archive.c
Thanks,
Tristan.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-04 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-29 17:54 [PATCH] implement correct alignment for Mach-O archive entries Nathan Froyd
2013-10-29 17:54 ` [PATCH] don't include symbols from S_ATTR_NO_TOC sections in mach-o canonical symtab output Nathan Froyd
2013-11-04 9:17 ` Tristan Gingold
2013-10-29 18:11 ` [PATCH] implement correct alignment for Mach-O archive entries Matt Thomas
2013-10-29 18:15 ` Nathan Froyd
2013-11-04 9:19 ` Tristan Gingold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).