From patchwork Fri Mar 28 13:30:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 334694 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3B3E8140082 for ; Sat, 29 Mar 2014 00:32:08 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=baFQpmPu7LN7ptfc P6m32L0hW6K/EHYR0hI4XdZe/9yFEhXQLuFbhnTeOi/0HnFe1dJ8SpA/pH54mWVH KtcYpjzddkpBHrrBGC/zM2e8EZkG3X+0dpU1DBAlSyys3FuQUkL1Ax2Hg47uyLEL ub1heheNQzuoX4ld+vszqmUJBtU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=9tB5Ghs3Sgk+EUcyNRGt5S zJ6Yo=; b=tPfzJ+vKDYsIEJr6XLVjx26aBrSVUaWx3ew4Oah7nSKHt5/lJ5blWM 334oLgX67n9QhxzxJOIV1iGwV+ms3dfSe+6zd4vIQcseeafotmAjj2GN/oy+GSWb JRs5UgXMOE5ruLcREFrnQebBSJQPsaIF+9SrqmfqxlAZ9eS+dAVvE= Received: (qmail 4558 invoked by alias); 28 Mar 2014 13:32:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 4548 invoked by uid 89); 28 Mar 2014 13:32:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 28 Mar 2014 13:31:59 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2CDAFABE4; Fri, 28 Mar 2014 13:31:56 +0000 (UTC) Date: Fri, 28 Mar 2014 14:30:30 +0100 (CET) From: Richard Biener To: Ian Lance Taylor cc: gcc-patches Subject: Re: [PATCH] Handle short reads and EINTR in lto-plugin/simple-object In-Reply-To: Message-ID: References: User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 On Wed, 26 Mar 2014, Richard Biener wrote: > On March 26, 2014 4:51:58 PM CET, Ian Lance Taylor wrote: > >On Wed, Mar 26, 2014 at 8:38 AM, Richard Biener > >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 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 #endif #include +#include #include #include #include @@ -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