diff mbox

[lto] Fix PR81487

Message ID 8df05c04-e1d9-ae44-be21-2356746f4776@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay July 20, 2017, 1:18 p.m. UTC
Hi, this patch fixes some minor problems in lto-plugin:

Some older mingw32 host environments have broken asprintf.
As far as I can tell, the problem is that the mingw asprintf
implementation calls _vsnprintf (NULL, 0, ...) which always
returns -1 as length on the host.

The patch fixes this by using xasprintf from libiberty with
the additional benefit of easier use and unified error reporting
in the case when there is actually no more memory available, i.e.
it will use the same error massages like xmalloc then.

Moreover, the old implementation calls asprintf as

 > asprintf (&objname, "%s@0x%x%08x", file->name, lo, hi)

for large archives when the high 32 bits (hi) are non-zero.
This looks like a typo: the high part must come first to yiels
a proper 64-bit value.

Bootstrapped & re-tested on x86_64-linux gnu and also on
mingw32 which popped the "ld.exe: asprintf failed".

Ok for trunk?

Johann


lto-plugin/
	PR lto/81487
	* lto-plugin.c (claim_file_handler): Use xasprintf instead of
	asprintf.
	[hi!=0]: Swap hi and lo arguments supplied to xasprintf.

Comments

Richard Biener July 21, 2017, 11:41 a.m. UTC | #1
On Thu, Jul 20, 2017 at 3:18 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> Hi, this patch fixes some minor problems in lto-plugin:
>
> Some older mingw32 host environments have broken asprintf.
> As far as I can tell, the problem is that the mingw asprintf
> implementation calls _vsnprintf (NULL, 0, ...) which always
> returns -1 as length on the host.
>
> The patch fixes this by using xasprintf from libiberty with
> the additional benefit of easier use and unified error reporting
> in the case when there is actually no more memory available, i.e.
> it will use the same error massages like xmalloc then.
>
> Moreover, the old implementation calls asprintf as
>
>> asprintf (&objname, "%s@0x%x%08x", file->name, lo, hi)
>
> for large archives when the high 32 bits (hi) are non-zero.
> This looks like a typo: the high part must come first to yiels
> a proper 64-bit value.
>
> Bootstrapped & re-tested on x86_64-linux gnu and also on
> mingw32 which popped the "ld.exe: asprintf failed".
>
> Ok for trunk?

Ok.

I wonder if any of the other plain asprintf calls in GCC are
problematic.  From a quick look they are all inside code
only exercised when dumping/debugging.  But maybe we
should replace those as well.

Richard.

> Johann
>
>
> lto-plugin/
>         PR lto/81487
>         * lto-plugin.c (claim_file_handler): Use xasprintf instead of
>         asprintf.
>         [hi!=0]: Swap hi and lo arguments supplied to xasprintf.
Georg-Johann Lay July 21, 2017, 3:53 p.m. UTC | #2
On 21.07.2017 13:41, Richard Biener wrote:
> On Thu, Jul 20, 2017 at 3:18 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>> Hi, this patch fixes some minor problems in lto-plugin:
>>
>> Some older mingw32 host environments have broken asprintf.
>> As far as I can tell, the problem is that the mingw asprintf
>> implementation calls _vsnprintf (NULL, 0, ...) which always
>> returns -1 as length on the host.
>>
>> The patch fixes this by using xasprintf from libiberty with
>> the additional benefit of easier use and unified error reporting
>> in the case when there is actually no more memory available, i.e.
>> it will use the same error massages like xmalloc then.
>>
>> Moreover, the old implementation calls asprintf as
>>
>>> asprintf (&objname, "%s@0x%x%08x", file->name, lo, hi)
>>
>> for large archives when the high 32 bits (hi) are non-zero.
>> This looks like a typo: the high part must come first to yiels
>> a proper 64-bit value.
>>
>> Bootstrapped & re-tested on x86_64-linux gnu and also on
>> mingw32 which popped the "ld.exe: asprintf failed".
>>
>> Ok for trunk?
> 
> Ok.
> 
> I wonder if any of the other plain asprintf calls in GCC are
> problematic.  From a quick look they are all inside code
> only exercised when dumping/debugging.  But maybe we
> should replace those as well.
> 
> Richard.


Maybe I just didn't run into such spots yet.  I just started using
a different cross-toolchain for canadian cross builds. With my old
i386-mingw32 (3.4.5) I never saw such problems, but the new v8
(maybe also v7) no more gave sane build result, i.e. the generated
cross-compiler to run under mingw just hangs.  Must be somewhere
around tree dump no. 62, but I couldn't even find out which pass
actually hangs.  So I used 4.9.3 x64 -> mingw cross compiler in
the hope that it works better than good old 3.4.5 which did a very
good job for a really long time.

Johann

>>
>> lto-plugin/
>>          PR lto/81487
>>          * lto-plugin.c (claim_file_handler): Use xasprintf instead of
>>          asprintf.
>>          [hi!=0]: Swap hi and lo arguments supplied to xasprintf.
diff mbox

Patch

Index: lto-plugin/lto-plugin.c
===================================================================
--- lto-plugin/lto-plugin.c	(revision 250302)
+++ lto-plugin/lto-plugin.c	(working copy)
@@ -975,17 +975,16 @@  claim_file_handler (const struct ld_plug
 
   if (file->offset != 0)
     {
-      char *objname;
       /* We pass the offset of the actual file, not the archive header.
          Can't use PRIx64, because that's C99, so we have to print the
-	 64-bit hex int as two 32-bit ones. */
-      int lo, hi, t;
+	 64-bit hex int as two 32-bit ones.  Use xasprintf instead of
+	 asprintf because asprintf doesn't work as expected on some older
+	 mingw32 hosts.  */
+      int lo, hi;
       lo = file->offset & 0xffffffff;
       hi = ((int64_t)file->offset >> 32) & 0xffffffff;
-      t = hi ? asprintf (&objname, "%s@0x%x%08x", file->name, lo, hi)
-	     : asprintf (&objname, "%s@0x%x", file->name, lo);
-      check (t >= 0, LDPL_FATAL, "asprintf failed");
-      lto_file.name = objname;
+      lto_file.name = hi ? xasprintf ("%s@0x%x%08x", file->name, hi, lo)
+      			 : xasprintf ("%s@0x%x", file->name, lo);
     }
   else
     {