public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nelson Chu <nelson@rivosinc.com>
To: binutils@sourceware.org
Cc: nelson@rivosinc.com
Subject: [committed] RISC-V: PR28509, the default visibility symbol cannot be referenced by R_RISCV_JAL.
Date: Mon, 12 Sep 2022 11:59:44 +0800	[thread overview]
Message-ID: <20220912035944.22717-1-nelson@rivosinc.com> (raw)

When generating the shared object, the default visibility symbols may bind
externally, which means they will be exported to the dynamic symbol table,
and are preemptible by default.  These symbols cannot be referenced by the
non-pic R_RISCV_JAL and R_RISCV_RVC_JUMP.  However, consider that linker
may relax the R_RISCV_CALL relocations to R_RISCV_JAL or R_RISCV_RVC_JUMP,
if these relocations are relocated to the plt entries, then we won't report
error for them.  Perhaps we also need the similar checks for the
R_RISCV_BRANCH and R_RISCV_RVC_BRANCH relocations.

After applying this patch, and revert the following glibc patch,
riscv: Fix incorrect jal with HIDDEN_JUMPTARGET
https://sourceware.org/git/?p=glibc.git;a=commit;h=68389203832ab39dd0dbaabbc4059e7fff51c29b

I get the expected errors as follows,
ld: relocation R_RISCV_RVC_JUMP against `__sigsetjmp' which may bind externally can not be used when making a shared object; recompile with -fPIC
ld: relocation R_RISCV_JAL against `exit' which may bind externally can not be used when making a shared object; recompile with -fPIC

Besides, we also have similar changes for libgcc,
RISC-V: jal cannot refer to a default visibility symbol for shared object
https://github.com/gcc-mirror/gcc/commit/45116f342057b7facecd3d05c2091ce3a77eda59

bfd/
	pr 28509
	* elfnn-riscv.c (riscv_elf_relocate_section): Report errors when
	makeing a shard object, and the referenced symbols of R_RISCV_JAL
	relocations are default visibility.  Besides, we should handle most
	of the cases here, so don't need the unresolvable check later for
	R_RISCV_JAL and R_RISCV_RVC_JUMP.
ld/
	pr 28509
	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
	* testsuite/ld-riscv-elf/lib-nopic-01a.s: Removed.
	* testsuite/ld-riscv-elf/lib-nopic-01b.d: Likewise.
	* testsuite/ld-riscv-elf/lib-nopic-01b.s: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-01.d: New testcase.
	* testsuite/ld-riscv-elf/shared-lib-nopic-01.s: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-02.d: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-02.s: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-03.d: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-03.s: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-04.d: Likewise.
	* testsuite/ld-riscv-elf/shared-lib-nopic-04.s: Likewise.
---
 bfd/elfnn-riscv.c                             | 71 +++++++++++--------
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp    | 10 ++-
 ld/testsuite/ld-riscv-elf/lib-nopic-01a.s     |  9 ---
 ld/testsuite/ld-riscv-elf/lib-nopic-01b.d     |  5 --
 ld/testsuite/ld-riscv-elf/lib-nopic-01b.s     |  9 ---
 .../ld-riscv-elf/shared-lib-nopic-01.d        |  4 ++
 .../ld-riscv-elf/shared-lib-nopic-01.s        |  3 +
 .../ld-riscv-elf/shared-lib-nopic-02.d        |  4 ++
 .../ld-riscv-elf/shared-lib-nopic-02.s        | 12 ++++
 .../ld-riscv-elf/shared-lib-nopic-03.d        | 14 ++++
 .../ld-riscv-elf/shared-lib-nopic-03.s        | 13 ++++
 .../ld-riscv-elf/shared-lib-nopic-04.d        | 15 ++++
 .../ld-riscv-elf/shared-lib-nopic-04.s        | 14 ++++
 13 files changed, 126 insertions(+), 57 deletions(-)
 delete mode 100644 ld/testsuite/ld-riscv-elf/lib-nopic-01a.s
 delete mode 100644 ld/testsuite/ld-riscv-elf/lib-nopic-01b.d
 delete mode 100644 ld/testsuite/ld-riscv-elf/lib-nopic-01b.s
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.d
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.s
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.d
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.s
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.d
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.s
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.d
 create mode 100644 ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.s

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 86cb207b573..0e0a0b09e24 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -2457,12 +2457,44 @@ riscv_elf_relocate_section (bfd *output_bfd,
 
 	case R_RISCV_JAL:
 	case R_RISCV_RVC_JUMP:
-	  /* This line has to match the check in _bfd_riscv_relax_section.  */
-	  if (bfd_link_pic (info) && h != NULL && h->plt.offset != MINUS_ONE)
+	  if (bfd_link_pic (info) && h != NULL)
 	    {
-	      /* Refer to the PLT entry.  */
-	      relocation = sec_addr (htab->elf.splt) + h->plt.offset;
-	      unresolved_reloc = false;
+	      if (h->plt.offset != MINUS_ONE)
+		{
+		  /* Refer to the PLT entry.  This check has to match the
+		     check in _bfd_riscv_relax_section.  */
+		  relocation = sec_addr (htab->elf.splt) + h->plt.offset;
+		  unresolved_reloc = false;
+		}
+	      else if (!SYMBOL_REFERENCES_LOCAL (info, h)
+		       && (input_section->flags & SEC_ALLOC) != 0
+		       && (input_section->flags & SEC_READONLY) != 0
+		       && ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
+		{
+		  /* PR 28509, when generating the shared object, these
+		     referenced symbols may bind externally, which means
+		     they will be exported to the dynamic symbol table,
+		     and are preemptible by default.  These symbols cannot
+		     be referenced by the non-pic relocations, like
+		     R_RISCV_JAL and R_RISCV_RVC_JUMP relocations.
+
+		     However, consider that linker may relax the R_RISCV_CALL
+		     relocations to R_RISCV_JAL or R_RISCV_RVC_JUMP, if
+		     these relocations are relocated to the plt entries,
+		     then we won't report error for them.
+
+		     Perhaps we also need the similar checks for the
+		     R_RISCV_BRANCH and R_RISCV_RVC_BRANCH relocations.  */
+		  if (asprintf (&msg_buf,
+				_("%%X%%P: relocation %s against `%s' which "
+				  "may bind externally can not be used when "
+				  "making a shared object; recompile "
+				  "with -fPIC\n"),
+				howto->name, h->root.root.string) == -1)
+		    msg_buf = NULL;
+		  msg = msg_buf;
+		  r = bfd_reloc_notsupported;
+		}
 	    }
 	  break;
 
@@ -2755,29 +2787,12 @@ riscv_elf_relocate_section (bfd *output_bfd,
 	  && _bfd_elf_section_offset (output_bfd, info, input_section,
 				      rel->r_offset) != (bfd_vma) -1)
 	{
-	  switch (r_type)
-	    {
-	    case R_RISCV_JAL:
-	    case R_RISCV_RVC_JUMP:
-	      if (asprintf (&msg_buf,
-			    _("%%X%%P: relocation %s against `%s' can "
-			      "not be used when making a shared object; "
-			      "recompile with -fPIC\n"),
-			    howto->name,
-			    h->root.root.string) == -1)
-		msg_buf = NULL;
-	      break;
-
-	    default:
-	      if (asprintf (&msg_buf,
-			    _("%%X%%P: unresolvable %s relocation against "
-			      "symbol `%s'\n"),
-			    howto->name,
-			    h->root.root.string) == -1)
-		msg_buf = NULL;
-	      break;
-	    }
-
+	  if (asprintf (&msg_buf,
+			_("%%X%%P: unresolvable %s relocation against "
+			  "symbol `%s'\n"),
+			howto->name,
+			h->root.root.string) == -1)
+	    msg_buf = NULL;
 	  msg = msg_buf;
 	  r = bfd_reloc_notsupported;
 	}
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 272424b33e3..df89e0ee68b 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -210,12 +210,10 @@ if [istarget "riscv*-*-*"] {
 				    "gp-test-${abi}"]]
     }
 
-    run_ld_link_tests {
-	{ "Link non-pic code into a shared library (setup)"
-	    "-shared" "" "" {lib-nopic-01a.s}
-	    {} "lib-nopic-01a.so" }
-    }
-    run_dump_test "lib-nopic-01b"
+    run_dump_test "shared-lib-nopic-01"
+    run_dump_test "shared-lib-nopic-02"
+    run_dump_test "shared-lib-nopic-03"
+    run_dump_test "shared-lib-nopic-04"
 
     # IFUNC testcases.
     # Check IFUNC by single type relocs.
diff --git a/ld/testsuite/ld-riscv-elf/lib-nopic-01a.s b/ld/testsuite/ld-riscv-elf/lib-nopic-01a.s
deleted file mode 100644
index 632875d423a..00000000000
--- a/ld/testsuite/ld-riscv-elf/lib-nopic-01a.s
+++ /dev/null
@@ -1,9 +0,0 @@
-        .option nopic
-	.text
-	.align  1
-	.globl  func1
-	.type   func1, @function
-func1:
-	jal	func2
-	jr      ra
-	.size   func1, .-func1
diff --git a/ld/testsuite/ld-riscv-elf/lib-nopic-01b.d b/ld/testsuite/ld-riscv-elf/lib-nopic-01b.d
deleted file mode 100644
index 1c2c907fc30..00000000000
--- a/ld/testsuite/ld-riscv-elf/lib-nopic-01b.d
+++ /dev/null
@@ -1,5 +0,0 @@
-#name: link non-pic code into a shared library
-#source: lib-nopic-01b.s
-#as:
-#ld: -shared tmpdir/lib-nopic-01a.so
-#error: .*relocation R_RISCV_JAL against `func1' can not be used when making a shared object; recompile with -fPIC
diff --git a/ld/testsuite/ld-riscv-elf/lib-nopic-01b.s b/ld/testsuite/ld-riscv-elf/lib-nopic-01b.s
deleted file mode 100644
index ea7b0298895..00000000000
--- a/ld/testsuite/ld-riscv-elf/lib-nopic-01b.s
+++ /dev/null
@@ -1,9 +0,0 @@
-        .option nopic
-	.text
-	.align  1
-	.globl  func2
-	.type   func2, @function
-func2:
-	jal	func1
-	jr      ra
-	.size   func2, .-func2
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.d b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.d
new file mode 100644
index 00000000000..2ea851247f6
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.d
@@ -0,0 +1,4 @@
+#source: shared-lib-nopic-01.s
+#as:
+#ld: -shared
+#error: .*relocation R_RISCV_JAL against `foo' which may bind externally can not be used when making a shared object; recompile with -fPIC
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.s b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.s
new file mode 100644
index 00000000000..8d85c17cbdf
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-01.s
@@ -0,0 +1,3 @@
+        .option nopic
+	.text
+	jal	foo	# undefined
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.d b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.d
new file mode 100644
index 00000000000..f866d017362
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.d
@@ -0,0 +1,4 @@
+#source: shared-lib-nopic-02.s
+#as:
+#ld: -shared
+#error: .*relocation R_RISCV_JAL against `foo_default' which may bind externally can not be used when making a shared object; recompile with -fPIC
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.s b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.s
new file mode 100644
index 00000000000..61a8cc1a87b
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-02.s
@@ -0,0 +1,12 @@
+        .option nopic
+	.text
+	.align  1
+
+	jal	foo_default
+
+	.globl  foo_default
+	.type   foo_default, @function
+foo_default:
+	nop
+	ret
+	.size   foo_default, .-foo_default
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.d b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.d
new file mode 100644
index 00000000000..182a557bab8
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.d
@@ -0,0 +1,14 @@
+#source: shared-lib-nopic-03.s
+#as:
+#ld: -shared
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section \.text:
+
+#...
+.*:[ 	]+[0-9a-f]+[ 	]+jal[ 	]+.* <foo_default>
+
+0+[0-9a-f]+ <foo_default>:
+#...
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.s b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.s
new file mode 100644
index 00000000000..d1855f8ff30
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-03.s
@@ -0,0 +1,13 @@
+        .option nopic
+	.text
+	.align  1
+
+	jal	foo_default
+
+	.globl  foo_default
+	.hidden	foo_default
+	.type   foo_default, @function
+foo_default:
+	nop
+	ret
+	.size   foo_default, .-foo_default
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.d b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.d
new file mode 100644
index 00000000000..e66b721a137
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.d
@@ -0,0 +1,15 @@
+#source: shared-lib-nopic-04.s
+#as:
+#ld: -shared
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+#...
+Disassembly of section \.text:
+#...
+.*:[ 	]+[0-9a-f]+[ 	]+jal[ 	]+.* <foo_default@plt>
+.*:[ 	]+[0-9a-f]+[ 	]+jal[ 	]+.* <foo_default@plt>
+
+0+[0-9a-f]+ <foo_default>:
+#...
diff --git a/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.s b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.s
new file mode 100644
index 00000000000..46760f809df
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/shared-lib-nopic-04.s
@@ -0,0 +1,14 @@
+        .option nopic
+	.text
+	.align  1
+
+	call	foo_default
+	jal	foo_default
+
+
+	.globl  foo_default
+	.type   foo_default, @function
+foo_default:
+	nop
+	ret
+	.size   foo_default, .-foo_default
-- 
2.32.1 (Apple Git-133)


                 reply	other threads:[~2022-09-12  3:59 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220912035944.22717-1-nelson@rivosinc.com \
    --to=nelson@rivosinc.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).