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

x/arch/arm/armasm: MSR instruction is not supported #20762

Closed
benshi001 opened this issue Jun 23, 2017 · 9 comments
Closed

x/arch/arm/armasm: MSR instruction is not supported #20762

benshi001 opened this issue Jun 23, 2017 · 9 comments

Comments

@benshi001
Copy link
Member

@benshi001 benshi001 commented Jun 23, 2017

decode_test.go:66: Decode(01f02ce1|) [plan9] = error: unknown instruction, 0, want MSR R1, CPSR
decode_test.go:66: Decode(fff02ce3|) [plan9] = error: unknown instruction, 0, want MSR $255, CPSR
decode_test.go:66: Decode(fff42ce3|) [plan9] = error: unknown instruction, 0, want MSR $4278190080, CPSR

All those are common ARM instructions, which should be correctly decoded.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Jun 23, 2017

@benshi001 what package is this issue for? Is it for x/arch/arm/armasm or for cmd/internal/objfile? Could we update the title to something that can easily be groked?
Thanks.

/cc @cherrymui @randall77 @josharian

Loading

@benshi001 benshi001 changed the title defects of the disassembler defects of arm's disassembler Jun 23, 2017
@benshi001 benshi001 changed the title defects of arm's disassembler https://go.googlesource.com/arch/+/master/arm/armasm/: defects of arm's disassembler Jun 23, 2017
@benshi001
Copy link
Member Author

@benshi001 benshi001 commented Jun 23, 2017

01f02ce1|	1	plan9	MSR R1, CPSR
fff02ce3|	1	plan9	MSR $255, CPSR
fff42ce3|	1	plan9	MSR $4278190080, CPSR

add to arm/armasm/testdata/decode.txt

Loading

@odeke-em odeke-em changed the title https://go.googlesource.com/arch/+/master/arm/armasm/: defects of arm's disassembler x/arch/arm/armasm: incorrect disassembly of some instructions on plan9 Jun 23, 2017
@benshi001
Copy link
Member Author

@benshi001 benshi001 commented Jun 23, 2017

here is gnu objdump's result

10: e12cf001 msr CPSR_fs, r1
14: e32cfff0 msr CPSR_fs, # 240, 30 ; 0x3c0
18: e32cfff4 msr CPSR_fs, # 244, 30 ; 0x3d0

Loading

@bradfitz bradfitz added this to the Go1.10 milestone Jun 23, 2017
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jun 23, 2017

@odeke-em it is x/arch/arm/armasm.

All those are common ARM instructions

They are rarely used in Go. Given that it doesn't disassemble to some other instruction, I would say it is "unsupported" yet, instead of "incorrect".

Does it work in GNU syntax? (I mean, GNU syntax in x/arch/arm/armasm, not GNU objudump).

Loading

@cherrymui cherrymui added this to the Unreleased milestone Jun 23, 2017
@cherrymui cherrymui removed this from the Go1.10 milestone Jun 23, 2017
@cherrymui cherrymui changed the title x/arch/arm/armasm: incorrect disassembly of some instructions on plan9 x/arch/arm/armasm: MSR instruction is not supported Jun 23, 2017
@benshi001
Copy link
Member Author

@benshi001 benshi001 commented Jun 24, 2017

No. it is not supported in GNU syntax x/arch/arm/armasm

Loading

@benshi001
Copy link
Member Author

@benshi001 benshi001 commented Jul 10, 2017

The register list order is wrong when decoding QADD/QDADD/QSUB.

0xe1086055 should be decoded to "QADD R6, R8, R5" in GNU syntax, but actually "QADD R6, R5, R8". So do QDADD and QSUB.

Loading

@benshi001
Copy link
Member Author

@benshi001 benshi001 commented Jul 13, 2017

Sorry, I made a mistake again. The QADD/QSUB/QDSUB/QDADD are right in register order. Theirs are indeed different from QADD16/QSUB16/QADD8/QSUB8.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 13, 2017

CL https://golang.org/cl/48310 mentions this issue.

Loading

@gopherbot gopherbot closed this in ec150e0 Jul 15, 2017
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 27, 2017

Change https://golang.org/cl/49530 mentions this issue: cmd/vendor/golang.org/x/arch: pull updates from x repo

Loading

gopherbot pushed a commit that referenced this issue Aug 16, 2017
Vendor from golang.org/x/arch (commit f185940).

Implements #19157

Updates #12840
Updates #20762
Updates #20897
Updates #20096
Updates #20766
Updates #20752
Updates #20096
Updates #19142

Change-Id: Idefb8ba2c355dc07f3b9e8dcf5f00173256a0f0f
Reviewed-on: https://go-review.googlesource.com/49530
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Jul 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants