On Wed, 26 Mar 2014, Richard Biener wrote:
> On March 26, 2014 4:51:58 PM CET, Ian Lance Taylor <i...@google.com> wrote:
> >On Wed, Mar 26, 2014 at 8:38 AM, Richard Biener <rguent...@suse.de>
> >wrote:
> >>
> >> - got = read (descriptor, buffer, size);
> >> - if (got < 0)
> >> + do
> >> {
> >> - *errmsg = "read";
> >> - *err = errno;
> >> - return 0;
> >> + got = read (descriptor, buffer, size);
> >> + if (got < 0
> >> + && errno != EINTR)
> >> + {
> >> + *errmsg = "read";
> >> + *err = errno;
> >> + return 0;
> >> + }
> >> + else
> >> + {
> >> + buffer += got;
> >> + size -= got;
> >> + }
> >
> >This appears to do the wrong thing if got < 0 && errno == EINTR. In
> >that case it should not add got to buffer and size.
>
> Uh, indeed. Will fix.
>
> >> - if (offset != lseek (obj->file->fd, offset, SEEK_SET)
> >> - || length != read (obj->file->fd, secdata, length))
> >> + if (!simple_object_internal_read (obj->file->fd, offset,
> >> + secdata, length, &errmsg, &err))
> >
> >Hmmm, internal_read is meant to be, well, internal. It's not declared
> >anywhere as far as I can see.
>
> I can duplicate the stuff as well.
>
> >Are you really seeing EINTR reads here? That seems very odd to me,
> >since we are always just reading a local file. But if you are seeing
> >it, I guess we should handle it.
>
> Well, it's a shot in the dark... I definitely know short reads and EINTR
> happens more in virtual machines though. So handling it is an improvement.
>
> I'll see if it fixes my problems and report back.
Ok, updated patch as below. The patch _does_ fix the issues I run into
(well, previously 1 out of 4 compiles succeeded, now 4 out of 4 succeed,
whatever that proves ;))
LTO bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
Thanks,
Richard.
2014-03-26 Richard Biener <rguent...@suse.de>
libiberty/
* simple-object.c (simple_object_internal_read): Handle
EINTR and short reads.
lto-plugin/
* lto-plugin.c (process_symtab): Use simple_object_internal_read.
Index: libiberty/simple-object.c
===================================================================
--- libiberty/simple-object.c (revision 208812)
+++ libiberty/simple-object.c (working copy)
@@ -63,8 +63,6 @@ simple_object_internal_read (int descrip
unsigned char *buffer, size_t size,
const char **errmsg, int *err)
{
- ssize_t got;
-
if (lseek (descriptor, offset, SEEK_SET) < 0)
{
*errmsg = "lseek";
@@ -72,15 +70,26 @@ simple_object_internal_read (int descrip
return 0;
}
- got = read (descriptor, buffer, size);
- if (got < 0)
+ do
{
- *errmsg = "read";
- *err = errno;
- return 0;
+ ssize_t got = read (descriptor, buffer, size);
+ if (got == 0)
+ break;
+ else if (got > 0)
+ {
+ buffer += got;
+ size -= got;
+ }
+ else if (errno != EINTR)
+ {
+ *errmsg = "read";
+ *err = errno;
+ return 0;
+ }
}
+ while (size > 0);
- if ((size_t) got < size)
+ if (size > 0)
{
*errmsg = "file too short";
*err = 0;
Index: lto-plugin/lto-plugin.c
===================================================================
--- lto-plugin/lto-plugin.c (revision 208812)
+++ lto-plugin/lto-plugin.c (working copy)
@@ -39,6 +39,7 @@ along with this program; see the file CO
#include <stdint.h>
#endif
#include <assert.h>
+#include <errno.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
@@ -817,7 +818,7 @@ process_symtab (void *data, const char *
{
struct plugin_objfile *obj = (struct plugin_objfile *)data;
char *s;
- char *secdata;
+ char *secdatastart, *secdata;
if (strncmp (name, LTO_SECTION_PREFIX, LTO_SECTION_PREFIX_LEN) != 0)
return 1;
@@ -825,23 +826,40 @@ process_symtab (void *data, const char *
s = strrchr (name, '.');
if (s)
sscanf (s, ".%" PRI_LL "x", &obj->out->id);
- secdata = xmalloc (length);
+ secdata = secdatastart = xmalloc (length);
offset += obj->file->offset;
- if (offset != lseek (obj->file->fd, offset, SEEK_SET)
- || length != read (obj->file->fd, secdata, length))
+ if (offset != lseek (obj->file->fd, offset, SEEK_SET))
+ goto err;
+
+ do
{
- if (message)
- message (LDPL_FATAL, "%s: corrupt object file", obj->file->name);
- /* Force claim_file_handler to abandon this file. */
- obj->found = 0;
- free (secdata);
- return 0;
+ ssize_t got = read (obj->file->fd, secdata, length);
+ if (got == 0)
+ break;
+ else if (got > 0)
+ {
+ secdata += got;
+ length -= got;
+ }
+ else if (errno != EINTR)
+ goto err;
}
+ while (length > 0);
+ if (length > 0)
+ goto err;
- translate (secdata, secdata + length, obj->out);
+ translate (secdatastart, secdata, obj->out);
obj->found++;
- free (secdata);
+ free (secdatastart);
return 1;
+
+err:
+ if (message)
+ message (LDPL_FATAL, "%s: corrupt object file", obj->file->name);
+ /* Force claim_file_handler to abandon this file. */
+ obj->found = 0;
+ free (secdatastart);
+ return 0;
}
/* Callback used by gold to check if the plugin will claim FILE. Writes
