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

Jump to bottom

cmd/internal/obj/x86: improve asm error messages #21860

Open
quasilyte opened this issue Sep 13, 2017 · 4 comments
Open

cmd/internal/obj/x86: improve asm error messages #21860

quasilyte opened this issue Sep 13, 2017 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@quasilyte
Copy link
Contributor

quasilyte commented Sep 13, 2017 •
edited

Some assembler error messages are misleading and miss context info.
99% of errors programmer get is invalid instruction which does not help that much.

Go (or plan9) asm has its peculiar points that make initial experience, coupled with such error messages,
very unpleasant.

There are multiple aspects of error messages that can be improved, but for the very start,
some obviously horrible things can be changed.

I present some cases where error message could be more precise/informative.
If applicable, clang asm error message for the same case is provided.

This is a proposal to start a discussion regarding this topic.
Maybe someone will point important pitfalls, or provide more cases
that worth a fix.

Scope of this one restricted to x86 assembler backend.

TEXT main·asmErrors(SB),$0
        // 1
        // Case: immediate operand overflow.
        // Note: base16->base10 constant conversion in error message
        //       is annoying too, but it is separate issue.
        //
        // Have: "invalid instruction: ADDQ $-2415919103, AX"
        // Want: "ADDQ(imm32,r): constant -2415919103 overflows imm32"
        ADDQ $-0x8FFFFFFF, AX
        // 2
        // Case: invalid operand types combination.
        //
        // Have: "invalid instruction: MOVL (R8), (R9)"
        // Want: "MOVL with (mem,mem) operands combination does not exist"
        //
        // Clang:
        // > invalid operand for instruction
        // >   movl (%R8), (%R9)
        // >               ^~~~~
        MOVL (R8), (R9)
        // 3
        // Case: negative immediate for unsigned operand.
        //
        // Have: "invalid instruction: PSHUFL: $-10, X0, X1"
        // Want: "PSHUFL(uint8,xmm,xmm): invalid negative immediate value -10"
        PSHUFL $-10, X0, X1
        // 4
        // Case: using "SP" register as "index" in SIB addressing.
        // Note: same goes for XMM and other non-suitable for index registers.
        //
        // Have: "invalid instruction: MOVL (AX)(SP*2), AX"
        // Want: "SP register can't be used as index in SIB addressing"
        MOVL (AX)(SP*2), AX
        // 5
        // Case: SIB without explicit "scale".
        // See:  #13282.
        //
        // Have[1]: "invalid instruction: MOVL foo<>(SB)(AX), AX"
        // Have[2]: "asmidx: bad address 0/2064/2064"
        // Want:    "SIB addressing without explicit scale is forbidden"
        MOVL foo<>(SB)(AX), AX // [1]
        MOVL (AX)(AX), AX      // [2]

        RET
@quasilyte
Copy link
Contributor Author

The error messages in Want section provided for demonstration purposes.
I think it is more important to collect as many weird cases as possible, to evaluate how much improvement can be brought there.

This investigation lead me to 2 distantly related issues.

  1. BYTE accepts uint64 argument, but always writes only single byte. Silently.
  2. Errors like asmidx: blah-blah do not provide error location (source file + line).

@NightTsarina NightTsarina mentioned this issue Oct 24, 2017
Closed
@quasilyte quasilyte mentioned this issue Dec 6, 2017
Closed
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 30, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 30, 2018
@quasilyte quasilyte mentioned this issue Apr 15, 2018
Closed
@quasilyte
Copy link
Contributor Author

The "illegal instruction" error message is quite fabulous:
https://blog.sgmansfield.com/2017/04/a-foray-into-go-assembly-programming/

I've almost prepared a CL that fixes most of "invalid instruction" error messages to more precise and descriptive strings.

@gopherbot
Copy link

Change https://golang.org/cl/108515 mentions this issue: cmd/internal/obj/x86: improve error messages

@WojciechMula
Copy link

I think it is more important to collect as many weird cases as possible, to evaluate how much improvement can be brought there.

I came across the following case:

TEXT ·test(SB), NOSPLIT, $0
    // valid: LEAQ 0(CX)(R14*1), R15
    LEAQ 0(CX)(R14), R15
    RET

The message is:

asm: asmidx: bad address 0/2078/2065
asm: assembly failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@ianlancetaylor @WojciechMula @quasilyte @gopherbot