ld: Properly check static link of dynamic object

Message ID 20200311235752.406955-1-hjl.tools@gmail.com
State New
Headers show
Series
  • ld: Properly check static link of dynamic object
Related show

Commit Message

Jose E. Marchesi via Binutils March 11, 2020, 11:57 p.m.
When -static is passed to gcc, gcc passes it to linker before any input
files to create static executable.  If -Bdynamic is also passed to linker
from command-line, linker should issue an error if dynamic object is used.

include/

	PR ld/24920
	* bfdlink.h (bfd_link_info): Add static_exec.

ld/

	PR ld/24920
	* ldfile.c (ldfile_try_open_bfd): Also check link_info.static_exec
	for static link of dynamic object.
	* lexsup.c (parse_args): Set link_info.static_exec to true if we
	see -static before any input files.
	* testsuite/ld-elf/pr24920.err: New file.
	* testsuite/ld-elf/shared.exp: Run ld/24920 tests.
---
 include/bfdlink.h               |  3 +++
 ld/ldfile.c                     |  6 ++++--
 ld/lexsup.c                     |  4 ++++
 ld/testsuite/ld-elf/pr24920.err |  1 +
 ld/testsuite/ld-elf/shared.exp  | 18 ++++++++++++++++++
 5 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/pr24920.err

-- 
2.24.1

Comments

Fangrui Song March 12, 2020, 4:09 a.m. | #1
> When -static is passed to gcc, gcc passes it to linker before any input

> files to create static executable.  If -Bdynamic is also passed to linker

> from command-line, linker should issue an error if dynamic object is used.

> 

> include/

> 

> 	PR ld/24920

> 	* bfdlink.h (bfd_link_info): Add static_exec.

> 

> ld/

> 

> 	PR ld/24920

> 	* ldfile.c (ldfile_try_open_bfd): Also check link_info.static_exec

> 	for static link of dynamic object.

> 	* lexsup.c (parse_args): Set link_info.static_exec to true if we

> 	see -static before any input files.

> 	* testsuite/ld-elf/pr24920.err: New file.

> 	* testsuite/ld-elf/shared.exp: Run ld/24920 tests.

> ---

>  include/bfdlink.h               |  3 +++

>  ld/ldfile.c                     |  6 ++++--

>  ld/lexsup.c                     |  4 ++++

>  ld/testsuite/ld-elf/pr24920.err |  1 +

>  ld/testsuite/ld-elf/shared.exp  | 18 ++++++++++++++++++

>  5 files changed, 30 insertions(+), 2 deletions(-)

>  create mode 100644 ld/testsuite/ld-elf/pr24920.err

> 

> diff --git a/include/bfdlink.h b/include/bfdlink.h

> index 8d85530e39..a850c6102c 100644

> --- a/include/bfdlink.h

> +++ b/include/bfdlink.h

> @@ -322,6 +322,9 @@ struct bfd_link_info

>    /* Output type.  */

>    ENUM_BITFIELD (output_type) type : 2;

>  

> +  /* TRUE if building a static executable.  */

> +  unsigned int static_exec : 1;

> +

>    /* TRUE if BFD should pre-bind symbols in a shared object.  */

>    unsigned int symbolic: 1;

>  

> diff --git a/ld/ldfile.c b/ld/ldfile.c

> index aa84906992..7b57bd9fb7 100644

> --- a/ld/ldfile.c

> +++ b/ld/ldfile.c

> @@ -164,7 +164,8 @@ ldfile_try_open_bfd (const char *attempt,

>       checks out compatible, do not exit early returning TRUE, or

>       the plugins will not get a chance to claim the file.  */

>  

> -  if (entry->flags.search_dirs || !entry->flags.dynamic)

> +  if (entry->flags.search_dirs

> +      || (link_info.static_exec || !entry->flags.dynamic))

>      {

>        bfd *check;

>  

> @@ -274,7 +275,8 @@ ldfile_try_open_bfd (const char *attempt,

>  	      goto success;

>  	    }

>  

> -	  if (!entry->flags.dynamic && (entry->the_bfd->flags & DYNAMIC) != 0)

> +	  if ((link_info.static_exec || !entry->flags.dynamic)

> +	      && (entry->the_bfd->flags & DYNAMIC) != 0)

>  	    {

>  	      einfo (_("%F%P: attempted static link of dynamic object `%s'\n"),

>  		     attempt);

> diff --git a/ld/lexsup.c b/ld/lexsup.c

> index 3d15cc491d..aeddc52466 100644

> --- a/ld/lexsup.c

> +++ b/ld/lexsup.c

> @@ -786,6 +786,10 @@ parse_args (unsigned argc, char **argv)

>  	  break;

>  	case OPTION_NON_SHARED:

>  	  input_flags.dynamic = FALSE;

> +	  /* If we see -static before any input files, we are building

> +	     a static executable.  */

> +	  if (!lang_has_input_file)

> +	    link_info.static_exec = TRUE;

>  	  break;

>  	case OPTION_CREF:

>  	  command_line.cref = TRUE;

> diff --git a/ld/testsuite/ld-elf/pr24920.err b/ld/testsuite/ld-elf/pr24920.err

> new file mode 100644

> index 0000000000..8f5cab9167

> --- /dev/null

> +++ b/ld/testsuite/ld-elf/pr24920.err

> @@ -0,0 +1 @@

> +.*: attempted static link of dynamic object `tmpdir/pr24920.so'

> diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp

> index b1762aff9b..0b8024dd1a 100644

> --- a/ld/testsuite/ld-elf/shared.exp

> +++ b/ld/testsuite/ld-elf/shared.exp

> @@ -114,6 +114,24 @@ run_ld_link_tests [list \

>  	{} \

>  	"pr22649-1.so" \

>      ] \

> +    [list \

> +	"Build pr24920.so" \

> +	"$LFLAGS -shared" \

> +	"" \

> +	"$AFLAGS_PIC" \

> +	{dummy.s} \

> +	{} \

> +	"pr24920.so" \

> +    ] \

> +    [list \

> +	"Build pr24920" \

> +	"$LFLAGS -static " \

> +	"-Bdynamic tmpdir/pr24920.so" \

> +	"" \

> +	{start.s} \

> +	{{ld pr24920.err}} \

> +	"pr24920" \

> +    ] \

>  ]

>  

>  if { [check_gc_sections_available] } {

> -- 

> 2.24.1


Does the patch reject valid use case like:

ld -Bstatic a.o -Bdynamic b.so

If the gcc driver detects the input file type, I think such an error should be emitted on its side.?


-static = -Bstatic. There is no need to teach it to know that the first -static/-Bstatic is different.
Jose E. Marchesi via Binutils March 12, 2020, 4:19 a.m. | #2
On Wed, Mar 11, 2020 at 04:57:52PM -0700, H.J. Lu via Binutils wrote:
> When -static is passed to gcc, gcc passes it to linker before any input

> files to create static executable.  If -Bdynamic is also passed to linker

> from command-line, linker should issue an error if dynamic object is used.


I can't say I like the idea of giving -static special meaning when it
occurs before input files, making its effect sticky.  The sticky
effect just seems wrong to me and liable to break some project
somewhere.

PR 24920 is really just a user being surprised when they misuse their
tools.  That and the default dynamic linker being wrong.  Why not fix
the x86_64 default dynamic linker?

-- 
Alan Modra
Australia Development Lab, IBM
Jose E. Marchesi via Binutils March 12, 2020, 9:36 a.m. | #3
On Wed, Mar 11, 2020 at 9:19 PM Alan Modra <amodra@gmail.com> wrote:
>

> On Wed, Mar 11, 2020 at 04:57:52PM -0700, H.J. Lu via Binutils wrote:

> > When -static is passed to gcc, gcc passes it to linker before any input

> > files to create static executable.  If -Bdynamic is also passed to linker

> > from command-line, linker should issue an error if dynamic object is used.

>

> I can't say I like the idea of giving -static special meaning when it

> occurs before input files, making its effect sticky.  The sticky

> effect just seems wrong to me and liable to break some project

> somewhere.

>

> PR 24920 is really just a user being surprised when they misuse their

> tools.  That and the default dynamic linker being wrong.  Why not fix

> the x86_64 default dynamic linker?

>


I want to use the generic x86-64 ELF backend for Linux.  I will see
if I can just do -static for x86.

Besides, when -static is passed to GCC driver, I don't think -Wl,-Bdynamic
can completely override it at the linker command-line.

-- 
H.J.

Patch

diff --git a/include/bfdlink.h b/include/bfdlink.h
index 8d85530e39..a850c6102c 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -322,6 +322,9 @@  struct bfd_link_info
   /* Output type.  */
   ENUM_BITFIELD (output_type) type : 2;
 
+  /* TRUE if building a static executable.  */
+  unsigned int static_exec : 1;
+
   /* TRUE if BFD should pre-bind symbols in a shared object.  */
   unsigned int symbolic: 1;
 
diff --git a/ld/ldfile.c b/ld/ldfile.c
index aa84906992..7b57bd9fb7 100644
--- a/ld/ldfile.c
+++ b/ld/ldfile.c
@@ -164,7 +164,8 @@  ldfile_try_open_bfd (const char *attempt,
      checks out compatible, do not exit early returning TRUE, or
      the plugins will not get a chance to claim the file.  */
 
-  if (entry->flags.search_dirs || !entry->flags.dynamic)
+  if (entry->flags.search_dirs
+      || (link_info.static_exec || !entry->flags.dynamic))
     {
       bfd *check;
 
@@ -274,7 +275,8 @@  ldfile_try_open_bfd (const char *attempt,
 	      goto success;
 	    }
 
-	  if (!entry->flags.dynamic && (entry->the_bfd->flags & DYNAMIC) != 0)
+	  if ((link_info.static_exec || !entry->flags.dynamic)
+	      && (entry->the_bfd->flags & DYNAMIC) != 0)
 	    {
 	      einfo (_("%F%P: attempted static link of dynamic object `%s'\n"),
 		     attempt);
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 3d15cc491d..aeddc52466 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -786,6 +786,10 @@  parse_args (unsigned argc, char **argv)
 	  break;
 	case OPTION_NON_SHARED:
 	  input_flags.dynamic = FALSE;
+	  /* If we see -static before any input files, we are building
+	     a static executable.  */
+	  if (!lang_has_input_file)
+	    link_info.static_exec = TRUE;
 	  break;
 	case OPTION_CREF:
 	  command_line.cref = TRUE;
diff --git a/ld/testsuite/ld-elf/pr24920.err b/ld/testsuite/ld-elf/pr24920.err
new file mode 100644
index 0000000000..8f5cab9167
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr24920.err
@@ -0,0 +1 @@ 
+.*: attempted static link of dynamic object `tmpdir/pr24920.so'
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index b1762aff9b..0b8024dd1a 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -114,6 +114,24 @@  run_ld_link_tests [list \
 	{} \
 	"pr22649-1.so" \
     ] \
+    [list \
+	"Build pr24920.so" \
+	"$LFLAGS -shared" \
+	"" \
+	"$AFLAGS_PIC" \
+	{dummy.s} \
+	{} \
+	"pr24920.so" \
+    ] \
+    [list \
+	"Build pr24920" \
+	"$LFLAGS -static " \
+	"-Bdynamic tmpdir/pr24920.so" \
+	"" \
+	{start.s} \
+	{{ld pr24920.err}} \
+	"pr24920" \
+    ] \
 ]
 
 if { [check_gc_sections_available] } {