Re: PING #2 [Patch, driver] PR41594, [Patch, c++] PR41596 -static-libstdc++ issues.

[Richard Guenther <richard dot guenther at gmail dot com>]

On Sat, Apr 3, 2010 at 1:06 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> On Fri, Mar 26, 2010 at 08:00:54PM +0000, IainS wrote:
>> Hi all,
>>
>> These two patches have had some discussion amongst interested parties..
>> ... but they seem to have stalled without conclusion, or a specific
>> yes/no from the maintainers involved.
>>
>> PR41594 appears trivial - get the driver to recognize an option parsed
>> by g++spec.c
>>
>> PR41596 is slightly more involved.
>>
>> Without these changes:
>> (a) it's not possible to run the testsuite "-static-libstdc++" ?which
>> makes a comprehensive test of that option somewhat tricky.
>> (b) PCH is not correctly generated if -static-libstdc++ is given on the
>> CL.
>>
>> cheers,
>> Iain
>>
>> Updated patches:
>>
>
>> Index: gcc/gcc.c
>> ===================================================================
>> --- gcc/gcc.c (revision 157731)
>> +++ gcc/gcc.c (working copy)
>> @@ -4558,12 +4558,14 @@
>> ? ? ? ? switches[n_switches].validated = 0;
>> ? ? ? ? switches[n_switches].ordering = 0;
>> ? ? ? ? /* These are always valid, since gcc.c itself understands the
>> - ? ? ? ? ?first four and gfortranspec.c understands -static-libgfortran. ?*/
>> + ? ? ? ? ?first four, gfortranspec.c understands -static-libgfortran
>> + ? ? ? ? ?and g++spec.c understands -static-libstdc++ */
>> ? ? ? ? if (!strcmp (p, "save-temps")
>> ? ? ? ? ? ? || !strcmp (p, "static-libgcc")
>> ? ? ? ? ? ? || !strcmp (p, "shared-libgcc")
>> ? ? ? ? ? ? || !strcmp (p, "pipe")
>> - ? ? ? ? ? || !strcmp (p, "static-libgfortran"))
>> + ? ? ? ? ? || !strcmp (p, "static-libgfortran")
>> + ? ? ? ? ? || !strcmp (p, "static-libstdc++"))
>> ? ? ? ? ? switches[n_switches].validated = 1;
>> ? ? ? ? else
>> ? ? ? ? ? {
>
>>
>>
>>
>> 2010-03-26 ?Iain Sandoe ?<iain.sandoe@sandoe-acoustics.co.uk>
>>
>> ? ? ? PR driver/41594
>> ? ? ? * gcc/gcc.c: Add -static-libstdc++ to list of recognized options.
>>
>>
>>
>
>> Index: gcc/cp/g++spec.c
>> ===================================================================
>> --- gcc/cp/g++spec.c ?(revision 157761)
>> +++ gcc/cp/g++spec.c ?(working copy)
>> @@ -60,10 +60,12 @@
>> ? ?int saw_profile_flag = 0;
>>
>> ? ?/* What do with libstdc++:
>> - ? ? -1 means we should not link in libstdc++
>> - ? ? 0 ?means we should link in libstdc++ if it is needed
>> - ? ? 1 ?means libstdc++ is needed and should be linked in.
>> - ? ? 2 ?means libstdc++ is needed and should be linked statically. ?*/
>> + ? ? -1 means a forced "don't link" libstdc++,
>> + ? ? 0 ?means we don't need libstdc++,
>> + ? ? bit 0 = 1 means libstdc++ is needed and should be linked in.
>> + ? ? bit 1 = 1 means when libstdc++ is needed it should be linked statically.
>> + ? ? during re-writing the args the -1 case is set to 0.
>> + ? ? */
>> ? ?int library = 0;
>>
>> ? ?/* The number of arguments being added to what's in argv, other than
>> @@ -158,21 +160,21 @@
>> ? ? ? ? ? ? else if ((argv[i+1]) != NULL)
>> ? ? ? ? ? ? ? /* We need to swallow arg on next loop. ?*/
>> ? ? ? ? ? ? ? quote = arg = argv[i+1];
>> - ? ? ? ? ? else ?/* Error condition, message will be printed later. ?*/
>> + ? ? ? ? ? else ?/* Error condition, message will be printed later. ?*/
>> ? ? ? ? ? ? ? arg = "";
>> - ? ? ? ? ? if (library == 0
>> + ? ? ? ? ? if (library >= 0
>> ? ? ? ? ? ? ? ? && (strcmp (arg, "c++") == 0
>> ? ? ? ? ? ? ? ? ? ? || strcmp (arg, "c++-cpp-output") == 0
>> ? ? ? ? ? ? ? ? ? ? || strcmp (arg, "objective-c++") == 0
>> ? ? ? ? ? ? ? ? ? ? || strcmp (arg, "objective-c++-cpp-output") == 0))
>> - ? ? ? ? ? ? library = 1;
>> + ? ? ? ? ? ? library |= 1;
>>
>> ? ? ? ? ? ? saw_speclang = 1;
>> ? ? ? ? ? }
>> ? ? ? ? else if (strcmp (argv[i], "-ObjC++") == 0)
>> ? ? ? ? ? {
>> - ? ? ? ? ? if (library == 0)
>> - ? ? ? ? ? ? library = 1;
>> + ? ? ? ? ? if (library >= 0)
>> + ? ? ? ? ? ? library |= 1;
>> ? ? ? ? ? ? saw_speclang = 1;
>> ? ? ? ? ? }
>> ? ? ? ? /* Arguments that go directly to the linker might be .o files,
>> @@ -180,14 +182,14 @@
>> ? ? ? ? else if (strcmp (argv[i], "-Xlinker") == 0)
>> ? ? ? ? ? {
>> ? ? ? ? ? ? quote = argv[i];
>> - ? ? ? ? ? if (library == 0)
>> - ? ? ? ? ? ? library = 1;
>> + ? ? ? ? ? if (library >= 0)
>> + ? ? ? ? ? ? library |= 1;
>> ? ? ? ? ? }
>> ? ? ? ? else if (strncmp (argv[i], "-Wl,", 4) == 0)
>> - ? ? ? ? library = (library == 0) ? 1 : library;
>> + ? ? ? ? library |= (library >= 0) ? 1 : library;
>> ? ? ? ? /* Unrecognized libraries (e.g. -lfoo) may require libstdc++. ?*/
>> ? ? ? ? else if (strncmp (argv[i], "-l", 2) == 0)
>> - ? ? ? ? library = (library == 0) ? 1 : library;
>> + ? ? ? ? library |= (library >= 0) ? 1 : library;
>> ? ? ? ? else if (((argv[i][2] == '\0'
>> ? ? ? ? ? ? ? ? ? ?&& strchr ("bBVDUoeTuIYmLiA", argv[i][1]) != NULL)
>> ? ? ? ? ? ? ? ? ? || strcmp (argv[i], "-Tdata") == 0))
>> @@ -207,7 +209,7 @@
>> ? ? ? ? ? shared_libgcc = 0;
>> ? ? ? ? else if (strcmp (argv[i], "-static-libstdc++") == 0)
>> ? ? ? ? ? {
>> - ? ? ? ? ? library = library >= 0 ? 2 : library;
>> + ? ? ? ? ? library |= library >= 0 ? 2 : library;
>> ? ? ? ? ? ? args[i] |= SKIPOPT;
>> ? ? ? ? ? }
>> ? ? ? ? else if (DEFAULT_WORD_SWITCH_TAKES_ARG (&argv[i][1]))
>> @@ -241,7 +243,7 @@
>>
>> ? ? ? ? /* If we don't know that this is a header file, we might
>> ? ? ? ? ? ?need to be linking in the libraries. ?*/
>> - ? ? ? if (library == 0)
>> + ? ? ? if (library >= 0 && (library & 1) == 0)
>> ? ? ? ? ? {
>> ? ? ? ? ? ? if ((len <= 2 || strcmp (argv[i] + (len - 2), ".H") != 0)
>> ? ? ? ? ? ? ? ? && (len <= 2 || strcmp (argv[i] + (len - 2), ".h") != 0)
>> @@ -252,7 +254,7 @@
>> ? ? ? ? ? ? ? ? && (len <= 4 || strcmp (argv[i] + (len - 4), ".HPP") != 0)
>> ? ? ? ? ? ? ? ? && (len <= 4 || strcmp (argv[i] + (len - 4), ".tcc") != 0)
>> ? ? ? ? ? ? ? ? && (len <= 3 || strcmp (argv[i] + (len - 3), ".hh") != 0))
>> - ? ? ? ? ? ? library = 1;
>> + ? ? ? ? ? ? library |= 1;
>> ? ? ? ? ? }
>> ? ? ? }
>> ? ? ?}
>> @@ -260,6 +262,10 @@
>> ? ?if (quote)
>> ? ? ?fatal ("argument to '%s' missing\n", quote);
>>
>> + ?/* if library < 0 it means it was forced off, so don't include it */
>> + ?if (library < 0)
>> + ? ?library = 0;
>> +
>> ? ?/* There's no point adding -shared-libgcc if we don't have a shared
>> ? ? ? libgcc. ?*/
>> ?#ifndef ENABLE_SHARED_LIBGCC
>> @@ -284,13 +290,13 @@
>>
>> ? ? ? ?/* Make sure -lstdc++ is before the math library, since libstdc++
>> ? ? ? ?itself uses those math routines. ?*/
>> - ? ? ?if (!saw_math && (args[i] & MATHLIB) && library > 0)
>> + ? ? ?if (!saw_math && (args[i] & MATHLIB) && (library & 1) != 0)
>> ? ? ? {
>> ? ? ? ? --j;
>> ? ? ? ? saw_math = argv[i];
>> ? ? ? }
>>
>> - ? ? ?if (!saw_libc && (args[i] & WITHLIBC) && library > 0)
>> + ? ? ?if (!saw_libc && (args[i] & WITHLIBC) && (library & 1) != 0)
>> ? ? ? {
>> ? ? ? ? --j;
>> ? ? ? ? saw_libc = argv[i];
>> @@ -327,21 +333,23 @@
>> ? ? ?}
>>
>> ? ?/* Add `-lstdc++' if we haven't already done so. ?*/
>> - ?if (library > 0)
>> + ?if ((library & 1) != 0)
>> ? ? ?{
>> + ? ? ?if ((library & 2) && !static_link)
>> + ? ? {
>> + ? ? ? arglist[j] = "-static-libstdc++"; /* allow for link spec substitution */
>> + ? ? ? j++;
>> ?#ifdef HAVE_LD_STATIC_DYNAMIC
>> - ? ? ?if (library > 1 && !static_link)
>> - ? ? {
>> ? ? ? ? arglist[j] = "-Wl,-Bstatic";
>> ? ? ? ? j++;
>> +#endif
>> ? ? ? }
>> -#endif
>> ? ? ? ?arglist[j] = saw_profile_flag ? LIBSTDCXX_PROFILE : LIBSTDCXX;
>> ? ? ? ?if (arglist[j][0] != '-' || arglist[j][1] == 'l')
>> ? ? ? added_libraries++;
>> ? ? ? ?j++;
>> ? ? ? ?/* Add target-dependent static library, if necessary. ?*/
>> - ? ? ?if ((static_link || library > 1) && LIBSTDCXX_STATIC != NULL)
>> + ? ? ?if ((static_link || (library & 2)) && LIBSTDCXX_STATIC != NULL)
>> ? ? ? {
>> ? ? ? ? arglist[j] = LIBSTDCXX_STATIC;
>> ? ? ? ? if (arglist[j][0] != '-' || arglist[j][1] == 'l')
>> @@ -349,7 +357,7 @@
>> ? ? ? ? j++;
>> ? ? ? }
>> ?#ifdef HAVE_LD_STATIC_DYNAMIC
>> - ? ? ?if (library > 1 && !static_link)
>> + ? ? ?if ((library & 2) && !static_link)
>> ? ? ? {
>> ? ? ? ? arglist[j] = "-Wl,-Bdynamic";
>> ? ? ? ? j++;
>> @@ -358,7 +366,7 @@
>> ? ? ?}
>> ? ?if (saw_math)
>> ? ? ?arglist[j++] = saw_math;
>> - ?else if (library > 0 && need_math)
>> + ?else if ((library & 1) && need_math)
>> ? ? ?{
>> ? ? ? ?arglist[j] = saw_profile_flag ? MATH_LIBRARY_PROFILE : MATH_LIBRARY;
>> ? ? ? ?if (arglist[j][0] != '-' || arglist[j][1] == 'l')
>
>>
>>
>>
>> 2010-03-26 ?Iain Sandoe ?<iain.sandoe@sandoe-acoustics.co.uk>
>>
>> ? ? ? PR c++/41596
>> ? ? ? *gcc/cp/g++spec.c(lang_specific_driver): ?Do not insert -lstdc++ unless
>> we
>> ? ? ? really need it. ?Insert 'static-libstdc++' for platforms without
>> Bstatic/Bdynamic to
>> ? ? ? allow link spec substitution.
>>
>
>
> Richard,
> ? This patch...
>
> http://gcc.gnu.org/ml/gcc-patches/2010-03/msg01268.html
>
> ?was already approved...
>
> http://gcc.gnu.org/ml/gcc-patches/2010-03/msg01269.html
>
> Can we get it committed for gcc 4.5?

It doesn't look like it fixes a regression, nor is it a complete solution
for the issues mentioned (or even remotely looks important to me).

Please leave it for 4.6 and eventually  4.5.1.

Thanks,
Richard.

> ? ? ? ? ? ? ?Jack
>