On Wed, Aug 22, 2018 at 2:56 PM Iain Sandoe <i...@sandoe.co.uk> wrote:
>
>
> > On 20 Aug 2018, at 11:01, Richard Biener <richard.guent...@gmail.com> wrote:
> >
> > On Sat, Aug 18, 2018 at 9:00 PM Iain Sandoe <i...@sandoe.co.uk> wrote:
> >>
>
> >> I plan on making Darwin default to fno-lto for link unless there’s an
> >> “flto*” on the link line, since otherwise there’s a process launch for
> >> every object on the c/l to do “nm”, which is quite heavy weight. At
> >> present, ISTM if the compiler is configured with —enable-lto, the link
> >> line defaults to assuming that every object needs to be checked.
> >
> > I think we wanted to transparently handle LTO objects (similar to how
> > it works with a linker plugin).
>
> At present, we are not only calling nm for every object, but also dsymutil
> sometimes unnecessarily - since the assumption on the “maybe_lto” path is
> that an temporary file will be generated.
>
> as a headline - when I did the simplistic “default is to assume fno-lto” that
> took 10% off the bootstrap time (with a stage#1 compiler without the
> optimisation).
>
> - I have a patch under test as below + better fidelity of avoiding dsymutil
> runs.
>
> > Ideally collect2 wouldn't use nm but simple-object to inspect files
> > (but that doesn't have symbol table query support
>
> Hrm.my WIP had grow symtab awareness to support debug-section copy on mach-o
> (and the ELF impl. looks like it understands both symtab and relocs)
> - so maybe that’s not as far away as we might think.
>
> > though looking for LTO specific sections would have been better in the
> > first place - I'd suggest .gnu.lto_.symtab).
>
> I’ve done this noting that the symtab seems to be the last emitted section -
> is that why you chose it (for security, rather than just stopping as soon as
> we see a .gnu.lto_. section?)
Ah, any .gnu.lto_.section would work as well I guess - I just picked
one that should be always
created. .gnu.lto_.opts is another one.
> bootstrapped (Darwin and Linux) along with a third patch to make collect2
> respond to -save-temps.
>
> comments?
OK. Feel free to stop when recognizing any LTO section. Btw, as you
include lto-section-names.h
you should probably use LTO_SECTION_NAME_PREFIX instead of hard-coding
.gnu.lto_.
I'm not sure if/how we should handle offload sections and if those
work at all without a
linker plugin currently?
Richard.
> thanks
> Iain
>
> gcc/
>
> * collect2.c (maybe_run_lto_and_relink): Don’t say we have a temp file
> unless we actually did some LTO.
> (has_lto_symtab, is_lto_object_file): New.
> (maybe_lto_object_file): Remove.
> (scan_prog_file): Use is_lto_object_file() instead of scanning the
> output
> of nm.
>
> ---
> gcc/collect2.c | 116 ++++++++++++++++++++++---------------------------
> 1 file changed, 53 insertions(+), 63 deletions(-)
>
> diff --git a/gcc/collect2.c b/gcc/collect2.c
> index 9ead5a6a1d..4b0f191ad3 100644
> --- a/gcc/collect2.c
> +++ b/gcc/collect2.c
> @@ -30,6 +30,8 @@ along with GCC; see the file COPYING3. If not see
> #include "tm.h"
> #include "filenames.h"
> #include "file-find.h"
> +#include "simple-object.h"
> +#include "lto-section-names.h"
>
> /* TARGET_64BIT may be defined to use driver specific functionality. */
> #undef TARGET_64BIT
> @@ -804,7 +806,9 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char
> **object_lst,
> /* Run the linker again, this time replacing the object files
> optimized by the LTO with the temporary file generated by the LTO.
> */
> fork_execute ("ld", out_lto_ld_argv, HAVE_GNU_LD && at_file_supplied);
> - post_ld_pass (true);
> + /* We assume that temp files were created, and therefore we need to
> take
> + that into account (maybe run dsymutil). */
> + post_ld_pass (/*temp_file*/true);
> free (lto_ld_argv);
>
> maybe_unlink_list (lto_o_files);
> @@ -814,10 +818,11 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char
> **object_lst,
> /* Our caller is relying on us to do the link
> even though there is no LTO back end work to be done. */
> fork_execute ("ld", lto_ld_argv, HAVE_GNU_LD && at_file_supplied);
> - post_ld_pass (false);
> + /* No LTO objects were found, so no new temp file. */
> + post_ld_pass (/*temp_file*/false);
> }
> else
> - post_ld_pass (true);
> + post_ld_pass (false); /* No LTO objects were found, no temp file. */
> }
>
> /* Main program. */
> @@ -1677,7 +1682,7 @@ main (int argc, char **argv)
> if (lto_mode != LTO_MODE_NONE)
> maybe_run_lto_and_relink (ld1_argv, object_lst, object, false);
> else
> - post_ld_pass (false);
> + post_ld_pass (/*temp_file*/false);
>
> maybe_unlink (c_file);
> maybe_unlink (o_file);
> @@ -1749,7 +1754,7 @@ main (int argc, char **argv)
> #ifdef COLLECT_EXPORT_LIST
> maybe_unlink (export_file);
> #endif
> - post_ld_pass (false);
> + post_ld_pass (/*temp_file*/false);
>
> maybe_unlink (c_file);
> maybe_unlink (o_file);
> @@ -1843,7 +1848,7 @@ main (int argc, char **argv)
> else
> {
> fork_execute ("ld", ld2_argv, HAVE_GNU_LD && at_file_supplied);
> - post_ld_pass (false);
> + post_ld_pass (/*temp_file*/false);
> }
>
> /* Let scan_prog_file do any final mods (OSF/rose needs this for
> @@ -2300,38 +2305,48 @@ write_aix_file (FILE *stream, struct id *list)
>
> /* Check to make sure the file is an LTO object file. */
>
> +static int
> +has_lto_symtab (void *data, const char *name ATTRIBUTE_UNUSED,
> + off_t offset ATTRIBUTE_UNUSED,
> + off_t length ATTRIBUTE_UNUSED)
> +{
> + int *found = (int *) data;
> +
> + if (strncmp (name, ".gnu.lto_.symtab.",
> + sizeof (".gnu.lto_.symtab.") - 1) != 0)
> + return 1;
> +
> + *found = 1;
> +
> + /* Stop iteration. */
> + return 0;
> +}
> +
> static bool
> -maybe_lto_object_file (const char *prog_name)
> +is_lto_object_file (const char *prog_name)
> {
> - FILE *f;
> - unsigned char buf[4];
> - int i;
> + const char *errmsg;
> + int err;
> + int found = 0;
> + off_t inoff = 0;
> + int infd = open (prog_name, O_RDONLY | O_BINARY);
>
> - static unsigned char elfmagic[4] = { 0x7f, 'E', 'L', 'F' };
> - static unsigned char coffmagic[2] = { 0x4c, 0x01 };
> - static unsigned char coffmagic_x64[2] = { 0x64, 0x86 };
> - static unsigned char machomagic[4][4] = {
> - { 0xcf, 0xfa, 0xed, 0xfe },
> - { 0xce, 0xfa, 0xed, 0xfe },
> - { 0xfe, 0xed, 0xfa, 0xcf },
> - { 0xfe, 0xed, 0xfa, 0xce }
> - };
> + if (infd == -1)
> + return false;
>
> - f = fopen (prog_name, "rb");
> - if (f == NULL)
> + simple_object_read *inobj = simple_object_start_read (infd, inoff,
> + LTO_SEGMENT_NAME,
> + &errmsg, &err);
> + if (!inobj)
> return false;
> - if (fread (buf, sizeof (buf), 1, f) != 1)
> - buf[0] = 0;
> - fclose (f);
>
> - if (memcmp (buf, elfmagic, sizeof (elfmagic)) == 0
> - || memcmp (buf, coffmagic, sizeof (coffmagic)) == 0
> - || memcmp (buf, coffmagic_x64, sizeof (coffmagic_x64)) == 0)
> + errmsg = simple_object_find_sections (inobj, has_lto_symtab,
> + (void *) &found, &err);
> + if (! errmsg && found)
> return true;
> - for (i = 0; i < 4; i++)
> - if (memcmp (buf, machomagic[i], sizeof (machomagic[i])) == 0)
> - return true;
>
> + if (errmsg)
> + fatal_error (0, "%s: %s\n", errmsg, xstrerror (err));
> return false;
> }
>
> @@ -2354,7 +2369,6 @@ scan_prog_file (const char *prog_name, scanpass
> which_pass,
> int err;
> char *p, buf[1024];
> FILE *inf;
> - int found_lto = 0;
>
> if (which_pass == PASS_SECOND)
> return;
> @@ -2362,8 +2376,13 @@ scan_prog_file (const char *prog_name, scanpass
> which_pass,
> /* LTO objects must be in a known format. This check prevents
> us from accepting an archive containing LTO objects, which
> gcc cannot currently handle. */
> - if (which_pass == PASS_LTOINFO && !maybe_lto_object_file (prog_name))
> - return;
> + if (which_pass == PASS_LTOINFO)
> + {
> + if(is_lto_object_file (prog_name)) {
> + add_lto_object (<o_objects, prog_name);
> + }
> + return;
> + }
>
> /* If we do not have an `nm', complain. */
> if (nm_file_name == 0)
> @@ -2418,12 +2437,7 @@ scan_prog_file (const char *prog_name, scanpass
> which_pass,
> fatal_error (input_location, "can't open nm output: %m");
>
> if (debug)
> - {
> - if (which_pass == PASS_LTOINFO)
> - fprintf (stderr, "\nnm output with LTO info marker symbol.\n");
> - else
> - fprintf (stderr, "\nnm output with constructors/destructors.\n");
> - }
> + fprintf (stderr, "\nnm output with constructors/destructors.\n");
>
> /* Read each line of nm output. */
> while (fgets (buf, sizeof buf, inf) != (char *) 0)
> @@ -2434,30 +2448,6 @@ scan_prog_file (const char *prog_name, scanpass
> which_pass,
> if (debug)
> fprintf (stderr, "\t%s\n", buf);
>
> - if (which_pass == PASS_LTOINFO)
> - {
> - if (found_lto)
> - continue;
> -
> - /* Look for the LTO info marker symbol, and add filename to
> - the LTO objects list if found. */
> - for (p = buf; (ch = *p) != '\0' && ch != '\n'; p++)
> - if (ch == ' ' && p[1] == '_' && p[2] == '_'
> - && (strncmp (p + (p[3] == '_' ? 2 : 1), "__gnu_lto_v1", 12)
> == 0)
> - && ISSPACE (p[p[3] == '_' ? 14 : 13]))
> - {
> - add_lto_object (<o_objects, prog_name);
> -
> - /* We need to read all the input, so we can't just
> - return here. But we can avoid useless work. */
> - found_lto = 1;
> -
> - break;
> - }
> -
> - continue;
> - }
> -
> /* If it contains a constructor or destructor name, add the name
> to the appropriate list unless this is a kind of symbol we're
> not supposed to even consider. */
> --
> 2.17.1
>
>
