Bug 1692486 - clang fails to compile assembly with "error: unknown token in expression"
Summary: clang fails to compile assembly with "error: unknown token in expression"
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: clang
Version: 30
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: serge_sans_paille
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-03-25 17:10 UTC by Omair Majid
Modified: 2019-07-16 08:33 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2019-07-16 08:33:50 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Omair Majid 2019-03-25 17:10:24 UTC
clang fails to compile https://github.com/dotnet/coreclr/blob/master/src/vm/amd64/jithelpers_fast.S

The errors are all of the form "unknown token in expression":

  /home/omajid/source-build/src/coreclr/src/vm/amd64/jithelpers_fast.S:84:21: error: unknown token in expression
          shr rax, 0Ch
                      ^


It seems like you can reproduce the error using just:

void square() {
    asm("add rdi, 8h");
}

See https://godbolt.org/z/RvrdvU


Version-Release number of selected component (if applicable):
llvm-8.0.0-0.5.rc3.fc30.x86_64
clang-8.0.0-0.5.rc3.fc30.x86_64

Comment 1 serge_sans_paille 2019-03-25 18:03:59 UTC
This syntax is not documented in gnu-as: https://sourceware.org/binutils/docs-2.32/as/Integers.html#Integers, but they are documented in nasm: https://nasm.us/doc/nasmdoc3.html.

Comment 2 Omair Majid 2019-03-25 22:55:41 UTC
A couple of things I noticed:

On Fedora 30, adding -fno-integrated-as seems to change the error messages. Here's what the original compile command produces:

$ bash ./test.sh

/home/omajid/coreclr/src/vm/amd64/jithelpers_fast.S:84:21: error: unknown token in expression 
        shr rax, 0Ch                                                
                    ^                                              
/home/omajid/coreclr/src/vm/amd64/jithelpers_fast.S:88:31: error: unknown token in expression 
        cmp byte ptr [rax], 0h                                      
                              ^                                     

And this is what adding -fno-integrated-as does:

$ bash ./test.sh 
/home/omajid/coreclr/src/vm/amd64/jithelpers_fast.S: Assembler messages:
/home/omajid/coreclr/src/vm/amd64/jithelpers_fast.S:84: Error: junk `Ch' after expression
/home/omajid/coreclr/src/vm/amd64/jithelpers_fast.S:88: Error: junk `h' after expression

From the change in output, it looks like the backend indeed changed. Could it be that assembler backend of the new GCC rejects this syntax too?

I went digging into sources upstream and they made a commit here: https://github.com/dotnet/coreclr/pull/22810/commits/bdd0408f25a285deae0a69da659f4bc4d4f272f7. It is part of a series of fixes for supporting GCC (!) as a compiler. So it sounds like even GCC (or some other part of the GCC stack) rejects this code?

Comment 3 Omair Majid 2019-03-25 22:56:39 UTC
I can't see the GCC failure here, though: https://godbolt.org/z/mC6UE5

Comment 4 serge_sans_paille 2019-03-26 09:31:50 UTC
Patch submitted upstream: https://reviews.llvm.org/D59810

Comment 5 serge_sans_paille 2019-04-03 07:18:53 UTC
Upstream's answer in https://reviews.llvm.org/D59810 is relatively clear: the goal for clang here is to match gcc behavior, and as you pointed out, gcc does not accept the nasm syntax either, even within intel_syntax mode.

As you pointed out, https://github.com/dotnet/coreclr/pull/22810/commits/bdd0408f25a285deae0a69da659f4bc4d4f272f7 is a compatibility patch for gcc, and it should fix clang compatibility too.

Comment 6 serge_sans_paille 2019-05-22 07:10:32 UTC
Concerning the lack of GCC failure in https://godbolt.org/z/mC6UE5, it seems GCC is not making the syntax check on constants, but if you provide an assembly file to the GNU assembler, it indeed chokes on it: https://godbolt.org/z/9J8y2-

According to me, we should close this bug as WONTFIX.


Note You need to log in before you can comment on or make changes to this bug.