[PATCH 0/2] elf: Implement single global definition marker

[PATCH 0/2] elf: Implement single global definition marker

[H.J. Lu]

On systems with copy relocation:
* A copy in executable is created for the definition in a shared library
at run-time by ld.so.
* The copy is referenced by executable and shared libraries.
* Executable can access the copy directly.

Issues are:
* Overhead of a copy, time and space, may be visible at run-time.
* Read-only data in the shared library becomes read-write copy in
executable at run-time.
* Local access to data with the STV_PROTECTED visibility in the shared
library must use GOT.

On systems without function descriptor, function pointers vary depending
on where and how the functions are defined.
* If the function is defined in executable, it can be the address of
function body.
* If the function, including the function with STV_PROTECTED visibility,
is defined in the shared library, it can be the address of the PLT entry
in executable or shared library.

Issues are:
* The address of function body may not be used as its function pointer.
* ld.so needs to search loaded shared libraries for the function pointer
of the function with STV_PROTECTED visibility.

Here is a proposal to remove copy relocation and use canonical function
pointer:

1. Accesses, including in PIE and non-PIE, to undefined symbols must
use GOT.
  a. Linker may optimize out GOT access if the data is defined in PIE or
  non-PIE.
2. Read-only data in the shared library remain read-only at run-time
3. Address of global data with the STV_PROTECTED visibility in the shared
library is the address of data body.
  a. Can use IP-relative access.
  b. May need GOT without IP-relative access.
4. For systems without function descriptor,
  a. All global function pointers of undefined functions in PIE and
  non-PIE must use GOT.  Linker may optimize out GOT access if the
  function is defined in PIE or non-PIE.
  b. Function pointer of functions with the STV_PROTECTED visibility in
  executable and shared library is the address of function body.
   i. Can use IP-relative access.
   ii. May need GOT without IP-relative access.
   iii. Branches to undefined functions may use PLT.
5. Single global definition marker:

Add GNU_PROPERTY_1_NEEDED:

#define GNU_PROPERTY_1_NEEDED GNU_PROPERTY_UINT32_OR_LO

to indicate the needed properties by the object file.

Add GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION:

#define GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION (1U << 0)

to indicate that the object file requires canonical function pointers and
cannot be used with copy relocation.

  a. Copy relocation should be disallowed at link-time and run-time.
  b. Canonical function pointers are required at link-time and run-tima

Linker change:

If any relocatable input files contain the single global definition
marker:
* Generate the single global definition marker in output.
* Avoid copy relocation if possible.
* Access to symbols with the STV_PROTECTED visibility is the same as
local access.
* For systems without function descriptor, function pointer is the address
of function body.

H.J. Lu (2):
  elf: Add GNU_PROPERTY_1_NEEDED
  elf: Add GNU_PROPERTY_1_NEEDED check

 bfd/elf-properties.c                          | 103 ++++++++++++--
 bfd/elflink.c                                 |   4 +
 binutils/readelf.c                            |  39 ++++++
 include/bfdlink.h                             |   6 +
 include/elf/common.h                          |   7 +
 ld/NEWS                                       |   3 +
 ld/emultempl/elf.em                           |   4 +
 ld/ld.texi                                    |  12 ++
 ld/ldmain.c                                   |   1 +
 ld/lexsup.c                                   |   5 +
 ld/testsuite/ld-elf/property-1_needed-1.s     |  15 ++
 ld/testsuite/ld-elf/property-1_needed-1a.d    |  17 +++
 ld/testsuite/ld-elf/property-1_needed-1b.d    |  16 +++
 ld/testsuite/ld-elf/property-1_needed-1c.d    |  17 +++
 ld/testsuite/ld-x86-64/protected-data-1.h     |  11 ++
 ld/testsuite/ld-x86-64/protected-data-1a.c    |  40 ++++++
 ld/testsuite/ld-x86-64/protected-data-1b.c    |  59 ++++++++
 ld/testsuite/ld-x86-64/protected-data-2a.S    | 109 +++++++++++++++
 ld/testsuite/ld-x86-64/protected-data-2b.S    | 119 ++++++++++++++++
 ld/testsuite/ld-x86-64/protected-func-2a.S    |  68 +++++++++
 ld/testsuite/ld-x86-64/protected-func-2b.S    |  83 +++++++++++
 ld/testsuite/ld-x86-64/protected-func-2c.c    |  29 ++++
 .../ld-x86-64/single-global-definition.rd     |   6 +
 ld/testsuite/ld-x86-64/x86-64.exp             | 131 ++++++++++++++++++
 24 files changed, 889 insertions(+), 15 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1.s
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1a.d
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1b.d
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1c.d
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1.h
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1a.c
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1b.c
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-2a.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-2b.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2a.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2b.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2c.c
 create mode 100644 ld/testsuite/ld-x86-64/single-global-definition.rd

-- 
2.31.1

[PATCH 1/2] elf: Add GNU_PROPERTY_1_NEEDED

[H.J. Lu]

Add GNU_PROPERTY_1_NEEDED:

 #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO

to indicate the needed properties by the object file.

Add GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION:

 #define GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION  (1U << 0)

to indicate that the object file requires canonical function pointers and
cannot be used with copy relocation.

binutils/

	* readelf.c (decode_1_needed): New.
	(print_gnu_property_note): Handle GNU_PROPERTY_1_NEEDED.

include/

	* elf/common.h (GNU_PROPERTY_1_NEEDED): New.
	(GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION): Likewise.

ld/

	* testsuite/ld-elf/property-1_needed-1a.d: New file.
	* testsuite/ld-elf/property-1_needed-1.s: Likewise.
---
 binutils/readelf.c                         | 39 ++++++++++++++++++++++
 include/elf/common.h                       |  7 ++++
 ld/testsuite/ld-elf/property-1_needed-1.s  | 15 +++++++++
 ld/testsuite/ld-elf/property-1_needed-1a.d | 17 ++++++++++
 4 files changed, 78 insertions(+)
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1.s
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1a.d

diff --git a/binutils/readelf.c b/binutils/readelf.c
index f7c64329f37..505d3685670 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -19319,6 +19319,28 @@ decode_aarch64_feature_1_and (unsigned int bitmask)
     }
 }
 
+static void
+decode_1_needed (unsigned int bitmask)
+{
+  while (bitmask)
+    {
+      unsigned int bit = bitmask & (- bitmask);
+
+      bitmask &= ~ bit;
+      switch (bit)
+	{
+	case GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION:
+	  printf ("single global definition");
+	  break;
+	default:
+	  printf (_("<unknown: %x>"), bit);
+	  break;
+	}
+      if (bitmask)
+	printf (", ");
+    }
+}
+
 static void
 print_gnu_property_note (Filedata * filedata, Elf_Internal_Note * pnote)
 {
@@ -19512,6 +19534,23 @@ print_gnu_property_note (Filedata * filedata, Elf_Internal_Note * pnote)
 		  || (type >= GNU_PROPERTY_UINT32_OR_LO
 		      && type <= GNU_PROPERTY_UINT32_OR_HI))
 		{
+		  switch (type)
+		    {
+		    case GNU_PROPERTY_1_NEEDED:
+		      if (datasz != 4)
+			printf (_("1_needed: <corrupt length: %#x> "),
+				datasz);
+		      else
+			{
+			  unsigned int bitmask = byte_get (ptr, 4);
+			  printf ("1_needed: ");
+			  decode_1_needed (bitmask);
+			}
+		      goto next;
+
+		    default:
+		      break;
+		    }
 		  if (type <= GNU_PROPERTY_UINT32_AND_HI)
 		    printf (_("UINT32_AND (%#x): "), type);
 		  else
diff --git a/include/elf/common.h b/include/elf/common.h
index 0cca28673dd..f352d2a28cb 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -792,6 +792,13 @@
 #define GNU_PROPERTY_UINT32_OR_LO	0xb0008000
 #define GNU_PROPERTY_UINT32_OR_HI	0xb000ffff
 
+/* The needed properties by the object file.  */
+#define GNU_PROPERTY_1_NEEDED		GNU_PROPERTY_UINT32_OR_LO
+
+/* Set if the object file requires canonical function pointers and
+   cannot be used with copy relocation.  */
+#define GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION	(1U << 0)
+
 /* Processor-specific semantics, lo */
 #define GNU_PROPERTY_LOPROC  0xc0000000
 /* Processor-specific semantics, hi */
diff --git a/ld/testsuite/ld-elf/property-1_needed-1.s b/ld/testsuite/ld-elf/property-1_needed-1.s
new file mode 100644
index 00000000000..5d5cdc87744
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-1_needed-1.s
@@ -0,0 +1,15 @@
+	.section ".note.gnu.property", "a"
+	.p2align ALIGN
+	.long 1f - 0f		/* name length */
+	.long 5f - 2f		/* data length */
+	.long 5			/* note type */
+0:	.asciz "GNU"		/* vendor name */
+1:
+	.p2align ALIGN
+2:	.long 0xb0008000	/* pr_type.  */
+	.long 4f - 3f		/* pr_datasz.  */
+3:
+	.long 0x3
+4:
+	.p2align ALIGN
+5:
diff --git a/ld/testsuite/ld-elf/property-1_needed-1a.d b/ld/testsuite/ld-elf/property-1_needed-1a.d
new file mode 100644
index 00000000000..272a0997cd5
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-1_needed-1a.d
@@ -0,0 +1,17 @@
+#source: empty.s
+#source: property-1_needed-1.s
+#as:
+#ld: -shared
+#readelf: -n
+#xfail: ![check_shared_lib_support]
+#notarget: am33_2.0-*-* hppa*-*-hpux* mn10300-*-*
+# Assembly source file for the HPPA assembler is renamed and modifed by
+# sed.  mn10300 has relocations in .note.gnu.property section which
+# elf_parse_notes doesn't support.
+
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: 1_needed: single global definition, <unknown: 2>
+#pass
-- 
2.31.1

[PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
relocatable files:

1. Don't generate copy relocations.
2. Turn off extern_protected_data.
3. Treate reference to protected symbols with single global definition
as local.
4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
5. Add -z [no]single-global-definition to control single global definition.

bfd/

	* elf-properties.c (_bfd_elf_link_setup_gnu_properties): Handle
	GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION for
	-z single-global-definition.  Set nocopyreloc to true and
	extern_protected_data to false for single global definiton.
	* elflink.c (_bfd_elf_symbol_refs_local_p): Return true for
	STV_PROTECTED symbols with single global definition.

include/

	* bfdlink.h (bfd_link_info): Add single_global_definition.

ld/

	* NEWS: Mention -z [no]single-global-definition.
	* ld.texi: Document -z [no]single-global-definition.
	* ldmain.c (main): Initialize link_info.single_global_definition
	to -1.
	* lexsup.c (elf_shlib_list_options): Add
	-z [no]single-global-definition.
	* emultempl/elf.em (gld${EMULATION_NAME}_handle_optio): Handle
	-z [no]single-global-definition.
	* testsuite/ld-elf/property-1_needed-1b.d: New file.
	* testsuite/ld-elf/property-1_needed-1c.d: Likewise.
	* testsuite/ld-x86-64/protected-data-1.h: Likewise.
	* testsuite/ld-x86-64/protected-data-1a.c: Likewise.
	* testsuite/ld-x86-64/protected-data-1b.c: Likewise.
	* testsuite/ld-x86-64/protected-data-2a.S: Likewise.
	* testsuite/ld-x86-64/protected-data-2b.S: Likewise.
	* testsuite/ld-x86-64/protected-func-2a.S: Likewise.
	* testsuite/ld-x86-64/protected-func-2b.S: Likewise.
	* testsuite/ld-x86-64/protected-func-2c.c: Likewise.
	* testsuite/ld-x86-64/single-global-definition.rd: Likewise.
	* testsuite/ld-x86-64/x86-64.exp: Run tests for protected
	function and data with single global definition.
---
 bfd/elf-properties.c                          | 103 ++++++++++++--
 bfd/elflink.c                                 |   4 +
 include/bfdlink.h                             |   6 +
 ld/NEWS                                       |   3 +
 ld/emultempl/elf.em                           |   4 +
 ld/ld.texi                                    |  12 ++
 ld/ldmain.c                                   |   1 +
 ld/lexsup.c                                   |   5 +
 ld/testsuite/ld-elf/property-1_needed-1b.d    |  16 +++
 ld/testsuite/ld-elf/property-1_needed-1c.d    |  17 +++
 ld/testsuite/ld-x86-64/protected-data-1.h     |  11 ++
 ld/testsuite/ld-x86-64/protected-data-1a.c    |  40 ++++++
 ld/testsuite/ld-x86-64/protected-data-1b.c    |  59 ++++++++
 ld/testsuite/ld-x86-64/protected-data-2a.S    | 109 +++++++++++++++
 ld/testsuite/ld-x86-64/protected-data-2b.S    | 119 ++++++++++++++++
 ld/testsuite/ld-x86-64/protected-func-2a.S    |  68 +++++++++
 ld/testsuite/ld-x86-64/protected-func-2b.S    |  83 +++++++++++
 ld/testsuite/ld-x86-64/protected-func-2c.c    |  29 ++++
 .../ld-x86-64/single-global-definition.rd     |   6 +
 ld/testsuite/ld-x86-64/x86-64.exp             | 131 ++++++++++++++++++
 20 files changed, 811 insertions(+), 15 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1b.d
 create mode 100644 ld/testsuite/ld-elf/property-1_needed-1c.d
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1.h
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1a.c
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-1b.c
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-2a.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-data-2b.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2a.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2b.S
 create mode 100644 ld/testsuite/ld-x86-64/protected-func-2c.c
 create mode 100644 ld/testsuite/ld-x86-64/single-global-definition.rd

diff --git a/bfd/elf-properties.c b/bfd/elf-properties.c
index 56ee91d7786..50c20964006 100644
--- a/bfd/elf-properties.c
+++ b/bfd/elf-properties.c
@@ -598,7 +598,7 @@ elf_write_gnu_properties (bfd *abfd, bfd_byte *contents,
 bfd *
 _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
 {
-  bfd *abfd, *first_pbfd = NULL;
+  bfd *abfd, *first_pbfd = NULL, *elf_bfd = NULL;
   elf_property_list *list;
   asection *sec;
   bool has_properties = false;
@@ -606,32 +606,75 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
     = get_elf_backend_data (info->output_bfd);
   unsigned int elfclass = bed->s->elfclass;
   int elf_machine_code = bed->elf_machine_code;
+  elf_property *p;
 
   /* Find the first relocatable ELF input with GNU properties.  */
   for (abfd = info->input_bfds; abfd != NULL; abfd = abfd->link.next)
     if (bfd_get_flavour (abfd) == bfd_target_elf_flavour
 	&& (abfd->flags & DYNAMIC) == 0
-	&& elf_properties (abfd) != NULL)
+	&& (elf_machine_code
+	    == get_elf_backend_data (abfd)->elf_machine_code)
+	&& (elfclass == get_elf_backend_data (abfd)->s->elfclass))
       {
-	has_properties = true;
-
 	/* Ignore GNU properties from ELF objects with different machine
 	   code or class.  Also skip objects without a GNU_PROPERTY note
 	   section.  */
-	if ((elf_machine_code
-	     == get_elf_backend_data (abfd)->elf_machine_code)
-	    && (elfclass
-		== get_elf_backend_data (abfd)->s->elfclass)
-	    && bfd_get_section_by_name (abfd,
-					NOTE_GNU_PROPERTY_SECTION_NAME) != NULL
-	    )
+	elf_bfd = abfd;
+
+	if (elf_properties (abfd) != NULL)
 	  {
-	    /* Keep .note.gnu.property section in FIRST_PBFD.  */
-	    first_pbfd = abfd;
-	    break;
+	    has_properties = true;
+
+	    if (bfd_get_section_by_name (abfd,
+					 NOTE_GNU_PROPERTY_SECTION_NAME)
+		!= NULL)
+	      {
+		/* Keep .note.gnu.property section in FIRST_PBFD.  */
+		first_pbfd = abfd;
+		break;
+	      }
 	  }
       }
 
+  if (info->single_global_definition > 0 && elf_bfd != NULL)
+    {
+      /* Support -z single-global-definition.  */
+      if (first_pbfd == NULL)
+	{
+	  sec = bfd_make_section_with_flags (elf_bfd,
+					     NOTE_GNU_PROPERTY_SECTION_NAME,
+					     (SEC_ALLOC
+					      | SEC_LOAD
+					      | SEC_IN_MEMORY
+					      | SEC_READONLY
+					      | SEC_HAS_CONTENTS
+					      | SEC_DATA));
+	  if (sec == NULL)
+	    info->callbacks->einfo (_("%F%P: failed to create GNU property section\n"));
+
+	  if (!bfd_set_section_alignment (sec,
+					  elfclass == ELFCLASS64 ? 3 : 2))
+	    info->callbacks->einfo (_("%F%pA: failed to align section\n"),
+				    sec);
+
+	  elf_section_type (sec) = SHT_NOTE;
+	  first_pbfd = elf_bfd;
+	  has_properties = true;
+	}
+
+      p = _bfd_elf_get_property (first_pbfd, GNU_PROPERTY_1_NEEDED, 4);
+      if (p->pr_kind == property_unknown)
+	{
+	  /* Create GNU_PROPERTY_1_NEEDED.  */
+	  p->u.number
+	    = GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION;
+	  p->pr_kind = property_number;
+	}
+      else
+	p->u.number
+	  |= GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION;
+    }
+
   /* Do nothing if there is no .note.gnu.property section.  */
   if (!has_properties)
     return NULL;
@@ -695,7 +738,6 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
 	 if N > 0.  */
       if (info->stacksize > 0)
 	{
-	  elf_property *p;
 	  bfd_vma stacksize = info->stacksize;
 
 	  p = _bfd_elf_get_property (first_pbfd, GNU_PROPERTY_STACK_SIZE,
@@ -737,6 +779,29 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
       sec->size = size;
       contents = (bfd_byte *) bfd_zalloc (first_pbfd, size);
 
+      if (info->single_global_definition <= 0)
+	{
+	  /* Get GNU_PROPERTY_1_NEEDED properties.  */
+	  p = elf_find_and_remove_property (&elf_properties (first_pbfd),
+					    GNU_PROPERTY_1_NEEDED, false);
+	  if (p != NULL)
+	    {
+	      if (info->single_global_definition < 0)
+		{
+		  /* Turn on single global definition based on input
+		     properties.  */
+		  if ((p->u.number
+		       & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION)
+		      != 0)
+		    info->single_global_definition = 1;
+		}
+	      else
+		/* Turn off single global definition.  */
+		p->u.number
+		  &= ~GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION;
+	    }
+	}
+
       elf_write_gnu_properties (first_pbfd, contents, list, size,
 				align_size);
 
@@ -747,6 +812,14 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
 	 symbol is defined in the shared object.  */
       if (elf_has_no_copy_on_protected (first_pbfd))
 	info->extern_protected_data = false;
+
+      if (info->single_global_definition > 0)
+	{
+	  /* For single global definition, don't generate copy
+	     relocations.  */
+	  info->nocopyreloc = true;
+	  info->extern_protected_data = false;
+	}
     }
 
   return first_pbfd;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9a05208253c..21a77bf67bd 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3331,6 +3331,10 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
   if (!is_elf_hash_table (&hash_table->root))
     return true;
 
+  /* STV_PROTECTED symbols with single global definition are local. */
+  if (info->single_global_definition > 0)
+    return true;
+
   bed = get_elf_backend_data (hash_table->dynobj);
 
   /* If extern_protected_data is false, STV_PROTECTED non-function
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 7f1b12dbf37..815780091c5 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -639,6 +639,12 @@ struct bfd_link_info
      the backend decide.  */
   int dynamic_undefined_weak;
 
+  /* GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION control:
+       > 0: Turn on.
+         0: Turn off.
+       < 0: Turn on if it is set on any inputs.  */
+  int single_global_definition;
+
   /* Non-zero if auto-import thunks for DATA items in pei386 DLLs
      should be generated/linked against.  Set to 1 if this feature
      is explicitly requested by the user, -1 if enabled by default.  */
diff --git a/ld/NEWS b/ld/NEWS
index a5ed9058c04..aa74d4fd6a0 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Add -z single-global-definition/-z nosingle-global-definition to control
+  canonical function pointers and copy relocation.
+
 * arm-symbianelf support removed.
 
 * Add -z report-relative-reloc to x86 ELF linker to report dynamic
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index bfaf8130a3e..cdf5b26621a 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -848,6 +848,10 @@ fragment <<EOF
 	link_info.textrel_check = textrel_check_none;
       else if (strcmp (optarg, "textoff") == 0)
 	link_info.textrel_check = textrel_check_none;
+      else if (strcmp (optarg, "single-global-definition") == 0)
+	link_info.single_global_definition = 1;
+      else if (strcmp (optarg, "nosingle-global-definition") == 0)
+	link_info.single_global_definition = 0;
 EOF
 fi
 
diff --git a/ld/ld.texi b/ld/ld.texi
index dd8f571d4e4..b847e78ee7f 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1442,6 +1442,18 @@ Generate GNU_PROPERTY_X86_FEATURE_1_SHSTK in .note.gnu.property section
 to indicate compatibility with Intel Shadow Stack.  Supported for
 Linux/i386 and Linux/x86_64.
 
+@item single-global-definition
+@itemx nosingle-global-definition
+Generate GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION in
+.note.gnu.property section to indicate that object file requires
+canonical function pointers and cannot be used with copy relocation.
+This option also implies @option{noextern-protected-data} and
+@option{nocopyreloc}.
+
+@option{nosingle-global-definition} removes
+GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION from .note.gnu.property
+section.
+
 @item stack-size=@var{value}
 Specify a stack size for an ELF @code{PT_GNU_STACK} segment.
 Specifying zero will override any default non-zero sized
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 42660eb9a3c..99a1b0cf5f3 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -346,6 +346,7 @@ main (int argc, char **argv)
   link_info.relax_pass = 1;
   link_info.extern_protected_data = -1;
   link_info.dynamic_undefined_weak = -1;
+  link_info.single_global_definition = -1;
   link_info.pei386_auto_import = -1;
   link_info.spare_dynamic_tags = 5;
   link_info.path_separator = ':';
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 00274c500d0..abfc3766c7c 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2100,6 +2100,11 @@ elf_shlib_list_options (FILE *file)
       fprintf (file, _("\
   -z textoff                  Don't treat DT_TEXTREL in output as error\n"));
     }
+  fprintf (file, _("\
+  -z single-global-definition Enable single global definition\n"));
+  fprintf (file, _("\
+  -z nosingle-global-definition\n\
+                              Disable single global definition\n"));
 }
 
 static void
diff --git a/ld/testsuite/ld-elf/property-1_needed-1b.d b/ld/testsuite/ld-elf/property-1_needed-1b.d
new file mode 100644
index 00000000000..2e2517f0d4f
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-1_needed-1b.d
@@ -0,0 +1,16 @@
+#source: empty.s
+#as:
+#ld: -shared -z single-global-definition
+#readelf: -n
+#xfail: ![check_shared_lib_support]
+#notarget: am33_2.0-*-* hppa*-*-hpux* mn10300-*-*
+# Assembly source file for the HPPA assembler is renamed and modifed by
+# sed.  mn10300 has relocations in .note.gnu.property section which
+# elf_parse_notes doesn't support.
+
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: 1_needed: single global definition
+#pass
diff --git a/ld/testsuite/ld-elf/property-1_needed-1c.d b/ld/testsuite/ld-elf/property-1_needed-1c.d
new file mode 100644
index 00000000000..d26bce62edd
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-1_needed-1c.d
@@ -0,0 +1,17 @@
+#source: empty.s
+#source: property-1_needed-1.s
+#as:
+#ld: -shared -z nosingle-global-definition
+#readelf: -n
+#xfail: ![check_shared_lib_support]
+#notarget: am33_2.0-*-* hppa*-*-hpux* mn10300-*-*
+# Assembly source file for the HPPA assembler is renamed and modifed by
+# sed.  mn10300 has relocations in .note.gnu.property section which
+# elf_parse_notes doesn't support.
+
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: 1_needed: <unknown: 2>
+#pass
diff --git a/ld/testsuite/ld-x86-64/protected-data-1.h b/ld/testsuite/ld-x86-64/protected-data-1.h
new file mode 100644
index 00000000000..a80c9769d37
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-data-1.h
@@ -0,0 +1,11 @@
+extern int protected_data_1a;
+extern int protected_data_1b;
+
+extern int *protected_data_1a_p ();
+extern int *protected_data_1b_p ();
+
+extern void set_protected_data_1a (int);
+extern void set_protected_data_1b (int);
+
+extern int check_protected_data_1a (int);
+extern int check_protected_data_1b (int);
diff --git a/ld/testsuite/ld-x86-64/protected-data-1a.c b/ld/testsuite/ld-x86-64/protected-data-1a.c
new file mode 100644
index 00000000000..6942426ecb3
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-data-1a.c
@@ -0,0 +1,40 @@
+#include "protected-data-1.h"
+
+int protected_data_1a __attribute__ ((visibility("protected"))) = 1;
+int protected_data_1b __attribute__ ((visibility("protected"))) = 2;
+
+int *
+protected_data_1a_p (void)
+{
+  return &protected_data_1a;
+}
+
+int *
+protected_data_1b_p (void)
+{
+  return &protected_data_1b;
+}
+
+void
+set_protected_data_1a (int i)
+{
+  protected_data_1a = i;
+}
+
+void
+set_protected_data_1b (int i)
+{
+  protected_data_1b = i;
+}
+
+int
+check_protected_data_1a (int i)
+{
+  return protected_data_1a == i ? 0 : 1;
+}
+
+int
+check_protected_data_1b (int i)
+{
+  return protected_data_1b == i ? 0 : 1;
+}
diff --git a/ld/testsuite/ld-x86-64/protected-data-1b.c b/ld/testsuite/ld-x86-64/protected-data-1b.c
new file mode 100644
index 00000000000..a4756ee08da
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-data-1b.c
@@ -0,0 +1,59 @@
+#include <stdio.h>
+
+#include "protected-data-1.h"
+
+int protected_data_1b = 3;
+
+int
+main (void)
+{
+  int res = 0;
+
+  /* Check if we get the same address for the protected data symbol.  */
+  if (&protected_data_1a != protected_data_1a_p ())
+    {
+      puts ("'protected_data_1a' in main and shared library doesn't have same address");
+      res = 1;
+    }
+
+  protected_data_1a = -1;
+  if (check_protected_data_1a (-1))
+    {
+      puts ("'protected_data_1a' in main and shared library doesn't have same value");
+      res = 1;
+    }
+
+  set_protected_data_1a (-3);
+  if (protected_data_1a != -3)
+    {
+      puts ("'protected_data_1a' in main and shared library doesn't have same value");
+      res = 1;
+    }
+
+  /* Check if we get the different addresses for the protected data
+     symbol.  */
+  if (&protected_data_1b == protected_data_1b_p ())
+    {
+      puts ("'protected_data_1b' in main and shared library has same address");
+      res = 1;
+    }
+
+  protected_data_1b = -10;
+  if (check_protected_data_1b (2))
+    {
+      puts ("'protected_data_1b' in main and shared library has same address");
+      res = 1;
+    }
+
+  set_protected_data_1b (-30);
+  if (protected_data_1b != -10)
+    {
+      puts ("'protected_data_1b' in main and shared library has same address");
+      res = 1;
+    }
+
+  if (!res)
+    puts ("PASS");
+
+  return res;
+}
diff --git a/ld/testsuite/ld-x86-64/protected-data-2a.S b/ld/testsuite/ld-x86-64/protected-data-2a.S
new file mode 100644
index 00000000000..dcc857c165d
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-data-2a.S
@@ -0,0 +1,109 @@
+	.text
+	.p2align 4
+	.protected	protected_data_1a
+	.globl	protected_data_1a_p
+	.type	protected_data_1a_p, @function
+protected_data_1a_p:
+.LFB0:
+	.cfi_startproc
+	leaq	protected_data_1a(%rip), %rax
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	protected_data_1a_p, .-protected_data_1a_p
+	.p2align 4
+	.protected	protected_data_1b
+	.globl	protected_data_1b_p
+	.type	protected_data_1b_p, @function
+protected_data_1b_p:
+.LFB1:
+	.cfi_startproc
+	leaq	protected_data_1b(%rip), %rax
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	protected_data_1b_p, .-protected_data_1b_p
+	.p2align 4
+	.globl	set_protected_data_1a
+	.type	set_protected_data_1a, @function
+set_protected_data_1a:
+.LFB2:
+	.cfi_startproc
+	movl	%edi, protected_data_1a(%rip)
+	ret
+	.cfi_endproc
+.LFE2:
+	.size	set_protected_data_1a, .-set_protected_data_1a
+	.p2align 4
+	.globl	set_protected_data_1b
+	.type	set_protected_data_1b, @function
+set_protected_data_1b:
+.LFB3:
+	.cfi_startproc
+	movl	%edi, protected_data_1b(%rip)
+	ret
+	.cfi_endproc
+.LFE3:
+	.size	set_protected_data_1b, .-set_protected_data_1b
+	.p2align 4
+	.globl	check_protected_data_1a
+	.type	check_protected_data_1a, @function
+check_protected_data_1a:
+.LFB4:
+	.cfi_startproc
+	xorl	%eax, %eax
+	cmpl	%edi, protected_data_1a(%rip)
+	setne	%al
+	ret
+	.cfi_endproc
+.LFE4:
+	.size	check_protected_data_1a, .-check_protected_data_1a
+	.p2align 4
+	.globl	check_protected_data_1b
+	.type	check_protected_data_1b, @function
+check_protected_data_1b:
+.LFB5:
+	.cfi_startproc
+	xorl	%eax, %eax
+	cmpl	%edi, protected_data_1b(%rip)
+	setne	%al
+	ret
+	.cfi_endproc
+.LFE5:
+	.size	check_protected_data_1b, .-check_protected_data_1b
+	.globl	protected_data_1b
+	.data
+	.align 4
+	.type	protected_data_1b, @object
+	.size	protected_data_1b, 4
+protected_data_1b:
+	.long	2
+	.globl	protected_data_1a
+	.align 4
+	.type	protected_data_1a, @object
+	.size	protected_data_1a, 4
+protected_data_1a:
+	.long	1
+	.section	.note.GNU-stack,"",@progbits
+#ifdef USE_GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
+# ifdef __LP64__
+#  define ALIGN 3
+# else
+#  define ALIGN 2
+# endif
+	.section ".note.gnu.property", "a"
+	.p2align ALIGN
+	.long 1f - 0f		/* name length */
+	.long 5f - 2f		/* data length */
+	.long 5			/* note type */
+0:	.asciz "GNU"		/* vendor name */
+1:
+	.p2align ALIGN
+2:	.long 0xb0008000	/* pr_type.  */
+	.long 4f - 3f		/* pr_datasz.  */
+3:
+	.long 0x1
+4:
+	.p2align ALIGN
+5:
+#endif
diff --git a/ld/testsuite/ld-x86-64/protected-data-2b.S b/ld/testsuite/ld-x86-64/protected-data-2b.S
new file mode 100644
index 00000000000..da8956139fe
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-data-2b.S
@@ -0,0 +1,119 @@
+	.section	.rodata.str1.8,"aMS",@progbits,1
+	.align 8
+.LC0:
+	.string	"'protected_data_1a' in main and shared library doesn't have same address"
+	.align 8
+.LC1:
+	.string	"'protected_data_1a' in main and shared library doesn't have same value"
+	.align 8
+.LC2:
+	.string	"'protected_data_1b' in main and shared library has same address"
+	.section	.rodata.str1.1,"aMS",@progbits,1
+.LC3:
+	.string	"PASS"
+	.section	.text.startup,"ax",@progbits
+	.p2align 4,,15
+	.globl	main
+	.type	main, @function
+main:
+.LFB11:
+	.cfi_startproc
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	xorl	%eax, %eax
+	pushq	%rbx
+	.cfi_def_cfa_offset 24
+	.cfi_offset 3, -24
+	xorl	%ebx, %ebx
+	subq	$8, %rsp
+	.cfi_def_cfa_offset 32
+	call	protected_data_1a_p
+	movq	protected_data_1a@GOTPCREL(%rip), %rbp
+	cmpq	%rbp, %rax
+	je	.L2
+	leaq	.LC0(%rip), %rdi
+	movb	$1, %bl
+	call	puts
+.L2:
+	movl	$-1, %edi
+	movl	$-1, 0(%rbp)
+	call	check_protected_data_1a
+	testl	%eax, %eax
+	jne	.L17
+.L3:
+	movl	$-3, %edi
+	call	set_protected_data_1a
+	cmpl	$-3, 0(%rbp)
+	je	.L4
+	leaq	.LC1(%rip), %rdi
+	movl	$1, %ebx
+	call	puts
+.L4:
+	xorl	%eax, %eax
+	call	protected_data_1b_p
+	leaq	protected_data_1b(%rip), %rdx
+	cmpq	%rdx, %rax
+	je	.L18
+.L5:
+	movl	$2, %edi
+	movl	$-10, protected_data_1b(%rip)
+	call	check_protected_data_1b
+	testl	%eax, %eax
+	jne	.L19
+	movl	$-30, %edi
+	call	set_protected_data_1b
+	cmpl	$-10, protected_data_1b(%rip)
+	je	.L9
+.L7:
+	leaq	.LC2(%rip), %rdi
+	movl	$1, %ebx
+	call	puts
+.L8:
+	addq	$8, %rsp
+	.cfi_remember_state
+	.cfi_def_cfa_offset 24
+	movl	%ebx, %eax
+	popq	%rbx
+	.cfi_def_cfa_offset 16
+	popq	%rbp
+	.cfi_def_cfa_offset 8
+	ret
+.L9:
+	.cfi_restore_state
+	testl	%ebx, %ebx
+	jne	.L11
+	leaq	.LC3(%rip), %rdi
+	call	puts
+	jmp	.L8
+.L19:
+	leaq	.LC2(%rip), %rdi
+	call	puts
+	movl	$-30, %edi
+	call	set_protected_data_1b
+	cmpl	$-10, protected_data_1b(%rip)
+	jne	.L7
+.L11:
+	movl	$1, %ebx
+	jmp	.L8
+.L17:
+	leaq	.LC1(%rip), %rdi
+	movl	$1, %ebx
+	call	puts
+	jmp	.L3
+.L18:
+	leaq	.LC2(%rip), %rdi
+	movl	$1, %ebx
+	call	puts
+	jmp	.L5
+	.cfi_endproc
+.LFE11:
+	.size	main, .-main
+	.globl	protected_data_1b
+	.data
+	.align 4
+	.type	protected_data_1b, @object
+	.size	protected_data_1b, 4
+protected_data_1b:
+	.long	3
+	.section	.note.GNU-stack,"",@progbits
diff --git a/ld/testsuite/ld-x86-64/protected-func-2a.S b/ld/testsuite/ld-x86-64/protected-func-2a.S
new file mode 100644
index 00000000000..35c9cd121bd
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-func-2a.S
@@ -0,0 +1,68 @@
+	.text
+	.p2align 4
+	.protected	protected_func_1a
+	.globl	protected_func_1a
+	.type	protected_func_1a, @function
+protected_func_1a:
+.LFB0:
+	.cfi_startproc
+	movl	$1, %eax
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	protected_func_1a, .-protected_func_1a
+	.p2align 4
+	.protected	protected_func_1b
+	.globl	protected_func_1b
+	.type	protected_func_1b, @function
+protected_func_1b:
+.LFB1:
+	.cfi_startproc
+	movl	$2, %eax
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	protected_func_1b, .-protected_func_1b
+	.p2align 4
+	.globl	protected_func_1a_p
+	.type	protected_func_1a_p, @function
+protected_func_1a_p:
+.LFB2:
+	.cfi_startproc
+	leaq	protected_func_1a(%rip), %rax
+	ret
+	.cfi_endproc
+.LFE2:
+	.size	protected_func_1a_p, .-protected_func_1a_p
+	.p2align 4
+	.globl	protected_func_1b_p
+	.type	protected_func_1b_p, @function
+protected_func_1b_p:
+.LFB3:
+	.cfi_startproc
+	leaq	protected_func_1b(%rip), %rax
+	ret
+	.cfi_endproc
+.LFE3:
+	.size	protected_func_1b_p, .-protected_func_1b_p
+	.section	.note.GNU-stack,"",@progbits
+#ifdef __LP64__
+# define ALIGN 3
+#else
+# define ALIGN 2
+#endif
+	.section ".note.gnu.property", "a"
+	.p2align ALIGN
+	.long 1f - 0f		/* name length */
+	.long 5f - 2f		/* data length */
+	.long 5			/* note type */
+0:	.asciz "GNU"		/* vendor name */
+1:
+	.p2align ALIGN
+2:	.long 0xb0008000	/* pr_type.  */
+	.long 4f - 3f		/* pr_datasz.  */
+3:
+	.long 0x1
+4:
+	.p2align ALIGN
+5:
diff --git a/ld/testsuite/ld-x86-64/protected-func-2b.S b/ld/testsuite/ld-x86-64/protected-func-2b.S
new file mode 100644
index 00000000000..8fa4cbf09d6
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-func-2b.S
@@ -0,0 +1,83 @@
+	.text
+	.p2align 4
+	.globl	protected_func_1b
+	.type	protected_func_1b, @function
+protected_func_1b:
+.LFB11:
+	.cfi_startproc
+	movl	$3, %eax
+	ret
+	.cfi_endproc
+.LFE11:
+	.size	protected_func_1b, .-protected_func_1b
+	.section	.rodata.str1.8,"aMS",@progbits,1
+	.align 8
+.LC0:
+	.string	"'protected_func_1a' in main and shared library doesn't have same address"
+	.align 8
+.LC1:
+	.string	"'protected_func_1a' doesn't return the correct value"
+	.align 8
+.LC2:
+	.string	"'protected_func_1b' in main and shared library has same address"
+	.section	.rodata.str1.1,"aMS",@progbits,1
+.LC3:
+	.string	"PASS"
+	.section	.text.startup,"ax",@progbits
+	.p2align 4
+	.globl	main
+	.type	main, @function
+main:
+.LFB12:
+	.cfi_startproc
+	pushq	%r12
+	.cfi_def_cfa_offset 16
+	.cfi_offset 12, -16
+	xorl	%r12d, %r12d
+	call	protected_func_1a_p
+	cmpq	protected_func_1a@GOTPCREL(%rip), %rax
+	je	.L4
+	leaq	.LC0(%rip), %rdi
+	movl	$1, %r12d
+	call	puts
+.L4:
+	call	protected_func_1a
+	cmpl	$1, %eax
+	jne	.L13
+	call	protected_func_1b_p
+	leaq	protected_func_1b(%rip), %rdx
+	cmpq	%rax, %rdx
+	je	.L6
+	testl	%r12d, %r12d
+	jne	.L12
+	leaq	.LC3(%rip), %rdi
+	call	puts
+	movl	%r12d, %eax
+	popq	%r12
+	.cfi_remember_state
+	.cfi_def_cfa_offset 8
+	ret
+.L13:
+	.cfi_restore_state
+	leaq	.LC1(%rip), %rdi
+	call	puts
+	call	protected_func_1b_p
+	leaq	protected_func_1b(%rip), %rdx
+	cmpq	%rax, %rdx
+	je	.L6
+.L12:
+	movl	$1, %r12d
+	movl	%r12d, %eax
+	popq	%r12
+	.cfi_remember_state
+	.cfi_def_cfa_offset 8
+	ret
+.L6:
+	.cfi_restore_state
+	leaq	.LC2(%rip), %rdi
+	call	puts
+	jmp	.L12
+	.cfi_endproc
+.LFE12:
+	.size	main, .-main
+	.section	.note.GNU-stack,"",@progbits
diff --git a/ld/testsuite/ld-x86-64/protected-func-2c.c b/ld/testsuite/ld-x86-64/protected-func-2c.c
new file mode 100644
index 00000000000..c5ec1eca17e
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected-func-2c.c
@@ -0,0 +1,29 @@
+#include "protected-func-1.h"
+
+protected_func_type protected_func_1a_ptr = protected_func_1a;
+
+__attribute__ ((visibility("protected")))
+int
+protected_func_1a (void)
+{
+  return 1;
+}
+
+__attribute__ ((visibility("protected")))
+int
+protected_func_1b (void)
+{
+  return 2;
+}
+
+protected_func_type
+protected_func_1a_p (void)
+{
+  return protected_func_1a;
+}
+
+protected_func_type
+protected_func_1b_p (void)
+{
+  return protected_func_1b;
+}
diff --git a/ld/testsuite/ld-x86-64/single-global-definition.rd b/ld/testsuite/ld-x86-64/single-global-definition.rd
new file mode 100644
index 00000000000..139dca0d6c6
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/single-global-definition.rd
@@ -0,0 +1,6 @@
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: 1_needed: single global definition
+#pass
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 3bf62504cf9..5fabbec6fa7 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -1334,6 +1334,47 @@ if { [isnative] && [check_compiler_available] } {
 	    {} \
 	    "libprotected-func-1.so" \
 	] \
+	[list \
+	    "Build libprotected-func-2a.so" \
+	    "-shared" \
+	    "-fPIC -Wa,-mx86-used-note=yes" \
+	    { protected-func-2a.S } \
+	    {{readelf -n single-global-definition.rd}}  \
+	    "libprotected-func-2a.so" \
+	] \
+	[list \
+	    "Build libprotected-func-2b.so" \
+	    "-shared -z single-global-definition" \
+	    "-fPIC -Wa,-mx86-used-note=yes" \
+	    { protected-func-2c.c } \
+	    {{readelf -n single-global-definition.rd}}  \
+	    "libprotected-func-2b.so" \
+	] \
+	[list \
+	    "Build libprotected-data-1.so" \
+	    "-shared" \
+	    "-fPIC -Wa,-mx86-used-note=yes" \
+	    { protected-data-1a.c } \
+	    {} \
+	    "libprotected-data-1.so" \
+	] \
+	[list \
+	    "Build libprotected-data-2a.so" \
+	    "-shared" \
+	    "-fPIC -Wa,-mx86-used-note=yes \
+	     -DUSE_GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION" \
+	    { protected-data-2a.S } \
+	    {{readelf -n single-global-definition.rd}}  \
+	    "libprotected-data-2a.so" \
+	] \
+	[list \
+	    "Build libprotected-data-2b.so" \
+	    "-shared -z single-global-definition" \
+	    "-fPIC -Wa,-mx86-used-note=yes" \
+	    { protected-data-2a.S } \
+	    {{readelf -n single-global-definition.rd}}  \
+	    "libprotected-data-2b.so" \
+	] \
     ]
 
     if  {[istarget "x86_64-*-linux*-gnux32"]} {
@@ -1761,6 +1802,96 @@ if { [isnative] && [check_compiler_available] } {
 	    "pass.out" \
 	    "-fPIE" \
 	] \
+	[list \
+	    "Run protected-func-2a without PIE" \
+	    "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-func-2a.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-func-2b.S } \
+	    "protected-func-2a" \
+	    "pass.out" \
+	    "$NOPIE_CFLAGS" \
+	] \
+	[list \
+	    "Run protected-func-2b with PIE" \
+	    "-Wl,--no-as-needed -pie tmpdir/libprotected-func-2a.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-func-2b.S } \
+	    "protected-func-2b" \
+	    "pass.out" \
+	    "-fPIE" \
+	] \
+	[list \
+	    "Run protected-func-2c without PIE" \
+	    "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-func-2b.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-func-2b.S } \
+	    "protected-func-2c" \
+	    "pass.out" \
+	    "$NOPIE_CFLAGS" \
+	] \
+	[list \
+	    "Run protected-func-2d with PIE" \
+	    "-Wl,--no-as-needed -pie tmpdir/libprotected-func-2b.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-func-2b.S } \
+	    "protected-func-2d" \
+	    "pass.out" \
+	    "-fPIE" \
+	] \
+	[list \
+	    "Run protected-data-1 without PIE" \
+	    "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-data-1.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-1b.c } \
+	    "protected-data-1a" \
+	    "pass.out" \
+	    "$NOPIE_CFLAGS" \
+	] \
+	[list \
+	    "Run protected-data-1 with PIE" \
+	    "-Wl,--no-as-needed -pie tmpdir/libprotected-data-1.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-1b.c } \
+	    "protected-data-1b" \
+	    "pass.out" \
+	    "-fPIE" \
+	] \
+	[list \
+	    "Run protected-data-2a without PIE" \
+	    "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-data-2a.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-2b.S } \
+	    "protected-data-2a" \
+	    "pass.out" \
+	    "$NOPIE_CFLAGS" \
+	] \
+	[list \
+	    "Run protected-data-2b with PIE" \
+	    "-Wl,--no-as-needed -pie tmpdir/libprotected-data-2a.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-2b.S } \
+	    "protected-data-2b" \
+	    "pass.out" \
+	    "-fPIE" \
+	] \
+	[list \
+	    "Run protected-data-2c without PIE" \
+	    "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-data-2b.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-2b.S } \
+	    "protected-data-2c" \
+	    "pass.out" \
+	    "$NOPIE_CFLAGS" \
+	] \
+	[list \
+	    "Run protected-data-2d with PIE" \
+	    "-Wl,--no-as-needed -pie tmpdir/libprotected-data-2b.so" \
+	    "-Wa,-mx86-used-note=yes" \
+	    { protected-data-2b.S } \
+	    "protected-data-2d" \
+	    "pass.out" \
+	    "-fPIE" \
+	] \
     ]
 
     # Run-time tests which require working ifunc attribute support.
-- 
2.31.1

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -3331,6 +3331,10 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
>    if (!is_elf_hash_table (&hash_table->root))
>      return true;
>  
> +  /* STV_PROTECTED symbols with single global definition are local. */
> +  if (info->single_global_definition > 0)
> +    return true;
> +
>    bed = get_elf_backend_data (hash_table->dynobj);
>  
>    /* If extern_protected_data is false, STV_PROTECTED non-function

With single_global_definition set you will require access from an
executable to a protected visibility symbol defined in a shared
library to go through the GOT, and you won't be using dynbss copies.
How can it be correct for symbol_refs_local_p to return true here for
such an executable reference?

It seems logically incorrect, since such an access is clearly not
local.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Sun, Jun 20, 2021 at 7:16 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > --- a/bfd/elflink.c
> > +++ b/bfd/elflink.c
> > @@ -3331,6 +3331,10 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
> >    if (!is_elf_hash_table (&hash_table->root))
> >      return true;
> >
> > +  /* STV_PROTECTED symbols with single global definition are local. */
> > +  if (info->single_global_definition > 0)
> > +    return true;
> > +
> >    bed = get_elf_backend_data (hash_table->dynobj);
> >
> >    /* If extern_protected_data is false, STV_PROTECTED non-function
>
> With single_global_definition set you will require access from an
> executable to a protected visibility symbol defined in a shared
> library to go through the GOT, and you won't be using dynbss copies.
> How can it be correct for symbol_refs_local_p to return true here for
> such an executable reference?

We don't merge symbol visibility from shared libraries.    A symbol
with STV_PROTECTED visibility must be defined locally.

[hjl@gnu-cfl-1 tmp]$ cat foo.s
.data
.globl foo
.protected foo
foo:
.byte 0
[hjl@gnu-cfl-1 tmp]$ cat bar.s
.data
.globl bar
.protected foo
bar:
.dc.a foo
[hjl@gnu-cfl-1 tmp]$ gcc -c foo.s bar.s
[hjl@gnu-cfl-1 tmp]$ ld -shared -o libfoo.so foo.o
[hjl@gnu-cfl-1 tmp]$ ld -shared -o libbar.so bar.o libfoo.so
ld: bar.o: in function `bar':
(.data+0x0): undefined reference to `foo'
ld: libbar.so: protected symbol `foo' isn't defined
ld: final link failed: bad value
[hjl@gnu-cfl-1 tmp]$ ld -shared -o libbar.so bar.o foo.o
[hjl@gnu-cfl-1 tmp]$

> It seems logically incorrect, since such an access is clearly not
> local.
>
> --
> Alan Modra
> Australia Development Lab, IBM



--
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

On Sun, Jun 20, 2021 at 07:31:45PM -0700, H.J. Lu wrote:
> We don't merge symbol visibility from shared libraries.

Ah, true, I'd forgotten about that.  elf_merge_st_other if anyone is
wondering about it.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

I'm happy with the direction of this patch series, but do consult with
ARM maintainers before committing.

On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> relocatable files:
> 
> 1. Don't generate copy relocations.
> 2. Turn off extern_protected_data.
> 3. Treate reference to protected symbols with single global definition
> as local.
> 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> 5. Add -z [no]single-global-definition to control single global definition.

This doesn't seem a good name.  I think the name should have
"protected" in it somewhere, since what you are doing here affects the
way the x86 and arm toolchains treat protected visibility symbols.

Hmm, do you even need a new linker option?  Powerpc toolchains behave
as you are proposing for x86, without needing special linker options.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
>
> I'm happy with the direction of this patch series, but do consult with
> ARM maintainers before committing.

Nick, Richard, what do you think?

> On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > relocatable files:
> >
> > 1. Don't generate copy relocations.
> > 2. Turn off extern_protected_data.
> > 3. Treate reference to protected symbols with single global definition
> > as local.
> > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > 5. Add -z [no]single-global-definition to control single global definition.
>
> This doesn't seem a good name.  I think the name should have
> "protected" in it somewhere, since what you are doing here affects the
> way the x86 and arm toolchains treat protected visibility symbols.

Removing copy relocation and canonical function pointers do help
protected symbols in shared libraries.  But their impacts on executable
are unrelated to protected symbols.  Do you have any suggestions?

> Hmm, do you even need a new linker option?  Powerpc toolchains behave
> as you are proposing for x86, without needing special linker options.
>

I added these linker options to use them in glibc tests:

https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html

If the linker options are x86 specific, these tests will only run on
x86 targets.

Thanks.

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > I'm happy with the direction of this patch series, but do consult with
> > ARM maintainers before committing.
> 
> Nick, Richard, what do you think?
> 
> > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > > relocatable files:
> > >
> > > 1. Don't generate copy relocations.
> > > 2. Turn off extern_protected_data.
> > > 3. Treate reference to protected symbols with single global definition
> > > as local.
> > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > > 5. Add -z [no]single-global-definition to control single global definition.
> >
> > This doesn't seem a good name.  I think the name should have
> > "protected" in it somewhere, since what you are doing here affects the
> > way the x86 and arm toolchains treat protected visibility symbols.
> 
> Removing copy relocation and canonical function pointers do help
> protected symbols in shared libraries.

Right.

>  But their impacts on executable
> are unrelated to protected symbols.

Obviously you do need some option to control code generated by gcc
(and the gcc option name also doesn't seem right to me).  I understand
what you're trying to say in the name but it won't convey much to
users.

>  Do you have any suggestions?

Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
with the first that code is respecting the ELF gABI, while with the
second you're being tricky with dynbss copies and copy relocations.

> > Hmm, do you even need a new linker option?  Powerpc toolchains behave
> > as you are proposing for x86, without needing special linker options.
> >
> 
> I added these linker options to use them in glibc tests:

But you are trying to drive the linker via notes too, so why is it
necessary to also have a linker command line option?

> https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
> 
> If the linker options are x86 specific, these tests will only run on
> x86 targets.

I think that indeed any new linker option should be treated just like
-z noextern-protected-data and be enabled using another file like
emulparams/extern_protected_data.sh.  I don't want a -z linker option
on powerpc that won't be useful.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > I'm happy with the direction of this patch series, but do consult with
> > > ARM maintainers before committing.
> >
> > Nick, Richard, what do you think?
> >
> > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > > > relocatable files:
> > > >
> > > > 1. Don't generate copy relocations.
> > > > 2. Turn off extern_protected_data.
> > > > 3. Treate reference to protected symbols with single global definition
> > > > as local.
> > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > > > 5. Add -z [no]single-global-definition to control single global definition.
> > >
> > > This doesn't seem a good name.  I think the name should have
> > > "protected" in it somewhere, since what you are doing here affects the
> > > way the x86 and arm toolchains treat protected visibility symbols.
> >
> > Removing copy relocation and canonical function pointers do help
> > protected symbols in shared libraries.
>
> Right.
>
> >  But their impacts on executable
> > are unrelated to protected symbols.
>
> Obviously you do need some option to control code generated by gcc
> (and the gcc option name also doesn't seem right to me).  I understand
> what you're trying to say in the name but it won't convey much to
> users.
>
> >  Do you have any suggestions?
>
> Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
> options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
> with the first that code is respecting the ELF gABI, while with the
> second you're being tricky with dynbss copies and copy relocations.

This also impacts function pointers.  "copy" doesn't cover it.   Florian
also points out that we need a different behavior in executable.   Since
linker has all information, it may be able to set/clear the bit properly
based on input relocations.

> > > Hmm, do you even need a new linker option?  Powerpc toolchains behave
> > > as you are proposing for x86, without needing special linker options.
> > >
> >
> > I added these linker options to use them in glibc tests:
>
> But you are trying to drive the linker via notes too, so why is it
> necessary to also have a linker command line option?
>
> > https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
> >
> > If the linker options are x86 specific, these tests will only run on
> > x86 targets.
>
> I think that indeed any new linker option should be treated just like
> -z noextern-protected-data and be enabled using another file like
> emulparams/extern_protected_data.sh.  I don't want a -z linker option

I will make the change.

> on powerpc that won't be useful.

Thanks.

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> > > >
> > > > I'm happy with the direction of this patch series, but do consult with
> > > > ARM maintainers before committing.
> > >
> > > Nick, Richard, what do you think?
> > >
> > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > > > > relocatable files:
> > > > >
> > > > > 1. Don't generate copy relocations.
> > > > > 2. Turn off extern_protected_data.
> > > > > 3. Treate reference to protected symbols with single global definition
> > > > > as local.
> > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > > > > 5. Add -z [no]single-global-definition to control single global definition.
> > > >
> > > > This doesn't seem a good name.  I think the name should have
> > > > "protected" in it somewhere, since what you are doing here affects the
> > > > way the x86 and arm toolchains treat protected visibility symbols.
> > >
> > > Removing copy relocation and canonical function pointers do help
> > > protected symbols in shared libraries.
> >
> > Right.
> >
> > >  But their impacts on executable
> > > are unrelated to protected symbols.
> >
> > Obviously you do need some option to control code generated by gcc
> > (and the gcc option name also doesn't seem right to me).  I understand
> > what you're trying to say in the name but it won't convey much to
> > users.
> >
> > >  Do you have any suggestions?
> >
> > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
> > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
> > with the first that code is respecting the ELF gABI, while with the
> > second you're being tricky with dynbss copies and copy relocations.
>
> This also impacts function pointers.  "copy" doesn't cover it.   Florian
> also points out that we need a different behavior in executable.   Since
> linker has all information, it may be able to set/clear the bit properly
> based on input relocations.

How about

-fprotected-abi=extern
-fprotected-abi=extern-data
-fprotected-abi=extern-function
-fprotected-abi=local
-fprotected-abi=local-data
-fprotected-abi=local-function

> > > > Hmm, do you even need a new linker option?  Powerpc toolchains behave
> > > > as you are proposing for x86, without needing special linker options.
> > > >
> > >
> > > I added these linker options to use them in glibc tests:
> >
> > But you are trying to drive the linker via notes too, so why is it
> > necessary to also have a linker command line option?
> >
> > > https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
> > >
> > > If the linker options are x86 specific, these tests will only run on
> > > x86 targets.
> >
> > I think that indeed any new linker option should be treated just like
> > -z noextern-protected-data and be enabled using another file like
> > emulparams/extern_protected_data.sh.  I don't want a -z linker option
>
> I will make the change.
>
> > on powerpc that won't be useful.
>
> Thanks.
>
> --
> H.J.



-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 2:32 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> > > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> > > > >
> > > > > I'm happy with the direction of this patch series, but do consult with
> > > > > ARM maintainers before committing.
> > > >
> > > > Nick, Richard, what do you think?
> > > >
> > > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > > > > > relocatable files:
> > > > > >
> > > > > > 1. Don't generate copy relocations.
> > > > > > 2. Turn off extern_protected_data.
> > > > > > 3. Treate reference to protected symbols with single global definition
> > > > > > as local.
> > > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > > > > > 5. Add -z [no]single-global-definition to control single global definition.
> > > > >
> > > > > This doesn't seem a good name.  I think the name should have
> > > > > "protected" in it somewhere, since what you are doing here affects the
> > > > > way the x86 and arm toolchains treat protected visibility symbols.
> > > >
> > > > Removing copy relocation and canonical function pointers do help
> > > > protected symbols in shared libraries.
> > >
> > > Right.
> > >
> > > >  But their impacts on executable
> > > > are unrelated to protected symbols.
> > >
> > > Obviously you do need some option to control code generated by gcc
> > > (and the gcc option name also doesn't seem right to me).  I understand
> > > what you're trying to say in the name but it won't convey much to
> > > users.
> > >
> > > >  Do you have any suggestions?
> > >
> > > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
> > > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
> > > with the first that code is respecting the ELF gABI, while with the
> > > second you're being tricky with dynbss copies and copy relocations.
> >
> > This also impacts function pointers.  "copy" doesn't cover it.   Florian
> > also points out that we need a different behavior in executable.   Since
> > linker has all information, it may be able to set/clear the bit properly
> > based on input relocations.
>
> How about
>
> -fprotected-abi=extern
> -fprotected-abi=extern-data
> -fprotected-abi=extern-function
> -fprotected-abi=local
> -fprotected-abi=local-data
> -fprotected-abi=local-function

-fprotected-abi=extern
-fprotected-abi=local

is simpler.

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Fangrui Song]

On 2021-06-21, H.J. Lu via Binutils wrote:
>On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
>> >
>> > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
>> > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
>> > > >
>> > > > I'm happy with the direction of this patch series, but do consult with
>> > > > ARM maintainers before committing.
>> > >
>> > > Nick, Richard, what do you think?
>> > >
>> > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
>> > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
>> > > > > relocatable files:
>> > > > >
>> > > > > 1. Don't generate copy relocations.
>> > > > > 2. Turn off extern_protected_data.
>> > > > > 3. Treate reference to protected symbols with single global definition
>> > > > > as local.
>> > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
>> > > > > 5. Add -z [no]single-global-definition to control single global definition.
>> > > >
>> > > > This doesn't seem a good name.  I think the name should have
>> > > > "protected" in it somewhere, since what you are doing here affects the
>> > > > way the x86 and arm toolchains treat protected visibility symbols.
>> > >
>> > > Removing copy relocation and canonical function pointers do help
>> > > protected symbols in shared libraries.
>> >
>> > Right.
>> >
>> > >  But their impacts on executable
>> > > are unrelated to protected symbols.
>> >
>> > Obviously you do need some option to control code generated by gcc
>> > (and the gcc option name also doesn't seem right to me).  I understand
>> > what you're trying to say in the name but it won't convey much to
>> > users.
>> >
>> > >  Do you have any suggestions?
>> >
>> > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
>> > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
>> > with the first that code is respecting the ELF gABI, while with the
>> > second you're being tricky with dynbss copies and copy relocations.
>>
>> This also impacts function pointers.  "copy" doesn't cover it.   Florian
>> also points out that we need a different behavior in executable.   Since
>> linker has all information, it may be able to set/clear the bit properly
>> based on input relocations.
>
>How about
>
>-fprotected-abi=extern
>-fprotected-abi=extern-data
>-fprotected-abi=extern-function
>-fprotected-abi=local
>-fprotected-abi=local-data
>-fprotected-abi=local-function

I don't think the option name needs to mention "protected".

First, -fpie and -fpic use GOT for external data/function today, no need
for a new option.

The option is used with -fno-pic to prevent interaction issues with
protected definitions in shared objects, but the option itself doesn't
do anything with protected. For instance, the option can fix the pointer
equality issue with a -Bsymbolic or --dynamic-list linked shared object as well.
So I don't think the option name needs to mention "protected".

clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
The GCC feature request is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112.

When taking an external function address in -fno-pic code, I suggest
-fno-direct-access-extern-function
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593). Actually, for
many arches I suggest that we just use GOT by default, no need for a
toggle.

For x86-64 -fpie, we should just apply
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html

>> > > > Hmm, do you even need a new linker option?  Powerpc toolchains behave
>> > > > as you are proposing for x86, without needing special linker options.
>> > > >
>> > >
>> > > I added these linker options to use them in glibc tests:
>> >
>> > But you are trying to drive the linker via notes too, so why is it
>> > necessary to also have a linker command line option?
>> >
>> > > https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
>> > >
>> > > If the linker options are x86 specific, these tests will only run on
>> > > x86 targets.
>> >
>> > I think that indeed any new linker option should be treated just like
>> > -z noextern-protected-data and be enabled using another file like
>> > emulparams/extern_protected_data.sh.  I don't want a -z linker option
>>
>> I will make the change.
>>
>> > on powerpc that won't be useful.
>>
>> Thanks.
>>
>> --
>> H.J.
>
>
>
>-- 
>H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 3:34 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2021-06-21, H.J. Lu via Binutils wrote:
> >On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
> >> >
> >> > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> >> > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> >> > > >
> >> > > > I'm happy with the direction of this patch series, but do consult with
> >> > > > ARM maintainers before committing.
> >> > >
> >> > > Nick, Richard, what do you think?
> >> > >
> >> > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> >> > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> >> > > > > relocatable files:
> >> > > > >
> >> > > > > 1. Don't generate copy relocations.
> >> > > > > 2. Turn off extern_protected_data.
> >> > > > > 3. Treate reference to protected symbols with single global definition
> >> > > > > as local.
> >> > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> >> > > > > 5. Add -z [no]single-global-definition to control single global definition.
> >> > > >
> >> > > > This doesn't seem a good name.  I think the name should have
> >> > > > "protected" in it somewhere, since what you are doing here affects the
> >> > > > way the x86 and arm toolchains treat protected visibility symbols.
> >> > >
> >> > > Removing copy relocation and canonical function pointers do help
> >> > > protected symbols in shared libraries.
> >> >
> >> > Right.
> >> >
> >> > >  But their impacts on executable
> >> > > are unrelated to protected symbols.
> >> >
> >> > Obviously you do need some option to control code generated by gcc
> >> > (and the gcc option name also doesn't seem right to me).  I understand
> >> > what you're trying to say in the name but it won't convey much to
> >> > users.
> >> >
> >> > >  Do you have any suggestions?
> >> >
> >> > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
> >> > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
> >> > with the first that code is respecting the ELF gABI, while with the
> >> > second you're being tricky with dynbss copies and copy relocations.
> >>
> >> This also impacts function pointers.  "copy" doesn't cover it.   Florian
> >> also points out that we need a different behavior in executable.   Since
> >> linker has all information, it may be able to set/clear the bit properly
> >> based on input relocations.
> >
> >How about
> >
> >-fprotected-abi=extern
> >-fprotected-abi=extern-data
> >-fprotected-abi=extern-function
> >-fprotected-abi=local
> >-fprotected-abi=local-data
> >-fprotected-abi=local-function
>
> I don't think the option name needs to mention "protected".
>
> First, -fpie and -fpic use GOT for external data/function today, no need
> for a new option.
>
> The option is used with -fno-pic to prevent interaction issues with
> protected definitions in shared objects, but the option itself doesn't
> do anything with protected. For instance, the option can fix the pointer
> equality issue with a -Bsymbolic or --dynamic-list linked shared object as well.
> So I don't think the option name needs to mention "protected".

It sounds reasonable.

> clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> The GCC feature request is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112.
>
> When taking an external function address in -fno-pic code, I suggest
> -fno-direct-access-extern-function
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593). Actually, for
> many arches I suggest that we just use GOT by default, no need for a
> toggle.
>
> For x86-64 -fpie, we should just apply
> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>

I'd like to have a single option to use GOT to access external symbols
and make protected symbols local.   Programmer doesn't have to know
if PIE is enabled by default nor if copy relocation is used by default.

-fsingle-global-definition isn't a good name.  But I can't find a better one.

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 3:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 3:34 PM Fangrui Song <i@maskray.me> wrote:
> >
> > On 2021-06-21, H.J. Lu via Binutils wrote:
> > >On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >>
> > >> On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
> > >> >
> > >> > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
> > >> > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
> > >> > > >
> > >> > > > I'm happy with the direction of this patch series, but do consult with
> > >> > > > ARM maintainers before committing.
> > >> > >
> > >> > > Nick, Richard, what do you think?
> > >> > >
> > >> > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
> > >> > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
> > >> > > > > relocatable files:
> > >> > > > >
> > >> > > > > 1. Don't generate copy relocations.
> > >> > > > > 2. Turn off extern_protected_data.
> > >> > > > > 3. Treate reference to protected symbols with single global definition
> > >> > > > > as local.
> > >> > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
> > >> > > > > 5. Add -z [no]single-global-definition to control single global definition.
> > >> > > >
> > >> > > > This doesn't seem a good name.  I think the name should have
> > >> > > > "protected" in it somewhere, since what you are doing here affects the
> > >> > > > way the x86 and arm toolchains treat protected visibility symbols.
> > >> > >
> > >> > > Removing copy relocation and canonical function pointers do help
> > >> > > protected symbols in shared libraries.
> > >> >
> > >> > Right.
> > >> >
> > >> > >  But their impacts on executable
> > >> > > are unrelated to protected symbols.
> > >> >
> > >> > Obviously you do need some option to control code generated by gcc
> > >> > (and the gcc option name also doesn't seem right to me).  I understand
> > >> > what you're trying to say in the name but it won't convey much to
> > >> > users.
> > >> >
> > >> > >  Do you have any suggestions?
> > >> >
> > >> > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
> > >> > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
> > >> > with the first that code is respecting the ELF gABI, while with the
> > >> > second you're being tricky with dynbss copies and copy relocations.
> > >>
> > >> This also impacts function pointers.  "copy" doesn't cover it.   Florian
> > >> also points out that we need a different behavior in executable.   Since
> > >> linker has all information, it may be able to set/clear the bit properly
> > >> based on input relocations.
> > >
> > >How about
> > >
> > >-fprotected-abi=extern
> > >-fprotected-abi=extern-data
> > >-fprotected-abi=extern-function
> > >-fprotected-abi=local
> > >-fprotected-abi=local-data
> > >-fprotected-abi=local-function
> >
> > I don't think the option name needs to mention "protected".
> >
> > First, -fpie and -fpic use GOT for external data/function today, no need
> > for a new option.
> >
> > The option is used with -fno-pic to prevent interaction issues with
> > protected definitions in shared objects, but the option itself doesn't
> > do anything with protected. For instance, the option can fix the pointer
> > equality issue with a -Bsymbolic or --dynamic-list linked shared object as well.
> > So I don't think the option name needs to mention "protected".
>
> It sounds reasonable.
>
> > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> > The GCC feature request is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112.
> >
> > When taking an external function address in -fno-pic code, I suggest
> > -fno-direct-access-extern-function
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593). Actually, for
> > many arches I suggest that we just use GOT by default, no need for a
> > toggle.
> >
> > For x86-64 -fpie, we should just apply
> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> >
>
> I'd like to have a single option to use GOT to access external symbols
> and make protected symbols local.   Programmer doesn't have to know
> if PIE is enabled by default nor if copy relocation is used by default.
>
> -fsingle-global-definition isn't a good name.  But I can't find a better one.
>

How about -fsymbolic, similar to linker -Bsymbolic?

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Fangrui Song]

On 2021-06-21, H.J. Lu wrote:
>On Mon, Jun 21, 2021 at 3:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Mon, Jun 21, 2021 at 3:34 PM Fangrui Song <i@maskray.me> wrote:
>> >
>> > On 2021-06-21, H.J. Lu via Binutils wrote:
>> > >On Mon, Jun 21, 2021 at 6:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> > >>
>> > >> On Mon, Jun 21, 2021 at 6:50 AM Alan Modra <amodra@gmail.com> wrote:
>> > >> >
>> > >> > On Mon, Jun 21, 2021 at 05:42:06AM -0700, H.J. Lu wrote:
>> > >> > > On Mon, Jun 21, 2021 at 3:46 AM Alan Modra <amodra@gmail.com> wrote:
>> > >> > > >
>> > >> > > > I'm happy with the direction of this patch series, but do consult with
>> > >> > > > ARM maintainers before committing.
>> > >> > >
>> > >> > > Nick, Richard, what do you think?
>> > >> > >
>> > >> > > > On Sun, Jun 20, 2021 at 03:50:29PM -0700, H.J. Lu via Binutils wrote:
>> > >> > > > > If GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION is set on any input
>> > >> > > > > relocatable files:
>> > >> > > > >
>> > >> > > > > 1. Don't generate copy relocations.
>> > >> > > > > 2. Turn off extern_protected_data.
>> > >> > > > > 3. Treate reference to protected symbols with single global definition
>> > >> > > > > as local.
>> > >> > > > > 4. Set GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION on output.
>> > >> > > > > 5. Add -z [no]single-global-definition to control single global definition.
>> > >> > > >
>> > >> > > > This doesn't seem a good name.  I think the name should have
>> > >> > > > "protected" in it somewhere, since what you are doing here affects the
>> > >> > > > way the x86 and arm toolchains treat protected visibility symbols.
>> > >> > >
>> > >> > > Removing copy relocation and canonical function pointers do help
>> > >> > > protected symbols in shared libraries.
>> > >> >
>> > >> > Right.
>> > >> >
>> > >> > >  But their impacts on executable
>> > >> > > are unrelated to protected symbols.
>> > >> >
>> > >> > Obviously you do need some option to control code generated by gcc
>> > >> > (and the gcc option name also doesn't seem right to me).  I understand
>> > >> > what you're trying to say in the name but it won't convey much to
>> > >> > users.
>> > >> >
>> > >> > >  Do you have any suggestions?
>> > >> >
>> > >> > Perhaps -fprotected-abi=nocopy and -fprotected-abi=copy for the gcc
>> > >> > options?  Or just -fprotected-abi and -fprotected-abi=copy.  Saying
>> > >> > with the first that code is respecting the ELF gABI, while with the
>> > >> > second you're being tricky with dynbss copies and copy relocations.
>> > >>
>> > >> This also impacts function pointers.  "copy" doesn't cover it.   Florian
>> > >> also points out that we need a different behavior in executable.   Since
>> > >> linker has all information, it may be able to set/clear the bit properly
>> > >> based on input relocations.
>> > >
>> > >How about
>> > >
>> > >-fprotected-abi=extern
>> > >-fprotected-abi=extern-data
>> > >-fprotected-abi=extern-function
>> > >-fprotected-abi=local
>> > >-fprotected-abi=local-data
>> > >-fprotected-abi=local-function
>> >
>> > I don't think the option name needs to mention "protected".
>> >
>> > First, -fpie and -fpic use GOT for external data/function today, no need
>> > for a new option.
>> >
>> > The option is used with -fno-pic to prevent interaction issues with
>> > protected definitions in shared objects, but the option itself doesn't
>> > do anything with protected. For instance, the option can fix the pointer
>> > equality issue with a -Bsymbolic or --dynamic-list linked shared object as well.
>> > So I don't think the option name needs to mention "protected".
>>
>> It sounds reasonable.

Thanks... We have moved toward getting on the same page.

>> > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
>> > The GCC feature request is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112.
>> >
>> > When taking an external function address in -fno-pic code, I suggest
>> > -fno-direct-access-extern-function
>> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593). Actually, for
>> > many arches I suggest that we just use GOT by default, no need for a
>> > toggle.

Sorry, I made a typo. s/extern/external/

>> > For x86-64 -fpie, we should just apply
>> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>> >
>>
>> I'd like to have a single option to use GOT to access external symbols
>> and make protected symbols local.   Programmer doesn't have to know
>> if PIE is enabled by default nor if copy relocation is used by default.
>>
>> -fsingle-global-definition isn't a good name.  But I can't find a better one.
>>
>
>How about -fsymbolic, similar to linker -Bsymbolic?

The linker -Bsymbolic changes the preemptible property of a **defined**
symbol.

The compiler option is about the behavior of accessing a non-definition
declaration in C/C++.

People may be confused by different symbol sets of -fsymbolic and
-Bsymbolic. Plus, I think option names should be direct about their
affects.

If this is only one option, it can be -fno-direct-access-external,
which includes -fno-direct-access-external-{function,data}.

And I'll argue that for many architectures they should just default to
-fno-direct-access-external-function.

Whether -fno-direct-access-external-data should gradually become the
default   depends on   how much people dislike copy relocations.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.

-fno-direct-access-extern-data or variations on that also seem good to
me.  -fpic-extern would also work.  I liked -fprotected-abi because
it shows the intent of correcting abi issues related to protected
visibility.  (Yes, it affects code for all undefined symbols because
the compiler clearly isn't seeing the entire program if there are
undefined symbols.)

The main thing that struck me about -fsingle-global-definition is that
the option doesn't do what it says.  You can still have multiple
global definitions of a given symbol, one in the executable and one in
each of the shared libraries making up the complete program.  Which of
course is no different to code without -fsingle-global-definition.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
>
> -fno-direct-access-extern-data or variations on that also seem good to
> me.  -fpic-extern would also work.  I liked -fprotected-abi because
> it shows the intent of correcting abi issues related to protected
> visibility.  (Yes, it affects code for all undefined symbols because
> the compiler clearly isn't seeing the entire program if there are
> undefined symbols.)

I need an option which can be turned on and off.   How about
-fextern-access=direct and -fextern-access=indirect?  It will cover
both data and function?

> The main thing that struck me about -fsingle-global-definition is that
> the option doesn't do what it says.  You can still have multiple
> global definitions of a given symbol, one in the executable and one in
> each of the shared libraries making up the complete program.  Which of
> course is no different to code without -fsingle-global-definition.


-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Alan Modra]

On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
> On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> >
> > -fno-direct-access-extern-data or variations on that also seem good to
> > me.  -fpic-extern would also work.  I liked -fprotected-abi because
> > it shows the intent of correcting abi issues related to protected
> > visibility.  (Yes, it affects code for all undefined symbols because
> > the compiler clearly isn't seeing the entire program if there are
> > undefined symbols.)
> 
> I need an option which can be turned on and off.   How about
> -fextern-access=direct and -fextern-access=indirect?  It will cover
> both data and function?

Yes, FWIW that option name for gcc also looks good to me.

Now as to the need for a corresponding linker option, I'm of the
opinion that it is ideal for the linker to be able to cope without
needing special options.  Can you show me a set of object files (or
just describe them) where ld cannot deduce from relocations and
dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
are needed?  I'm fairly sure I manage to do that for powerpc.

Note that I'm not against a new option to force the linker to go
against what it would do based on input object files (perhaps
reporting errors), but don't think we should have a new option without
some effort being made to see whether we really need it.

> > The main thing that struck me about -fsingle-global-definition is that
> > the option doesn't do what it says.  You can still have multiple
> > global definitions of a given symbol, one in the executable and one in
> > each of the shared libraries making up the complete program.  Which of
> > course is no different to code without -fsingle-global-definition.
> 
> 
> -- 
> H.J.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 9:16 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
> > On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> > > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> > >
> > > -fno-direct-access-extern-data or variations on that also seem good to
> > > me.  -fpic-extern would also work.  I liked -fprotected-abi because
> > > it shows the intent of correcting abi issues related to protected
> > > visibility.  (Yes, it affects code for all undefined symbols because
> > > the compiler clearly isn't seeing the entire program if there are
> > > undefined symbols.)
> >
> > I need an option which can be turned on and off.   How about
> > -fextern-access=direct and -fextern-access=indirect?  It will cover
> > both data and function?
>
> Yes, FWIW that option name for gcc also looks good to me.

I will change the gcc option to

-fextern-access=direct
-fextern-access=indirect

and change GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS

> Now as to the need for a corresponding linker option, I'm of the
> opinion that it is ideal for the linker to be able to cope without
> needing special options.  Can you show me a set of object files (or
> just describe them) where ld cannot deduce from relocations and
> dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
> are needed?  I'm fairly sure I manage to do that for powerpc.
>
> Note that I'm not against a new option to force the linker to go
> against what it would do based on input object files (perhaps

I'd like to turn it on in linker without any compiler changes, especially
when building shared libraries, kind of a subset of -Bsymbolic.

> reporting errors), but don't think we should have a new option without
> some effort being made to see whether we really need it.

Here is a glibc patch to use both linker options on some testcases:

https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html

> > > The main thing that struck me about -fsingle-global-definition is that
> > > the option doesn't do what it says.  You can still have multiple
> > > global definitions of a given symbol, one in the executable and one in
> > > each of the shared libraries making up the complete program.  Which of
> > > course is no different to code without -fsingle-global-definition.
> >
> >
> > --
> > H.J.
>
> --
> Alan Modra
> Australia Development Lab, IBM



-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Fangrui Song]

On 2021-06-21, H.J. Lu wrote:
>On Mon, Jun 21, 2021 at 9:16 PM Alan Modra <amodra@gmail.com> wrote:
>>
>> On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
>> > On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
>> > >
>> > > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
>> > > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
>> > >
>> > > -fno-direct-access-extern-data or variations on that also seem good to
>> > > me.  -fpic-extern would also work.  I liked -fprotected-abi because
>> > > it shows the intent of correcting abi issues related to protected
>> > > visibility.  (Yes, it affects code for all undefined symbols because
>> > > the compiler clearly isn't seeing the entire program if there are
>> > > undefined symbols.)
>> >
>> > I need an option which can be turned on and off.   How about
>> > -fextern-access=direct and -fextern-access=indirect?  It will cover
>> > both data and function?

-fno-direct-access-external-data and -fdirect-access-external-data can turn on/off the bit.

clang -fno-pic -fno-direct-access-external-data  works for x86-64 and aarch64.

We can add a -fno-direct-access-external

>> Yes, FWIW that option name for gcc also looks good to me.
>
>I will change the gcc option to
>
>-fextern-access=direct
>-fextern-access=indirect
>
>and change GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
>to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS

Note that this will be a glibc + GNU ld specific thing.

gold and ld.lld error for copy relocations on protected data symbols by default.

>> Now as to the need for a corresponding linker option, I'm of the
>> opinion that it is ideal for the linker to be able to cope without
>> needing special options.  Can you show me a set of object files (or
>> just describe them) where ld cannot deduce from relocations and
>> dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
>> are needed?  I'm fairly sure I manage to do that for powerpc.
>>
>> Note that I'm not against a new option to force the linker to go
>> against what it would do based on input object files (perhaps
>
>I'd like to turn it on in linker without any compiler changes, especially
>when building shared libraries, kind of a subset of -Bsymbolic.
>
>> reporting errors), but don't think we should have a new option without
>> some effort being made to see whether we really need it.
>
>Here is a glibc patch to use both linker options on some testcases:
>
>https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
>
>> > > The main thing that struck me about -fsingle-global-definition is that
>> > > the option doesn't do what it says.  You can still have multiple
>> > > global definitions of a given symbol, one in the executable and one in
>> > > each of the shared libraries making up the complete program.  Which of
>> > > course is no different to code without -fsingle-global-definition.
>> >
>> >
>> > --
>> > H.J.
>>
>> --
>> Alan Modra
>> Australia Development Lab, IBM
>
>
>
>-- 
>H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Mon, Jun 21, 2021 at 10:46 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2021-06-21, H.J. Lu wrote:
> >On Mon, Jun 21, 2021 at 9:16 PM Alan Modra <amodra@gmail.com> wrote:
> >>
> >> On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
> >> > On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
> >> > >
> >> > > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> >> > > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> >> > >
> >> > > -fno-direct-access-extern-data or variations on that also seem good to
> >> > > me.  -fpic-extern would also work.  I liked -fprotected-abi because
> >> > > it shows the intent of correcting abi issues related to protected
> >> > > visibility.  (Yes, it affects code for all undefined symbols because
> >> > > the compiler clearly isn't seeing the entire program if there are
> >> > > undefined symbols.)
> >> >
> >> > I need an option which can be turned on and off.   How about
> >> > -fextern-access=direct and -fextern-access=indirect?  It will cover
> >> > both data and function?
>
> -fno-direct-access-external-data and -fdirect-access-external-data can turn on/off the bit.
>
> clang -fno-pic -fno-direct-access-external-data  works for x86-64 and aarch64.
>
> We can add a -fno-direct-access-external

Since both clang and GCC will add a new option for both data and function
symbols, can we have an agreement on the new option name?  I am listing
options here:

1. -fdirect-access-external/-fno-direct-access-external
2. -fdirect-extern-access/-fno-direct-exern-access
3. -fdirect-external-access/-fno-direct-exernal-access
4. -fextern-access=direct/-fextern-access=indirect
5. -fexternal-access=direct/-fexternal-access=indirect

My order of preferences are 4, 5, 2, 3, 1.

> >> Yes, FWIW that option name for gcc also looks good to me.
> >
> >I will change the gcc option to
> >
> >-fextern-access=direct
> >-fextern-access=indirect
> >
> >and change GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
> >to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS
>
> Note that this will be a glibc + GNU ld specific thing.
>
> gold and ld.lld error for copy relocations on protected data symbols by default.

At run-time, there will be a mixture of components built with different tools
over time.  A marker will help glibc to avoid potential run-time failures due
to binary incompatibility.

> >> Now as to the need for a corresponding linker option, I'm of the
> >> opinion that it is ideal for the linker to be able to cope without
> >> needing special options.  Can you show me a set of object files (or
> >> just describe them) where ld cannot deduce from relocations and
> >> dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
> >> are needed?  I'm fairly sure I manage to do that for powerpc.
> >>
> >> Note that I'm not against a new option to force the linker to go
> >> against what it would do based on input object files (perhaps
> >
> >I'd like to turn it on in linker without any compiler changes, especially
> >when building shared libraries, kind of a subset of -Bsymbolic.
> >
> >> reporting errors), but don't think we should have a new option without
> >> some effort being made to see whether we really need it.
> >
> >Here is a glibc patch to use both linker options on some testcases:
> >
> >https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
> >
> >> > > The main thing that struck me about -fsingle-global-definition is that
> >> > > the option doesn't do what it says.  You can still have multiple
> >> > > global definitions of a given symbol, one in the executable and one in
> >> > > each of the shared libraries making up the complete program.  Which of
> >> > > course is no different to code without -fsingle-global-definition.
> >> >
> >> >
> >> > --
> >> > H.J.
> >>
> >> --
> >> Alan Modra
> >> Australia Development Lab, IBM
> >
> >
> >
> >--
> >H.J.



-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Michael Matz]

Hello,

On Mon, 21 Jun 2021, H.J. Lu via Binutils wrote:

> > Now as to the need for a corresponding linker option, I'm of the
> > opinion that it is ideal for the linker to be able to cope without
> > needing special options.  Can you show me a set of object files (or
> > just describe them) where ld cannot deduce from relocations and
> > dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
> > are needed?  I'm fairly sure I manage to do that for powerpc.
> >
> > Note that I'm not against a new option to force the linker to go
> > against what it would do based on input object files (perhaps
> 
> I'd like to turn it on in linker without any compiler changes, especially
> when building shared libraries, kind of a subset of -Bsymbolic.
> 
> > reporting errors), but don't think we should have a new option without
> > some effort being made to see whether we really need it.
> 
> Here is a glibc patch to use both linker options on some testcases:
> 
> https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html

But Alan asked for examples where the linker option is absolutely 
required.  AFAICS in the above examples the linker can determine the 
situation from the input files because (if either the right compiler 
options are used, or because that's the default for the architecture) 
they will contain only GOT relocations to the undefined symbols, and hence 
no copy relocs are needed or emitted.

Simply look at the busy work that you need to go through in one part of 
above patch:

+ifeq ($(have-fsingle-global-definition),yes)
+# Compile tst-prelink.c with -fno-single-global-definition to keepp COPY
+# relocation.
+CFLAGS-tst-prelink.c += -fno-single-global-definition
+endif
+ifeq ($(have-z-single-global-definition),yes)
+# Link tst-prelink with -z nosingle-global-definition to keepp COPY
+# relocation.
+LDFLAGS-tst-prelink += -Wl,-z,nosingle-global-definition
+endif

So, the only purpose of the linker flag seems to be to reflect what the 
compiler was already knowing, that it emitted a PC-relative reloc that 
ultimately requires a copy reloc.  The linker readily sees this, there's 
no need for an option, or is there?


Ciao,
Michael.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Tue, Jun 22, 2021 at 6:33 AM Michael Matz <matz@suse.de> wrote:
>
> Hello,
>
> On Mon, 21 Jun 2021, H.J. Lu via Binutils wrote:
>
> > > Now as to the need for a corresponding linker option, I'm of the
> > > opinion that it is ideal for the linker to be able to cope without
> > > needing special options.  Can you show me a set of object files (or
> > > just describe them) where ld cannot deduce from relocations and
> > > dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
> > > are needed?  I'm fairly sure I manage to do that for powerpc.
> > >
> > > Note that I'm not against a new option to force the linker to go
> > > against what it would do based on input object files (perhaps
> >
> > I'd like to turn it on in linker without any compiler changes, especially
> > when building shared libraries, kind of a subset of -Bsymbolic.
> >
> > > reporting errors), but don't think we should have a new option without
> > > some effort being made to see whether we really need it.
> >
> > Here is a glibc patch to use both linker options on some testcases:
> >
> > https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
>
> But Alan asked for examples where the linker option is absolutely
> required.  AFAICS in the above examples the linker can determine the
> situation from the input files because (if either the right compiler
> options are used, or because that's the default for the architecture)
> they will contain only GOT relocations to the undefined symbols, and hence
> no copy relocs are needed or emitted.

For new compilers, "-z single-global-definition" isn't needed.  But it can
be used with the old compilers to enable linker optimization.

> Simply look at the busy work that you need to go through in one part of
> above patch:
>
> +ifeq ($(have-fsingle-global-definition),yes)
> +# Compile tst-prelink.c with -fno-single-global-definition to keepp COPY
> +# relocation.
> +CFLAGS-tst-prelink.c += -fno-single-global-definition
> +endif
> +ifeq ($(have-z-single-global-definition),yes)
> +# Link tst-prelink with -z nosingle-global-definition to keepp COPY
> +# relocation.
> +LDFLAGS-tst-prelink += -Wl,-z,nosingle-global-definition
> +endif
>
> So, the only purpose of the linker flag seems to be to reflect what the
> compiler was already knowing, that it emitted a PC-relative reloc that
> ultimately requires a copy reloc.  The linker readily sees this, there's
> no need for an option, or is there?

For old compilers, "-z no-single-global-definition" isn't needed.   Linker
works better when it knows if copy relocation should be avoided at the
very beginning.   The property marker provides such info to linker.
"-z no-single-global-definition" tells linker to ignore such marker to
keep copy relocation.  For this particular testcase, the program runs
normally without copy relocation.  But the prelink test only works with
copy relocation.   We can either skip this test with the new tools or
keep copy relocation with the new tools using a linker option.  Since
I'd like to keep the test with the new tools, I use a linker option.

-- 
H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[Fangrui Song]

On 2021-06-22, H.J. Lu wrote:
>On Mon, Jun 21, 2021 at 10:46 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On 2021-06-21, H.J. Lu wrote:
>> >On Mon, Jun 21, 2021 at 9:16 PM Alan Modra <amodra@gmail.com> wrote:
>> >>
>> >> On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
>> >> > On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
>> >> > >
>> >> > > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
>> >> > > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
>> >> > >
>> >> > > -fno-direct-access-extern-data or variations on that also seem good to
>> >> > > me.  -fpic-extern would also work.  I liked -fprotected-abi because
>> >> > > it shows the intent of correcting abi issues related to protected
>> >> > > visibility.  (Yes, it affects code for all undefined symbols because
>> >> > > the compiler clearly isn't seeing the entire program if there are
>> >> > > undefined symbols.)
>> >> >
>> >> > I need an option which can be turned on and off.   How about
>> >> > -fextern-access=direct and -fextern-access=indirect?  It will cover
>> >> > both data and function?
>>
>> -fno-direct-access-external-data and -fdirect-access-external-data can turn on/off the bit.
>>
>> clang -fno-pic -fno-direct-access-external-data  works for x86-64 and aarch64.
>>
>> We can add a -fno-direct-access-external
>
>Since both clang and GCC will add a new option for both data and function
>symbols, can we have an agreement on the new option name?  I am listing
>options here:
>
>1. -fdirect-access-external/-fno-direct-access-external
>2. -fdirect-extern-access/-fno-direct-exern-access
>3. -fdirect-external-access/-fno-direct-exernal-access
>4. -fextern-access=direct/-fextern-access=indirect
>5. -fexternal-access=direct/-fexternal-access=indirect
>
>My order of preferences are 4, 5, 2, 3, 1.

Preferring "extern" to "external" looks fine to me. (`extern` is the C/C++ construct anyway and this option describes what to do with default visibility non-definition `extern int xxx`/`extern void foo()`).

-fextern-access=direct/-fextern-access=indirect and -fdirect-extern-access/-fno-direct-exern-access

look good to me.

I am happy to add aliases to clang if consensus is moving toward  -fextern-access=indirect or -fno-direct-extern-access.

>> >> Yes, FWIW that option name for gcc also looks good to me.
>> >
>> >I will change the gcc option to
>> >
>> >-fextern-access=direct
>> >-fextern-access=indirect
>> >
>> >and change GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
>> >to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS
>>
>> Note that this will be a glibc + GNU ld specific thing.
>>
>> gold and ld.lld error for copy relocations on protected data symbols by default.
>
>At run-time, there will be a mixture of components built with different tools
>over time.  A marker will help glibc to avoid potential run-time failures due
>to binary incompatibility.

glibc can perform the check without a GNU PROPERTY.


For a st_value!=0 && st_shndx==0 symbol lookup during relocation
processing, if the definition is found protected in a shared object,
ld.so can report an error and make a suggestion like
"consider recompiling the executable with -fno-direct-extern-access or
-fpie/-fpic"


I echo Michael's question in another thread
https://sourceware.org/pipermail/binutils/2021-June/117137.html

"IOW: which scenario do you want to not error on when you want to make the error conditional?"

For such rare cases, we can use a LD_* environment variable.

>> >> Now as to the need for a corresponding linker option, I'm of the
>> >> opinion that it is ideal for the linker to be able to cope without
>> >> needing special options.  Can you show me a set of object files (or
>> >> just describe them) where ld cannot deduce from relocations and
>> >> dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
>> >> are needed?  I'm fairly sure I manage to do that for powerpc.
>> >>
>> >> Note that I'm not against a new option to force the linker to go
>> >> against what it would do based on input object files (perhaps
>> >
>> >I'd like to turn it on in linker without any compiler changes, especially
>> >when building shared libraries, kind of a subset of -Bsymbolic.
>> >
>> >> reporting errors), but don't think we should have a new option without
>> >> some effort being made to see whether we really need it.
>> >
>> >Here is a glibc patch to use both linker options on some testcases:
>> >
>> >https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
>> >
>> >> > > The main thing that struck me about -fsingle-global-definition is that
>> >> > > the option doesn't do what it says.  You can still have multiple
>> >> > > global definitions of a given symbol, one in the executable and one in
>> >> > > each of the shared libraries making up the complete program.  Which of
>> >> > > course is no different to code without -fsingle-global-definition.
>> >> >
>> >> >
>> >> > --
>> >> > H.J.
>> >>
>> >> --
>> >> Alan Modra
>> >> Australia Development Lab, IBM
>> >
>> >
>> >
>> >--
>> >H.J.
>
>
>
>-- 
>H.J.

Re: [PATCH 2/2] elf: Add GNU_PROPERTY_1_NEEDED check

[H.J. Lu]

On Tue, Jun 22, 2021 at 11:15 AM Fangrui Song <i@maskray.me> wrote:
>
> On 2021-06-22, H.J. Lu wrote:
> >On Mon, Jun 21, 2021 at 10:46 PM Fangrui Song <i@maskray.me> wrote:
> >>
> >> On 2021-06-21, H.J. Lu wrote:
> >> >On Mon, Jun 21, 2021 at 9:16 PM Alan Modra <amodra@gmail.com> wrote:
> >> >>
> >> >> On Mon, Jun 21, 2021 at 07:12:02PM -0700, H.J. Lu wrote:
> >> >> > On Mon, Jun 21, 2021 at 5:06 PM Alan Modra <amodra@gmail.com> wrote:
> >> >> > >
> >> >> > > On Mon, Jun 21, 2021 at 03:34:38PM -0700, Fangrui Song wrote:
> >> >> > > > clang -fno-pic -fno-direct-access-extern-data  works with clang>=12.0.0 today.
> >> >> > >
> >> >> > > -fno-direct-access-extern-data or variations on that also seem good to
> >> >> > > me.  -fpic-extern would also work.  I liked -fprotected-abi because
> >> >> > > it shows the intent of correcting abi issues related to protected
> >> >> > > visibility.  (Yes, it affects code for all undefined symbols because
> >> >> > > the compiler clearly isn't seeing the entire program if there are
> >> >> > > undefined symbols.)
> >> >> >
> >> >> > I need an option which can be turned on and off.   How about
> >> >> > -fextern-access=direct and -fextern-access=indirect?  It will cover
> >> >> > both data and function?
> >>
> >> -fno-direct-access-external-data and -fdirect-access-external-data can turn on/off the bit.
> >>
> >> clang -fno-pic -fno-direct-access-external-data  works for x86-64 and aarch64.
> >>
> >> We can add a -fno-direct-access-external
> >
> >Since both clang and GCC will add a new option for both data and function
> >symbols, can we have an agreement on the new option name?  I am listing
> >options here:
> >
> >1. -fdirect-access-external/-fno-direct-access-external
> >2. -fdirect-extern-access/-fno-direct-exern-access
> >3. -fdirect-external-access/-fno-direct-exernal-access
> >4. -fextern-access=direct/-fextern-access=indirect
> >5. -fexternal-access=direct/-fexternal-access=indirect
> >
> >My order of preferences are 4, 5, 2, 3, 1.
>
> Preferring "extern" to "external" looks fine to me. (`extern` is the C/C++ construct anyway and this option describes what to do with default visibility non-definition `extern int xxx`/`extern void foo()`).
>
> -fextern-access=direct/-fextern-access=indirect and -fdirect-extern-access/-fno-direct-exern-access
>
> look good to me.

Let's go with -fdirect-extern-access/-fno-direct-exern-access

> I am happy to add aliases to clang if consensus is moving toward  -fextern-access=indirect or -fno-direct-extern-access.
>
> >> >> Yes, FWIW that option name for gcc also looks good to me.
> >> >
> >> >I will change the gcc option to
> >> >
> >> >-fextern-access=direct
> >> >-fextern-access=indirect
> >> >
> >> >and change GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION
> >> >to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS
> >>
> >> Note that this will be a glibc + GNU ld specific thing.
> >>
> >> gold and ld.lld error for copy relocations on protected data symbols by default.
> >
> >At run-time, there will be a mixture of components built with different tools
> >over time.  A marker will help glibc to avoid potential run-time failures due
> >to binary incompatibility.
>
> glibc can perform the check without a GNU PROPERTY.
>
>
> For a st_value!=0 && st_shndx==0 symbol lookup during relocation
> processing, if the definition is found protected in a shared object,
> ld.so can report an error and make a suggestion like
> "consider recompiling the executable with -fno-direct-extern-access or
> -fpie/-fpic"

There is no run-time issue when the shared object isn't built with
-fno-direct-extern-access.   That is what the marker is used for.

>
> I echo Michael's question in another thread
> https://sourceware.org/pipermail/binutils/2021-June/117137.html
>
> "IOW: which scenario do you want to not error on when you want to make the error conditional?"
>
> For such rare cases, we can use a LD_* environment variable.
>
> >> >> Now as to the need for a corresponding linker option, I'm of the
> >> >> opinion that it is ideal for the linker to be able to cope without
> >> >> needing special options.  Can you show me a set of object files (or
> >> >> just describe them) where ld cannot deduce from relocations and
> >> >> dynamic symbols what dynbss copies, plt stubs, and dynamic relocations
> >> >> are needed?  I'm fairly sure I manage to do that for powerpc.
> >> >>
> >> >> Note that I'm not against a new option to force the linker to go
> >> >> against what it would do based on input object files (perhaps
> >> >
> >> >I'd like to turn it on in linker without any compiler changes, especially
> >> >when building shared libraries, kind of a subset of -Bsymbolic.
> >> >
> >> >> reporting errors), but don't think we should have a new option without
> >> >> some effort being made to see whether we really need it.
> >> >
> >> >Here is a glibc patch to use both linker options on some testcases:
> >> >
> >> >https://sourceware.org/pipermail/libc-alpha/2021-June/127770.html
> >> >
> >> >> > > The main thing that struck me about -fsingle-global-definition is that
> >> >> > > the option doesn't do what it says.  You can still have multiple
> >> >> > > global definitions of a given symbol, one in the executable and one in
> >> >> > > each of the shared libraries making up the complete program.  Which of
> >> >> > > course is no different to code without -fsingle-global-definition.
> >> >> >
> >> >> >
> >> >> > --
> >> >> > H.J.
> >> >>
> >> >> --
> >> >> Alan Modra
> >> >> Australia Development Lab, IBM
> >> >
> >> >
> >> >
> >> >--
> >> >H.J.
> >
> >
> >
> >--
> >H.J.



-- 
H.J.