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 Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #12812)	Cavium?
94 ↗	(On Diff #12812)	Maybe `uint32_t` will be better to implicitly show the instruction length.
144 ↗	(On Diff #12812)	Maybe `switch` would be more readable.
168 ↗	(On Diff #12812)	Must we `panic()` here? We could return with error or place `KASSERT` here an return with error when invariants disabled.
179 ↗	(On Diff #12812)	Same here
198 ↗	(On Diff #12812)	Aaand here
255 ↗	(On Diff #12812)	`matchp != 0`
303 ↗	(On Diff #12812)	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 ↗	(On Diff #12812)	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 ↗	(On Diff #12812)	What is `4` and `0`?
67 ↗	(On Diff #12812)	`return (...);`
sys/arm64/arm64/disassem.c
41 ↗	(On Diff #12812)	Why not `static const char *w_reg[] = {`?
sys/arm64/include/disassem.h
30 ↗	(On Diff #12812)	`#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
287 ↗	(On Diff #12817)	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
29 ↗	(On Diff #12817)	No tab after `#ifndef` (style(9) is a little odd about this)
36 ↗	(On Diff #12817)	Does this need to be a typedef? It hides the details on what is an internal to arm64 struct.