darwin, d: Support outfile substitution for liphobos

Message ID 20211119083226.3095760-1-ibuclaw@gdcproject.org
State Committed
Headers
Series darwin, d: Support outfile substitution for liphobos |

Commit Message

Iain Buclaw Nov. 19, 2021, 8:32 a.m. UTC
  Hi,

This patch fixes a stage2 bootstrap failure in the D front-end on
darwin due to libgphobos being dynamically linked despite
-static-libphobos being on the command line.

In the gdc driver, this takes the previous fix for the Darwin D
bootstrap, and extends it to the -static-libphobos option as well.
Rather than pushing the -static-libphobos option back onto the command
line, the setting of SKIPOPT is instead conditionally removed.  The same
change has been repeated for -static-libstdc++ so there is now no need
to call generate_option to re-add it.

In the gcc driver, -static-libphobos has been added as a common option,
validated, and a new outfile substition added to config/darwin.h to
correctly replace -lgphobos with libgphobos.a.

Bootstrapped and regression tested on x86_64-linux-gnu and
x86_64-apple-darwin20.

OK for mainline?  This would also be fine for gcc-11 release branch too,
as well as earlier releases with D support.

Regards,
Iain.

---
gcc/ChangeLog:

	* common.opt (static-libphobos): Add option.
	* config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
	when linking statically.
	* gcc.c (driver_handle_option): Set -static-libphobos as always valid.

gcc/d/ChangeLog:

	* d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
	and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
	Remove generate_option to re-add -static-libstdc++.
---
 gcc/common.opt      |  4 ++++
 gcc/config/darwin.h |  1 +
 gcc/d/d-spec.cc     | 18 +++++++++++-------
 gcc/gcc.c           |  6 ++++--
 4 files changed, 20 insertions(+), 9 deletions(-)
  

Comments

Iain Sandoe Nov. 19, 2021, 9:21 a.m. UTC | #1
Hi Iain

> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:

> This patch fixes a stage2 bootstrap failure in the D front-end on
> darwin due to libgphobos being dynamically linked despite
> -static-libphobos being on the command line.
> 
> In the gdc driver, this takes the previous fix for the Darwin D
> bootstrap, and extends it to the -static-libphobos option as well.
> Rather than pushing the -static-libphobos option back onto the command
> line, the setting of SKIPOPT is instead conditionally removed.  The same
> change has been repeated for -static-libstdc++ so there is now no need
> to call generate_option to re-add it.
> 
> In the gcc driver, -static-libphobos has been added as a common option,
> validated, and a new outfile substition added to config/darwin.h to
> correctly replace -lgphobos with libgphobos.a.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu and
> x86_64-apple-darwin20.
> 
> OK for mainline?  This would also be fine for gcc-11 release branch too,
> as well as earlier releases with D support.

the Darwin parts are fine, thanks 

The SKIPOPT in d-spec, presumably means “skip removing this opt”?
otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
test).

Iain

> 
> Regards,
> Iain.
> 
> ---
> gcc/ChangeLog:
> 
> 	* common.opt (static-libphobos): Add option.
> 	* config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
> 	when linking statically.
> 	* gcc.c (driver_handle_option): Set -static-libphobos as always valid.
> 
> gcc/d/ChangeLog:
> 
> 	* d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
> 	and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
> 	Remove generate_option to re-add -static-libstdc++.
> ---
> gcc/common.opt      |  4 ++++
> gcc/config/darwin.h |  1 +
> gcc/d/d-spec.cc     | 18 +++++++++++-------
> gcc/gcc.c           |  6 ++++--
> 4 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index db6010e4e20..73c12d933f3 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3527,6 +3527,10 @@ static-libgfortran
> Driver
> ; Documented for Fortran, but always accepted by driver.
> 
> +static-libphobos
> +Driver
> +; Documented for D, but always accepted by driver.
> +
> static-libstdc++
> Driver
> 
> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
> index 7ed01efa694..c4ddd623e8b 100644
> --- a/gcc/config/darwin.h
> +++ b/gcc/config/darwin.h
> @@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
>                      %:replace-outfile(-lobjc libobjc-gnu.a%s); \
>                     :%:replace-outfile(-lobjc -lobjc-gnu )}}\
>    %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
> +   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
>    %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
>    %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
>    %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
> diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
> index b12d28f1047..73ecac3bbf1 100644
> --- a/gcc/d/d-spec.cc
> +++ b/gcc/d/d-spec.cc
> @@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> 
> 	case OPT_static_libstdc__:
> 	  saw_static_libcxx = true;
> +#ifndef HAVE_LD_STATIC_DYNAMIC
> +	  /* Remove -static-libstdc++ from the command only if target supports
> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> +	     back-end target can use outfile substitution.  */
> 	  args[i] |= SKIPOPT;
> +#endif
> 	  break;
> 
> 	case OPT_static_libphobos:
> 	  if (phobos_library != PHOBOS_NOLINK)
> 	    phobos_library = PHOBOS_STATIC;
> +#ifndef HAVE_LD_STATIC_DYNAMIC
> +	  /* Remove -static-libphobos from the command only if target supports
> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> +	     back-end target can use outfile substitution.  */
> 	  args[i] |= SKIPOPT;
> +#endif
> 	  break;
> 
> 	case OPT_shared_libphobos:
> @@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> #endif
>     }
> 
> -  if (saw_libcxx || need_stdcxx)
> +  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
>     {
> #ifdef HAVE_LD_STATIC_DYNAMIC
>       if (saw_static_libcxx && !static_link)
> @@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> 	  generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
> 			   &new_decoded_options[j++]);
> 	}
> -#else
> -      /* Push the -static-libstdc++ option back onto the command so that
> -	 a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
> -      if (saw_static_libcxx && !static_link)
> -	generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
> -			 &new_decoded_options[j++]);
> #endif
>       if (saw_libcxx)
> 	new_decoded_options[j++] = *saw_libcxx;
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 506c2acc282..fea6d049183 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
>     case OPT_static_libgcc:
>     case OPT_shared_libgcc:
>     case OPT_static_libgfortran:
> +    case OPT_static_libphobos:
>     case OPT_static_libstdc__:
>       /* These are always valid, since gcc.c itself understands the
> -	 first two, gfortranspec.c understands -static-libgfortran and
> -	 g++spec.c understands -static-libstdc++ */
> +	 first two, gfortranspec.c understands -static-libgfortran,
> +	 d-spec.cc understands -static-libphobos, and g++spec.c
> +	 understands -static-libstdc++ */
>       validated = true;
>       break;
> 
> -- 
> 2.30.2
>
  
Iain Buclaw Nov. 19, 2021, 9:57 a.m. UTC | #2
Excerpts from Iain Sandoe's message of November 19, 2021 10:21 am:
> Hi Iain
> 
>> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
> 
>> This patch fixes a stage2 bootstrap failure in the D front-end on
>> darwin due to libgphobos being dynamically linked despite
>> -static-libphobos being on the command line.
>> 
>> In the gdc driver, this takes the previous fix for the Darwin D
>> bootstrap, and extends it to the -static-libphobos option as well.
>> Rather than pushing the -static-libphobos option back onto the command
>> line, the setting of SKIPOPT is instead conditionally removed.  The same
>> change has been repeated for -static-libstdc++ so there is now no need
>> to call generate_option to re-add it.
>> 
>> In the gcc driver, -static-libphobos has been added as a common option,
>> validated, and a new outfile substition added to config/darwin.h to
>> correctly replace -lgphobos with libgphobos.a.
>> 
>> Bootstrapped and regression tested on x86_64-linux-gnu and
>> x86_64-apple-darwin20.
>> 
>> OK for mainline?  This would also be fine for gcc-11 release branch too,
>> as well as earlier releases with D support.
> 
> the Darwin parts are fine, thanks 
> 
> The SKIPOPT in d-spec, presumably means “skip removing this opt”?
> otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
> darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
> test).
> 

The inverse. SKIPOPT means "skip this option", so previously it was
being removed by the driver when constructing the new_decoded_options,
hence why your generate_option addition was necessary before.

Iain.

> Iain
> 
>> 
>> Regards,
>> Iain.
>> 
>> ---
>> gcc/ChangeLog:
>> 
>> 	* common.opt (static-libphobos): Add option.
>> 	* config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
>> 	when linking statically.
>> 	* gcc.c (driver_handle_option): Set -static-libphobos as always valid.
>> 
>> gcc/d/ChangeLog:
>> 
>> 	* d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
>> 	and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
>> 	Remove generate_option to re-add -static-libstdc++.
>> ---
>> gcc/common.opt      |  4 ++++
>> gcc/config/darwin.h |  1 +
>> gcc/d/d-spec.cc     | 18 +++++++++++-------
>> gcc/gcc.c           |  6 ++++--
>> 4 files changed, 20 insertions(+), 9 deletions(-)
>> 
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index db6010e4e20..73c12d933f3 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -3527,6 +3527,10 @@ static-libgfortran
>> Driver
>> ; Documented for Fortran, but always accepted by driver.
>> 
>> +static-libphobos
>> +Driver
>> +; Documented for D, but always accepted by driver.
>> +
>> static-libstdc++
>> Driver
>> 
>> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
>> index 7ed01efa694..c4ddd623e8b 100644
>> --- a/gcc/config/darwin.h
>> +++ b/gcc/config/darwin.h
>> @@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
>>                      %:replace-outfile(-lobjc libobjc-gnu.a%s); \
>>                     :%:replace-outfile(-lobjc -lobjc-gnu )}}\
>>    %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
>> +   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
>>    %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
>>    %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
>>    %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
>> diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
>> index b12d28f1047..73ecac3bbf1 100644
>> --- a/gcc/d/d-spec.cc
>> +++ b/gcc/d/d-spec.cc
>> @@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>> 
>> 	case OPT_static_libstdc__:
>> 	  saw_static_libcxx = true;
>> +#ifndef HAVE_LD_STATIC_DYNAMIC
>> +	  /* Remove -static-libstdc++ from the command only if target supports
>> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
>> +	     back-end target can use outfile substitution.  */
>> 	  args[i] |= SKIPOPT;
>> +#endif
>> 	  break;
>> 
>> 	case OPT_static_libphobos:
>> 	  if (phobos_library != PHOBOS_NOLINK)
>> 	    phobos_library = PHOBOS_STATIC;
>> +#ifndef HAVE_LD_STATIC_DYNAMIC
>> +	  /* Remove -static-libphobos from the command only if target supports
>> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
>> +	     back-end target can use outfile substitution.  */
>> 	  args[i] |= SKIPOPT;
>> +#endif
>> 	  break;
>> 
>> 	case OPT_shared_libphobos:
>> @@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>> #endif
>>     }
>> 
>> -  if (saw_libcxx || need_stdcxx)
>> +  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
>>     {
>> #ifdef HAVE_LD_STATIC_DYNAMIC
>>       if (saw_static_libcxx && !static_link)
>> @@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>> 	  generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
>> 			   &new_decoded_options[j++]);
>> 	}
>> -#else
>> -      /* Push the -static-libstdc++ option back onto the command so that
>> -	 a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
>> -      if (saw_static_libcxx && !static_link)
>> -	generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
>> -			 &new_decoded_options[j++]);
>> #endif
>>       if (saw_libcxx)
>> 	new_decoded_options[j++] = *saw_libcxx;
>> diff --git a/gcc/gcc.c b/gcc/gcc.c
>> index 506c2acc282..fea6d049183 100644
>> --- a/gcc/gcc.c
>> +++ b/gcc/gcc.c
>> @@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
>>     case OPT_static_libgcc:
>>     case OPT_shared_libgcc:
>>     case OPT_static_libgfortran:
>> +    case OPT_static_libphobos:
>>     case OPT_static_libstdc__:
>>       /* These are always valid, since gcc.c itself understands the
>> -	 first two, gfortranspec.c understands -static-libgfortran and
>> -	 g++spec.c understands -static-libstdc++ */
>> +	 first two, gfortranspec.c understands -static-libgfortran,
>> +	 d-spec.cc understands -static-libphobos, and g++spec.c
>> +	 understands -static-libstdc++ */
>>       validated = true;
>>       break;
>> 
>> -- 
>> 2.30.2
>> 
> 
>
  
Iain Buclaw Nov. 26, 2021, 12:51 p.m. UTC | #3
Excerpts from Iain Sandoe's message of November 19, 2021 10:21 am:
> Hi Iain
> 
>> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
> 
>> This patch fixes a stage2 bootstrap failure in the D front-end on
>> darwin due to libgphobos being dynamically linked despite
>> -static-libphobos being on the command line.
>> 
>> In the gdc driver, this takes the previous fix for the Darwin D
>> bootstrap, and extends it to the -static-libphobos option as well.
>> Rather than pushing the -static-libphobos option back onto the command
>> line, the setting of SKIPOPT is instead conditionally removed.  The same
>> change has been repeated for -static-libstdc++ so there is now no need
>> to call generate_option to re-add it.
>> 
>> In the gcc driver, -static-libphobos has been added as a common option,
>> validated, and a new outfile substition added to config/darwin.h to
>> correctly replace -lgphobos with libgphobos.a.
>> 
>> Bootstrapped and regression tested on x86_64-linux-gnu and
>> x86_64-apple-darwin20.
>> 
>> OK for mainline?  This would also be fine for gcc-11 release branch too,
>> as well as earlier releases with D support.
> 
> the Darwin parts are fine, thanks 
> 
> The SKIPOPT in d-spec, presumably means “skip removing this opt”?
> otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
> darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
> test).
> 

I've only just realised what you meant.  Yes you are of course right,
and it should have been #ifdef, attaching a fixed-up patch.

Iain.

---
    gcc/ChangeLog:
    
            * common.opt (static-libphobos): Add option.
            * config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
            when linking statically.
            * gcc.c (driver_handle_option): Set -static-libphobos as always valid.
    
    gcc/d/ChangeLog:
    
            * d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
            and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
            Remove generate_option to re-add -static-libstdc++.
    
    libphobos/ChangeLog:
    
            * testsuite/testsuite_flags.in: Add libphobos library directory as
            search path to --gdcldflags.

diff --git a/gcc/common.opt b/gcc/common.opt
index db6010e4e20..73c12d933f3 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3527,6 +3527,10 @@ static-libgfortran
 Driver
 ; Documented for Fortran, but always accepted by driver.
 
+static-libphobos
+Driver
+; Documented for D, but always accepted by driver.
+
 static-libstdc++
 Driver
 
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 7ed01efa694..c4ddd623e8b 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
                      %:replace-outfile(-lobjc libobjc-gnu.a%s); \
                     :%:replace-outfile(-lobjc -lobjc-gnu )}}\
    %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
+   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
    %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
    %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
    %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
index b12d28f1047..1304126a675 100644
--- a/gcc/d/d-spec.cc
+++ b/gcc/d/d-spec.cc
@@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
 
 	case OPT_static_libstdc__:
 	  saw_static_libcxx = true;
+#ifdef HAVE_LD_STATIC_DYNAMIC
+	  /* Remove -static-libstdc++ from the command only if target supports
+	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
+	     back-end target can use outfile substitution.  */
 	  args[i] |= SKIPOPT;
+#endif
 	  break;
 
 	case OPT_static_libphobos:
 	  if (phobos_library != PHOBOS_NOLINK)
 	    phobos_library = PHOBOS_STATIC;
+#ifdef HAVE_LD_STATIC_DYNAMIC
+	  /* Remove -static-libphobos from the command only if target supports
+	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
+	     back-end target can use outfile substitution.  */
 	  args[i] |= SKIPOPT;
+#endif
 	  break;
 
 	case OPT_shared_libphobos:
@@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
 #endif
     }
 
-  if (saw_libcxx || need_stdcxx)
+  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
     {
 #ifdef HAVE_LD_STATIC_DYNAMIC
       if (saw_static_libcxx && !static_link)
@@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
 	  generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
 			   &new_decoded_options[j++]);
 	}
-#else
-      /* Push the -static-libstdc++ option back onto the command so that
-	 a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
-      if (saw_static_libcxx && !static_link)
-	generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
-			 &new_decoded_options[j++]);
 #endif
       if (saw_libcxx)
 	new_decoded_options[j++] = *saw_libcxx;
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 506c2acc282..fea6d049183 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
     case OPT_static_libgcc:
     case OPT_shared_libgcc:
     case OPT_static_libgfortran:
+    case OPT_static_libphobos:
     case OPT_static_libstdc__:
       /* These are always valid, since gcc.c itself understands the
-	 first two, gfortranspec.c understands -static-libgfortran and
-	 g++spec.c understands -static-libstdc++ */
+	 first two, gfortranspec.c understands -static-libgfortran,
+	 d-spec.cc understands -static-libphobos, and g++spec.c
+	 understands -static-libstdc++ */
       validated = true;
       break;
 
diff --git a/libphobos/testsuite/testsuite_flags.in b/libphobos/testsuite/testsuite_flags.in
index bafd5ad4502..8e2f1eefd5b 100755
--- a/libphobos/testsuite/testsuite_flags.in
+++ b/libphobos/testsuite/testsuite_flags.in
@@ -46,6 +46,7 @@ case ${query} in
     --gdcldflags)
       GDCLDFLAGS="-B${BUILD_DIR}/src
                   -B${BUILD_DIR}/libdruntime/gcc
+                  -B${BUILD_DIR}/src/.libs
                   -L${BUILD_DIR}/src/.libs"
       echo ${GDCLDFLAGS}
       ;;
  
Iain Buclaw Nov. 30, 2021, 5:03 p.m. UTC | #4
Ping.

Are the common gcc parts OK (also for backporting)?

Iain.

Excerpts from Iain Buclaw's message of November 26, 2021 1:51 pm:
> Excerpts from Iain Sandoe's message of November 19, 2021 10:21 am:
>> Hi Iain
>> 
>>> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
>> 
>>> This patch fixes a stage2 bootstrap failure in the D front-end on
>>> darwin due to libgphobos being dynamically linked despite
>>> -static-libphobos being on the command line.
>>> 
>>> In the gdc driver, this takes the previous fix for the Darwin D
>>> bootstrap, and extends it to the -static-libphobos option as well.
>>> Rather than pushing the -static-libphobos option back onto the command
>>> line, the setting of SKIPOPT is instead conditionally removed.  The same
>>> change has been repeated for -static-libstdc++ so there is now no need
>>> to call generate_option to re-add it.
>>> 
>>> In the gcc driver, -static-libphobos has been added as a common option,
>>> validated, and a new outfile substition added to config/darwin.h to
>>> correctly replace -lgphobos with libgphobos.a.
>>> 
>>> Bootstrapped and regression tested on x86_64-linux-gnu and
>>> x86_64-apple-darwin20.
>>> 
>>> OK for mainline?  This would also be fine for gcc-11 release branch too,
>>> as well as earlier releases with D support.
>> 
>> the Darwin parts are fine, thanks 
>> 
>> The SKIPOPT in d-spec, presumably means “skip removing this opt”?
>> otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
>> darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
>> test).
>> 
> 
> I've only just realised what you meant.  Yes you are of course right,
> and it should have been #ifdef, attaching a fixed-up patch.
> 
> Iain.
> 
> ---
>     gcc/ChangeLog:
>     
>             * common.opt (static-libphobos): Add option.
>             * config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
>             when linking statically.
>             * gcc.c (driver_handle_option): Set -static-libphobos as always valid.
>     
>     gcc/d/ChangeLog:
>     
>             * d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
>             and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
>             Remove generate_option to re-add -static-libstdc++.
>     
>     libphobos/ChangeLog:
>     
>             * testsuite/testsuite_flags.in: Add libphobos library directory as
>             search path to --gdcldflags.
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index db6010e4e20..73c12d933f3 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3527,6 +3527,10 @@ static-libgfortran
>  Driver
>  ; Documented for Fortran, but always accepted by driver.
>  
> +static-libphobos
> +Driver
> +; Documented for D, but always accepted by driver.
> +
>  static-libstdc++
>  Driver
>  
> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
> index 7ed01efa694..c4ddd623e8b 100644
> --- a/gcc/config/darwin.h
> +++ b/gcc/config/darwin.h
> @@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
>                       %:replace-outfile(-lobjc libobjc-gnu.a%s); \
>                      :%:replace-outfile(-lobjc -lobjc-gnu )}}\
>     %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
> +   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
>     %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
>     %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
>     %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
> diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
> index b12d28f1047..1304126a675 100644
> --- a/gcc/d/d-spec.cc
> +++ b/gcc/d/d-spec.cc
> @@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>  
>  	case OPT_static_libstdc__:
>  	  saw_static_libcxx = true;
> +#ifdef HAVE_LD_STATIC_DYNAMIC
> +	  /* Remove -static-libstdc++ from the command only if target supports
> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> +	     back-end target can use outfile substitution.  */
>  	  args[i] |= SKIPOPT;
> +#endif
>  	  break;
>  
>  	case OPT_static_libphobos:
>  	  if (phobos_library != PHOBOS_NOLINK)
>  	    phobos_library = PHOBOS_STATIC;
> +#ifdef HAVE_LD_STATIC_DYNAMIC
> +	  /* Remove -static-libphobos from the command only if target supports
> +	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> +	     back-end target can use outfile substitution.  */
>  	  args[i] |= SKIPOPT;
> +#endif
>  	  break;
>  
>  	case OPT_shared_libphobos:
> @@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>  #endif
>      }
>  
> -  if (saw_libcxx || need_stdcxx)
> +  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
>      {
>  #ifdef HAVE_LD_STATIC_DYNAMIC
>        if (saw_static_libcxx && !static_link)
> @@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>  	  generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
>  			   &new_decoded_options[j++]);
>  	}
> -#else
> -      /* Push the -static-libstdc++ option back onto the command so that
> -	 a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
> -      if (saw_static_libcxx && !static_link)
> -	generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
> -			 &new_decoded_options[j++]);
>  #endif
>        if (saw_libcxx)
>  	new_decoded_options[j++] = *saw_libcxx;
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 506c2acc282..fea6d049183 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
>      case OPT_static_libgcc:
>      case OPT_shared_libgcc:
>      case OPT_static_libgfortran:
> +    case OPT_static_libphobos:
>      case OPT_static_libstdc__:
>        /* These are always valid, since gcc.c itself understands the
> -	 first two, gfortranspec.c understands -static-libgfortran and
> -	 g++spec.c understands -static-libstdc++ */
> +	 first two, gfortranspec.c understands -static-libgfortran,
> +	 d-spec.cc understands -static-libphobos, and g++spec.c
> +	 understands -static-libstdc++ */
>        validated = true;
>        break;
>  
> diff --git a/libphobos/testsuite/testsuite_flags.in b/libphobos/testsuite/testsuite_flags.in
> index bafd5ad4502..8e2f1eefd5b 100755
> --- a/libphobos/testsuite/testsuite_flags.in
> +++ b/libphobos/testsuite/testsuite_flags.in
> @@ -46,6 +46,7 @@ case ${query} in
>      --gdcldflags)
>        GDCLDFLAGS="-B${BUILD_DIR}/src
>                    -B${BUILD_DIR}/libdruntime/gcc
> +                  -B${BUILD_DIR}/src/.libs
>                    -L${BUILD_DIR}/src/.libs"
>        echo ${GDCLDFLAGS}
>        ;;
>
  
Richard Biener Dec. 1, 2021, 2:12 p.m. UTC | #5
On Tue, Nov 30, 2021 at 6:04 PM Iain Buclaw via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Ping.
>
> Are the common gcc parts OK (also for backporting)?

Yes, OK for backporting as well after a short burn in period.

Richard,

> Iain.
>
> Excerpts from Iain Buclaw's message of November 26, 2021 1:51 pm:
> > Excerpts from Iain Sandoe's message of November 19, 2021 10:21 am:
> >> Hi Iain
> >>
> >>> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
> >>
> >>> This patch fixes a stage2 bootstrap failure in the D front-end on
> >>> darwin due to libgphobos being dynamically linked despite
> >>> -static-libphobos being on the command line.
> >>>
> >>> In the gdc driver, this takes the previous fix for the Darwin D
> >>> bootstrap, and extends it to the -static-libphobos option as well.
> >>> Rather than pushing the -static-libphobos option back onto the command
> >>> line, the setting of SKIPOPT is instead conditionally removed.  The same
> >>> change has been repeated for -static-libstdc++ so there is now no need
> >>> to call generate_option to re-add it.
> >>>
> >>> In the gcc driver, -static-libphobos has been added as a common option,
> >>> validated, and a new outfile substition added to config/darwin.h to
> >>> correctly replace -lgphobos with libgphobos.a.
> >>>
> >>> Bootstrapped and regression tested on x86_64-linux-gnu and
> >>> x86_64-apple-darwin20.
> >>>
> >>> OK for mainline?  This would also be fine for gcc-11 release branch too,
> >>> as well as earlier releases with D support.
> >>
> >> the Darwin parts are fine, thanks
> >>
> >> The SKIPOPT in d-spec, presumably means “skip removing this opt”?
> >> otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
> >> darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
> >> test).
> >>
> >
> > I've only just realised what you meant.  Yes you are of course right,
> > and it should have been #ifdef, attaching a fixed-up patch.
> >
> > Iain.
> >
> > ---
> >     gcc/ChangeLog:
> >
> >             * common.opt (static-libphobos): Add option.
> >             * config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
> >             when linking statically.
> >             * gcc.c (driver_handle_option): Set -static-libphobos as always valid.
> >
> >     gcc/d/ChangeLog:
> >
> >             * d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
> >             and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
> >             Remove generate_option to re-add -static-libstdc++.
> >
> >     libphobos/ChangeLog:
> >
> >             * testsuite/testsuite_flags.in: Add libphobos library directory as
> >             search path to --gdcldflags.
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index db6010e4e20..73c12d933f3 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -3527,6 +3527,10 @@ static-libgfortran
> >  Driver
> >  ; Documented for Fortran, but always accepted by driver.
> >
> > +static-libphobos
> > +Driver
> > +; Documented for D, but always accepted by driver.
> > +
> >  static-libstdc++
> >  Driver
> >
> > diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
> > index 7ed01efa694..c4ddd623e8b 100644
> > --- a/gcc/config/darwin.h
> > +++ b/gcc/config/darwin.h
> > @@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
> >                       %:replace-outfile(-lobjc libobjc-gnu.a%s); \
> >                      :%:replace-outfile(-lobjc -lobjc-gnu )}}\
> >     %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
> > +   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
> >     %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
> >     %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
> >     %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
> > diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
> > index b12d28f1047..1304126a675 100644
> > --- a/gcc/d/d-spec.cc
> > +++ b/gcc/d/d-spec.cc
> > @@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> >
> >       case OPT_static_libstdc__:
> >         saw_static_libcxx = true;
> > +#ifdef HAVE_LD_STATIC_DYNAMIC
> > +       /* Remove -static-libstdc++ from the command only if target supports
> > +          LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> > +          back-end target can use outfile substitution.  */
> >         args[i] |= SKIPOPT;
> > +#endif
> >         break;
> >
> >       case OPT_static_libphobos:
> >         if (phobos_library != PHOBOS_NOLINK)
> >           phobos_library = PHOBOS_STATIC;
> > +#ifdef HAVE_LD_STATIC_DYNAMIC
> > +       /* Remove -static-libphobos from the command only if target supports
> > +          LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
> > +          back-end target can use outfile substitution.  */
> >         args[i] |= SKIPOPT;
> > +#endif
> >         break;
> >
> >       case OPT_shared_libphobos:
> > @@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> >  #endif
> >      }
> >
> > -  if (saw_libcxx || need_stdcxx)
> > +  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
> >      {
> >  #ifdef HAVE_LD_STATIC_DYNAMIC
> >        if (saw_static_libcxx && !static_link)
> > @@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
> >         generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
> >                          &new_decoded_options[j++]);
> >       }
> > -#else
> > -      /* Push the -static-libstdc++ option back onto the command so that
> > -      a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
> > -      if (saw_static_libcxx && !static_link)
> > -     generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
> > -                      &new_decoded_options[j++]);
> >  #endif
> >        if (saw_libcxx)
> >       new_decoded_options[j++] = *saw_libcxx;
> > diff --git a/gcc/gcc.c b/gcc/gcc.c
> > index 506c2acc282..fea6d049183 100644
> > --- a/gcc/gcc.c
> > +++ b/gcc/gcc.c
> > @@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
> >      case OPT_static_libgcc:
> >      case OPT_shared_libgcc:
> >      case OPT_static_libgfortran:
> > +    case OPT_static_libphobos:
> >      case OPT_static_libstdc__:
> >        /* These are always valid, since gcc.c itself understands the
> > -      first two, gfortranspec.c understands -static-libgfortran and
> > -      g++spec.c understands -static-libstdc++ */
> > +      first two, gfortranspec.c understands -static-libgfortran,
> > +      d-spec.cc understands -static-libphobos, and g++spec.c
> > +      understands -static-libstdc++ */
> >        validated = true;
> >        break;
> >
> > diff --git a/libphobos/testsuite/testsuite_flags.in b/libphobos/testsuite/testsuite_flags.in
> > index bafd5ad4502..8e2f1eefd5b 100755
> > --- a/libphobos/testsuite/testsuite_flags.in
> > +++ b/libphobos/testsuite/testsuite_flags.in
> > @@ -46,6 +46,7 @@ case ${query} in
> >      --gdcldflags)
> >        GDCLDFLAGS="-B${BUILD_DIR}/src
> >                    -B${BUILD_DIR}/libdruntime/gcc
> > +                  -B${BUILD_DIR}/src/.libs
> >                    -L${BUILD_DIR}/src/.libs"
> >        echo ${GDCLDFLAGS}
> >        ;;
> >
  

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index db6010e4e20..73c12d933f3 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3527,6 +3527,10 @@  static-libgfortran
 Driver
 ; Documented for Fortran, but always accepted by driver.
 
+static-libphobos
+Driver
+; Documented for D, but always accepted by driver.
+
 static-libstdc++
 Driver
 
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 7ed01efa694..c4ddd623e8b 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -443,6 +443,7 @@  extern GTY(()) int darwin_ms_struct;
                      %:replace-outfile(-lobjc libobjc-gnu.a%s); \
                     :%:replace-outfile(-lobjc -lobjc-gnu )}}\
    %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
+   %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
    %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
    %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
    %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
index b12d28f1047..73ecac3bbf1 100644
--- a/gcc/d/d-spec.cc
+++ b/gcc/d/d-spec.cc
@@ -253,13 +253,23 @@  lang_specific_driver (cl_decoded_option **in_decoded_options,
 
 	case OPT_static_libstdc__:
 	  saw_static_libcxx = true;
+#ifndef HAVE_LD_STATIC_DYNAMIC
+	  /* Remove -static-libstdc++ from the command only if target supports
+	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
+	     back-end target can use outfile substitution.  */
 	  args[i] |= SKIPOPT;
+#endif
 	  break;
 
 	case OPT_static_libphobos:
 	  if (phobos_library != PHOBOS_NOLINK)
 	    phobos_library = PHOBOS_STATIC;
+#ifndef HAVE_LD_STATIC_DYNAMIC
+	  /* Remove -static-libphobos from the command only if target supports
+	     LD_STATIC_DYNAMIC.  When not supported, it is left in so that a
+	     back-end target can use outfile substitution.  */
 	  args[i] |= SKIPOPT;
+#endif
 	  break;
 
 	case OPT_shared_libphobos:
@@ -460,7 +470,7 @@  lang_specific_driver (cl_decoded_option **in_decoded_options,
 #endif
     }
 
-  if (saw_libcxx || need_stdcxx)
+  if (saw_libcxx || saw_static_libcxx || need_stdcxx)
     {
 #ifdef HAVE_LD_STATIC_DYNAMIC
       if (saw_static_libcxx && !static_link)
@@ -468,12 +478,6 @@  lang_specific_driver (cl_decoded_option **in_decoded_options,
 	  generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
 			   &new_decoded_options[j++]);
 	}
-#else
-      /* Push the -static-libstdc++ option back onto the command so that
-	 a target without LD_STATIC_DYNAMIC can use outfile substitution.  */
-      if (saw_static_libcxx && !static_link)
-	generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
-			 &new_decoded_options[j++]);
 #endif
       if (saw_libcxx)
 	new_decoded_options[j++] = *saw_libcxx;
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 506c2acc282..fea6d049183 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4576,10 +4576,12 @@  driver_handle_option (struct gcc_options *opts,
     case OPT_static_libgcc:
     case OPT_shared_libgcc:
     case OPT_static_libgfortran:
+    case OPT_static_libphobos:
     case OPT_static_libstdc__:
       /* These are always valid, since gcc.c itself understands the
-	 first two, gfortranspec.c understands -static-libgfortran and
-	 g++spec.c understands -static-libstdc++ */
+	 first two, gfortranspec.c understands -static-libgfortran,
+	 d-spec.cc understands -static-libphobos, and g++spec.c
+	 understands -static-libstdc++ */
       validated = true;
       break;