[PATCH][LTO][RFC] AR archive support for LTO

[Richard Guenther-3]

This implements a simple way of retrofitting AR support into the
existing LTO ELF machinery.  It avoids breaking the number-of-files
== number-of-objects equality and thus avoids touching the various
places we iterate over files.  It also avoids that LTO would need
to think about partially LTOed archives.

The hack is that the LTO frontend now knows how to address a
single ELF object inside an ar archive.  lto1 gets passed
these objects as ARCHIVE@OBJECT, for example t.a@....  This
should avoid the need to extract the archive inside the linker
plugin and should allow straight-forward support for archives
inside collect2.

The patch re-introduces the problem of unaligned accesses as
ar archive members are only 2-byte aligned.  I guess strict-align
hosts would need to fall back to read instead of mmap here,
or we need to audit all places that read from the mmapped memory
with non-char pointers ...

The patch doesn't try to limit the users ability of shooting
himself in the foot, like when linking with --whole-archive
and mixed LTO / non-LTO archives.  As is I consider the patch
a cleanup for the existing AR support in the linker plugin.

Once again libelfs are broken.  libelf 0.8.12 doesn't have
elf_getaroff and at least elfutils is broken when iterating
through archive members (it repeats the last entry forever,
at increasing offsets).  I have worked around both issues...

Tested manually on archives up to two members (yay!).

Comments?

Volunteers to hack this support into the linker-plugin?

Thanks,
Richard.

2009-11-01  Richard Guenther  <rguenther@...>

        lto/
        * lto-elf.c (struct lto_elf_file): New off member.
        (lto_elf_build_section_table): Add offset.
        (struct ar_hdr): New.
        (lto_elf_file_open): Handle object files in ar archives
        named archive@object.

Index: gcc/lto/lto-elf.c
===================================================================
*** gcc/lto/lto-elf.c (revision 153774)
--- gcc/lto/lto-elf.c (working copy)
*************** struct lto_elf_file
*** 49,54 ****
--- 49,57 ----
    /* The libelf descriptor for the file.  */
    Elf *elf;
 
+   /* The offset of the file.  */
+   size_t off;
+
    /* Section number of string table used for section names.  */
    size_t sec_strtab;
 
*************** lto_elf_build_section_table (lto_file *l
*** 206,212 ****
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
--- 209,215 ----
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = elf_file->off + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
*************** init_ehdr (lto_elf_file *elf_file)
*** 530,535 ****
--- 533,548 ----
      }
  }
 
+ /* AR archive header.  */
+ struct ar_hdr
+ {
+   char ar_name[16];           /* Member file name, sometimes / terminated. */
+   char ar_date[12];           /* File date, decimal seconds since Epoch.  */
+   char ar_uid[6], ar_gid[6];  /* User and group IDs, in ASCII decimal.  */
+   char ar_mode[8];            /* File mode, in ASCII octal.  */
+   char ar_size[10];           /* File size, in ASCII decimal.  */
+   char ar_fmag[2];            /* Always contains ARFMAG.  */
+ };
 
  /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
     and, if necessary, created.  Otherwise, the file is opened for reading.
*************** lto_elf_file_open (const char *filename,
*** 540,557 ****
  {
    lto_elf_file *elf_file;
    lto_file *result;
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, filename);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", filename);
        goto fail;
      }
 
--- 553,583 ----
  {
    lto_elf_file *elf_file;
    lto_file *result;
+   const char *entry_name;
+   char *fname;
+
+   entry_name = strchr (filename, '@');
+   if (!entry_name)
+     fname = xstrdup (filename);
+   else
+     {
+       fname = (char *)xmalloc (entry_name - filename + 1);
+       memcpy (fname, filename, entry_name - filename);
+       fname[entry_name - filename] = '\0';
+       entry_name++;
+     }
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, fname);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", fname);
        goto fail;
      }
 
*************** lto_elf_file_open (const char *filename,
*** 571,576 ****
--- 597,663 ----
        goto fail;
      }
 
+   if (elf_kind (elf_file->elf) == ELF_K_AR)
+     {
+       Elf *e;
+       struct stat sbuf;
+       struct ar_hdr hdr;
+       int lfd;
+       off_t coff;
+
+       if (!entry_name)
+ {
+  error ("need ar entry name for %s", fname);
+  goto fail;
+ }
+
+       /* One libelf implementation lacks elf_getaroff.  */
+       lfd = dup (elf_file->fd);
+       coff = lseek (lfd, 8, SEEK_SET);
+       fstat (elf_file->fd, &sbuf);
+       do
+ {
+  Elf_Arhdr *arh;
+  read (lfd, &hdr, sizeof (struct ar_hdr));
+  coff += sizeof (struct ar_hdr);
+  e = elf_begin (elf_file->fd, ELF_C_READ, elf_file->elf);
+  if (!e)
+    break;
+  arh = elf_getarhdr (e);
+  if (arh
+      && strcmp (arh->ar_name, entry_name) == 0)
+    break;
+
+  /* ???  The iterator seems to stick on the last entry but
+     advances offsets, so bail out based on our parallel
+     parsing as well.  */
+  coff += (atoi(hdr.ar_size) + 1) & ~1;
+  if (lseek (lfd, (atoi(hdr.ar_size) + 1) & ~1, SEEK_CUR) != coff)
+    {
+      e = NULL;
+      break;
+    }
+  elf_next (e);
+  elf_end (e);
+ }
+       while (1);
+       if (!e)
+ {
+  error ("ar entry %s not found in archive %s", entry_name, fname);
+  goto fail;
+ }
+       elf_end (elf_file->elf);
+       elf_file->elf = e;
+       /* Once all libelf implementations support it we can simply use
+          elf_getaroff (elf_file->elf) + sizeof (struct ar_hdr) here.  */
+       elf_file->off = coff;
+     }
+   else if (entry_name)
+     {
+       error ("ar entry name specified for non-archive %s", fname);
+       goto fail;
+     }
+
    if (writable)
      {
        init_ehdr (elf_file);

[Richard Guenther-3]

On Sun, 1 Nov 2009, Richard Guenther wrote:

>
> This implements a simple way of retrofitting AR support into the
> existing LTO ELF machinery.  It avoids breaking the number-of-files
> == number-of-objects equality and thus avoids touching the various
> places we iterate over files.  It also avoids that LTO would need
> to think about partially LTOed archives.
>
> The hack is that the LTO frontend now knows how to address a
> single ELF object inside an ar archive.  lto1 gets passed
> these objects as ARCHIVE@OBJECT, for example t.a@....  This
> should avoid the need to extract the archive inside the linker
> plugin and should allow straight-forward support for archives
> inside collect2.
>
> The patch re-introduces the problem of unaligned accesses as
> ar archive members are only 2-byte aligned.  I guess strict-align
> hosts would need to fall back to read instead of mmap here,
> or we need to audit all places that read from the mmapped memory
> with non-char pointers ...
>
> The patch doesn't try to limit the users ability of shooting
> himself in the foot, like when linking with --whole-archive
> and mixed LTO / non-LTO archives.  As is I consider the patch
> a cleanup for the existing AR support in the linker plugin.
>
> Once again libelfs are broken.  libelf 0.8.12 doesn't have
> elf_getaroff and at least elfutils is broken when iterating
> through archive members (it repeats the last entry forever,
> at increasing offsets).  I have worked around both issues...
>
> Tested manually on archives up to two members (yay!).
>
> Comments?
>
> Volunteers to hack this support into the linker-plugin?

Actually I can avoid most of the mess by using elf_getbase like
the following.  The elfutils libelf bug persists though, so we
have to leave the workaround in.

Richard.

2009-11-01  Richard Guenther  <rguenther@...>

        lto/
        * lto-elf.c (lto_elf_build_section_table): Add the base offset.
        (lto_elf_file_open): Handle object files in ar archives
        named archive@object.

Index: gcc/lto/lto-elf.c
===================================================================
*** gcc/lto/lto-elf.c (revision 153774)
--- gcc/lto/lto-elf.c (working copy)
*************** lto_elf_build_section_table (lto_file *l
*** 167,175 ****
--- 167,177 ----
    lto_elf_file *elf_file = (lto_elf_file *)lto_file;
    htab_t section_hash_table;
    Elf_Scn *section;
+   size_t base_offset;
 
    section_hash_table = htab_create (37, hash_name, eq_name, free);
 
+   base_offset = elf_getbase (elf_file->elf);
    for (section = elf_getscn (elf_file->elf, 0);
         section;
         section = elf_nextscn (elf_file->elf, section))
*************** lto_elf_build_section_table (lto_file *l
*** 206,212 ****
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
--- 208,214 ----
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = base_offset + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
*************** init_ehdr (lto_elf_file *elf_file)
*** 530,536 ****
      }
  }
 
-
  /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
     and, if necessary, created.  Otherwise, the file is opened for reading.
     Returns the opened file.  */
--- 532,537 ----
*************** lto_elf_file_open (const char *filename,
*** 540,557 ****
  {
    lto_elf_file *elf_file;
    lto_file *result;
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, filename);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", filename);
        goto fail;
      }
 
--- 541,571 ----
  {
    lto_elf_file *elf_file;
    lto_file *result;
+   const char *entry_name;
+   char *fname;
+
+   entry_name = strchr (filename, '@');
+   if (!entry_name)
+     fname = xstrdup (filename);
+   else
+     {
+       fname = (char *)xmalloc (entry_name - filename + 1);
+       memcpy (fname, filename, entry_name - filename);
+       fname[entry_name - filename] = '\0';
+       entry_name++;
+     }
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, fname);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", fname);
        goto fail;
      }
 
*************** lto_elf_file_open (const char *filename,
*** 571,576 ****
--- 585,634 ----
        goto fail;
      }
 
+   if (elf_kind (elf_file->elf) == ELF_K_AR)
+     {
+       Elf *e;
+       struct stat sbuf;
+
+       if (!entry_name)
+ {
+  error ("need ar entry name for %s", fname);
+  goto fail;
+ }
+
+       fstat (elf_file->fd, &sbuf);
+       do
+ {
+  Elf_Arhdr *arh;
+  e = elf_begin (elf_file->fd, ELF_C_READ, elf_file->elf);
+  if (!e
+      /* ???  The iterator seems to stick on the last entry but
+ advances offsets for elfutils libelf.  */
+      || sbuf.st_size < elf_getbase (e))
+    break;
+  arh = elf_getarhdr (e);
+  if (arh
+      && strcmp (arh->ar_name, entry_name) == 0)
+    break;
+
+  elf_next (e);
+  elf_end (e);
+ }
+       while (1);
+       if (!e)
+ {
+  error ("ar entry %s not found in archive %s", entry_name, fname);
+  goto fail;
+ }
+       elf_end (elf_file->elf);
+       elf_file->elf = e;
+     }
+   else if (entry_name)
+     {
+       error ("ar entry name specified for non-archive %s", fname);
+       goto fail;
+     }
+
    if (writable)
      {
        init_ehdr (elf_file);

[Rafael Espindola-2]

> Comments?

Thanks!
Some general comments. Will try to review the patch on Monday:

*) The plugin knows the offsets and sizes of each object. Maybe we
could pass that to lto1 instead of having it understand the .a format.
*) Correct implementation in collect2 is not going to be simple, since
collect2 doesn't know which members are going to be used.
*) What is the problem with archives with mixed IL and regular ELF
files? The plugin should pass only the ones with IL to lto1.

> Volunteers to hack this support into the linker-plugin?

I can do it. Will removed the part of it that needed the most cleanup :-)

> Thanks,
> Richard.
>
> 2009-11-01  Richard Guenther  <rguenther@...>
>

Cheers,
--
Rafael Ávila de Espíndola

[Richard Guenther-3]

On Sun, 1 Nov 2009, Rafael Espindola wrote:

> > Comments?
>
> Thanks!
> Some general comments. Will try to review the patch on Monday:
>
> *) The plugin knows the offsets and sizes of each object. Maybe we
> could pass that to lto1 instead of having it understand the .a format.

Maybe - but that's going to make a collect2 implementation harder
as it would need to communicate via the command-line.

> *) Correct implementation in collect2 is not going to be simple, since
> collect2 doesn't know which members are going to be used.

Well, collect2 uses nm at the moment to decide which .o files have
lto data - it could as well use nm to decide which ar members have
lto data (which is what I would do here).  The alternative would
of course be to pass the whole archive to lto1 and let it discover
which members to use and which not.

> *) What is the problem with archives with mixed IL and regular ELF
> files? The plugin should pass only the ones with IL to lto1.

Right.  But see above - if we'd pass the whole archive lto1 would
need to figure this out.  The question is also what to pass to
the final link step - I suppose linking against the archive again
should work as long as you don't force in the whole archive.  And
is even necessary if regular ELF file members are in there.

> > Volunteers to hack this support into the linker-plugin?
>
> I can do it. Will removed the part of it that needed the most cleanup :-)

Yeah ;)  I'll investigate how difficult collect2 support would be.

Richard.

[Rafael Espindola-2]

> Maybe - but that's going to make a collect2 implementation harder
> as it would need to communicate via the command-line.

Can we at least have support for something like

foo.a@123:15

meaning the file in foo.a starting at offset 123 and having size 15? The
linker plugin currently doesn't know the file name and I would like to
avoid having the plugin parse the archive header.

>> *) Correct implementation in collect2 is not going to be simple, since
>> collect2 doesn't know which members are going to be used.
>
> Well, collect2 uses nm at the moment to decide which .o files have
> lto data - it could as well use nm to decide which ar members have
> lto data (which is what I would do here).  The alternative would
> of course be to pass the whole archive to lto1 and let it discover
> which members to use and which not.

My preferences is to keep as much stuff out of lto1 as possible, so I would
vote for having only IL files passed to lto1. Note that collect2 would have
to know the files without IL anyway so that it would pass them to the
final link.

Is it really unacceptable to require gold for using IL files in archives? Maybe
the best solution is to add plugin support to gnu LD. I already added a
minimal support to BFD so that nm and ar can use the IL symbol table.


> Yeah ;)  I'll investigate how difficult collect2 support would be.

Do consider patching GNU ld or moving to gold instead :-)

I will now thy to code a patch to the plugin for using foo.a@1234:15 and adapt
your patch for it. Lets see if works :-)

> Richard.
>

Cheers,
--
Rafael Ávila de Espíndola

[Richard Guenther-3]

On Mon, 2 Nov 2009, Rafael Espindola wrote:

> > Maybe - but that's going to make a collect2 implementation harder
> > as it would need to communicate via the command-line.
>
> Can we at least have support for something like
>
> foo.a@123:15
>
> meaning the file in foo.a starting at offset 123 and having size 15? The
> linker plugin currently doesn't know the file name and I would like to
> avoid having the plugin parse the archive header.

Sure, the size shouldn't even be needed, just use elf_rand to seek
to the archive member.

> >> *) Correct implementation in collect2 is not going to be simple, since
> >> collect2 doesn't know which members are going to be used.
> >
> > Well, collect2 uses nm at the moment to decide which .o files have
> > lto data - it could as well use nm to decide which ar members have
> > lto data (which is what I would do here).  The alternative would
> > of course be to pass the whole archive to lto1 and let it discover
> > which members to use and which not.
>
> My preferences is to keep as much stuff out of lto1 as possible, so I would
> vote for having only IL files passed to lto1. Note that collect2 would have
> to know the files without IL anyway so that it would pass them to the
> final link.

It would pass the full archive to the link if there is any file w/o
LTO data in the archive.

> Is it really unacceptable to require gold for using IL files in archives? Maybe
> the best solution is to add plugin support to gnu LD. I already added a
> minimal support to BFD so that nm and ar can use the IL symbol table.

Well, it's an unfortunate limitation that is difficult to explain ...
and the fact that you can't use both, GNU ld and gold side-by-side
doesn't make it easier.

> > Yeah ;)  I'll investigate how difficult collect2 support would be.
>
> Do consider patching GNU ld or moving to gold instead :-)

Heh.  I'm not really into the linker code(s).

> I will now thy to code a patch to the plugin for using foo.a@1234:15 and adapt
> your patch for it. Lets see if works :-)

Thanks - try to keep it simple and omit the size.

Here's my current patch btw, avoiding the elfutils bug workaround.

Richard.

2009-11-01  Richard Guenther  <rguenther@...>

        lto/
        * lto-elf.c (lto_elf_build_section_table): Add the base offset.
        (lto_elf_file_open): Handle object files in ar archives
        named archive@object.

Index: gcc/lto/lto-elf.c
===================================================================
*** gcc/lto/lto-elf.c.orig 2009-10-10 23:54:19.000000000 +0200
--- gcc/lto/lto-elf.c 2009-11-02 11:16:44.000000000 +0100
*************** lto_elf_build_section_table (lto_file *l
*** 167,175 ****
--- 167,177 ----
    lto_elf_file *elf_file = (lto_elf_file *)lto_file;
    htab_t section_hash_table;
    Elf_Scn *section;
+   size_t base_offset;
 
    section_hash_table = htab_create (37, hash_name, eq_name, free);
 
+   base_offset = elf_getbase (elf_file->elf);
    for (section = elf_getscn (elf_file->elf, 0);
         section;
         section = elf_nextscn (elf_file->elf, section))
*************** lto_elf_build_section_table (lto_file *l
*** 206,212 ****
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
--- 208,214 ----
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
!  new_slot->start = base_offset + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
*************** init_ehdr (lto_elf_file *elf_file)
*** 530,536 ****
      }
  }
 
-
  /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
     and, if necessary, created.  Otherwise, the file is opened for reading.
     Returns the opened file.  */
--- 532,537 ----
*************** lto_elf_file_open (const char *filename,
*** 540,557 ****
  {
    lto_elf_file *elf_file;
    lto_file *result;
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, filename);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", filename);
        goto fail;
      }
 
--- 541,571 ----
  {
    lto_elf_file *elf_file;
    lto_file *result;
+   const char *entry_name;
+   char *fname;
+
+   entry_name = strchr (filename, '@');
+   if (!entry_name)
+     fname = xstrdup (filename);
+   else
+     {
+       fname = (char *)xmalloc (entry_name - filename + 1);
+       memcpy (fname, filename, entry_name - filename);
+       fname[entry_name - filename] = '\0';
+       entry_name++;
+     }
 
    /* Set up.  */
    elf_file = XCNEW (lto_elf_file);
    result = (lto_file *) elf_file;
!   lto_file_init (result, fname);
    elf_file->fd = -1;
 
    /* Open the file.  */
!   elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
    if (elf_file->fd == -1)
      {
!       error ("could not open file %s", fname);
        goto fail;
      }
 
*************** lto_elf_file_open (const char *filename,
*** 571,576 ****
--- 585,630 ----
        goto fail;
      }
 
+   if (elf_kind (elf_file->elf) == ELF_K_AR)
+     {
+       Elf *e;
+       Elf_Cmd cmd = ELF_C_READ;
+
+       if (!entry_name)
+ {
+  error ("need ar entry name for %s", fname);
+  goto fail;
+ }
+
+       do
+ {
+  Elf_Arhdr *arh;
+  e = elf_begin (elf_file->fd, cmd, elf_file->elf);
+  if (!e)
+    break;
+  arh = elf_getarhdr (e);
+  if (arh
+      && strcmp (arh->ar_name, entry_name) == 0)
+    break;
+
+  cmd = elf_next (e);
+  elf_end (e);
+ }
+       while (1);
+       if (!e)
+ {
+  error ("ar entry %s not found in archive %s", entry_name, fname);
+  goto fail;
+ }
+       elf_end (elf_file->elf);
+       elf_file->elf = e;
+     }
+   else if (entry_name)
+     {
+       error ("ar entry name specified for non-archive %s", fname);
+       goto fail;
+     }
+
    if (writable)
      {
        init_ehdr (elf_file);

[Rafael Espindola-2]

> Sure, the size shouldn't even be needed, just use elf_rand to seek
> to the archive member.

Sure.

> It would pass the full archive to the link if there is any file w/o
> LTO data in the archive.

That is not the correct behavior even for archives with only IL in
them. If an archive has a.o and b.o but only b.o is used, a.o should
not be passed to lto1.

>> Do consider patching GNU ld or moving to gold instead :-)
>
> Heh.  I'm not really into the linker code(s).

I am not really into BFD code, but I might be tempted to take a look
to keep the lto1 interface simpler :-)

> Thanks - try to keep it simple and omit the size.
>
> Here's my current patch btw, avoiding the elfutils bug workaround.

Attached is a work in progress that merges your patch with the
necessary changes to the plugin. With this we can do

 ./gcc/xgcc   -O2 -flto -B ./gcc/ test.o test2.a -o t
-fuse-linker-plugin -B ./lto-plugin/.libs/

And lto1 correctly sees all the necessary IL files and no temporary .o
is created :-)

Things I am not 100% happy with in the patch:

*) The hack in gcc.c. What is the correct solution? Making gcc.c
ignore filenames with a @ in it?
*) Passing the offset of the archive header instead of the object
data. This is to make elf_rand happy, but is an interface pollution. I
can change the plugin to pass the offset of the object data itself,
but then I have to include ar.h in lto-elf.c to get sizeof (struct
ar_hdr). Is there an API in libelf to get an Elf* from a fd, offset
pair?

> Richard.
>

Cheers,
--
Rafael Ávila de Espíndola

[archive.patch]

---

diff --git a/gcc/gcc.c b/gcc/gcc.c
index b033d62..fa7bf8d 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4566,7 +4566,7 @@ process_command (int argc, const char **argv)
   argv[i] = convert_filename (argv[i], 0, access (argv[i], F_OK));
 #endif
 
-  if (strcmp (argv[i], "-") != 0 && access (argv[i], F_OK) < 0)
+  if (strcmp (argv[i], "-") != 0 && access (argv[i], F_OK) < 0 && 0)
     {
       perror_with_name (argv[i]);
       error_count++;
diff --git a/gcc/lto/lto-elf.c b/gcc/lto/lto-elf.c
index 190430b..9fd3252 100644
--- a/gcc/lto/lto-elf.c
+++ b/gcc/lto/lto-elf.c
@@ -167,9 +167,11 @@ lto_elf_build_section_table (lto_file *lto_file)
   lto_elf_file *elf_file = (lto_elf_file *)lto_file;
   htab_t section_hash_table;
   Elf_Scn *section;
+  size_t base_offset;
 
   section_hash_table = htab_create (37, hash_name, eq_name, free);
 
+  base_offset = elf_getbase (elf_file->elf);
   for (section = elf_getscn (elf_file->elf, 0);
        section;
        section = elf_nextscn (elf_file->elf, section))
@@ -206,7 +208,7 @@ lto_elf_build_section_table (lto_file *lto_file)
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
-  new_slot->start = shdr->sh_offset;
+  new_slot->start = base_offset + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
@@ -530,7 +532,6 @@ init_ehdr (lto_elf_file *elf_file)
     }
 }
 
-
 /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
    and, if necessary, created.  Otherwise, the file is opened for reading.
    Returns the opened file.  */
@@ -540,18 +541,36 @@ lto_elf_file_open (const char *filename, bool writable)
 {
   lto_elf_file *elf_file;
   lto_file *result;
+  off_t offset;
+  const char *offset_p;
+  char *fname;
+
+  offset_p = strchr (filename, '@');
+  if (!offset_p)
+    {
+      fname = xstrdup (filename);
+      offset = 0;
+    }
+  else
+    {
+      fname = (char *)xmalloc (offset_p - filename + 1);
+      memcpy (fname, filename, offset_p - filename);
+      fname[offset_p - filename] = '\0';
+      offset_p++;
+      sscanf(offset_p, "%" PRId64, &offset);
+    }
 
   /* Set up.  */
   elf_file = XCNEW (lto_elf_file);
   result = (lto_file *) elf_file;
-  lto_file_init (result, filename);
+  lto_file_init (result, fname);
   elf_file->fd = -1;
 
   /* Open the file.  */
-  elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
+  elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
   if (elf_file->fd == -1)
     {
-      error ("could not open file %s", filename);
+      error ("could not open file %s", fname);
       goto fail;
     }
 
@@ -571,6 +590,26 @@ lto_elf_file_open (const char *filename, bool writable)
       goto fail;
     }
 
+  if (offset != 0)
+    {
+      Elf *e;
+      off_t t = elf_rand(elf_file->elf, offset);
+      if (t != offset)
+        {
+          error ("could not seek in archive");
+          goto fail;
+        }
+
+      e = elf_begin(elf_file->fd, ELF_C_READ, elf_file->elf);
+      if (e == NULL)
+        {
+          error("could not find archive member");
+          goto fail;
+        }
+      elf_end (elf_file->elf);
+      elf_file->elf = e;
+    }
+
   if (writable)
     {
       init_ehdr (elf_file);
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index c92ac06..9a4a396 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -70,7 +70,6 @@ struct plugin_file_info
   char *name;
   void *handle;
   struct plugin_symtab symtab;
-  unsigned char temp;
 };
 
 
@@ -513,20 +512,9 @@ static enum ld_plugin_status
 cleanup_handler (void)
 {
   int t;
-  unsigned i;
   char *arguments;
   struct stat buf;
 
-  for (i = 0; i < num_claimed_files; i++)
-    {
-      struct plugin_file_info *info = &claimed_files[i];
-      if (info->temp)
- {
-  t = unlink (info->name);
-  check (t == 0, LDPL_FATAL, "could not unlink temporary file");
- }
-    }
-
   /* If we are being called from an error handler, it is possible
      that the arguments file is still exists. */
   t = asprintf (&arguments, "%s/arguments", temp_obj_dir_name);
@@ -555,49 +543,32 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   Elf *elf;
   struct plugin_file_info lto_file;
   Elf_Data *symtab;
-  int lto_file_fd;
 
   if (file->offset != 0)
     {
-      /* FIXME lto: lto1 should know how to handle archives. */
-      int fd;
-      off_t size = file->filesize;
-      off_t offset;
-
-      static int objnum = 0;
       char *objname;
-      int t = asprintf (&objname, "%s/obj%d.o",
- temp_obj_dir_name, objnum);
+      Elf *archive;
+      off_t offset = file->offset - sizeof(struct ar_hdr);
+      int t = asprintf (&objname, "%s@%" PRId64, file->name, offset);
       check (t >= 0, LDPL_FATAL, "asprintf failed");
-      objnum++;
-
-      fd = open (objname, O_RDWR | O_CREAT, 0666);
-      check (fd > 0, LDPL_FATAL, "could not open/create temporary file");
-      offset = lseek (file->fd, file->offset, SEEK_SET);
-      check (offset == file->offset, LDPL_FATAL, "could not seek");
-      while (size > 0)
- {
-  ssize_t r, written;
-  char buf[1000];
-  off_t s = sizeof (buf) < size ? sizeof (buf) : size;
-  r = read (file->fd, buf, s);
-  written = write (fd, buf, r);
-  check (written == r, LDPL_FATAL, "could not write to temporary file");
-  size -= r;
- }
       lto_file.name = objname;
-      lto_file_fd = fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 1;
+
+      archive = elf_begin (file->fd, ELF_C_READ, NULL);
+      check (elf_kind(archive) == ELF_K_AR, LDPL_FATAL,
+             "Not an archive and offset not 0");
+
+      check (offset == elf_rand(archive, offset), LDPL_FATAL,
+             "could not seek in archive");
+      elf = elf_begin(file->fd, ELF_C_READ, archive);
+      check (elf != NULL, LDPL_FATAL, "could not find archive member");
+      elf_end (archive);
     }
   else
     {
       lto_file.name = strdup (file->name);
-      lto_file_fd = file->fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 0;
+      elf = elf_begin (file->fd, ELF_C_READ, NULL);
     }
-  elf = elf_begin (lto_file_fd, ELF_C_READ, NULL);
+  lto_file.handle = file->handle;
 
   *claimed = 0;
 
@@ -624,20 +595,12 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   goto cleanup;
 
  err:
-  if (file->offset != 0)
-    {
-      int t = unlink (lto_file.name);
-      check (t == 0, LDPL_FATAL, "could not unlink file");
-    }
   free (lto_file.name);
 
  cleanup:
   if (elf)
     elf_end (elf);
 
-  if (file->offset != 0)
-    close (lto_file_fd);
-
   return LDPS_OK;
 }
 

---

[Rafael Espindola-2]

> Things I am not 100% happy with in the patch:

>
> *) The hack in gcc.c. What is the correct solution? Making gcc.c
> ignore filenames with a @ in it?
> *) Passing the offset of the archive header instead of the object
> data. This is to make elf_rand happy, but is an interface pollution. I
> can change the plugin to pass the offset of the object data itself,
> but then I have to include ar.h in lto-elf.c to get sizeof (struct
> ar_hdr). Is there an API in libelf to get an Elf* from a fd, offset
> pair?
>

OK. I fixed both. What do you think of the attached patch?

gcc/
2009-11-02  Richard Guenther  <rguenther@...>
            Rafael Avila de Espindola  <espindola@...>

        * gcc.c (process_command): Handle arguments name@offset.

lto-plugin/
2009-11-02  Richard Guenther  <rguenther@...>
            Rafael Avila de Espindola  <espindola@...>

        * lto-plugin.c (plugin_file_info): Remove temp field.
        (cleanup_handler): Don't delete temporary objects.
        (claim_file_handler): Don't create temporary objects.
       
lto/
2009-11-02  Richard Guenther  <rguenther@...>
            Rafael Avila de Espindola  <espindola@...>

       * lto-elf.c (lto_elf_build_section_table): Add the base offset.
       (lto_elf_file_open): Handle offsets in arguments name@offest.

Cheers,
--
Rafael Ávila de Espíndola

[archive.patch]

---

diff --git a/gcc/gcc.c b/gcc/gcc.c
index b033d62..b52b925 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4562,20 +4562,33 @@ process_command (int argc, const char **argv)
  }
       else
  {
+          const char *p = strchr (argv[i], '@');
+          char *fname;
 #ifdef HAVE_TARGET_OBJECT_SUFFIX
   argv[i] = convert_filename (argv[i], 0, access (argv[i], F_OK));
 #endif
+          if (!p)
+            fname = argv[i];
+          else
+            {
+              fname = (char *)xmalloc (p - argv[i] + 1);
+              memcpy (fname, argv[i], p - argv[i]);
+              fname[p - argv[i]] = '\0';
+            }
+
+          if (strcmp (fname, "-") != 0 && access (fname, F_OK) < 0)
+            {
+              perror_with_name (fname);
+              error_count++;
+            }
+          else
+            {
+              infiles[n_infiles].language = spec_lang;
+              infiles[n_infiles++].name = argv[i];
+            }
 
-  if (strcmp (argv[i], "-") != 0 && access (argv[i], F_OK) < 0)
-    {
-      perror_with_name (argv[i]);
-      error_count++;
-    }
-  else
-    {
-      infiles[n_infiles].language = spec_lang;
-      infiles[n_infiles++].name = argv[i];
-    }
+          if (p)
+            free (fname);
  }
     }
 
diff --git a/gcc/lto/lto-elf.c b/gcc/lto/lto-elf.c
index 190430b..5f6eb60 100644
--- a/gcc/lto/lto-elf.c
+++ b/gcc/lto/lto-elf.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "libiberty.h"
 #include "ggc.h"
 #include "lto-streamer.h"
+#include "ar.h"
 
 
 /* Initialize FILE, an LTO file object for FILENAME.  */
@@ -167,9 +168,11 @@ lto_elf_build_section_table (lto_file *lto_file)
   lto_elf_file *elf_file = (lto_elf_file *)lto_file;
   htab_t section_hash_table;
   Elf_Scn *section;
+  size_t base_offset;
 
   section_hash_table = htab_create (37, hash_name, eq_name, free);
 
+  base_offset = elf_getbase (elf_file->elf);
   for (section = elf_getscn (elf_file->elf, 0);
        section;
        section = elf_nextscn (elf_file->elf, section))
@@ -206,7 +209,7 @@ lto_elf_build_section_table (lto_file *lto_file)
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
-  new_slot->start = shdr->sh_offset;
+  new_slot->start = base_offset + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
@@ -530,7 +533,6 @@ init_ehdr (lto_elf_file *elf_file)
     }
 }
 
-
 /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
    and, if necessary, created.  Otherwise, the file is opened for reading.
    Returns the opened file.  */
@@ -540,18 +542,37 @@ lto_elf_file_open (const char *filename, bool writable)
 {
   lto_elf_file *elf_file;
   lto_file *result;
+  off_t offset;
+  const char *offset_p;
+  char *fname;
+
+  offset_p = strchr (filename, '@');
+  if (!offset_p)
+    {
+      fname = xstrdup (filename);
+      offset = 0;
+    }
+  else
+    {
+      fname = (char *)xmalloc (offset_p - filename + 1);
+      memcpy (fname, filename, offset_p - filename);
+      fname[offset_p - filename] = '\0';
+      offset_p++;
+      sscanf(offset_p, "%" PRId64, &offset);
+      offset -= sizeof(struct ar_hdr);
+    }
 
   /* Set up.  */
   elf_file = XCNEW (lto_elf_file);
   result = (lto_file *) elf_file;
-  lto_file_init (result, filename);
+  lto_file_init (result, fname);
   elf_file->fd = -1;
 
   /* Open the file.  */
-  elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
+  elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
   if (elf_file->fd == -1)
     {
-      error ("could not open file %s", filename);
+      error ("could not open file %s", fname);
       goto fail;
     }
 
@@ -571,6 +592,26 @@ lto_elf_file_open (const char *filename, bool writable)
       goto fail;
     }
 
+  if (offset != 0)
+    {
+      Elf *e;
+      off_t t = elf_rand(elf_file->elf, offset);
+      if (t != offset)
+        {
+          error ("could not seek in archive");
+          goto fail;
+        }
+
+      e = elf_begin(elf_file->fd, ELF_C_READ, elf_file->elf);
+      if (e == NULL)
+        {
+          error("could not find archive member");
+          goto fail;
+        }
+      elf_end (elf_file->elf);
+      elf_file->elf = e;
+    }
+
   if (writable)
     {
       init_ehdr (elf_file);
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index c92ac06..48a3b18 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -70,7 +70,6 @@ struct plugin_file_info
   char *name;
   void *handle;
   struct plugin_symtab symtab;
-  unsigned char temp;
 };
 
 
@@ -513,20 +512,9 @@ static enum ld_plugin_status
 cleanup_handler (void)
 {
   int t;
-  unsigned i;
   char *arguments;
   struct stat buf;
 
-  for (i = 0; i < num_claimed_files; i++)
-    {
-      struct plugin_file_info *info = &claimed_files[i];
-      if (info->temp)
- {
-  t = unlink (info->name);
-  check (t == 0, LDPL_FATAL, "could not unlink temporary file");
- }
-    }
-
   /* If we are being called from an error handler, it is possible
      that the arguments file is still exists. */
   t = asprintf (&arguments, "%s/arguments", temp_obj_dir_name);
@@ -555,49 +543,32 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   Elf *elf;
   struct plugin_file_info lto_file;
   Elf_Data *symtab;
-  int lto_file_fd;
 
   if (file->offset != 0)
     {
-      /* FIXME lto: lto1 should know how to handle archives. */
-      int fd;
-      off_t size = file->filesize;
-      off_t offset;
-
-      static int objnum = 0;
       char *objname;
-      int t = asprintf (&objname, "%s/obj%d.o",
- temp_obj_dir_name, objnum);
+      Elf *archive;
+      off_t offset = file->offset - sizeof(struct ar_hdr);
+      int t = asprintf (&objname, "%s@%" PRId64, file->name, file->offset);
       check (t >= 0, LDPL_FATAL, "asprintf failed");
-      objnum++;
-
-      fd = open (objname, O_RDWR | O_CREAT, 0666);
-      check (fd > 0, LDPL_FATAL, "could not open/create temporary file");
-      offset = lseek (file->fd, file->offset, SEEK_SET);
-      check (offset == file->offset, LDPL_FATAL, "could not seek");
-      while (size > 0)
- {
-  ssize_t r, written;
-  char buf[1000];
-  off_t s = sizeof (buf) < size ? sizeof (buf) : size;
-  r = read (file->fd, buf, s);
-  written = write (fd, buf, r);
-  check (written == r, LDPL_FATAL, "could not write to temporary file");
-  size -= r;
- }
       lto_file.name = objname;
-      lto_file_fd = fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 1;
+
+      archive = elf_begin (file->fd, ELF_C_READ, NULL);
+      check (elf_kind(archive) == ELF_K_AR, LDPL_FATAL,
+             "Not an archive and offset not 0");
+
+      check (offset == elf_rand(archive, offset), LDPL_FATAL,
+             "could not seek in archive");
+      elf = elf_begin(file->fd, ELF_C_READ, archive);
+      check (elf != NULL, LDPL_FATAL, "could not find archive member");
+      elf_end (archive);
     }
   else
     {
       lto_file.name = strdup (file->name);
-      lto_file_fd = file->fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 0;
+      elf = elf_begin (file->fd, ELF_C_READ, NULL);
     }
-  elf = elf_begin (lto_file_fd, ELF_C_READ, NULL);
+  lto_file.handle = file->handle;
 
   *claimed = 0;
 
@@ -624,20 +595,12 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   goto cleanup;
 
  err:
-  if (file->offset != 0)
-    {
-      int t = unlink (lto_file.name);
-      check (t == 0, LDPL_FATAL, "could not unlink file");
-    }
   free (lto_file.name);
 
  cleanup:
   if (elf)
     elf_end (elf);
 
-  if (file->offset != 0)
-    close (lto_file_fd);
-
   return LDPS_OK;
 }
 

---

[Richard Guenther-3]

On Mon, 2 Nov 2009, Rafael Espindola wrote:

> > Things I am not 100% happy with in the patch:
> >
> > *) The hack in gcc.c. What is the correct solution? Making gcc.c
> > ignore filenames with a @ in it?
> > *) Passing the offset of the archive header instead of the object
> > data. This is to make elf_rand happy, but is an interface pollution. I
> > can change the plugin to pass the offset of the object data itself,
> > but then I have to include ar.h in lto-elf.c to get sizeof (struct
> > ar_hdr). Is there an API in libelf to get an Elf* from a fd, offset
> > pair?
> >
>
> OK. I fixed both. What do you think of the attached patch?

It looks good apart from

+#include "ar.h"
...
+      offset -= sizeof(struct ar_hdr);

I don't think we can assume that ar.h is available, so I'd suggest
to hardcode 60 here with an appropriate comment.

Thus, the patch is ok with that change and if the plugin changes
are oked by Cary.

Thanks,
Richard.

> gcc/
> 2009-11-02  Richard Guenther  <rguenther@...>
>             Rafael Avila de Espindola  <espindola@...>
>
> * gcc.c (process_command): Handle arguments name@offset.
>
> lto-plugin/
> 2009-11-02  Richard Guenther  <rguenther@...>
>             Rafael Avila de Espindola  <espindola@...>
>
> * lto-plugin.c (plugin_file_info): Remove temp field.
> (cleanup_handler): Don't delete temporary objects.
> (claim_file_handler): Don't create temporary objects.
>
> lto/
> 2009-11-02  Richard Guenther  <rguenther@...>
>             Rafael Avila de Espindola  <espindola@...>
>
>        * lto-elf.c (lto_elf_build_section_table): Add the base offset.
>        (lto_elf_file_open): Handle offsets in arguments name@offest.
>
> Cheers,
>

--
Richard Guenther <rguenther@...>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

[Dave Korn-6]

Rafael Espindola wrote:
>> Things I am not 100% happy with in the patch:
>>
>> *) The hack in gcc.c. What is the correct solution? Making gcc.c
>> ignore filenames with a @ in it?

> OK. I fixed both. What do you think of the attached patch?

  Maybe it should take care not to match on a filename where the first
character is '@', to avoid any danger of clashing with the "@file"
command-line file-substitution syntax?  I'm not sure where that gets expanded
(possibly in libiberty?), but it would be more robust to guard against it than
to rely on it happening earlier... someone might refactor this code one day
without realising, for example.

    cheers,
      DaveK

[Rafael Espindola-2]

> I don't think we can assume that ar.h is available, so I'd suggest
> to hardcode 60 here with an appropriate comment.

Done. Updated patch attached.

> Thus, the patch is ok with that change and if the plugin changes
> are oked by Cary.
>
> Thanks,
> Richard.

Cheers,
--
Rafael Ávila de Espíndola

[archive.patch]

---

diff --git a/gcc/gcc.c b/gcc/gcc.c
index b033d62..b52b925 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4562,20 +4562,33 @@ process_command (int argc, const char **argv)
  }
       else
  {
+          const char *p = strchr (argv[i], '@');
+          char *fname;
 #ifdef HAVE_TARGET_OBJECT_SUFFIX
   argv[i] = convert_filename (argv[i], 0, access (argv[i], F_OK));
 #endif
+          if (!p)
+            fname = argv[i];
+          else
+            {
+              fname = (char *)xmalloc (p - argv[i] + 1);
+              memcpy (fname, argv[i], p - argv[i]);
+              fname[p - argv[i]] = '\0';
+            }
+
+          if (strcmp (fname, "-") != 0 && access (fname, F_OK) < 0)
+            {
+              perror_with_name (fname);
+              error_count++;
+            }
+          else
+            {
+              infiles[n_infiles].language = spec_lang;
+              infiles[n_infiles++].name = argv[i];
+            }
 
-  if (strcmp (argv[i], "-") != 0 && access (argv[i], F_OK) < 0)
-    {
-      perror_with_name (argv[i]);
-      error_count++;
-    }
-  else
-    {
-      infiles[n_infiles].language = spec_lang;
-      infiles[n_infiles++].name = argv[i];
-    }
+          if (p)
+            free (fname);
  }
     }
 
diff --git a/gcc/lto/lto-elf.c b/gcc/lto/lto-elf.c
index 190430b..e6194ce 100644
--- a/gcc/lto/lto-elf.c
+++ b/gcc/lto/lto-elf.c
@@ -167,9 +167,11 @@ lto_elf_build_section_table (lto_file *lto_file)
   lto_elf_file *elf_file = (lto_elf_file *)lto_file;
   htab_t section_hash_table;
   Elf_Scn *section;
+  size_t base_offset;
 
   section_hash_table = htab_create (37, hash_name, eq_name, free);
 
+  base_offset = elf_getbase (elf_file->elf);
   for (section = elf_getscn (elf_file->elf, 0);
        section;
        section = elf_nextscn (elf_file->elf, section))
@@ -206,7 +208,7 @@ lto_elf_build_section_table (lto_file *lto_file)
 
   new_slot->name = new_name;
   /* The offset into the file for this section.  */
-  new_slot->start = shdr->sh_offset;
+  new_slot->start = base_offset + shdr->sh_offset;
   new_slot->len = shdr->sh_size;
   *slot = new_slot;
  }
@@ -530,7 +532,6 @@ init_ehdr (lto_elf_file *elf_file)
     }
 }
 
-
 /* Open ELF file FILENAME.  If WRITABLE is true, the file is opened for write
    and, if necessary, created.  Otherwise, the file is opened for reading.
    Returns the opened file.  */
@@ -540,18 +541,40 @@ lto_elf_file_open (const char *filename, bool writable)
 {
   lto_elf_file *elf_file;
   lto_file *result;
+  off_t offset;
+  const char *offset_p;
+  char *fname;
+
+  offset_p = strchr (filename, '@');
+  if (!offset_p)
+    {
+      fname = xstrdup (filename);
+      offset = 0;
+    }
+  else
+    {
+      fname = (char *)xmalloc (offset_p - filename + 1);
+      memcpy (fname, filename, offset_p - filename);
+      fname[offset_p - filename] = '\0';
+      offset_p++;
+      sscanf(offset_p, "%" PRId64, &offset);
+      // elf_rand expects the offset to point to the ar header, not the
+      // object itself. Subtract the size of the ar header (60 bytes).
+      // We don't uses sizeof (struct ar_hd) to avoid including ar.h
+      offset -= 60;
+    }
 
   /* Set up.  */
   elf_file = XCNEW (lto_elf_file);
   result = (lto_file *) elf_file;
-  lto_file_init (result, filename);
+  lto_file_init (result, fname);
   elf_file->fd = -1;
 
   /* Open the file.  */
-  elf_file->fd = open (filename, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
+  elf_file->fd = open (fname, writable ? O_WRONLY|O_CREAT : O_RDONLY, 0666);
   if (elf_file->fd == -1)
     {
-      error ("could not open file %s", filename);
+      error ("could not open file %s", fname);
       goto fail;
     }
 
@@ -571,6 +594,26 @@ lto_elf_file_open (const char *filename, bool writable)
       goto fail;
     }
 
+  if (offset != 0)
+    {
+      Elf *e;
+      off_t t = elf_rand(elf_file->elf, offset);
+      if (t != offset)
+        {
+          error ("could not seek in archive");
+          goto fail;
+        }
+
+      e = elf_begin(elf_file->fd, ELF_C_READ, elf_file->elf);
+      if (e == NULL)
+        {
+          error("could not find archive member");
+          goto fail;
+        }
+      elf_end (elf_file->elf);
+      elf_file->elf = e;
+    }
+
   if (writable)
     {
       init_ehdr (elf_file);
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index c92ac06..48a3b18 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -70,7 +70,6 @@ struct plugin_file_info
   char *name;
   void *handle;
   struct plugin_symtab symtab;
-  unsigned char temp;
 };
 
 
@@ -513,20 +512,9 @@ static enum ld_plugin_status
 cleanup_handler (void)
 {
   int t;
-  unsigned i;
   char *arguments;
   struct stat buf;
 
-  for (i = 0; i < num_claimed_files; i++)
-    {
-      struct plugin_file_info *info = &claimed_files[i];
-      if (info->temp)
- {
-  t = unlink (info->name);
-  check (t == 0, LDPL_FATAL, "could not unlink temporary file");
- }
-    }
-
   /* If we are being called from an error handler, it is possible
      that the arguments file is still exists. */
   t = asprintf (&arguments, "%s/arguments", temp_obj_dir_name);
@@ -555,49 +543,32 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   Elf *elf;
   struct plugin_file_info lto_file;
   Elf_Data *symtab;
-  int lto_file_fd;
 
   if (file->offset != 0)
     {
-      /* FIXME lto: lto1 should know how to handle archives. */
-      int fd;
-      off_t size = file->filesize;
-      off_t offset;
-
-      static int objnum = 0;
       char *objname;
-      int t = asprintf (&objname, "%s/obj%d.o",
- temp_obj_dir_name, objnum);
+      Elf *archive;
+      off_t offset = file->offset - sizeof(struct ar_hdr);
+      int t = asprintf (&objname, "%s@%" PRId64, file->name, file->offset);
       check (t >= 0, LDPL_FATAL, "asprintf failed");
-      objnum++;
-
-      fd = open (objname, O_RDWR | O_CREAT, 0666);
-      check (fd > 0, LDPL_FATAL, "could not open/create temporary file");
-      offset = lseek (file->fd, file->offset, SEEK_SET);
-      check (offset == file->offset, LDPL_FATAL, "could not seek");
-      while (size > 0)
- {
-  ssize_t r, written;
-  char buf[1000];
-  off_t s = sizeof (buf) < size ? sizeof (buf) : size;
-  r = read (file->fd, buf, s);
-  written = write (fd, buf, r);
-  check (written == r, LDPL_FATAL, "could not write to temporary file");
-  size -= r;
- }
       lto_file.name = objname;
-      lto_file_fd = fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 1;
+
+      archive = elf_begin (file->fd, ELF_C_READ, NULL);
+      check (elf_kind(archive) == ELF_K_AR, LDPL_FATAL,
+             "Not an archive and offset not 0");
+
+      check (offset == elf_rand(archive, offset), LDPL_FATAL,
+             "could not seek in archive");
+      elf = elf_begin(file->fd, ELF_C_READ, archive);
+      check (elf != NULL, LDPL_FATAL, "could not find archive member");
+      elf_end (archive);
     }
   else
     {
       lto_file.name = strdup (file->name);
-      lto_file_fd = file->fd;
-      lto_file.handle = file->handle;
-      lto_file.temp = 0;
+      elf = elf_begin (file->fd, ELF_C_READ, NULL);
     }
-  elf = elf_begin (lto_file_fd, ELF_C_READ, NULL);
+  lto_file.handle = file->handle;
 
   *claimed = 0;
 
@@ -624,20 +595,12 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   goto cleanup;
 
  err:
-  if (file->offset != 0)
-    {
-      int t = unlink (lto_file.name);
-      check (t == 0, LDPL_FATAL, "could not unlink file");
-    }
   free (lto_file.name);
 
  cleanup:
   if (elf)
     elf_end (elf);
 
-  if (file->offset != 0)
-    close (lto_file_fd);
-
   return LDPS_OK;
 }
 

---

[Rafael Espindola-2]

>  Maybe it should take care not to match on a filename where the first
> character is '@', to avoid any danger of clashing with the "@file"
> command-line file-substitution syntax?  I'm not sure where that gets expanded
> (possibly in libiberty?), but it would be more robust to guard against it than
> to rely on it happening earlier... someone might refactor this code one day
> without realising, for example.

That expansion has to be done very early. If it is not, this code will
be broken in many ways. For example, we would miss any foo.a@132
listed in the args file.

>    cheers,
>      DaveK
>
>


Cheers,
--
Rafael Ávila de Espíndola

[Cary Coutant-2]

> > I don't think we can assume that ar.h is available, so I'd suggest
> > to hardcode 60 here with an appropriate comment.

> Done. Updated patch attached.

In lto-plugin.c:

+      Elf *archive;
+      off_t offset = file->offset - sizeof(struct ar_hdr);
+      int t = asprintf (&objname, "%s@%" PRId64, file->name, file->offset);

Is it still OK to include <ar.h> from lto-plugin.c? I'm not sure why
it was necessary to remove it from lto-elf.c, but it's OK for
lto-plugin.c.

I don't think using PRId64 with off_t is completely portable. I'd
suggest casting file->offset to (int64_t) here.

Also, I think a comment here is necessary to make it clear that you
really do mean to pass "file->offset" as part of the file name rather
than the adjusted "offset" computed on the line above -- i.e., that
lto will make the same adjustment when parsing the filename. (Or
perhaps move the computation of "offset" down further to where it's
actually used.)

+      check (elf_kind(archive) == ELF_K_AR, LDPL_FATAL,
+             "Not an archive and offset not 0");
+
+      check (offset == elf_rand(archive, offset), LDPL_FATAL,
+             "could not seek in archive");
+      elf = elf_begin(file->fd, ELF_C_READ, archive);

Need spaces after elf_kind, elf_rand, and elf_begin.

In lto-elf.c:

+  off_t offset;
...
+      sscanf(offset_p, "%" PRId64, &offset);

Same problem as above; I don't think there's a portable format
specifier for off_t. I'd suggest declaring offset as int64_t here and
casting to off_t when you need it, or assigning it to an off_t after
the sscanf.

+      fname = (char *)xmalloc (offset_p - filename + 1);

Space after the cast?

+      off_t t = elf_rand(elf_file->elf, offset);
+      if (t != offset)
+        {
+          error ("could not seek in archive");
+          goto fail;
+        }
+
+      e = elf_begin(elf_file->fd, ELF_C_READ, elf_file->elf);

Need spaces after elf_rand and elf_begin.

OK with those changes.

-cary

[Dave Korn-6]

Rafael Espindola wrote:

>>  Maybe it should take care not to match on a filename where the first
>> character is '@', to avoid any danger of clashing with the "@file"
>> command-line file-substitution syntax?  I'm not sure where that gets expanded
>> (possibly in libiberty?), but it would be more robust to guard against it than
>> to rely on it happening earlier... someone might refactor this code one day
>> without realising, for example.
>
> That expansion has to be done very early. If it is not, this code will
> be broken in many ways. For example, we would miss any foo.a@132
> listed in the args file.

  Fair enough.

    cheers,
      DaveK

[Andrew Pinski-2]

On Tue, Nov 3, 2009 at 3:02 PM, Cary Coutant <ccoutant@...> wrote:

> +  off_t offset;
> ...
> +      sscanf(offset_p, "%" PRId64, &offset);
>
> Same problem as above; I don't think there's a portable format
> specifier for off_t. I'd suggest declaring offset as int64_t here and
> casting to off_t when you need it, or assigning it to an off_t after
> the sscanf.

SCNd64 is the correct usage for sscanf but can't we have a atoll for
int64_t ?  Or rather why don't we use HWI here?
On darwin with -Werror turned on I get the following error:
/Users/apinski/src/local/gcc/gcc/lto/lto-elf.c:561:7: error: ISO C
does not support the 'q' gnu_scanf length modifier

Thanks,
Andrew Pinski

[Rainer Orth-2]

Andrew Pinski <pinskia@...> writes:

> On Tue, Nov 3, 2009 at 3:02 PM, Cary Coutant <ccoutant@...> wrote:
>
>> +  off_t offset;
>> ...
>> +      sscanf(offset_p, "%" PRId64, &offset);
>>
>> Same problem as above; I don't think there's a portable format
>> specifier for off_t. I'd suggest declaring offset as int64_t here and
>> casting to off_t when you need it, or assigning it to an off_t after
>> the sscanf.
>
> SCNd64 is the correct usage for sscanf but can't we have a atoll for
> int64_t ?  Or rather why don't we use HWI here?
> On darwin with -Werror turned on I get the following error:
> /Users/apinski/src/local/gcc/gcc/lto/lto-elf.c:561:7: error: ISO C
> does not support the 'q' gnu_scanf length modifier

Please be careful here: you need to provide a replacement for platforms
that lack PRId64/SCNd64, cf. PR bootstrap/41996.

     Rainer

--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

[Rafael Espindola-2]

> SCNd64 is the correct usage for sscanf but can't we have a atoll for
> int64_t ?  Or rather why don't we use HWI here?
> On darwin with -Werror turned on I get the following error:
> /Users/apinski/src/local/gcc/gcc/lto/lto-elf.c:561:7: error: ISO C
> does not support the 'q' gnu_scanf length modifier

Is the attached patch OK?

Rainer, can you check that it fixes 41996.

> Thanks,
> Andrew Pinski
>

Thanks,
--
Rafael Ávila de Espíndola

[atoll.patch]

---

diff --git a/gcc/lto/lto-elf.c b/gcc/lto/lto-elf.c
index ee587f7..a1ea884 100644
--- a/gcc/lto/lto-elf.c
+++ b/gcc/lto/lto-elf.c
@@ -553,13 +553,11 @@ lto_elf_file_open (const char *filename, bool writable)
     }
   else
     {
-      int64_t t;
       fname = (char *) xmalloc (offset_p - filename + 1);
       memcpy (fname, filename, offset_p - filename);
       fname[offset_p - filename] = '\0';
       offset_p++;
-      sscanf(offset_p, "%" PRId64 , &t);
-      offset = t;
+      offset = atoll(offset_p);
       /* elf_rand expects the offset to point to the ar header, not the
          object itself. Subtract the size of the ar header (60 bytes).
          We don't uses sizeof (struct ar_hd) to avoid including ar.h */

---

[Rainer Orth-2]

Rafael Espindola <espindola@...> writes:

>> SCNd64 is the correct usage for sscanf but can't we have a atoll for
>> int64_t ?  Or rather why don't we use HWI here?
>> On darwin with -Werror turned on I get the following error:
>> /Users/apinski/src/local/gcc/gcc/lto/lto-elf.c:561:7: error: ISO C
>> does not support the 'q' gnu_scanf length modifier
>
> Is the attached patch OK?
>
> Rainer, can you check that it fixes 41996.

I'm pretty certain that it works on IRIX 6.5, which has atoll in libc,
but what about platforms that don't?  I think you'll have to provide a
replacement in libiberty.

I'll fire off a bootstrap on mips-sgi-irix6.5 to make sure.

        Rainer

--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

[Rafael Espindola-2]

> I'm pretty certain that it works on IRIX 6.5, which has atoll in libc,
> but what about platforms that don't?  I think you'll have to provide a
> replacement in libiberty.
>
> I'll fire off a bootstrap on mips-sgi-irix6.5 to make sure.

I this works, I would suggest committing it and then implementing one
is libiberty.

Can it be copied from glibc?

>        Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>

Cheers,
--
Rafael Ávila de Espíndola

[Andrew Pinski-2]

On Mon, Nov 16, 2009 at 8:16 AM, Rafael Espindola <espindola@...> wrote:

>> I'm pretty certain that it works on IRIX 6.5, which has atoll in libc,
>> but what about platforms that don't?  I think you'll have to provide a
>> replacement in libiberty.
>>
>> I'll fire off a bootstrap on mips-sgi-irix6.5 to make sure.
>
> I this works, I would suggest committing it and then implementing one
> is libiberty.
>
> Can it be copied from glibc?

You can copy it from gnulib which is the location where some of the
functions in libiberty are grabbed from
<http://www.gnu.org/software/gnulib/>.

Thanks,
Andrew Pinski