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
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gas (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Stafford Horne
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-11 00:10 UTC by Rich Felker
Modified: 2020-05-19 13:31 UTC (History)
2 users (show)

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


Attachments

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.
Comment 2 Sourceware Commits 2020-05-19 11:41:57 UTC
The master branch has been updated by Stafford Horne <shorne@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ae440402f54c634baddc462f8561442befe2cafe

commit ae440402f54c634baddc462f8561442befe2cafe
Author: Stafford Horne <shorne@gmail.com>
Date:   Tue May 19 20:40:27 2020 +0900

    or1k: Remove 64-bit support, it's not used and it breaks 32-bit hosts
    
    Reported by Rich Felker when building on 32-bit hosts.  Backwards jump
    negative offsets were not calculated correctly due to improper 32-bit
    to 64-bit zero-extension.  The 64-bit fields are present because we
    are mixing 32-bit and 64-bit architectures in our cpu descriptions.
    
    Removing 64-bit fixes the issue.  We don't use 64-bit, there is an architecture
    spec for 64-bit but no implementations or simulators.  My thought is if
    we need them in the future we should do the proper work to support both
    32-bit and 64-bit implementations co-existing then.
    
    cpu/ChangeLog:
    
    yyyy-mm-dd  Stafford Horne  <shorne@gmail.com>
    
            PR 25184
            * or1k.cpu (arch or1k): Remove or64 and or64nd machs.
            (ORBIS-MACHS, ORFPX32-MACHS): Remove pmacros.
            (cpu or1k64bf, mach or64, mach or64nd): Remove definitions.
            * or1kcommon.cpu (h-fdr): Remove hardware.
            * or1korfpx.cpu (rDDF, rADF, rBDF): Remove operand definitions.
            (float-regreg-insn): Remove lf- mnemonic -d instruction pattern.
            (float-setflag-insn-base): Remove lf-sf mnemonic -d pattern.
            (float-cust-insn): Remove "lf-cust" cust-num "-d" pattern.
            (lf-rem-d, lf-itof-d, lf-ftoi-d, lf-madd-d): Remove.
Comment 3 Sourceware Commits 2020-05-19 11:42:02 UTC
The master branch has been updated by Stafford Horne <shorne@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a501eb446f5149c1133dbc99f86743b8dd614fa4

commit a501eb446f5149c1133dbc99f86743b8dd614fa4
Author: Stafford Horne <shorne@gmail.com>
Date:   Tue May 19 20:40:27 2020 +0900

    or1k: Regenerate opcodes after removing 32-bit support
    
    opcodes/ChangeLog:
    
    yyyy-mm-dd  Stafford Horne  <shorne@gmail.com>
    
            PR 25184
            * or1k-asm.c: Regenerate.
            * or1k-desc.c: Regenerate.
            * or1k-desc.h: Regenerate.
            * or1k-dis.c: Regenerate.
            * or1k-ibld.c: Regenerate.
            * or1k-opc.c: Regenerate.
            * or1k-opc.h: Regenerate.
            * or1k-opinst.c: Regenerate.
Comment 4 Stafford Horne 2020-05-19 13:31:25 UTC
The committed patches fix this for me.