Bug 1692486

Summary:	clang fails to compile assembly with "error: unknown token in expression"
Product:	[Fedora] Fedora	Reporter:	Omair Majid <omajid>
Component:	clang	Assignee:	serge_sans_paille <sguelton>
Status:	CLOSED WONTFIX	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	30	CC:	airlied, sbergman, sguelton, siddharth.kde, tstellar
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2019-07-16 08:33:50 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

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.