| Summary: | Unsigned arithmetic and %hi()/%low() operators | ||
|---|---|---|---|
| Product: | binutils | Reporter: | Orlando Arias <orlandoarias> |
| Component: | gas | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | amodra, nickc, orlandoarias |
| Priority: | P2 | ||
| Version: | 2.26 | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
| Attachments: |
Proposed patch
Revised patch extended patch |
||
|
Description
Orlando Arias
2016-04-05 14:13:39 UTC
Hi Orlando, This happens because the assembler does not support U as a suffix for integer constants. Reduced test case: % cat fred.s sethi %hi (((0x7FE3CU))), %fp sethi %hi (((0x7FE3C))), %fp sethi %hi (((0x7FE3CV))), %fp % as fred.s fred.s:1: Error: missing ')' fred.s:1: Error: missing ')' fred.s:3: Error: missing ')' fred.s:3: Error: missing ')' Obviously however the error message is bogus. So I will look into that. Cheers Nick (In reply to Nick Clifton from comment #1) Greetings: > This happens because the assembler does not support U as a suffix for > integer constants. Is there any chance that support for the suffix could be added to gas? Or is there an already existing suffix which can be used with both GCC and GAS to the same effect? I came across this issue when porting U-Boot for the LEON3 to run on a Xilinx KC705 board. The LEON3's main memory is mapped to start at 0x40000000 and the KC705 board provides 1GB of RAM. If leaving the constants as signed, computing the end of RAM an integer overflow occurs. GCC warns of this when the constants get expanded and non-working code is generated (U-Boot crashes on the board, have yet to debug to find out why, likely though because of the undefined behaviour generated by the overflow). I proceeded to add typing to the preprocessor directives and at that point the aforementioned error came up. > Obviously however the error message is bogus. So I will look into that. This should be fine as well. I can probably make two preprocessor directives, one to be used with the assembly sources and one to be used with the C sources. It is less than ideal, but it could work (although I am not sure as to how GAS would go on to handle the sum at that point). Thank you for looking into this. Cheers, Orlando. Created attachment 9154 [details]
Proposed patch
Hi Orlando,
Please try this patch, which should improve the error message somewhat.
Cheers
Nick
(In reply to Nick Clifton from comment #3) > Created attachment 9154 [details] > Proposed patch > > Hi Orlando, > > Please try this patch, which should improve the error message somewhat. > > Cheers > Nick Greetings, The patch works as expected, making the error much more clear. $ cat assembler.S sethi %hi(0x4000U), %g1 or %g0, %lo(0x4000U + 0x4000U), %g1 or %g0, %lo((0x4000U + 0x4000U)), %g1 $ sparc-leon3-linux-as -c assembler.S assembler.S: Assembler messages: assembler.S:3: Error: found 'U', expected: ')' However, if U as a suffix is not supported, the assembler should catch line 2 of the testcase as invalid as well. The assembler, however, parses it incorrectly: $ cat assembler.S sethi %hi(0x4000U), %g1 or %g0, %lo(0x4000U + 0x4000U), %g1 $ sparc-leon3-linux-as -c assembler.S $ sparc-leon3-linux-objdump -S a.out a.out: file format elf32-sparc Disassembly of section .text: 00000000 <.text>: 0: 03 00 00 10 sethi %hi(0x4000), %g1 4: 82 10 20 00 clr %g1 Manually expanding the pseudoinstruction, we see that it is actually doing: or %g0, 0, %g1 instead of throwing an error. Naturally, the immediate constant is incorrectly computed. Thank you for looking into this. Cheers, Orlando. > On 05 Apr 2016, at 18:04, orlandoarias at gmail dot com <sourceware-bugzilla@sourceware.org> wrote:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=19910
>
> --- Comment #2 from Orlando Arias <orlandoarias at gmail dot com> ---
> (In reply to Nick Clifton from comment #1)
> Greetings:
>
>> This happens because the assembler does not support U as a suffix for
>> integer constants.
>
> Is there any chance that support for the suffix could be added to gas? Or is
> there an already existing suffix which can be used with both GCC and GAS to the
> same effect? I came across this issue when porting U-Boot for the LEON3 to run
> on a Xilinx KC705 board.
You could use the preprocessor to add an U when preprocessing for C (and not
add it when preprocessing for assembly).
Hi Orlando, > Is there any chance that support for the suffix could be added to gas? Yes. In fact it is quite simple. The uploaded revised patch includes this change. > However, if U as a suffix is not supported, the assembler should catch line > 2 of the testcase as invalid as well. The assembler, however, parses it > incorrectly: This was a bug in the sparc backend of gas. The revised patch also takes care of this problem. Please try out the new patch and let me know if it works for you. Cheers Nick Created attachment 9155 [details]
Revised patch
Created attachment 9156 [details]
extended patch
For completeness sake, here is an extended version of the second patch which includes a couple of additions to the SPARC testsuite for gas. Just to make sure that the patch continues to work.
(In reply to Nick Clifton from comment #8) > Created attachment 9156 [details] > extended patch > > For completeness sake, here is an extended version of the second patch which > includes a couple of additions to the SPARC testsuite for gas. Just to make > sure that the patch continues to work. Greetings, This is excellent. I can confirm the patch works as expected. Thank you for your time. Cheers, Orlando. The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e140100a5da85568e83ffe8e77d3f5e4a59ddee8 commit e140100a5da85568e83ffe8e77d3f5e4a59ddee8 Author: Nick Clifton <nickc@redhat.com> Date: Thu Apr 7 12:34:06 2016 +0100 Allow integer contants to have a U suffix. Improve error reporting for missing closing parentheses. PR gas/19910 * config/tc-sparc.c (sparc_ip): Report an error if the expression inside a %-macro could not be fully parsed. * expr.c (integer_constant): Accept and ignore U suffixes to integers. (operand): When a missing closing parenthesis is encountered, report the character that was found instead. * testsuite/gas/mips/tls-ill.l: Update expected error message. * testsuite/gas/sparc/pr19910-1.d: New test driver. * testsuite/gas/sparc/pr19910-1.s: New test. * testsuite/gas/sparc/pr19910-2.l: Expected error output. * testsuite/gas/sparc/pr19910-2.s: New test. * testsuite/gas/sparc/sparc.exp: Run the new tests. Patch applied. sparc-aout +FAIL: PR19910 - make sure that U suffix is accepted sparc-coff +FAIL: PR19910 - make sure that U suffix is accepted .../gas/sparc/pr19910-2.s: Assembler messages: .../gas/sparc/pr19910-2.s:2: Error: Expression inside %hi could not be parsed .../gas/sparc/pr19910-2.s:5: Error: found 'V', expected: ')' .../gas/sparc/pr19910-2.s:5: Error: Expression inside %lo could not be parsed The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=992dc2c47069220ce5a94829a8d8fed3ee72a1d0 commit 992dc2c47069220ce5a94829a8d8fed3ee72a1d0 Author: Nick Clifton <nickc@redhat.com> Date: Fri Apr 8 10:36:49 2016 +0100 Fix regexps in PR19910 test to cope with COFF and AOUT sparc targets. PR target/19910 * testsuite/gas/sparc/pr19910-1.d: Adjust regexps to work with COFF and AOUT sparc targets. Hi Alan, > sparc-aout +FAIL: PR19910 - make sure that U suffix is accepted > sparc-coff +FAIL: PR19910 - make sure that U suffix is accepted Are we still supporting those targets ? I had no idea. :-) Oh well, thanks for letting me know. I have checked in a small update to the regexps in the pr19910 test so they pass now. Cheers Nick |