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

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
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@benshi001
Copy link
Member

benshi001 commented Jun 23, 2017 •
edited 

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 commented Jun 23, 2017 •
edited

@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

@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 commented Jun 23, 2017 •
edited 

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

@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 commented Jun 23, 2017 •
edited

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

@bradfitz bradfitz added this to the Go1.10 milestone Jun 23, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 23, 2017
@cherrymui
Copy link
Member

@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).

@cherrymui cherrymui modified the milestones: Unreleased, Go1.10 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

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

@benshi001
Copy link
Member Author

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.

@benshi001
Copy link
Member Author

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.

@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Aug 16, 2017
@cherrymui
cmd/vendor/golang.org/x/arch: pull updates from x repo
56b62c7 
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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @odeke-em @gopherbot @cherrymui @benshi001