public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nelson Chu <nelson@rivosinc.com>
To: binutils@sourceware.org, jim.wilson.gcc@gmail.com, palmer@dabbelt.com
Cc: nelson@rivosinc.com, Palmer Dabbelt <palmer@rivosinc.com>
Subject: [PATCH 3/3] RISC-V: PR28789, Reject R_RISCV_PCREL relocations with ABS symbol in PIC/PIE.
Date: Sat, 25 Mar 2023 08:41:13 +0800	[thread overview]
Message-ID: <20230325004113.22673-3-nelson@rivosinc.com> (raw)
In-Reply-To: <20230325004113.22673-1-nelson@rivosinc.com>

From: Palmer Dabbelt <palmer@rivosinc.com>

The non-preemptible SHN_ABS symbol with a pc-relative relocation should be
disallowed when generating shared object (pic and pie).  Generally, the
following cases, which refer to pr25749, will cause a symbol be
non-preemptible,

* -pie, or -shared with -symbolic
* STV_HIDDEN, STV_INTERNAL, STV_PROTECTED
* Have dynamic symbol table, but without the symbol
* VER_NDX_LOCAL

However, PCREL_HI20/LO12 relocs are always bind locally when generating
shared object, so not only the non-preemptible absolute symbol need to
be disallowed, all absolute symbol references need but except that they
are defined in linker script.  If we also disallow the absolute symbol
in linker script, then the glibc-linux toolchain build failed, so regard
them as pc-relative symbols, just like what x86 did.

Maybe we should add this check for all pc-relative relocations, rather
than just handle in R_RISCV_PCREL relocs.  Ideally, since the value of
SHN_ABS symbol is a constant, only S - A relocations should be allowed
in the shared object, so only BFD_RELOC_8/16/32/64 are allowed, which
means R_RISCV_32/R_RISCV_64.

bfd/
    PR 28789
    * elfnn-riscv.c (riscv_elf_check_relocs): The absolute symbol cannot be
    referneced with pc-relative relocation when generating shared object.
ld/
    PR 28789
    * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
    * ld/testsuite/ld-riscv-elf/pcrel-reloc*: New testcases.
---
 bfd/elfnn-riscv.c                             | 41 +++++++++++++++++++
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp    |  7 ++++
 .../ld-riscv-elf/pcrel-reloc-abs-nopie.d      | 14 +++++++
 .../ld-riscv-elf/pcrel-reloc-abs-pie.d        |  5 +++
 ld/testsuite/ld-riscv-elf/pcrel-reloc-abs.s   |  2 +
 .../ld-riscv-elf/pcrel-reloc-rel-nopie.d      | 14 +++++++
 .../ld-riscv-elf/pcrel-reloc-rel-pie.d        | 14 +++++++
 ld/testsuite/ld-riscv-elf/pcrel-reloc-rel.s   |  9 ++++
 ld/testsuite/ld-riscv-elf/pcrel-reloc.s       |  5 +++
 9 files changed, 111 insertions(+)
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-nopie.d
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-pie.d
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-abs.s
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-nopie.d
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-pie.d
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc-rel.s
 create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-reloc.s

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 00f034a6751..0dd9b27c8ae 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -862,6 +862,47 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 		 ifunc symbol.  */
 	      h->plt.refcount += 1;
 	    }
+
+	  /* The non-preemptible absolute symbol shouldn't be referneced with
+	     pc-relative relocation when generating shared object.  However,
+	     PCREL_HI20/LO12 relocs are always bind locally when generating
+	     shared object, so all absolute symbol referenced need to be
+	     disallowed, except they are defined in linker script.
+
+	     Maybe we should add this check for all pc-relative relocations,
+	     please see pr28789 and pr25749 for details.  */
+	  if (bfd_link_pic (info)
+	      /* (h == NULL || SYMBOL_REFERENCES_LOCAL (info, h))  */
+	      && is_abs_symbol)
+	    {
+	      if (h != NULL && (h)->root.ldscript_def)
+		/* Disallow the absolute symbol defined in linker script here
+		   will cause the glibc-linux toolchain build failed, so regard
+		   them as pc-relative symbols, just like what x86 did.  */
+		;
+	      else
+		{
+		  const char *name;
+		  if (h->root.root.string)
+		    name = h->root.root.string;
+		  else
+		    {
+		      Elf_Internal_Sym *sym;
+		      sym = bfd_sym_from_r_symndx (&htab->elf.sym_cache, abfd,
+						   r_symndx);
+		      name = bfd_elf_sym_name (abfd, symtab_hdr, sym, NULL);
+		    }
+
+		  reloc_howto_type *r_t =
+			riscv_elf_rtype_to_howto (abfd, r_type);
+		  _bfd_error_handler
+		    (_("%pB: relocation %s against absolute symbol `%s' can "
+		       "not be used when making a shared object"),
+		     abfd, r_t ? r_t->name : _("<unknown>"), name);
+		  bfd_set_error (bfd_error_bad_value);
+		  return false;
+		}
+	    }
 	  /* Fall through.  */
 
 	case R_RISCV_JAL:
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 1b2a5ce2cb2..43572c5286b 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -308,4 +308,11 @@ if [istarget "riscv*-*-*"] {
     run_dump_test "ifunc-seperate-plt-pic"
     run_dump_test "ifunc-seperate-pcrel-pie"
     run_dump_test "ifunc-seperate-pcrel-pic"
+
+    # Tests related to mixing medany code into position-independent targets,
+    # where it's not always possible to generate correct addressing sequences.
+    run_dump_test "pcrel-reloc-rel-nopie"
+    run_dump_test "pcrel-reloc-rel-pie"
+    run_dump_test "pcrel-reloc-abs-nopie"
+    run_dump_test "pcrel-reloc-abs-pie"
 }
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-nopie.d b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-nopie.d
new file mode 100644
index 00000000000..54026388b7a
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-nopie.d
@@ -0,0 +1,14 @@
+#source: pcrel-reloc.s
+#source: pcrel-reloc-abs.s
+#as: -march=rv64i -mabi=lp64
+#ld: -melf64lriscv --no-pie --no-relax
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section \.text:
+
+[0-9a-f]+ <_start>:
+.*auipc.*
+.*lw.*# [0-9a-f]* <sym>
+#pass
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-pie.d b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-pie.d
new file mode 100644
index 00000000000..7f5eaa321f7
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs-pie.d
@@ -0,0 +1,5 @@
+#source: pcrel-reloc.s
+#source: pcrel-reloc-abs.s
+#as: -march=rv64i -mabi=lp64
+#ld: -melf64lriscv --pie --no-relax
+#error: .*relocation R_RISCV_PCREL_HI20 against absolute symbol `sym' can not be used when making a shared objec.*t
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs.s b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs.s
new file mode 100644
index 00000000000..1df32a1a3fb
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-abs.s
@@ -0,0 +1,2 @@
+.global sym
+.set sym,0x8000
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-nopie.d b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-nopie.d
new file mode 100644
index 00000000000..ab2a3774cdd
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-nopie.d
@@ -0,0 +1,14 @@
+#source: pcrel-reloc.s
+#source: pcrel-reloc-rel.s
+#as: -march=rv64i -mabi=lp64
+#ld: -melf64lriscv --no-pie --no-relax
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section \.text:
+
+[0-9a-f]+ <_start>:
+.*auipc.*
+.*lw.*# [0-9a-f]* <sym>
+#pass
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-pie.d b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-pie.d
new file mode 100644
index 00000000000..aec612d4d2c
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel-pie.d
@@ -0,0 +1,14 @@
+#source: pcrel-reloc.s
+#source: pcrel-reloc-rel.s
+#as: -march=rv64i -mabi=lp64
+#ld: -melf64lriscv --pie --no-relax
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section \.text:
+
+[0-9a-f]+ <_start>:
+.*auipc.*
+.*lw.*# [0-9a-f]* <sym>
+#pass
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel.s b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel.s
new file mode 100644
index 00000000000..fb0e6c09f22
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc-rel.s
@@ -0,0 +1,9 @@
+.data
+# Makes sure "sym" doesn't end up at the beginning of ".data", as that makes it
+# tough to then later detect it from scripts.
+.global buf
+buf:
+    .fill 8192, 4, 1
+.global sym
+sym:
+    .fill 8192, 4, 2
diff --git a/ld/testsuite/ld-riscv-elf/pcrel-reloc.s b/ld/testsuite/ld-riscv-elf/pcrel-reloc.s
new file mode 100644
index 00000000000..db2103bafd1
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcrel-reloc.s
@@ -0,0 +1,5 @@
+.text
+.global _start
+_start:
+    auipc t0, %pcrel_hi(sym)
+    lw t0, %pcrel_lo(_start)(t0)
-- 
2.37.1 (Apple Git-137.1)


      parent reply	other threads:[~2023-03-25  0:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-25  0:41 [PATCH 1/3] RISC-V: Extract the ld code which are too complicated, and may be reused Nelson Chu
2023-03-25  0:41 ` [PATCH 2/3] RISC-V: Clarify link behaviors of R_RISCV_32/64 relocations with ABS symbol Nelson Chu
2023-03-30  0:12   ` Nelson Chu
2023-03-25  0:41 ` Nelson Chu [this message]

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=20230325004113.22673-3-nelson@rivosinc.com \
    --to=nelson@rivosinc.com \
    --cc=binutils@sourceware.org \
    --cc=jim.wilson.gcc@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=palmer@rivosinc.com \
    /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).