[PATCH] [RFC] ld: add new --enable-new-dtags-only flag

[PATCH] [RFC] ld: add new --enable-new-dtags-only flag

[Mike Frysinger]

With the --enable-new-dtags flag, you can generate DT_RUNPATH in
addition to DT_RPATH.  But the former tag isn't terribly useful
so long as the latter is still being generated.  So add a new
option to suppress that.

The proposed option name isn't great, but it's in line with the
existing --neable-new-dtags flag.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

[Being lazy on CL writing until we hammer out the proposal]
---
 bfd/elflink.c         | 14 ++++++++++----
 gold/layout.cc        |  3 ++-
 gold/options.h        |  3 +++
 include/bfdlink.h     |  3 +++
 ld/emultempl/elf32.em | 10 +++++++++-
 ld/ld.texinfo         |  8 ++++++--
 6 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 661b2eb..b70d995 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5748,13 +5748,19 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 
 	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
 				      TRUE);
-	  if (indx == (bfd_size_type) -1
-	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+	  if (indx == (bfd_size_type) -1)
 	    return FALSE;
 
-	  if  (info->new_dtags)
+	  if (!info->new_dtags_only)
+	    {
+	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+		return FALSE;
+	    }
+
+	  if (info->new_dtags)
 	    {
-	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
+	      if (!info->new_dtags_only)
+		_bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
 	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
 		return FALSE;
 	    }
diff --git a/gold/layout.cc b/gold/layout.cc
index f7f0e7e..5083e24 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -4645,7 +4645,8 @@ Layout::finish_dynamic_section(const Input_objects* input_objects,
 	    }
 	}
 
-      odyn->add_string(elfcpp::DT_RPATH, rpath_val);
+      if (!parameters->options().enable_new_dtags_only())
+	odyn->add_string(elfcpp::DT_RPATH, rpath_val);
       if (parameters->options().enable_new_dtags())
 	odyn->add_string(elfcpp::DT_RUNPATH, rpath_val);
     }
diff --git a/gold/options.h b/gold/options.h
index 38f0c00..65ff03b 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -908,6 +908,9 @@ class General_options
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
+  DEFINE_enable(new_dtags_only, options::EXACTLY_TWO_DASHES, '\0', false,
+		N_("Skip the use of DT_RPATH"), NULL);
+
   DEFINE_bool(noinhibit_exec, options::TWO_DASHES, '\0', false,
 	      N_("Create an output file even if errors occur"), NULL);
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bf44dee..3920d12 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -368,6 +368,9 @@ struct bfd_link_info
   /* TRUE if the new ELF dynamic tags are enabled. */
   unsigned int new_dtags: 1;
 
+  /* TRUE if only the new ELF dynamic tags are used (skips old tags).  */
+  unsigned int new_dtags_only: 1;
+
   /* FALSE if .eh_frame unwind info should be generated for PLT and other
      linker created sections, TRUE if it should be omitted.  */
   unsigned int no_ld_generated_unwind_info: 1;
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index d30a0ad..405cfc3 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -2120,7 +2120,8 @@ fragment <<EOF
 
 #define OPTION_DISABLE_NEW_DTAGS	(400)
 #define OPTION_ENABLE_NEW_DTAGS		(OPTION_DISABLE_NEW_DTAGS + 1)
-#define OPTION_GROUP			(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_ENABLE_NEW_DTAGS_ONLY	(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_GROUP			(OPTION_ENABLE_NEW_DTAGS_ONLY + 1)
 #define OPTION_EH_FRAME_HDR		(OPTION_GROUP + 1)
 #define OPTION_EXCLUDE_LIBS		(OPTION_EH_FRAME_HDR + 1)
 #define OPTION_HASH_STYLE		(OPTION_EXCLUDE_LIBS + 1)
@@ -2159,6 +2160,7 @@ fragment <<EOF
     {"depaudit", required_argument, NULL, 'P'},
     {"disable-new-dtags", no_argument, NULL, OPTION_DISABLE_NEW_DTAGS},
     {"enable-new-dtags", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS},
+    {"enable-new-dtags-only", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS_ONLY},
     {"eh-frame-hdr", no_argument, NULL, OPTION_EH_FRAME_HDR},
     {"exclude-libs", required_argument, NULL, OPTION_EXCLUDE_LIBS},
     {"hash-style", required_argument, NULL, OPTION_HASH_STYLE},
@@ -2222,6 +2224,10 @@ fragment <<EOF
       link_info.new_dtags = TRUE;
       break;
 
+    case OPTION_ENABLE_NEW_DTAGS_ONLY:
+      link_info.new_dtags_only = TRUE;
+      break;
+
     case OPTION_EH_FRAME_HDR:
       link_info.eh_frame_hdr = TRUE;
       break;
@@ -2402,6 +2408,8 @@ fragment <<EOF
   fprintf (file, _("\
   --enable-new-dtags          Enable new dynamic tags\n"));
   fprintf (file, _("\
+  --enable-new-dtags-only     Only use new dynamic tags (omit old dynamic tags)\n"));
+  fprintf (file, _("\
   --eh-frame-hdr              Create .eh_frame_hdr section\n"));
   fprintf (file, _("\
   --exclude-libs=LIBS         Make all symbols in LIBS hidden\n"));
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index c7ae2a5..bdab407 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -2099,15 +2099,19 @@ generated code sections like PLT.  This option is on by default
 if linker generated unwind info is supported.
 
 @kindex --enable-new-dtags
+@kindex --enable-new-dtags-only
 @kindex --disable-new-dtags
 @item --enable-new-dtags
+@itemx --enable-new-dtags-only
 @itemx --disable-new-dtags
 This linker can create the new dynamic tags in ELF. But the older ELF
 systems may not understand them. If you specify
 @option{--enable-new-dtags}, the dynamic tags will be created as needed.
 If you specify @option{--disable-new-dtags}, no new dynamic tags will be
-created. By default, the new dynamic tags are not created. Note that
-those options are only available for ELF systems.
+created. If you specify @option{--enable-new-dtags-only} in conjunction
+with @option{--enable-new-dtags}, then older dynamic tags will be omitted
+when new dynamic tags replace them. By default, the new dynamic tags are
+not created. Note that those options are only available for ELF systems.
 
 @kindex --hash-size=@var{number}
 @item --hash-size=@var{number}
-- 
1.8.0

Re: [PATCH] [RFC] ld: add new --enable-new-dtags-only flag

[Alan Modra]

On Tue, Dec 25, 2012 at 02:30:00AM -0500, Mike Frysinger wrote:
> With the --enable-new-dtags flag, you can generate DT_RUNPATH in
> addition to DT_RPATH.  But the former tag isn't terribly useful
> so long as the latter is still being generated.

Huh?  I thought that if DT_RUNPATH is present, DT_RPATH is ignored.

Also, if you're going to name the new option --enable-new-dtags-only,
I'd be inclined to make it actually enable the new dtags.  ie. don't
require --enable-new-dtags as well.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] [RFC] ld: add new --enable-new-dtags-only flag

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 759 bytes --]

On Tuesday 08 January 2013 04:31:53 Alan Modra wrote:
> On Tue, Dec 25, 2012 at 02:30:00AM -0500, Mike Frysinger wrote:
> > With the --enable-new-dtags flag, you can generate DT_RUNPATH in
> > addition to DT_RPATH.  But the former tag isn't terribly useful
> > so long as the latter is still being generated.
> 
> Huh?  I thought that if DT_RUNPATH is present, DT_RPATH is ignored.

it is ignored.  i meant to point out generating both is pointless and has been 
for quite a long time now.

> Also, if you're going to name the new option --enable-new-dtags-only,
> I'd be inclined to make it actually enable the new dtags.  ie. don't
> require --enable-new-dtags as well.

i'm fine with that behavior.  i was trying to be conservative.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v2] ld: add new --enable-new-dtags-only flag

[Mike Frysinger]

With the --enable-new-dtags flag, you can generate DT_RUNPATH in
addition to DT_RPATH.  But the former tag isn't terribly useful
so long as the latter is still being generated.  So add a new
option to suppress that.

The proposed option name isn't great, but it's in line with the
existing --neable-new-dtags flag.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

[Being lazy on CL writing until we hammer out the proposal]
---
v2
	- allow --enable-new-dtags-only to imply --enable-new-dtags

 bfd/elflink.c         | 14 ++++++++++----
 gold/layout.cc        |  6 ++++--
 gold/options.h        |  3 +++
 include/bfdlink.h     |  3 +++
 ld/emultempl/elf32.em | 10 +++++++++-
 ld/ld.texinfo         |  8 ++++++--
 6 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 661b2eb..b70d995 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5748,13 +5748,19 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 
 	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
 				      TRUE);
-	  if (indx == (bfd_size_type) -1
-	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+	  if (indx == (bfd_size_type) -1)
 	    return FALSE;
 
-	  if  (info->new_dtags)
+	  if (!info->new_dtags_only)
+	    {
+	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+		return FALSE;
+	    }
+
+	  if (info->new_dtags)
 	    {
-	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
+	      if (!info->new_dtags_only)
+		_bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
 	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
 		return FALSE;
 	    }
diff --git a/gold/layout.cc b/gold/layout.cc
index f7f0e7e..4e3b0a5 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -4645,8 +4645,10 @@ Layout::finish_dynamic_section(const Input_objects* input_objects,
 	    }
 	}
 
-      odyn->add_string(elfcpp::DT_RPATH, rpath_val);
-      if (parameters->options().enable_new_dtags())
+      if (!parameters->options().enable_new_dtags_only())
+	odyn->add_string(elfcpp::DT_RPATH, rpath_val);
+      if (parameters->options().enable_new_dtags()
+	  || parameters->options().enable_new_dtags_only())
 	odyn->add_string(elfcpp::DT_RUNPATH, rpath_val);
     }
 
diff --git a/gold/options.h b/gold/options.h
index 38f0c00..65ff03b 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -908,6 +908,9 @@ class General_options
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
+  DEFINE_enable(new_dtags_only, options::EXACTLY_TWO_DASHES, '\0', false,
+		N_("Skip the use of DT_RPATH"), NULL);
+
   DEFINE_bool(noinhibit_exec, options::TWO_DASHES, '\0', false,
 	      N_("Create an output file even if errors occur"), NULL);
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bf44dee..3920d12 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -368,6 +368,9 @@ struct bfd_link_info
   /* TRUE if the new ELF dynamic tags are enabled. */
   unsigned int new_dtags: 1;
 
+  /* TRUE if only the new ELF dynamic tags are used (skips old tags).  */
+  unsigned int new_dtags_only: 1;
+
   /* FALSE if .eh_frame unwind info should be generated for PLT and other
      linker created sections, TRUE if it should be omitted.  */
   unsigned int no_ld_generated_unwind_info: 1;
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index d30a0ad..5de2b21 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -2120,7 +2120,8 @@ fragment <<EOF
 
 #define OPTION_DISABLE_NEW_DTAGS	(400)
 #define OPTION_ENABLE_NEW_DTAGS		(OPTION_DISABLE_NEW_DTAGS + 1)
-#define OPTION_GROUP			(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_ENABLE_NEW_DTAGS_ONLY	(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_GROUP			(OPTION_ENABLE_NEW_DTAGS_ONLY + 1)
 #define OPTION_EH_FRAME_HDR		(OPTION_GROUP + 1)
 #define OPTION_EXCLUDE_LIBS		(OPTION_EH_FRAME_HDR + 1)
 #define OPTION_HASH_STYLE		(OPTION_EXCLUDE_LIBS + 1)
@@ -2159,6 +2160,7 @@ fragment <<EOF
     {"depaudit", required_argument, NULL, 'P'},
     {"disable-new-dtags", no_argument, NULL, OPTION_DISABLE_NEW_DTAGS},
     {"enable-new-dtags", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS},
+    {"enable-new-dtags-only", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS_ONLY},
     {"eh-frame-hdr", no_argument, NULL, OPTION_EH_FRAME_HDR},
     {"exclude-libs", required_argument, NULL, OPTION_EXCLUDE_LIBS},
     {"hash-style", required_argument, NULL, OPTION_HASH_STYLE},
@@ -2218,6 +2220,10 @@ fragment <<EOF
       link_info.new_dtags = FALSE;
       break;
 
+    case OPTION_ENABLE_NEW_DTAGS_ONLY:
+      link_info.new_dtags_only = TRUE;
+      /* Fall through.  */
+
     case OPTION_ENABLE_NEW_DTAGS:
       link_info.new_dtags = TRUE;
       break;
@@ -2402,6 +2408,8 @@ fragment <<EOF
   fprintf (file, _("\
   --enable-new-dtags          Enable new dynamic tags\n"));
   fprintf (file, _("\
+  --enable-new-dtags-only     Only use new dynamic tags (omit old dynamic tags)\n"));
+  fprintf (file, _("\
   --eh-frame-hdr              Create .eh_frame_hdr section\n"));
   fprintf (file, _("\
   --exclude-libs=LIBS         Make all symbols in LIBS hidden\n"));
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index c7ae2a5..859118f 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -2099,15 +2099,19 @@ generated code sections like PLT.  This option is on by default
 if linker generated unwind info is supported.
 
 @kindex --enable-new-dtags
+@kindex --enable-new-dtags-only
 @kindex --disable-new-dtags
 @item --enable-new-dtags
+@itemx --enable-new-dtags-only
 @itemx --disable-new-dtags
 This linker can create the new dynamic tags in ELF. But the older ELF
 systems may not understand them. If you specify
 @option{--enable-new-dtags}, the dynamic tags will be created as needed.
 If you specify @option{--disable-new-dtags}, no new dynamic tags will be
-created. By default, the new dynamic tags are not created. Note that
-those options are only available for ELF systems.
+created. If you specify @option{--enable-new-dtags-only}, then older
+dynamic tags will be omitted when new dynamic tags replace them. By
+default, the new dynamic tags are not created. Note that those options
+are only available for ELF systems.
 
 @kindex --hash-size=@var{number}
 @item --hash-size=@var{number}
-- 
1.8.0.2

Re: [PATCH v2] ld: add new --enable-new-dtags-only flag

[John Marino]

On 1/11/2013 07:05, Mike Frysinger wrote:
> With the --enable-new-dtags flag, you can generate DT_RUNPATH in
> addition to DT_RPATH.  But the former tag isn't terribly useful
> so long as the latter is still being generated.  So add a new
> option to suppress that.
>
> The proposed option name isn't great, but it's in line with the
> existing --neable-new-dtags flag.
>
> Signed-off-by: Mike Frysinger<vapier@gentoo.org>
>
> [Being lazy on CL writing until we hammer out the proposal]
> ---

I don't think the premise is true.
If both DT_RUNPATH and DT_RPATH dtags are present, DT_RPATH dtag is 
ignored by the dynamic linker.

I don't know of a dynamic linker that behaves differently.
Are you sure there's a problem here?

John

Re: [PATCH v2] ld: add new --enable-new-dtags-only flag

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 853 bytes --]

On Friday 11 January 2013 04:21:58 John Marino wrote:
> On 1/11/2013 07:05, Mike Frysinger wrote:
> > With the --enable-new-dtags flag, you can generate DT_RUNPATH in
> > addition to DT_RPATH.  But the former tag isn't terribly useful
> > so long as the latter is still being generated.  So add a new
> > option to suppress that.
> > 
> > The proposed option name isn't great, but it's in line with the
> > existing --neable-new-dtags flag.
> > 
> > Signed-off-by: Mike Frysinger<vapier@gentoo.org>
> > 
> > [Being lazy on CL writing until we hammer out the proposal]
> > ---
> 
> I don't think the premise is true.
> If both DT_RUNPATH and DT_RPATH dtags are present, DT_RPATH dtag is
> ignored by the dynamic linker.

yes, as mentioned in the first patchset, that's not what i meant.  i just 
forgot to update the message.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

The "new" dtags options have been around for 14+ years, and for all the
targets that gold supports, these flags have always exist.  So enable
them by default.

Having behavior be different from ld.bfd isn't new, and this behavior
is the "better" one, so there shouldn't be a problem based on that.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* options.h (General_options): Change default to true for new_dtags
	and new_dtags_only.
---
 gold/options.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gold/options.h b/gold/options.h
index ec3f823..9f7b5ae 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -904,11 +904,11 @@ class General_options
 	      N_("Do not page align data, do not make text readonly"),
 	      N_("Page align data, make text readonly"));
 
-  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', false,
+  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', true,
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
-  DEFINE_enable(new_dtags_only, options::EXACTLY_TWO_DASHES, '\0', false,
+  DEFINE_enable(new_dtags_only, options::EXACTLY_TWO_DASHES, '\0', true,
 		N_("Skip the use of DT_RPATH"),
 		N_("Do not skip the use of DT_RPATH"));
 
-- 
1.8.0.2

[PATCH 1/2 v3] ld: add new --{dis,en}able-new-dtags-only flag

[Mike Frysinger]

With the --enable-new-dtags flag, you can generate DT_RUNPATH in
addition to DT_RPATH.  But the latter tag isn't useful while the
former tag is being generated on systems, and DT_RUNPATH has been
around for at least 14 years, so it's probably about time we stop
assuming it's "new".

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

bfd/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* elflink.c (bfd_elf_size_dynamic_sections): Add DT_RPATH entry
	when new_dtags_only is false.  Add DT_RUNPATH when new_dtags is
	true.

gold/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* layout.cc (Layout::finish_dynamic_section): Add DT_RPATH entry
	when enable_new_dtags_only is false.  Add DT_RUNPATH when
	enable_new_dtags or enable_new_dtags_only is true.
	* options.h (General_options): Add new_dtags_only option.

include/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* bfdlink.h (struct bfd_link_info): Add new_dtags_only field.

ld/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* emultempl/elf32.em (OPTION_ENABLE_NEW_DTAGS_ONLY): Define.
	(OPTION_DISABLE_NEW_DTAGS_ONLY): Likewise.
	(OPTION_GROUP): Define based on OPTION_DISABLE_NEW_DTAGS_ONLY.
	(xtra_long): Add disable-new-dtags-only and enable-new-dtags-only.
	(_handle_option): Handle OPTION_DISABLE_NEW_DTAGS_ONLY and
	OPTION_ENABLE_NEW_DTAGS_ONLY.
	(_list_options): Add --disable-new-dtags-only and
	--enable-new-dtags-only.
	* ld.texinfo (Options): Document --disable-new-dtags-only and
	--enable-new-dtags-only.
---
v3
	- add --disable-new-dtags-only
	- fix up commit message
	- add changelog

 bfd/elflink.c         | 14 ++++++++++----
 gold/layout.cc        |  6 ++++--
 gold/options.h        |  4 ++++
 include/bfdlink.h     |  3 +++
 ld/emultempl/elf32.em | 18 +++++++++++++++++-
 ld/ld.texinfo         | 11 +++++++++--
 6 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 7861946..58d0caa 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5751,13 +5751,19 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 
 	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
 				      TRUE);
-	  if (indx == (bfd_size_type) -1
-	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+	  if (indx == (bfd_size_type) -1)
 	    return FALSE;
 
-	  if  (info->new_dtags)
+	  if (!info->new_dtags_only)
+	    {
+	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+		return FALSE;
+	    }
+
+	  if (info->new_dtags)
 	    {
-	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
+	      if (!info->new_dtags_only)
+		_bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
 	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
 		return FALSE;
 	    }
diff --git a/gold/layout.cc b/gold/layout.cc
index f7f0e7e..4e3b0a5 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -4645,8 +4645,10 @@ Layout::finish_dynamic_section(const Input_objects* input_objects,
 	    }
 	}
 
-      odyn->add_string(elfcpp::DT_RPATH, rpath_val);
-      if (parameters->options().enable_new_dtags())
+      if (!parameters->options().enable_new_dtags_only())
+	odyn->add_string(elfcpp::DT_RPATH, rpath_val);
+      if (parameters->options().enable_new_dtags()
+	  || parameters->options().enable_new_dtags_only())
 	odyn->add_string(elfcpp::DT_RUNPATH, rpath_val);
     }
 
diff --git a/gold/options.h b/gold/options.h
index 1eb2da2..ec3f823 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -908,6 +908,10 @@ class General_options
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
+  DEFINE_enable(new_dtags_only, options::EXACTLY_TWO_DASHES, '\0', false,
+		N_("Skip the use of DT_RPATH"),
+		N_("Do not skip the use of DT_RPATH"));
+
   DEFINE_bool(noinhibit_exec, options::TWO_DASHES, '\0', false,
 	      N_("Create an output file even if errors occur"), NULL);
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bf44dee..3920d12 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -368,6 +368,9 @@ struct bfd_link_info
   /* TRUE if the new ELF dynamic tags are enabled. */
   unsigned int new_dtags: 1;
 
+  /* TRUE if only the new ELF dynamic tags are used (skips old tags).  */
+  unsigned int new_dtags_only: 1;
+
   /* FALSE if .eh_frame unwind info should be generated for PLT and other
      linker created sections, TRUE if it should be omitted.  */
   unsigned int no_ld_generated_unwind_info: 1;
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 53d4e24..7ddd7a3 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -2121,7 +2121,9 @@ fragment <<EOF
 
 #define OPTION_DISABLE_NEW_DTAGS	(400)
 #define OPTION_ENABLE_NEW_DTAGS		(OPTION_DISABLE_NEW_DTAGS + 1)
-#define OPTION_GROUP			(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_ENABLE_NEW_DTAGS_ONLY	(OPTION_ENABLE_NEW_DTAGS + 1)
+#define OPTION_DISABLE_NEW_DTAGS_ONLY	(OPTION_ENABLE_NEW_DTAGS_ONLY + 1)
+#define OPTION_GROUP			(OPTION_DISABLE_NEW_DTAGS_ONLY + 1)
 #define OPTION_EH_FRAME_HDR		(OPTION_GROUP + 1)
 #define OPTION_EXCLUDE_LIBS		(OPTION_EH_FRAME_HDR + 1)
 #define OPTION_HASH_STYLE		(OPTION_EXCLUDE_LIBS + 1)
@@ -2160,6 +2162,8 @@ fragment <<EOF
     {"depaudit", required_argument, NULL, 'P'},
     {"disable-new-dtags", no_argument, NULL, OPTION_DISABLE_NEW_DTAGS},
     {"enable-new-dtags", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS},
+    {"disable-new-dtags-only", no_argument, NULL, OPTION_DISABLE_NEW_DTAGS_ONLY},
+    {"enable-new-dtags-only", no_argument, NULL, OPTION_ENABLE_NEW_DTAGS_ONLY},
     {"eh-frame-hdr", no_argument, NULL, OPTION_EH_FRAME_HDR},
     {"exclude-libs", required_argument, NULL, OPTION_EXCLUDE_LIBS},
     {"hash-style", required_argument, NULL, OPTION_HASH_STYLE},
@@ -2219,6 +2223,14 @@ fragment <<EOF
       link_info.new_dtags = FALSE;
       break;
 
+    case OPTION_DISABLE_NEW_DTAGS_ONLY:
+      link_info.new_dtags_only = FALSE;
+      break;
+
+    case OPTION_ENABLE_NEW_DTAGS_ONLY:
+      link_info.new_dtags_only = TRUE;
+      /* Fall through.  */
+
     case OPTION_ENABLE_NEW_DTAGS:
       link_info.new_dtags = TRUE;
       break;
@@ -2403,6 +2415,10 @@ fragment <<EOF
   fprintf (file, _("\
   --enable-new-dtags          Enable new dynamic tags\n"));
   fprintf (file, _("\
+  --disable-new-dtags-only    Use new and old dynamic tags\n"));
+  fprintf (file, _("\
+  --enable-new-dtags-only     Only use new dynamic tags (omit old dynamic tags)\n"));
+  fprintf (file, _("\
   --eh-frame-hdr              Create .eh_frame_hdr section\n"));
   fprintf (file, _("\
   --exclude-libs=LIBS         Make all symbols in LIBS hidden\n"));
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 4777ad5..414ccff 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -2101,15 +2101,22 @@ generated code sections like PLT.  This option is on by default
 if linker generated unwind info is supported.
 
 @kindex --enable-new-dtags
+@kindex --enable-new-dtags-only
 @kindex --disable-new-dtags
+@kindex --disable-new-dtags-only
 @item --enable-new-dtags
+@itemx --enable-new-dtags-only
 @itemx --disable-new-dtags
+@itemx --disable-new-dtags-only
 This linker can create the new dynamic tags in ELF. But the older ELF
 systems may not understand them. If you specify
 @option{--enable-new-dtags}, the dynamic tags will be created as needed.
 If you specify @option{--disable-new-dtags}, no new dynamic tags will be
-created. By default, the new dynamic tags are not created. Note that
-those options are only available for ELF systems.
+created. If you specify @option{--enable-new-dtags-only}, then older
+dynamic tags will be omitted when new dynamic tags replace them
+(and @option{--disable-new-dtags-only} can disable that behavior). By
+default, the new dynamic tags are not created. Note that those options
+are only available for ELF systems.
 
 @kindex --hash-size=@var{number}
 @item --hash-size=@var{number}
-- 
1.8.0.2

Re: [PATCH 2/2] gold: enable new dtags by default

[Ian Lance Taylor]

On Fri, Jan 11, 2013 at 10:32 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> The "new" dtags options have been around for 14+ years, and for all the
> targets that gold supports, these flags have always exist.  So enable
> them by default.
>
> Having behavior be different from ld.bfd isn't new, and this behavior
> is the "better" one, so there shouldn't be a problem based on that.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
>
>         * options.h (General_options): Change default to true for new_dtags
>         and new_dtags_only.

I'm not aware of any --new-dtags-only option in gold.

The patch changing the default for --new-dtags to true is OK.

Thanks.

Ian

Re: [PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 924 bytes --]

On Tuesday 15 January 2013 09:47:08 Ian Lance Taylor wrote:
> On Fri, Jan 11, 2013 at 10:32 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > The "new" dtags options have been around for 14+ years, and for all the
> > targets that gold supports, these flags have always exist.  So enable
> > them by default.
> > 
> > Having behavior be different from ld.bfd isn't new, and this behavior
> > is the "better" one, so there shouldn't be a problem based on that.
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > 
> > 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
> > 
> >         * options.h (General_options): Change default to true for
> >         new_dtags and new_dtags_only.
> 
> I'm not aware of any --new-dtags-only option in gold.

[patch 1/2] adds the option

> The patch changing the default for --new-dtags to true is OK.

k k ... i'll wait for the first patch to merge
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] gold: enable new dtags by default

[Ian Lance Taylor]

On Tue, Jan 15, 2013 at 9:41 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 15 January 2013 09:47:08 Ian Lance Taylor wrote:
>
>> I'm not aware of any --new-dtags-only option in gold.
>
> [patch 1/2] adds the option
>
>> The patch changing the default for --new-dtags to true is OK.
>
> k k ... i'll wait for the first patch to merge

I don't know that that patch is going to be approved.  I also don't
know that it makes sense to have it default to true.  That said with
regard to --new-dtags-only I'm willing to have gold do whatever GNU ld
does.

For --new-dtags I'm willing to have the default be true even if the
default for GNU ld is false.

Ian

Re: [PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1210 bytes --]

On Tuesday 15 January 2013 14:06:00 Ian Lance Taylor wrote:
> On Tue, Jan 15, 2013 at 9:41 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Tuesday 15 January 2013 09:47:08 Ian Lance Taylor wrote:
> >> I'm not aware of any --new-dtags-only option in gold.
> > 
> > [patch 1/2] adds the option
> > 
> >> The patch changing the default for --new-dtags to true is OK.
> > 
> > k k ... i'll wait for the first patch to merge
> 
> I don't know that that patch is going to be approved.  I also don't
> know that it makes sense to have it default to true.  That said with
> regard to --new-dtags-only I'm willing to have gold do whatever GNU ld
> does.
> 
> For --new-dtags I'm willing to have the default be true even if the
> default for GNU ld is false.

patch 1/2 doesn't change any defaults ... just adds a new flag

patch 2/2 makes gold default to true because gold is so new and is easy to do

i'd like to do something with ld.bfd to default new-dtags-only to true for 
linux systems, but that's a bit harder to pull off in the current ld.bfd 
framework.  so i posted the current code as is since they're usable already.

in Gentoo, i plan on changing the ld.bfd default
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] gold: enable new dtags by default

[Ian Lance Taylor]

On Tue, Jan 15, 2013 at 11:35 AM, Mike Frysinger <vapier@gentoo.org> wrote:
>
> patch 1/2 doesn't change any defaults ... just adds a new flag

Understood.  I only want to add that flag to gold if it is added to
GNU ld.  That is, I am deferring the decision about the flag to the
other binutils maintainers.

Ian

Re: [PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 416 bytes --]

On Tuesday 15 January 2013 15:02:31 Ian Lance Taylor wrote:
> On Tue, Jan 15, 2013 at 11:35 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > patch 1/2 doesn't change any defaults ... just adds a new flag
> 
> Understood.  I only want to add that flag to gold if it is added to
> GNU ld.  That is, I am deferring the decision about the flag to the
> other binutils maintainers.

perfectly reasonable
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] gold: enable new dtags by default

[Alan Modra]

On Tue, Jan 15, 2013 at 12:02:31PM -0800, Ian Lance Taylor wrote:
> On Tue, Jan 15, 2013 at 11:35 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> >
> > patch 1/2 doesn't change any defaults ... just adds a new flag
> 
> Understood.  I only want to add that flag to gold if it is added to
> GNU ld.  That is, I am deferring the decision about the flag to the
> other binutils maintainers.

On further thinking about this, I'd be happy with changing the
existing flag, --enable-new-dtags, to have the behaviour proposed for
--enable-new-dtags-only.  As Mike said, the "new" dtags have been
around for a mighty long time, and emitting both old and new tags was
really only for backward compatibility.  Is anyone running a system
with a 14 year old glibc?  If there is, do they also want the latest
binutils?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1066 bytes --]

On Wednesday 16 January 2013 22:42:17 Alan Modra wrote:
> On Tue, Jan 15, 2013 at 12:02:31PM -0800, Ian Lance Taylor wrote:
> > On Tue, Jan 15, 2013 at 11:35 AM, Mike Frysinger wrote:
> > > patch 1/2 doesn't change any defaults ... just adds a new flag
> > 
> > Understood.  I only want to add that flag to gold if it is added to
> > GNU ld.  That is, I am deferring the decision about the flag to the
> > other binutils maintainers.
> 
> On further thinking about this, I'd be happy with changing the
> existing flag, --enable-new-dtags, to have the behaviour proposed for
> --enable-new-dtags-only.  As Mike said, the "new" dtags have been
> around for a mighty long time, and emitting both old and new tags was
> really only for backward compatibility.  Is anyone running a system
> with a 14 year old glibc?  If there is, do they also want the latest
> binutils?

i'm happy with that course.  the new -only flag was merely to keep from rocking 
the boat.

how do you feel about also enabling --enable-new-dtags by default in ld.bfd ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] gold: enable new dtags by default

[Alan Modra]

On Wed, Jan 16, 2013 at 11:12:21PM -0500, Mike Frysinger wrote:
> On Wednesday 16 January 2013 22:42:17 Alan Modra wrote:
> > On Tue, Jan 15, 2013 at 12:02:31PM -0800, Ian Lance Taylor wrote:
> > > On Tue, Jan 15, 2013 at 11:35 AM, Mike Frysinger wrote:
> > > > patch 1/2 doesn't change any defaults ... just adds a new flag
> > > 
> > > Understood.  I only want to add that flag to gold if it is added to
> > > GNU ld.  That is, I am deferring the decision about the flag to the
> > > other binutils maintainers.
> > 
> > On further thinking about this, I'd be happy with changing the
> > existing flag, --enable-new-dtags, to have the behaviour proposed for
> > --enable-new-dtags-only.  As Mike said, the "new" dtags have been
> > around for a mighty long time, and emitting both old and new tags was
> > really only for backward compatibility.  Is anyone running a system
> > with a 14 year old glibc?  If there is, do they also want the latest
> > binutils?
> 
> i'm happy with that course.

OK, consider it approved.

>  the new -only flag was merely to keep from rocking 
> the boat.

:)

> how do you feel about also enabling --enable-new-dtags by default in ld.bfd ?

I'm not so comfortable with this.  After all, the new dtags do have
slightly different meaning to the old ones.  I think it would be
better to leave the default as is.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] gold: enable new dtags by default

[Michael Matz]

Hi,

On Thu, 17 Jan 2013, Alan Modra wrote:

> > how do you feel about also enabling --enable-new-dtags by default in 
> > ld.bfd ?
> 
> I'm not so comfortable with this.  After all, the new dtags do have 
> slightly different meaning to the old ones.  I think it would be better 
> to leave the default as is.

FWIW SUSE enabled the new dtags in our binutils package by default since, 
pfff, I think 2004/2005 or so.  Don't know about others.


Ciao,
Michael.

[PATCH 2/2 v4] gold: enable new dtags by default

[Mike Frysinger]

The "new" dtags options have been around for 14+ years, and for all the
targets that gold supports, these flags have always existed.  So enable
them by default.

Having behavior be different from ld.bfd isn't new, and this behavior
is the "better" one, so there shouldn't be a problem based on that.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* options.h (General_options): Change default to true for new_dtags.
---
v4
	- dropped --enable-new-dtags-only logic

 gold/options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gold/options.h b/gold/options.h
index 1eb2da2..9e65e8d 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -904,7 +904,7 @@ class General_options
 	      N_("Do not page align data, do not make text readonly"),
 	      N_("Page align data, make text readonly"));
 
-  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', false,
+  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', true,
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
-- 
1.8.0.2

[PATCH 1/2 v4] ld: change --enable-new-dtags to only generate new dtags

[Mike Frysinger]

The "new" dtags options have been around for 14+ years, so there
shouldn't be a need to generate both new & old tags anymore.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

bfd/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* elflink.c (bfd_elf_size_dynamic_sections): Only add DT_RPATH
	when new_dtags is false.  Only add DT_RUNPATH when new_dtags is
	true.

gold/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* layout.cc (Layout::finish_dynamic_section): Only add DT_RPATH
	when enable_new_dtags is false.  Only add DT_RUNPATH when
	enable_new_dtags is true.

ld/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* NEWS: Mention change in behavior with --enable-new-dtags.
	* ld.texinfo (Options): Clarify --enable-new-dtags behavior.
---
v4
	- drop --enable-new-dtags-only and have --enable-new-dtags omit new dtags

 bfd/elflink.c  | 10 +++++++---
 gold/layout.cc |  5 +++--
 ld/NEWS        |  2 ++
 ld/ld.texinfo  |  3 ++-
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 6985786..dcb529a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5733,11 +5733,15 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 
 	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
 				      TRUE);
-	  if (indx == (bfd_size_type) -1
-	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+	  if (indx == (bfd_size_type) -1)
 	    return FALSE;
 
-	  if  (info->new_dtags)
+	  if (!info->new_dtags)
+	    {
+	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+		return FALSE;
+	    }
+	  else
 	    {
 	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
 	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
diff --git a/gold/layout.cc b/gold/layout.cc
index 576d44b..250782a 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -4647,8 +4647,9 @@ Layout::finish_dynamic_section(const Input_objects* input_objects,
 	    }
 	}
 
-      odyn->add_string(elfcpp::DT_RPATH, rpath_val);
-      if (parameters->options().enable_new_dtags())
+      if (!parameters->options().enable_new_dtags())
+	odyn->add_string(elfcpp::DT_RPATH, rpath_val);
+      else
 	odyn->add_string(elfcpp::DT_RUNPATH, rpath_val);
     }
 
diff --git a/ld/NEWS b/ld/NEWS
index 9b9f4a6..1e07605 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -2,6 +2,8 @@
 
 * Add support for the Imagination Technologies Meta processor.
 
+* --enable-new-dtags no longer generates old dtags in addition to new dtags.
+
 Changes in 2.23:
 
 * Enable compressed debug section feature for x86/x86_64 pe-coff.
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 2429668..194f56a 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -2112,7 +2112,8 @@ if linker generated unwind info is supported.
 @itemx --disable-new-dtags
 This linker can create the new dynamic tags in ELF. But the older ELF
 systems may not understand them. If you specify
-@option{--enable-new-dtags}, the dynamic tags will be created as needed.
+@option{--enable-new-dtags}, the new dynamic tags will be created as needed
+and older dynamic tags will be omitted.
 If you specify @option{--disable-new-dtags}, no new dynamic tags will be
 created. By default, the new dynamic tags are not created. Note that
 those options are only available for ELF systems.
-- 
1.8.0.2

Re: [PATCH 2/2] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 476 bytes --]

On Wednesday 16 January 2013 23:41:37 Alan Modra wrote:
> On Wed, Jan 16, 2013 at 11:12:21PM -0500, Mike Frysinger wrote:
> > how do you feel about also enabling --enable-new-dtags by default in
> > ld.bfd ?
> 
> I'm not so comfortable with this.  After all, the new dtags do have
> slightly different meaning to the old ones.  I think it would be
> better to leave the default as is.

for all targets ?  i would be happy even if we did it just for linux ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2 v4] ld: change --enable-new-dtags to only generate new dtags

[Alan Modra]

On Thu, Jan 17, 2013 at 02:22:20PM -0500, Mike Frysinger wrote:
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -5733,11 +5733,15 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
>  
>  	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
>  				      TRUE);
> -	  if (indx == (bfd_size_type) -1
> -	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
> +	  if (indx == (bfd_size_type) -1)
>  	    return FALSE;
>  
> -	  if  (info->new_dtags)
> +	  if (!info->new_dtags)
> +	    {
> +	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
> +		return FALSE;
> +	    }
> +	  else
>  	    {
>  	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
>  	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))

That _bfd_elf_strtab_addref will cause assertion failures when you get
around to testing this patch, I think.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] gold: enable new dtags by default

[Alan Modra]

On Thu, Jan 17, 2013 at 02:23:06PM -0500, Mike Frysinger wrote:
> On Wednesday 16 January 2013 23:41:37 Alan Modra wrote:
> > On Wed, Jan 16, 2013 at 11:12:21PM -0500, Mike Frysinger wrote:
> > > how do you feel about also enabling --enable-new-dtags by default in
> > > ld.bfd ?
> > 
> > I'm not so comfortable with this.  After all, the new dtags do have
> > slightly different meaning to the old ones.  I think it would be
> > better to leave the default as is.
> 
> for all targets ?  i would be happy even if we did it just for linux ...

Enabling for linux should be OK.  It was the thought of omitting old
dtag for all targets that I didn't like.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 1/2 v4] ld: change --enable-new-dtags to only generate new dtags

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 777 bytes --]

On Thursday 17 January 2013 16:33:05 Alan Modra wrote:
> On Thu, Jan 17, 2013 at 02:22:20PM -0500, Mike Frysinger wrote:
> > --- a/bfd/elflink.c
> > +++ b/bfd/elflink.c
> > 
> > -	  if  (info->new_dtags)
> > +	  if (!info->new_dtags)
> > +	    {
> > +	      if (!_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
> > +		return FALSE;
> > +	    }
> > +	  else
> >  	    {
> >  	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
> >  	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
> 
> That _bfd_elf_strtab_addref will cause assertion failures when you get
> around to testing this patch, I think.

err i fixed that locally.  i must have uploaded the wrong version :/.

i did verify `make check` still worked ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/2 v5] ld: change --enable-new-dtags to only generate new dtags

[Mike Frysinger]

The "new" dtags options have been around for 14+ years, so there
shouldn't be a need to generate both new & old tags anymore.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

bfd/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* elflink.c (bfd_elf_size_dynamic_sections): Only add DT_RPATH
	when new_dtags is false.  Only add DT_RUNPATH when new_dtags is
	true.

gold/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* layout.cc (Layout::finish_dynamic_section): Only add DT_RPATH
	when enable_new_dtags is false.  Only add DT_RUNPATH when
	enable_new_dtags is true.

ld/:
2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* NEWS: Mention change in behavior with --enable-new-dtags.
	* ld.texinfo (Options): Clarify --enable-new-dtags behavior.
---
v5
	- post correct patch this time ...

 bfd/elflink.c  | 13 +++++--------
 gold/layout.cc |  5 +++--
 ld/NEWS        |  2 ++
 ld/ld.texinfo  |  3 ++-
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 6985786..d336730 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5730,19 +5730,16 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
       if (rpath != NULL)
 	{
 	  bfd_size_type indx;
+	  bfd_vma tag;
 
 	  indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
 				      TRUE);
-	  if (indx == (bfd_size_type) -1
-	      || !_bfd_elf_add_dynamic_entry (info, DT_RPATH, indx))
+	  if (indx == (bfd_size_type) -1)
 	    return FALSE;
 
-	  if  (info->new_dtags)
-	    {
-	      _bfd_elf_strtab_addref (elf_hash_table (info)->dynstr, indx);
-	      if (!_bfd_elf_add_dynamic_entry (info, DT_RUNPATH, indx))
-		return FALSE;
-	    }
+	  tag = info->new_dtags ? DT_RUNPATH : DT_RPATH;
+	  if (!_bfd_elf_add_dynamic_entry (info, tag, indx))
+	    return FALSE;
 	}
 
       if (filter_shlib != NULL)
diff --git a/gold/layout.cc b/gold/layout.cc
index 576d44b..250782a 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -4647,8 +4647,9 @@ Layout::finish_dynamic_section(const Input_objects* input_objects,
 	    }
 	}
 
-      odyn->add_string(elfcpp::DT_RPATH, rpath_val);
-      if (parameters->options().enable_new_dtags())
+      if (!parameters->options().enable_new_dtags())
+	odyn->add_string(elfcpp::DT_RPATH, rpath_val);
+      else
 	odyn->add_string(elfcpp::DT_RUNPATH, rpath_val);
     }
 
diff --git a/ld/NEWS b/ld/NEWS
index 9b9f4a6..1e07605 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -2,6 +2,8 @@
 
 * Add support for the Imagination Technologies Meta processor.
 
+* --enable-new-dtags no longer generates old dtags in addition to new dtags.
+
 Changes in 2.23:
 
 * Enable compressed debug section feature for x86/x86_64 pe-coff.
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 2429668..194f56a 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -2112,7 +2112,8 @@ if linker generated unwind info is supported.
 @itemx --disable-new-dtags
 This linker can create the new dynamic tags in ELF. But the older ELF
 systems may not understand them. If you specify
-@option{--enable-new-dtags}, the dynamic tags will be created as needed.
+@option{--enable-new-dtags}, the new dynamic tags will be created as needed
+and older dynamic tags will be omitted.
 If you specify @option{--disable-new-dtags}, no new dynamic tags will be
 created. By default, the new dynamic tags are not created. Note that
 those options are only available for ELF systems.
-- 
1.8.0.2

[PATCH 2/2 v5] gold: enable new dtags by default

[Mike Frysinger]

The "new" dtags options have been around for 14+ years, and for all the
targets that gold supports, these flags have always existed.  So enable
them by default.

Having behavior be different from ld.bfd isn't new, and this behavior
is the "better" one, so there shouldn't be a problem based on that.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2013-01-12  Mike Frysinger  <vapier@gentoo.org>

	* options.h (General_options): Change default to true for new_dtags.
---
v5
	- no changes

 gold/options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gold/options.h b/gold/options.h
index 1eb2da2..9e65e8d 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -904,7 +904,7 @@ class General_options
 	      N_("Do not page align data, do not make text readonly"),
 	      N_("Page align data, make text readonly"));
 
-  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', false,
+  DEFINE_enable(new_dtags, options::EXACTLY_TWO_DASHES, '\0', true,
 		N_("Enable use of DT_RUNPATH and DT_FLAGS"),
 		N_("Disable use of DT_RUNPATH and DT_FLAGS"));
 
-- 
1.8.0.2

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Ian Lance Taylor]

On Thu, Jan 17, 2013 at 8:26 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> The "new" dtags options have been around for 14+ years, and for all the
> targets that gold supports, these flags have always existed.  So enable
> them by default.
>
> Having behavior be different from ld.bfd isn't new, and this behavior
> is the "better" one, so there shouldn't be a problem based on that.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
>
>         * options.h (General_options): Change default to true for new_dtags.

This is OK.

Thanks.

Ian

Re: [PATCH 1/2 v5] ld: change --enable-new-dtags to only generate new dtags

[Alan Modra]

On Thu, Jan 17, 2013 at 11:26:40PM -0500, Mike Frysinger wrote:
> bfd/:
> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* elflink.c (bfd_elf_size_dynamic_sections): Only add DT_RPATH
> 	when new_dtags is false.  Only add DT_RUNPATH when new_dtags is
> 	true.
> 
> gold/:
> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* layout.cc (Layout::finish_dynamic_section): Only add DT_RPATH
> 	when enable_new_dtags is false.  Only add DT_RUNPATH when
> 	enable_new_dtags is true.
> 
> ld/:
> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* NEWS: Mention change in behavior with --enable-new-dtags.
> 	* ld.texinfo (Options): Clarify --enable-new-dtags behavior.

Please commit.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH] ld: enable new dtags by default for linux/gnu targets

[Mike Frysinger]

The "new" dtags options have been around for 14+ years now, so for Linux
and GNU targets, enable them by default.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2012-01-18  Mike Frysinger  <vapier@gentoo.org>

	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Set
	link_info.new_dtags to TRUE for linux/gnu targets.
	* NEWS: Mention new dtags default.
---
 ld/NEWS               |  2 ++
 ld/emultempl/elf32.em | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/ld/NEWS b/ld/NEWS
index 1e07605..6b30b2f 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -4,6 +4,8 @@
 
 * --enable-new-dtags no longer generates old dtags in addition to new dtags.
 
+* For Linux/GNU targets, new dtags is now the default.
+
 Changes in 2.23:
 
 * Enable compressed debug section feature for x86/x86_64 pe-coff.
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 53d4e24..60611f9 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -105,6 +105,16 @@ gld${EMULATION_NAME}_before_parse (void)
   input_flags.dynamic = ${DYNAMIC_LINK-TRUE};
   config.has_shared = `if test -n "$GENERATE_SHLIB_SCRIPT" ; then echo TRUE ; else echo FALSE ; fi`;
   config.separate_code = `if test "x${SEPARATE_CODE}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
+EOF
+
+case ${target} in
+  *-*-linux-* | *-*-k*bsd*-* | *-*-gnu*)
+    fragment <<EOF
+  link_info.new_dtags = TRUE;
+EOF
+    ;;
+esac
+fragment <<EOF
 }
 
 EOF
-- 
1.8.0.2

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Alan Modra]

On Fri, Jan 18, 2013 at 04:06:28PM -0500, Mike Frysinger wrote:
> 2012-01-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Set
> 	link_info.new_dtags to TRUE for linux/gnu targets.
> 	* NEWS: Mention new dtags default.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Andreas Schwab]

Mike Frysinger <vapier@gentoo.org> writes:

> +case ${target} in
> +  *-*-linux-* | *-*-k*bsd*-* | *-*-gnu*)

I don't think you should match k*bsd* here.  You only want to use it for
GNU/k*bsd (which is covered by gnu*), not other systems that happen to
use the *bsd kernel.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 598 bytes --]

On Saturday 19 January 2013 03:40:16 Andreas Schwab wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > +case ${target} in
> > +  *-*-linux-* | *-*-k*bsd*-* | *-*-gnu*)
> 
> I don't think you should match k*bsd* here.  You only want to use it for
> GNU/k*bsd (which is covered by gnu*), not other systems that happen to
> use the *bsd kernel.

i want to enable new dtags when the userland is using glibc.  i copy & pasted 
this set of tuples because it appears many times in this file to select "is it 
a glibc userland".  are all of the checks in this file incorrect then ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 835 bytes --]

On Saturday 19 January 2013 11:16:19 Mike Frysinger wrote:
> On Saturday 19 January 2013 03:40:16 Andreas Schwab wrote:
> > Mike Frysinger <vapier@gentoo.org> writes:
> > > +case ${target} in
> > > +  *-*-linux-* | *-*-k*bsd*-* | *-*-gnu*)
> > 
> > I don't think you should match k*bsd* here.  You only want to use it for
> > GNU/k*bsd (which is covered by gnu*), not other systems that happen to
> > use the *bsd kernel.
> 
> i want to enable new dtags when the userland is using glibc.  i copy &
> pasted this set of tuples because it appears many times in this file to
> select "is it a glibc userland".  are all of the checks in this file
> incorrect then ?

i've committed this patch as originally posted.  if we feel the k*bsd* is 
incorrect, we can drop it from the whole file rather than this one spot.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Hans-Peter Nilsson]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Fri, 18 Jan 2013 22:06:28 +0100

> The "new" dtags options have been around for 14+ years now, so for Linux
> and GNU targets, enable them by default.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> 2012-01-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Set
> 	link_info.new_dtags to TRUE for linux/gnu targets.
> 	* NEWS: Mention new dtags default.

You forgot to test affected targets and adjust regressions.  Any
change that alters default behavior is (should be) a red flag
that this is necessary.  Committed after checking that
cris-axis-elf and cris-axis-linux-gnu are again FAIL-free.

ld/testsuite:

	* ld-cris/libdso-13.d: Adjust for --enable-new-dtags now
	default for *-*-linux-*.

(Two obvious choices; I went for changing the expected output
and adding --enable-new-dtags explicitly instead of adding
--disable-new-dtags.  N.B. the pre-existing "-m crislinux"
emulation forces the test to behave the same for cris-elf.)

Index: ld-cris/libdso-13.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-cris/libdso-13.d,v
retrieving revision 1.4
diff -p -u -r1.4 libdso-13.d
--- ld-cris/libdso-13.d	15 Aug 2005 15:39:45 -0000	1.4
+++ ld-cris/libdso-13.d	31 Jan 2013 00:30:45 -0000
@@ -12,7 +12,7 @@
 # generally disabled.)  Split out the expected readelf output
 # into a separate test using that option.
 
-Dynamic section at offset 0x[0-9a-f][0-9a-f][0-9a-f] contains 10 entries:
+Dynamic section at offset 0x[0-9a-f][0-9a-f][0-9a-f] contains 11 entries:
   Tag[ 	]+Type[ 	]+Name/Value
  0x0+4 \(HASH\)[ 	]+0x94
  0x0+5 \(STRTAB\)[ 	]+0x[12][0-9a-f][0-9a-f]
@@ -23,6 +23,7 @@ Dynamic section at offset 0x[0-9a-f][0-9
  0x0+8 \(RELASZ\)[ 	]+12 \(bytes\)
  0x0+9 \(RELAENT\)[ 	]+12 \(bytes\)
  0x0+16 \(TEXTREL\)[ 	]+0x0
+ 0x0+1e \(FLAGS\)[ 	]+TEXTREL
  0x0+ \(NULL\)[ 	]+0x0
 
 Relocation section '\.rela\.text' at offset 0x[12][0-9a-f][0-9a-f] contains 1 entries:

brgds, H-P

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 31 Jan 2013 01:47:06 +0100

> I went for changing the expected output
> and adding --enable-new-dtags explicitly instead of adding
> --disable-new-dtags.

Except I didn't have to add it *explicitly*...

(Bah, fallout from switching back-and-forth between email
editing and test-suite tweaking and missing the last iteration.)

brgds, H-P

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 31 Jan 2013 01:58:40 +0100

> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Thu, 31 Jan 2013 01:47:06 +0100
> 
> > I went for changing the expected output
> > and adding --enable-new-dtags explicitly instead of adding
> > --disable-new-dtags.
> 
> Except I didn't have to add it *explicitly*...

Except I did have to, as pointed out by my autotester.
(The "crislinux" emulation doesn't catch everything enabled for
*-*-linux-* targets.)

> (Bah, fallout from switching back-and-forth between email
> editing and test-suite tweaking and missing the last iteration.)

And then fumbling or misreading test-results.  I blame the flu.

Committed; I just adjusted my last ChangeLog entry which
already had the right date.

	* ld-cris/libdso-13.d: Adjust for --enable-new-dtags now
	default for *-*-linux-* by passing explicitly for all targets.

Index: ld-cris/libdso-13.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-cris/libdso-13.d,v
retrieving revision 1.5
diff -p -u -r1.5 libdso-13.d
--- ld-cris/libdso-13.d	31 Jan 2013 00:35:24 -0000	1.5
+++ ld-cris/libdso-13.d	31 Jan 2013 09:53:21 -0000
@@ -1,7 +1,7 @@
 #source: dso-1.s
 #source: dsov32-3.s
 #as: --pic --no-underscore --march=v32 --em=criself
-#ld: --shared -m crislinux -z nocombreloc
+#ld: --shared -m crislinux -z nocombreloc --enable-new-dtags
 #readelf: -d -r
 #warning: relocation R_CRIS_32_PCREL should not be used in a shared object; recompile with -fPIC
 

brgds, H-P

Re: [PATCH] ld: enable new dtags by default for linux/gnu targets

[H.J. Lu]

On Fri, Jan 18, 2013 at 1:06 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> The "new" dtags options have been around for 14+ years now, so for Linux
> and GNU targets, enable them by default.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> 2012-01-18  Mike Frysinger  <vapier@gentoo.org>
>
>         * emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Set
>         link_info.new_dtags to TRUE for linux/gnu targets.
>         * NEWS: Mention new dtags default.
> ---
>  ld/NEWS               |  2 ++
>  ld/emultempl/elf32.em | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/ld/NEWS b/ld/NEWS
> index 1e07605..6b30b2f 100644
> --- a/ld/NEWS
> +++ b/ld/NEWS
> @@ -4,6 +4,8 @@
>
>  * --enable-new-dtags no longer generates old dtags in addition to new dtags.
>
> +* For Linux/GNU targets, new dtags is now the default.
> +
>  Changes in 2.23:
>
>  * Enable compressed debug section feature for x86/x86_64 pe-coff.
> diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
> index 53d4e24..60611f9 100644
> --- a/ld/emultempl/elf32.em
> +++ b/ld/emultempl/elf32.em
> @@ -105,6 +105,16 @@ gld${EMULATION_NAME}_before_parse (void)
>    input_flags.dynamic = ${DYNAMIC_LINK-TRUE};
>    config.has_shared = `if test -n "$GENERATE_SHLIB_SCRIPT" ; then echo TRUE ; else echo FALSE ; fi`;
>    config.separate_code = `if test "x${SEPARATE_CODE}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
> +EOF
> +
> +case ${target} in
> +  *-*-linux-* | *-*-k*bsd*-* | *-*-gnu*)
> +    fragment <<EOF
> +  link_info.new_dtags = TRUE;
> +EOF
> +    ;;
> +esac
> +fragment <<EOF
>  }
>
>  EOF
> --
> 1.8.0.2
>

This breaks -rpath on Linux:

http://sourceware.org/bugzilla/show_bug.cgi?id=15096

Since DT_RPATH != DT_RUNPATH,  we shouldn't generate
DT_RPATH for -rpath.

-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Thu, Jan 17, 2013 at 10:24 PM, Ian Lance Taylor <iant@google.com> wrote:
> On Thu, Jan 17, 2013 at 8:26 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> The "new" dtags options have been around for 14+ years, and for all the
>> targets that gold supports, these flags have always existed.  So enable
>> them by default.
>>
>> Having behavior be different from ld.bfd isn't new, and this behavior
>> is the "better" one, so there shouldn't be a problem based on that.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>
>> 2013-01-12  Mike Frysinger  <vapier@gentoo.org>
>>
>>         * options.h (General_options): Change default to true for new_dtags.
>
> This is OK.
>

This caused:

http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098

I changed BFD linker not set new dtags with -rpath.

-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Ian Lance Taylor]

On Mon, Feb 4, 2013 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
> This caused:
>
> http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098
>
> I changed BFD linker not set new dtags with -rpath.

I don't see why that is the right fix.  Since DT_RPATH/DT_RUNPATH are
only ever set by the linker's -rpath option, it seems like the right
fix is to always use DT_RPATH and never use DT_RUNPATH.

Of course, since the only thing --new-dtags does in gold is select
DT_RUNPATH rather than DT_RPATH, this makes new--dtags completely
useless in gold.

It seems that we have made sensible-seeming decisions to wind up in an
absurd place.  It seems that we should now make --new-dtags a no-op
and drop all support for generating DT_RUNPATH.  Which makes me wonder
why DT_RUNPATH was invented in the first place.

Ian

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Alan Modra]

On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
> It seems that we have made sensible-seeming decisions to wind up in an
> absurd place.

Right, so where did we go wrong?  I think it was in assuming that we
could default to --enable-new-dtags.  Users were affected.  (I'm
assuming HJ's PR ld/15096 came from a real world problem.)

I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
patches.  That will leave us with just Mike's 2013-01-18 change to
disable old dtags when new dtags are selected.  If people want new
dtags by default, do so via the gcc driver.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Ian Lance Taylor]

On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
>> It seems that we have made sensible-seeming decisions to wind up in an
>> absurd place.
>
> Right, so where did we go wrong?  I think it was in assuming that we
> could default to --enable-new-dtags.  Users were affected.  (I'm
> assuming HJ's PR ld/15096 came from a real world problem.)
>
> I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
> patches.  That will leave us with just Mike's 2013-01-18 change to
> disable old dtags when new dtags are selected.  If people want new
> dtags by default, do so via the gcc driver.

Sounds reasonable to me.

Ian

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Ian Lance Taylor]

On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
>> It seems that we have made sensible-seeming decisions to wind up in an
>> absurd place.
>
> Right, so where did we go wrong?  I think it was in assuming that we
> could default to --enable-new-dtags.  Users were affected.  (I'm
> assuming HJ's PR ld/15096 came from a real world problem.)
>
> I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
> patches.  That will leave us with just Mike's 2013-01-18 change to
> disable old dtags when new dtags are selected.  If people want new
> dtags by default, do so via the gcc driver.

I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.

Ian

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 5, 2013 at 6:19 AM, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
>>> It seems that we have made sensible-seeming decisions to wind up in an
>>> absurd place.
>>
>> Right, so where did we go wrong?  I think it was in assuming that we
>> could default to --enable-new-dtags.  Users were affected.  (I'm
>> assuming HJ's PR ld/15096 came from a real world problem.)
>>
>> I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
>> patches.  That will leave us with just Mike's 2013-01-18 change to
>> disable old dtags when new dtags are selected.  If people want new
>> dtags by default, do so via the gcc driver.
>
> I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.
>

This is a good idea.


-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
>> It seems that we have made sensible-seeming decisions to wind up in an
>> absurd place.
>
> Right, so where did we go wrong?  I think it was in assuming that we
> could default to --enable-new-dtags.  Users were affected.  (I'm
> assuming HJ's PR ld/15096 came from a real world problem.)
>
> I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
> patches.  That will leave us with just Mike's 2013-01-18 change to
> disable old dtags when new dtags are selected.  If people want new
> dtags by default, do so via the gcc driver.
>

Or we can just revert my change and add --runpath.   -rpath will always
generate DT_RPATH and --runpath will generate DT_RUNPATH.   What
should happen when both -rpath and --runpath are on command line?

-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 5, 2013 at 8:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Mon, Feb 04, 2013 at 09:43:04PM -0800, Ian Lance Taylor wrote:
>>> It seems that we have made sensible-seeming decisions to wind up in an
>>> absurd place.
>>
>> Right, so where did we go wrong?  I think it was in assuming that we
>> could default to --enable-new-dtags.  Users were affected.  (I'm
>> assuming HJ's PR ld/15096 came from a real world problem.)
>>
>> I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
>> patches.  That will leave us with just Mike's 2013-01-18 change to
>> disable old dtags when new dtags are selected.  If people want new
>> dtags by default, do so via the gcc driver.
>>
>
> Or we can just revert my change and add --runpath.   -rpath will always
> generate DT_RPATH and --runpath will generate DT_RUNPATH.   What
> should happen when both -rpath and --runpath are on command line?
>
Something like this.   -runpath will enable DT_RUNPATH.  Also
-runpath=PATH can also be used to specify DT_RUNPATH.


-- 
H.J.
---
diff --git a/bfd/elflink.c b/bfd/elflink.c
index d336730..60cf76f 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5737,7 +5737,7 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 	  if (indx == (bfd_size_type) -1)
 	    return FALSE;

-	  tag = info->new_dtags ? DT_RUNPATH : DT_RPATH;
+	  tag = info->runpath ? DT_RUNPATH : DT_RPATH;
 	  if (!_bfd_elf_add_dynamic_entry (info, tag, indx))
 	    return FALSE;
 	}
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bf44dee..9c6fcbd 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -368,6 +368,9 @@ struct bfd_link_info
   /* TRUE if the new ELF dynamic tags are enabled. */
   unsigned int new_dtags: 1;

+  /* TRUE if DT_RUNPATH instead of DT_RPATH should be generated. */
+  unsigned int runpath: 1;
+
   /* FALSE if .eh_frame unwind info should be generated for PLT and other
      linker created sections, TRUE if it should be omitted.  */
   unsigned int no_ld_generated_unwind_info: 1;
diff --git a/ld/ldlex.h b/ld/ldlex.h
index 99f4282..74cb774 100644
--- a/ld/ldlex.h
+++ b/ld/ldlex.h
@@ -56,6 +56,7 @@ enum option_values
   OPTION_RETAIN_SYMBOLS_FILE,
   OPTION_RPATH,
   OPTION_RPATH_LINK,
+  OPTION_RUNPATH,
   OPTION_SHARED,
   OPTION_SONAME,
   OPTION_SORT_COMMON,
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 2f71750..40a669b 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -395,10 +395,14 @@ static const struct ld_option ld_options[] =
      OPTION_RETAIN_SYMBOLS_FILE},
     '\0', N_("FILE"), N_("Keep only symbols listed in FILE"), TWO_DASHES },
   { {"rpath", required_argument, NULL, OPTION_RPATH},
-    '\0', N_("PATH"), N_("Set runtime shared library search path"), ONE_DASH },
+    '\0', N_("PATH"), N_("Set runtime shared library search path (DT_RPATH"),
+    ONE_DASH },
   { {"rpath-link", required_argument, NULL, OPTION_RPATH_LINK},
     '\0', N_("PATH"), N_("Set link time shared library search path"),
     ONE_DASH },
+  { {"runpath", optional_argument, NULL, OPTION_RUNPATH},
+    '\0', N_("PATH"), N_("Set runtime shared library search path (DT_RUNPATH"),
+    ONE_DASH },
   { {"shared", no_argument, NULL, OPTION_SHARED},
     '\0', NULL, N_("Create a shared library"), ONE_DASH },
   { {"Bshareable", no_argument, NULL, OPTION_SHARED }, /* FreeBSD.  */
@@ -990,6 +994,11 @@ parse_args (unsigned argc, char **argv)
 	  config.text_read_only = FALSE;
 	  input_flags.dynamic = FALSE;
 	  break;
+	case OPTION_RUNPATH:
+	  link_info.runpath = TRUE;
+	  if (optarg == NULL)
+	    break;
+	  goto handle_rpath;
 	case 'R':
 	  /* The GNU linker traditionally uses -R to mean to include
 	     only the symbols from a file.  The Solaris linker uses -R
@@ -1012,6 +1021,7 @@ parse_args (unsigned argc, char **argv)
 	  }
 	  /* Fall through.  */
 	case OPTION_RPATH:
+handle_rpath:
 	  if (command_line.rpath == NULL)
 	    command_line.rpath = xstrdup (optarg);
 	  else

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Alan Modra]

On Tue, Feb 05, 2013 at 06:19:06AM -0800, Ian Lance Taylor wrote:
> On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
> > I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
> > patches.  That will leave us with just Mike's 2013-01-18 change to
> > disable old dtags when new dtags are selected.  If people want new
> > dtags by default, do so via the gcc driver.
> 
> Sounds reasonable to me.

Done.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Alan Modra]

On Tue, Feb 05, 2013 at 08:32:13AM -0800, H.J. Lu wrote:
> On Tue, Feb 5, 2013 at 6:19 AM, Ian Lance Taylor <iant@google.com> wrote:
> > I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.

> This is a good idea.

Isn't it more or less an alias for -rpath, --enable-new-dtags?  Given
that is the case, people won't use the option in Makefiles if they
care about ld compatibility!  So why add an option that potentially
causes trouble for careless people?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Andreas Schwab]

Alan Modra <amodra@gmail.com> writes:

> On Tue, Feb 05, 2013 at 08:32:13AM -0800, H.J. Lu wrote:
>> On Tue, Feb 5, 2013 at 6:19 AM, Ian Lance Taylor <iant@google.com> wrote:
>> > I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.
>
>> This is a good idea.
>
> Isn't it more or less an alias for -rpath, --enable-new-dtags?  Given
> that is the case, people won't use the option in Makefiles if they
> care about ld compatibility!  So why add an option that potentially
> causes trouble for careless people?

OTOH, --enable-new-dtags is really misnamed, and it might be a good
opportunity to deprecate it.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 5, 2013 at 1:49 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Feb 05, 2013 at 06:19:06AM -0800, Ian Lance Taylor wrote:
>> On Tue, Feb 5, 2013 at 1:38 AM, Alan Modra <amodra@gmail.com> wrote:
>> > I propose reverting HJ's patch, and Mike's and Roland's 2013-02-21
>> > patches.  That will leave us with just Mike's 2013-01-18 change to
>> > disable old dtags when new dtags are selected.  If people want new
>> > dtags by default, do so via the gcc driver.
>>
>> Sounds reasonable to me.
>
> Done.
>

I checked in this patch to add some DT_XXX tests.

-- 
H.J.
---
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index c6f26a8..e172a1b 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2013-02-06  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* ld-elf/now-1.d: New file.
+	* ld-elf/now-2.d: Likewise.
+	* ld-elf/now-3.d: Likewise.
+	* ld-elf/now-4.d: Likewise.
+	* ld-elf/rpath-1.d: Likewise.
+	* ld-elf/rpath-2.d: Likewise.
+	* ld-elf/runpath-1.d: Likewise.
+	* ld-elf/runpath-2.d: Likewise.
+
 2013-02-06  Alan Modra  <amodra@gmail.com>

 	PR ld/15096
diff --git a/ld/testsuite/ld-elf/now-1.d b/ld/testsuite/ld-elf/now-1.d
new file mode 100644
index 0000000..9c7d5fa
--- /dev/null
+++ b/ld/testsuite/ld-elf/now-1.d
@@ -0,0 +1,9 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -z now --enable-new-dtags
+#target: *-*-linux* *-*-gnu*
+
+#failif
+#...
+ 0x[0-9a-f]+ +\(BIND_NOW\) +
+#...
diff --git a/ld/testsuite/ld-elf/now-2.d b/ld/testsuite/ld-elf/now-2.d
new file mode 100644
index 0000000..1430bc4
--- /dev/null
+++ b/ld/testsuite/ld-elf/now-2.d
@@ -0,0 +1,8 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -z now --enable-new-dtags
+#target: *-*-linux* *-*-gnu*
+
+#...
+ 0x[0-9a-f]+ +\(FLAGS\) +BIND_NOW
+#pass
diff --git a/ld/testsuite/ld-elf/now-3.d b/ld/testsuite/ld-elf/now-3.d
new file mode 100644
index 0000000..687885a
--- /dev/null
+++ b/ld/testsuite/ld-elf/now-3.d
@@ -0,0 +1,9 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -z now
+#target: *-*-linux* *-*-gnu*
+
+#failif
+#...
+ 0x[0-9a-f]+ +\(FLAGS\) +BIND_NOW
+#pass
diff --git a/ld/testsuite/ld-elf/now-4.d b/ld/testsuite/ld-elf/now-4.d
new file mode 100644
index 0000000..8d9d02f
--- /dev/null
+++ b/ld/testsuite/ld-elf/now-4.d
@@ -0,0 +1,8 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -z now
+#target: *-*-linux* *-*-gnu*
+
+#...
+ 0x[0-9a-f]+ +\(BIND_NOW\) +
+#pass
diff --git a/ld/testsuite/ld-elf/rpath-1.d b/ld/testsuite/ld-elf/rpath-1.d
new file mode 100644
index 0000000..918a326
--- /dev/null
+++ b/ld/testsuite/ld-elf/rpath-1.d
@@ -0,0 +1,9 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -rpath .
+#target: *-*-linux* *-*-gnu*
+
+#failif
+#...
+ +0x[0-9a-f]+ +\(RUNPATH\) +Library runpath: +\[.\]
+#...
diff --git a/ld/testsuite/ld-elf/rpath-2.d b/ld/testsuite/ld-elf/rpath-2.d
new file mode 100644
index 0000000..17be86d
--- /dev/null
+++ b/ld/testsuite/ld-elf/rpath-2.d
@@ -0,0 +1,8 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -rpath .
+#target: *-*-linux* *-*-gnu*
+
+#...
+ +0x[0-9a-f]+ +\(RPATH\) +Library rpath: +\[.\]
+#pass
diff --git a/ld/testsuite/ld-elf/runpath-1.d b/ld/testsuite/ld-elf/runpath-1.d
new file mode 100644
index 0000000..4d06639
--- /dev/null
+++ b/ld/testsuite/ld-elf/runpath-1.d
@@ -0,0 +1,9 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -rpath . --enable-new-dtags
+#target: *-*-linux* *-*-gnu*
+
+#failif
+#...
+ +0x[0-9a-f]+ +\(RPATH\) +Library rpath: +\[.\]
+#...
diff --git a/ld/testsuite/ld-elf/runpath-2.d b/ld/testsuite/ld-elf/runpath-2.d
new file mode 100644
index 0000000..0df8af6
--- /dev/null
+++ b/ld/testsuite/ld-elf/runpath-2.d
@@ -0,0 +1,8 @@
+#source: start.s
+#readelf: -d -W
+#ld: -shared -rpath . --enable-new-dtags
+#target: *-*-linux* *-*-gnu*
+
+#...
+ +0x[0-9a-f]+ +\(RUNPATH\) +Library runpath: +\[.\]
+#pass

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 5, 2013 at 1:57 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Feb 05, 2013 at 08:32:13AM -0800, H.J. Lu wrote:
>> On Tue, Feb 5, 2013 at 6:19 AM, Ian Lance Taylor <iant@google.com> wrote:
>> > I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.
>
>> This is a good idea.
>
> Isn't it more or less an alias for -rpath, --enable-new-dtags?  Given
> that is the case, people won't use the option in Makefiles if they
> care about ld compatibility!  So why add an option that potentially
> causes trouble for careless people?
>

The gABI has DT_FLAGS to replace DT_SYMBOLIC, DT_BIND_NOW,
and DT_TEXTREL.  --enable-new-dtags can be used to enable DT_FLAGS.
DT_RUNPATH is independent of DT_FLAGS.   We may want DT_RPATH
with DT_FLAGS.  But there is no way to do that today.

-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Wed, Feb 6, 2013 at 12:24 AM, Andreas Schwab <schwab@suse.de> wrote:
> Alan Modra <amodra@gmail.com> writes:
>
>> On Tue, Feb 05, 2013 at 08:32:13AM -0800, H.J. Lu wrote:
>>> On Tue, Feb 5, 2013 at 6:19 AM, Ian Lance Taylor <iant@google.com> wrote:
>>> > I wonder if it would make sense to add a -runpath option to set DT_RUNPATH.
>>
>>> This is a good idea.
>>
>> Isn't it more or less an alias for -rpath, --enable-new-dtags?  Given
>> that is the case, people won't use the option in Makefiles if they
>> care about ld compatibility!  So why add an option that potentially
>> causes trouble for careless people?
>
> OTOH, --enable-new-dtags is really misnamed, and it might be a good
> opportunity to deprecate it.
>

Maybe we should add ---enable-dt-flags and --enable-dt-runpath.


-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Alan Modra]

On Wed, Feb 06, 2013 at 08:28:25AM -0800, H.J. Lu wrote:
> The gABI has DT_FLAGS to replace DT_SYMBOLIC, DT_BIND_NOW,
> and DT_TEXTREL.

Hmm, and we still emit DT_TEXTREL and DT_SYMBOLIC with --enable-new-dtags.

>  --enable-new-dtags can be used to enable DT_FLAGS.
> DT_RUNPATH is independent of DT_FLAGS.   We may want DT_RPATH
> with DT_FLAGS.  But there is no way to do that today.

Fair enough.  I also agree with Andreas that --enable-new-dtags is a
badly named option.  --enable-dt-flags and --enable-dt-runpath do
sound like a good idea.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2791 bytes --]

On Tuesday 05 February 2013 00:43:04 Ian Lance Taylor wrote:
> On Mon, Feb 4, 2013 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > This caused:
> > 
> > http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098
> > 
> > I changed BFD linker not set new dtags with -rpath.
> 
> I don't see why that is the right fix.  Since DT_RPATH/DT_RUNPATH are
> only ever set by the linker's -rpath option, it seems like the right
> fix is to always use DT_RPATH and never use DT_RUNPATH.
> 
> Of course, since the only thing --new-dtags does in gold is select
> DT_RUNPATH rather than DT_RPATH, this makes new--dtags completely
> useless in gold.
> 
> It seems that we have made sensible-seeming decisions to wind up in an
> absurd place.  It seems that we should now make --new-dtags a no-op
> and drop all support for generating DT_RUNPATH.  Which makes me wonder
> why DT_RUNPATH was invented in the first place.

sorry, but my main comp hd died, so i've been offline for a while.  it seems 
that the current status is that the linker no longer defaults to --enable-new-
dtags, but bfd still only specifies DT_RUNPATH if the flag is enabled (rather 
than using both).  is that correct ?

DT_RUNPATH is preferable to DT_RPATH because the latter is searched *before* 
LD_LIBRARY_PATH which is bad.  most of the use cases i've seen with rpath fall 
into two categories:
 - people want to generate libraries with a custom path to loadable plugins -- 
DT_RUNPATH works great
 - people want their application to search a local path for all of its libs -- 
DT_RPATH works here w/$ORIGIN
 - people want to install their shared libs into a non-searchable path and 
have their application use it -- DT_RUNPATH works here

i've seen build cases where DT_RPATH actively causes problems when there is a 
version already installed.  they compile their local binary and its shared 
libs, then attempt to use LD_LIBRARY_PATH to force the binary to use the local 
libs.  unfortunately, the DT_RPATH kicks in and loads everything from / 
instead and it falls down.

considering Gentoo has been defaulting to --enable-new-dtags since at least 
2004 and i have yet to see a bug report related to it, i wonder what actually 
broke that caused you to notice this ?  and if it's a minor case, is a better 
answer to tell people to use --disable-new-dtags if they really don't want the 
new DT_RUNPATH behavior ?  seems like the DT_RPATH behavior is the exception 
rather than the rule ... the only thing it has going for it is historical 
precedence.

similarly, i don't think it generally makes sense for libraries to utilize 
DT_RPATH.  dare i suggest that a middle ground might be to default to 
DT_RUNPATH when -shared is in use, and DT_RPATH otherwise ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[H.J. Lu]

On Tue, Feb 19, 2013 at 10:15 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 05 February 2013 00:43:04 Ian Lance Taylor wrote:
>> On Mon, Feb 4, 2013 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > This caused:
>> >
>> > http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098
>> >
>> > I changed BFD linker not set new dtags with -rpath.
>>
>> I don't see why that is the right fix.  Since DT_RPATH/DT_RUNPATH are
>> only ever set by the linker's -rpath option, it seems like the right
>> fix is to always use DT_RPATH and never use DT_RUNPATH.
>>
>> Of course, since the only thing --new-dtags does in gold is select
>> DT_RUNPATH rather than DT_RPATH, this makes new--dtags completely
>> useless in gold.
>>
>> It seems that we have made sensible-seeming decisions to wind up in an
>> absurd place.  It seems that we should now make --new-dtags a no-op
>> and drop all support for generating DT_RUNPATH.  Which makes me wonder
>> why DT_RUNPATH was invented in the first place.
>
> sorry, but my main comp hd died, so i've been offline for a while.  it seems
> that the current status is that the linker no longer defaults to --enable-new-
> dtags, but bfd still only specifies DT_RUNPATH if the flag is enabled (rather
> than using both).  is that correct ?
>
> DT_RUNPATH is preferable to DT_RPATH because the latter is searched *before*
> LD_LIBRARY_PATH which is bad.  most of the use cases i've seen with rpath fall
> into two categories:
>  - people want to generate libraries with a custom path to loadable plugins --
> DT_RUNPATH works great
>  - people want their application to search a local path for all of its libs --
> DT_RPATH works here w/$ORIGIN
>  - people want to install their shared libs into a non-searchable path and
> have their application use it -- DT_RUNPATH works here
>
> i've seen build cases where DT_RPATH actively causes problems when there is a
> version already installed.  they compile their local binary and its shared
> libs, then attempt to use LD_LIBRARY_PATH to force the binary to use the local
> libs.  unfortunately, the DT_RPATH kicks in and loads everything from /
> instead and it falls down.
>
> considering Gentoo has been defaulting to --enable-new-dtags since at least
> 2004 and i have yet to see a bug report related to it, i wonder what actually
> broke that caused you to notice this ?  and if it's a minor case, is a better
> answer to tell people to use --disable-new-dtags if they really don't want the
> new DT_RUNPATH behavior ?  seems like the DT_RPATH behavior is the exception
> rather than the rule ... the only thing it has going for it is historical
> precedence.
>
> similarly, i don't think it generally makes sense for libraries to utilize
> DT_RPATH.  dare i suggest that a middle ground might be to default to
> DT_RUNPATH when -shared is in use, and DT_RPATH otherwise ?
> -mike

Since DT_RPATH != DT_RUNPATH. we need a new option to
specify DT_RUNPATH.

-- 
H.J.

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 3801 bytes --]

On Wednesday 20 February 2013 12:16:51 H.J. Lu wrote:
> On Tue, Feb 19, 2013 at 10:15 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Tuesday 05 February 2013 00:43:04 Ian Lance Taylor wrote:
> >> On Mon, Feb 4, 2013 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >> > This caused:
> >> > 
> >> > http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098
> >> > 
> >> > I changed BFD linker not set new dtags with -rpath.
> >> 
> >> I don't see why that is the right fix.  Since DT_RPATH/DT_RUNPATH are
> >> only ever set by the linker's -rpath option, it seems like the right
> >> fix is to always use DT_RPATH and never use DT_RUNPATH.
> >> 
> >> Of course, since the only thing --new-dtags does in gold is select
> >> DT_RUNPATH rather than DT_RPATH, this makes new--dtags completely
> >> useless in gold.
> >> 
> >> It seems that we have made sensible-seeming decisions to wind up in an
> >> absurd place.  It seems that we should now make --new-dtags a no-op
> >> and drop all support for generating DT_RUNPATH.  Which makes me wonder
> >> why DT_RUNPATH was invented in the first place.
> > 
> > sorry, but my main comp hd died, so i've been offline for a while.  it
> > seems that the current status is that the linker no longer defaults to
> > --enable-new- dtags, but bfd still only specifies DT_RUNPATH if the flag
> > is enabled (rather than using both).  is that correct ?
> > 
> > DT_RUNPATH is preferable to DT_RPATH because the latter is searched
> > *before* LD_LIBRARY_PATH which is bad.  most of the use cases i've seen
> > with rpath fall
> > 
> > into two categories:
> >  - people want to generate libraries with a custom path to loadable
> >  plugins --
> > 
> > DT_RUNPATH works great
> > 
> >  - people want their application to search a local path for all of its
> >  libs --
> > 
> > DT_RPATH works here w/$ORIGIN
> > 
> >  - people want to install their shared libs into a non-searchable path
> >  and
> > 
> > have their application use it -- DT_RUNPATH works here
> > 
> > i've seen build cases where DT_RPATH actively causes problems when there
> > is a version already installed.  they compile their local binary and its
> > shared libs, then attempt to use LD_LIBRARY_PATH to force the binary to
> > use the local libs.  unfortunately, the DT_RPATH kicks in and loads
> > everything from / instead and it falls down.
> > 
> > considering Gentoo has been defaulting to --enable-new-dtags since at
> > least 2004 and i have yet to see a bug report related to it, i wonder
> > what actually broke that caused you to notice this ?  and if it's a
> > minor case, is a better answer to tell people to use --disable-new-dtags
> > if they really don't want the new DT_RUNPATH behavior ?  seems like the
> > DT_RPATH behavior is the exception rather than the rule ... the only
> > thing it has going for it is historical precedence.
> > 
> > similarly, i don't think it generally makes sense for libraries to
> > utilize DT_RPATH.  dare i suggest that a middle ground might be to
> > default to DT_RUNPATH when -shared is in use, and DT_RPATH otherwise ?
> 
> Since DT_RPATH != DT_RUNPATH. we need a new option to
> specify DT_RUNPATH.

i'm aware "DT_RPATH != DT_RUNPATH" is a true statement.  however, my point 
still stands that for the majority of cases, people want runpath tags to 
specify custom paths for loading libraries and in that regard, DT_RUNPATH is 
the same as DT_RPATH.  imo, the example you posted is the exception rather 
than the rule when it comes to expected behavior and already works w/the 
patches i posted -- if you want that behavior, use -rpath --disable-new-dtags.  
hence the idea is to improve the default rather than requiring everyone to 
change flags.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Cary Coutant]

Where did we end up on this issue? As far as I can tell, the
discussion just died with no clear consensus, and I'm still not sure
what changes I should make to gold.

It sounds like the community really isn't ready to move away from
DT_RPATH, but I'm not sure why. As I understand it, there are two
differences between DT_RPATH and DT_RUNPATH:

(1) DT_RUNPATH is searched after LD_LIBRARY_PATH, while DT_RPATH is
searched before. The DT_RUNPATH behavior makes sense to me -- having
an environment variable that can't override the search path embedded
in the binary seems useless.

(2) DT_RUNPATH is used only to search for direct dependencies of the
object it's contained in, while DT_RPATH is used for indirect
dependencies as well. (This difference is *not* mentioned in the
gABI.) Again, the DT_RUNPATH behavior makes sense -- an object
shouldn't have any awareness of its indirect dependencies, which may
change over time.

It seems to me that the cases where DT_RUNPATH causes breakage are
broken as intended, and the second-level libraries that depend on
third-level libraries should have an embedded DT_RUNPATH that says
where to find them. Of course, since they've been working for so long,
we need an option to allow them to continue working, but I don't see
why we shouldn't change the default.

In gold, --enable-new-dtags applies only to DT_RUNPATH vs. DT_RPATH.
We always generate DT_FLAGS, but also DT_TEXTREL and DT_SYMBOLIC when
those bits are set. I have no problem with renaming the option to
--{en,dis}able-dt-runpath.

-cary


On Wed, Feb 20, 2013 at 10:40 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 20 February 2013 12:16:51 H.J. Lu wrote:
>> On Tue, Feb 19, 2013 at 10:15 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> > On Tuesday 05 February 2013 00:43:04 Ian Lance Taylor wrote:
>> >> On Mon, Feb 4, 2013 at 5:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >> > This caused:
>> >> >
>> >> > http://www.sourceware.org/bugzilla/show_bug.cgi?id=15098
>> >> >
>> >> > I changed BFD linker not set new dtags with -rpath.
>> >>
>> >> I don't see why that is the right fix.  Since DT_RPATH/DT_RUNPATH are
>> >> only ever set by the linker's -rpath option, it seems like the right
>> >> fix is to always use DT_RPATH and never use DT_RUNPATH.
>> >>
>> >> Of course, since the only thing --new-dtags does in gold is select
>> >> DT_RUNPATH rather than DT_RPATH, this makes new--dtags completely
>> >> useless in gold.
>> >>
>> >> It seems that we have made sensible-seeming decisions to wind up in an
>> >> absurd place.  It seems that we should now make --new-dtags a no-op
>> >> and drop all support for generating DT_RUNPATH.  Which makes me wonder
>> >> why DT_RUNPATH was invented in the first place.
>> >
>> > sorry, but my main comp hd died, so i've been offline for a while.  it
>> > seems that the current status is that the linker no longer defaults to
>> > --enable-new- dtags, but bfd still only specifies DT_RUNPATH if the flag
>> > is enabled (rather than using both).  is that correct ?
>> >
>> > DT_RUNPATH is preferable to DT_RPATH because the latter is searched
>> > *before* LD_LIBRARY_PATH which is bad.  most of the use cases i've seen
>> > with rpath fall
>> >
>> > into two categories:
>> >  - people want to generate libraries with a custom path to loadable
>> >  plugins --
>> >
>> > DT_RUNPATH works great
>> >
>> >  - people want their application to search a local path for all of its
>> >  libs --
>> >
>> > DT_RPATH works here w/$ORIGIN
>> >
>> >  - people want to install their shared libs into a non-searchable path
>> >  and
>> >
>> > have their application use it -- DT_RUNPATH works here
>> >
>> > i've seen build cases where DT_RPATH actively causes problems when there
>> > is a version already installed.  they compile their local binary and its
>> > shared libs, then attempt to use LD_LIBRARY_PATH to force the binary to
>> > use the local libs.  unfortunately, the DT_RPATH kicks in and loads
>> > everything from / instead and it falls down.
>> >
>> > considering Gentoo has been defaulting to --enable-new-dtags since at
>> > least 2004 and i have yet to see a bug report related to it, i wonder
>> > what actually broke that caused you to notice this ?  and if it's a
>> > minor case, is a better answer to tell people to use --disable-new-dtags
>> > if they really don't want the new DT_RUNPATH behavior ?  seems like the
>> > DT_RPATH behavior is the exception rather than the rule ... the only
>> > thing it has going for it is historical precedence.
>> >
>> > similarly, i don't think it generally makes sense for libraries to
>> > utilize DT_RPATH.  dare i suggest that a middle ground might be to
>> > default to DT_RUNPATH when -shared is in use, and DT_RPATH otherwise ?
>>
>> Since DT_RPATH != DT_RUNPATH. we need a new option to
>> specify DT_RUNPATH.
>
> i'm aware "DT_RPATH != DT_RUNPATH" is a true statement.  however, my point
> still stands that for the majority of cases, people want runpath tags to
> specify custom paths for loading libraries and in that regard, DT_RUNPATH is
> the same as DT_RPATH.  imo, the example you posted is the exception rather
> than the rule when it comes to expected behavior and already works w/the
> patches i posted -- if you want that behavior, use -rpath --disable-new-dtags.
> hence the idea is to improve the default rather than requiring everyone to
> change flags.
> -mike

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Joseph S. Myers]

On Tue, 4 Feb 2014, Cary Coutant wrote:

> (2) DT_RUNPATH is used only to search for direct dependencies of the
> object it's contained in, while DT_RPATH is used for indirect
> dependencies as well. (This difference is *not* mentioned in the
> gABI.) Again, the DT_RUNPATH behavior makes sense -- an object
> shouldn't have any awareness of its indirect dependencies, which may
> change over time.
> 
> It seems to me that the cases where DT_RUNPATH causes breakage are
> broken as intended, and the second-level libraries that depend on
> third-level libraries should have an embedded DT_RUNPATH that says
> where to find them. Of course, since they've been working for so long,

Say you have a sysroot containing what's eventually intended to be some 
system's root filesystem.  None of the libraries therein should have RPATH 
or RUNPATH set - paths on the host or the system used for testing wouldn't 
work on the final target system, and on the final system the libraries 
will be in standard directories such as /usr/lib.  To build a test binary 
that uses the sysroot before it's installed in a root filesystem, you use 
-Wl,-dynamic-linker,/sysroot/<ld.so path> 
-Wl,-rpath,/sysroot/lib:/sysroot/usr/lib.  For this (which is the main 
purpose I use -rpath for at all, building programs to use non-installed 
libraries) to work reliably relies on all dependencies being found via 
that RPATH, both direct and indirect.  I consider this part of the 
semantics of the -rpath option; that is, generating DT_RUNPATH should be a 
different option such as -runpath.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Cary Coutant]

>> It seems to me that the cases where DT_RUNPATH causes breakage are
>> broken as intended, and the second-level libraries that depend on
>> third-level libraries should have an embedded DT_RUNPATH that says
>> where to find them. Of course, since they've been working for so long,
>
> Say you have a sysroot containing what's eventually intended to be some
> system's root filesystem.  None of the libraries therein should have RPATH
> or RUNPATH set - paths on the host or the system used for testing wouldn't
> work on the final target system, and on the final system the libraries
> will be in standard directories such as /usr/lib.  To build a test binary
> that uses the sysroot before it's installed in a root filesystem, you use
> -Wl,-dynamic-linker,/sysroot/<ld.so path>
> -Wl,-rpath,/sysroot/lib:/sysroot/usr/lib.  For this (which is the main
> purpose I use -rpath for at all, building programs to use non-installed
> libraries) to work reliably relies on all dependencies being found via
> that RPATH, both direct and indirect.  I consider this part of the
> semantics of the -rpath option; that is, generating DT_RUNPATH should be a
> different option such as -runpath.

Why is LD_LIBRARY_PATH not the right way to do this? For the same
reason that you don't want to build the libraries in the sysroot
specially, I'd think that you'd want to build test binaries exactly
the way they'd be built on the test system, rather than build them
with a special linker option.

For this scenario, maybe what we really need is an LD_SYSROOT
environment variable. When the dynamic linker starts up, if it's not
under the sysroot, it could re-exec the sysrooted dynamic linker, and
all library searches would be subject to sysroot substitution.

-cary

Re: [PATCH 2/2 v5] gold: enable new dtags by default

[Joseph S. Myers]

On Wed, 5 Feb 2014, Cary Coutant wrote:

> Why is LD_LIBRARY_PATH not the right way to do this? For the same

I don't think there is just one "the right way" to do this; I just think 
that supporting this is part of the semantics of -rpath.  (But in general, 
for some purposes a binary that uses the sysroot libraries without needing 
anything special in its environment is better than a binary that will only 
work when run with an environment variable set.)

-- 
Joseph S. Myers
joseph@codesourcery.com