RISC-V: Force linker error exit after unresolvable reloc.

Message ID 20190830221610.2559-1-jimw@sifive.com
State New
Headers show
Series
  • RISC-V: Force linker error exit after unresolvable reloc.
Related show

Commit Message

Jim Wilson Aug. 30, 2019, 10:16 p.m.
This was noticed while trying to test the compiler -msave-restore support.
Putting non-pic code in a shared library gives a linker error, but doesn't
stop the build.

rohan:2030$ cat libtmp.c
extern int sub2 (int);
int sub (int i) { return sub2 (i + 10); }
rohan:2031$ cat libtmp2.c
extern int sub (int);
int sub2 (int i) { return sub (i + 10); }
rohan:2032$ riscv64-unknown-linux-gnu-gcc --shared -o libtmp.so libtmp.c
rohan:2033$ riscv64-unknown-linux-gnu-gcc --shared -o libtmp2.so libtmp2.c libtmp.so
/home/jimw/FOSS/install-riscv64/lib/gcc/riscv64-unknown-linux-gnu/8.3.0/../../../../riscv64-unknown-linux-gnu/bin/ld: /tmp/cctrsIBe.o(.text+0x18): unresolvable R_RISCV_CALL relocation against symbol `sub'
rohan:2034$ echo $?
0
rohan:2035$ ls -lt libtmp2.so
-rwxr-xr-x 1 jimw jimw 6912 Aug 30 14:32 libtmp2.so
rohan:2036$

The patch fixes this by forcing a linker error.  I now get this.

ohan:2059$ sh tmp.script
/home/jimw/FOSS/BINUTILS/X-riscv64-linux/ld/ld-new: libtmp2.o(.text+0x18): unresolvable R_RISCV_CALL relocation against symbol `sub'
/home/jimw/FOSS/BINUTILS/X-riscv64-linux/ld/ld-new: final link failed: bad value
rohan:2060$ echo $?
1
rohan:2061$ ls -lt libtmp2.so
ls: cannot access 'libtmp2.so': No such file or directory

This was tested with 32/64-bit elf/linux cross binutils build and checks.
There were no regressions.  It also works for the original problem.

Committed.

Jim

	bfd/
	* elfnn-riscv.c (riscv_elf_relocate_section): For unresolvable reloc
	error, call bfd_set_error, set ret to FALSE, and goto out label.
---
 bfd/elfnn-riscv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Maciej W. Rozycki Sept. 30, 2019, 12:39 p.m. | #1
Jim,

> The patch fixes this by forcing a linker error.  I now get this.

> 

> ohan:2059$ sh tmp.script

> /home/jimw/FOSS/BINUTILS/X-riscv64-linux/ld/ld-new: libtmp2.o(.text+0x18): unresolvable R_RISCV_CALL relocation against symbol `sub'

> /home/jimw/FOSS/BINUTILS/X-riscv64-linux/ld/ld-new: final link failed: bad value

> rohan:2060$ echo $?

> 1

> rohan:2061$ ls -lt libtmp2.so

> ls: cannot access 'libtmp2.so': No such file or directory

[...]
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c

> index 4729bae09a..ef2471eb99 100644

> --- a/bfd/elfnn-riscv.c

> +++ b/bfd/elfnn-riscv.c

> @@ -2297,7 +2297,10 @@ riscv_elf_relocate_section (bfd *output_bfd,

>  	     (uint64_t) rel->r_offset,

>  	     howto->name,

>  	     h->root.root.string);

> -	  continue;

> +

> +	  bfd_set_error (bfd_error_bad_value);

> +	  ret = FALSE;

> +	  goto out;


 FYI, it's better done with one the percent-codes to `_bfd_error_handler' 
rather than aborting the link right away, so that any further link errors 
are also reported and you don't have to shake them out one by one.  I've 
done such an improvement for several error cases in the MIPS backend.

 Since none was attached to this change of yours, is there a test case (as 
one obviously should) already present in the LD test suite that covers 
this error?  If so, then I can start from there and provide you with a 
quick change to show you what I mean.

  Maciej
Jim Wilson Sept. 30, 2019, 8:38 p.m. | #2
On Mon, Sep 30, 2019 at 5:39 AM Maciej W. Rozycki <macro@wdc.com> wrote:
>  FYI, it's better done with one the percent-codes to `_bfd_error_handler'

> rather than aborting the link right away, so that any further link errors

> are also reported and you don't have to shake them out one by one.  I've

> done such an improvement for several error cases in the MIPS backend.


I was pressed for time.  I found this by accident while working on a
bigger and more important problem.  The right way to fix this is
presumably the same way I fixed the bfd_reloc_dangerous support later
in the same function.

I don't see the practical use for emitting more linker errors in this
case though.  If you accidentally linked non-pic code into a shared
library, then you are just going to get a lot of copies of the exact
same error message.  The fix is to recompile PIC.  You don't need more
than one error message to figure this out.

>  Since none was attached to this change of yours, is there a test case (as

> one obviously should) already present in the LD test suite that covers

> this error?  If so, then I can start from there and provide you with a

> quick change to show you what I mean.


There is a testcase in the email.  There isn't one in the ld testsuite
because I was pressed for time.

I don't see how this helps me.  I already know how to fix it
correctly.  I just don't have time to do that.  I can put finishing
this on my to do list, but it may be a very long wait before I have
time to work on this again.  Unless maybe I can get someone else
inside SiFive to work on this, we do have a new guy that is doing some
binutils work that might be able to help.

Jim

Patch

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 4729bae09a..ef2471eb99 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -2297,7 +2297,10 @@  riscv_elf_relocate_section (bfd *output_bfd,
 	     (uint64_t) rel->r_offset,
 	     howto->name,
 	     h->root.root.string);
-	  continue;
+
+	  bfd_set_error (bfd_error_bad_value);
+	  ret = FALSE;
+	  goto out;
 	}
 
       if (r == bfd_reloc_ok)