Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operands of the form '0b' in jump instructions in X86 Intel dialect inline assembly are not recognised as valid. #35492

Closed
llvmbot opened this issue Jan 30, 2018 · 5 comments
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 30, 2018

Bugzilla Link 36144
Resolution FIXED
Resolved on Oct 24, 2018 13:35
Version trunk
OS MacOS X
Blocks #11360
Attachments C++ source file, compilation of which with Clang reproduces the issue
Reporter LLVM Bugzilla Contributor
CC @topperc,@francisvm,@JDevlieghere,@RKSimon,@noloader,@rnk
Fixed by commit(s) r345189

Extended Description

Overview:
Using operands of the form '0b' (a numbered label appearing earlier in source relative to current source position) in jump instructions (any jump instruction, jmp, je, jne, jz, jnz etc.) in inline
intel dialect x86 assembly causes an 'Invalid operand for instruction' compilation error. In earlier versions (specific versions listed below) of clang this compiled correctly, producing code with the jmp target replaced with the correct label generated from the numbered label in the inline assembly.

Steps to Reproduce:

  1. Download the attached source 'x86IntelInlineAsmJmpToLabelRelativeTest.cpp'
  2. Using a version of clang 5.0 or greater (including trunk), attempt to compile this using: clang x86IntelInlineAsmJmpToLabelRelativeTest.cpp -o x86IntelInlineAsmJmpToLabelRelativeTest
  3. Observe the compilation error on line 17 of x86IntelInlineAsmJmpToLabelRelativeTest.cpp - "Invalid operand for instruction"

Actual Results:
The program fails to compile.

Expected Results:
The program compiles successfully, with the target of the jump instruction replaced with the correct label generated from the numbered label in the inline assembly.

Build Date & Hardware where bug was first encountered:
26 Jan 2018 - Xcode 9.3 Beta 1 (9Q98q), Apple LLVM version 9.1.0 (clang-902.0.30) - Mac OS 10.13.3 (17D47)

Additional Builds and Platforms:
Clang 5.0.0 (non-Xcode version) release reproduced the issue.
Clang 4.0.0 and 4.0.1 (non-Xcode version) releases did not reproduce the issue.
Locally compiled build of Clang on trunk (@ SVN revision 323529) reproduced the issue.

Additional information:
The attached code performs the same operation twice, first using Intel syntax, then using AT&T syntax to demonstrate the issue exists only in the Intel syntax path.

A brief investigation seems to show this arises from an ambiguity when parsing operands to instructions in Intel syntax after handling of MASM style Intel syntax was added in r280555. Because MASM allows integer literals of the form '011010b', '0b' is a valid integer literal representing value 0. The code in lib/MC/MCParser/AsmLexer.cpp - llvm::AsmLexer::LexDigit() with MASM style Intel assembly handling consumes the 'b' suffix on the literal. This means the special handling of positionally relative jump targets in lib/Target/X86/AsmParser/X86AsmParser.cpp - X86AsmParser::ParseIntelExpression() can no longer correctly detect this form of jump target, and incorrectly identifies the jump instruction's operand as just an integer which causes a compilation error.

@noloader
Copy link
Mannequin

noloader mannequin commented Apr 12, 2018

...
Using operands of the form '0b' (a numbered label appearing earlier in
source relative to current source position) in jump instructions (any jump
instruction, jmp, je, jne, jz, jnz etc.) in inline
intel dialect x86 assembly causes an 'Invalid operand for instruction'
compilation error. In earlier versions (specific versions listed below) of
clang this compiled correctly, producing code with the jmp target replaced
with the correct label generated from the numbered label in the inline
assembly.

The problem is bigger than just 0b. Crypto++ is experiencing it with 1b, also:

"1:\n"
...
"jnz 1b;\n"

Thanks to @​mouse07410 and @​alanbirtles at weidai11/cryptopp#636 .

@francisvm
Copy link
Collaborator

My understanding of this is that https://reviews.llvm.org/rL301390 sets

getParser().setParsingInlineAsm(true);

whenever it sees .intel_syntax. From my reading of the previous comments in various reviews / bugs, I understand that we rely on isParsingInlineAsm to actually check if we're parsing MSVC-style Intel syntax asm.

This breaks both things like the example Jeffrey attached:

1:
jnz 1b

and any binary immediate like:

.intel_syntax
and edi, 0b010101

What is the preferred way to fix this? Should we introduce a new flag for clang to set in ParseMicrosoftAsmAsmStatement and provide a hidden flag in LLVM to be able to test it independently?

@francisvm
Copy link
Collaborator

Ping? I think this is important to fix or revert as it's a pretty basic thing to support.

@rnk
Copy link
Collaborator

rnk commented Oct 22, 2018

I applied https://reviews.llvm.org/D53535 and it seems to fix the issue.

@rnk
Copy link
Collaborator

rnk commented Oct 24, 2018

Fixed by r345189.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants