Hi
This got stuck in my stack of patches for LTO debug support, and I forgot to
ping it…
> On 22 Aug 2018, at 14:20, Richard Biener <richard.guent...@gmail.com> wrote:
>
> 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.
>> 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_.
So I just look for the LTO_SECTION_NAME_PREFIX
> I'm not sure if/how we should handle offload sections and if those
> work at all without a
> linker plugin currently?
In this version, I look for the OFFLOAD_SECTION_NAME_PREFIX as well,
although AFAICS collect2 itself has no specific handling of offload, the
detection
is done in lto-wrapper.
OK for trunk now?
thanks
Iain
From: Iain Sandoe <i...@sandoe.co.uk>
Date: Tue, 21 Aug 2018 12:55:02 +0100
Subject: [PATCH] [PATCH, collect2] Use simple-object to find out if objects
contain LTO.
This replaces the use of nm to search for the LTO common symbol marker
and uses simple object to see if there's a section starting with
".gnu.lto_." or .gnu.offload_lto_
---
gcc/collect2.c | 120 +++++++++++++++++++++++--------------------------
1 file changed, 57 insertions(+), 63 deletions(-)
diff --git a/gcc/collect2.c b/gcc/collect2.c
index 60269682ec..dcbd3e18a6 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. */
@@ -1710,7 +1715,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);
return 0;
}
@@ -1780,7 +1785,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);
@@ -1874,7 +1879,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
@@ -2332,38 +2337,52 @@ write_aix_file (FILE *stream, struct id *list)
/* Check to make sure the file is an LTO object file. */
+static int
+has_lto_section (void *data, const char *name ATTRIBUTE_UNUSED,
+ off_t offset ATTRIBUTE_UNUSED,
+ off_t length ATTRIBUTE_UNUSED)
+{
+ int *found = (int *) data;
+
+ if (strncmp (name, LTO_SECTION_NAME_PREFIX,
+ sizeof (LTO_SECTION_NAME_PREFIX) - 1) != 0)
+ {
+ if (strncmp (name, OFFLOAD_SECTION_NAME_PREFIX,
+ sizeof (OFFLOAD_SECTION_NAME_PREFIX) - 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_section,
+ (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;
}
@@ -2386,7 +2405,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;
@@ -2394,8 +2412,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)
@@ -2450,12 +2473,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)
@@ -2466,30 +2484,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
