Bug 25184 - or1k gas errors on any backwards jump on 32-bit host
Summary: or1k gas errors on any backwards jump on 32-bit host
Alias: None
Product: binutils
Classification: Unclassified
Component: gas (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Stafford Horne
Depends on:
Reported: 2019-11-11 00:10 UTC by Rich Felker
Modified: 2019-12-02 22:25 UTC (History)
2 users (show)

See Also:
Target: or1k-*-*
Last reconfirmed: 2019-12-02 00:00:00


Note You need to log in before you can comment on or make changes to this bug.
Description Rich Felker 2019-11-11 00:10:59 UTC
I don't understand the lisp-like language the opcode C files are generated from, so I don't have a root cause for this, but the problem is in opcodes/or1k-ibld.c, or1k_cgen_insert_operand. For at least the DISP26 case, and probably others, arithmetic that's logically signed is performed as unsigned. On 64-bit hosts, prior unsigned arithmetic (long-bfd_vma, where bfd_vma is unsigned long) has left 32 high 1 bits, so that unsigned right-shift followed by truncation gives the right result. But on 32-bit hosts, the top two bits end up zero, causing, for example, an offset of -8 to be interpreted as 0x3ffffff8, producing an "operand out of range" error.

Minimal test case:

1:      l.nop
        l.j 1b

There may also be integer overflows in insert_normal (1L shifted left up to 31 bits, I think) but these don't seem to be getting optimized in an unsafe manner with the host compilers I've used.
Comment 1 Stafford Horne 2019-12-02 21:42:52 UTC
I have reproduced this.  There are a few issues here:

  1. One reason for the truncations seems to be related to including 64-bit and 32-bit targets in the OR1K architecture definitions.  If I remove the 64-bit bit definitions everything seems to work fine.
  2. The other reason is that the PC Address type is 'unsigned int'.  Doing the computation of ((int)value - (unsigned int)pc)  results in an unsigned int.  We want for PC to be casted to a signed int, However it seems CGEN doesn't support that.  It may required a patch to cgen.

My thought it to remove the 64-bit definitions, we don't really use them (2- lines).  If we need it we can add them back as a separate architecture.