Framework for ARM64 instruction disassembler
ClosedPublic
Actions

Authored by wma on Jan 29 2016, 7:39 AM.

Details

Reviewers

zbb
emaste
andrew
imp

Commits

rS295038: Framework for ARM64 instruction disassembler

Summary

A proposition of user-readable instruction disassembler for ARM64. It's only a framework and requires adding some the most common instruction opcodes.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

wma updated this revision to Diff 12812.Jan 29 2016, 7:39 AM

wma retitled this revision from to Framework for ARM64 instruction disassembler.

wma updated this object.

wma edited the test plan for this revision. (Show Details)

wma added reviewers: emaste, andrew, zbb, imp.

wma set the repository for this revision to rS FreeBSD src repository - subversion.

wma added a subscriber: arm64.

zbb accepted this revision.Jan 29 2016, 9:04 AM

zbb edited edge metadata.

zbb added inline comments.

sys/arm64/arm64/disassem.c
2	Cavium?
94	Maybe `uint32_t` will be better to implicitly show the instruction length.
144	Maybe `switch` would be more readable.
168	Must we `panic()` here? We could return with error or place `KASSERT` here an return with error when invariants disabled.
179	Same here
198	Aaand here
255	`matchp != 0`
303	Hmm if this is needed very early, etc. we may want to set a global variable indicating that this was already done. This would be of course necessary if it is possible to jump to the KDB before masks are generated (during boot).
sys/arm64/include/disassem.h
2	Cavium?

This revision is now accepted and ready to land.Jan 29 2016, 9:04 AM

andrew added inline comments.Jan 29 2016, 10:21 AM

sys/arm64/arm64/db_disasm.c
53	What is `4` and `0`?
67	`return (...);`
sys/arm64/arm64/disassem.c
41	Why not `static const char *w_reg[] = {`?
sys/arm64/include/disassem.h
30	`#define<tab>`

wma updated this revision to Diff 12817.Jan 29 2016, 10:49 AM

wma edited edge metadata.

This revision now requires review to proceed.Jan 29 2016, 10:49 AM

Done

andrew added inline comments.Jan 29 2016, 11:06 AM

sys/arm64/arm64/disassem.c
288	This looks a little odd given the distance between `ret = 0` above and here. Why not just make this `ret = arm64_...`, and not set it above?
sys/arm64/include/disassem.h
30	No tab after `#ifndef` (style(9) is a little odd about this)
37	Does this need to be a typedef? It hides the details on what is an internal to arm64 struct.