Closed
Bug 1125236
Opened 9 years ago
Closed 9 years ago
IonMonkey: a minimal x86 disassembler
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: sunfish, Assigned: sunfish)
References
Details
Attachments
(2 files)
25.30 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
11.41 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
In support of bug 986981, the following patches implement a minimal x86/x64 disassembler. A builtin disassembler could have a variety of uses in the future, so I made some attempt to keep the code general-purpose.
Attachment #8553862 -
Flags: review?(jdemooij)
Assignee | ||
Comment 1•9 years ago
|
||
Also anticipating bug 986981, this patch adds code to the x64 Codegen to disassemble every heap load and store and verify that the disassembly matches what Codegen intended.
Attachment #8553863 -
Flags: review?(jdemooij)
Comment 2•9 years ago
|
||
Comment on attachment 8553862 [details] [diff] [review] x86-disassembler.patch Review of attachment 8553862 [details] [diff] [review]: ----------------------------------------------------------------- Nice, looks clean and thanks for adding so many assertions. ::: js/src/jit/Disassembler.h @@ +69,5 @@ > + MOZ_ASSERT(reinterpret_cast<const void *>(uintptr_t(disp_)) == addr); > + } > + > + explicit ComplexAddress(const Operand &op) > + { Style nit: these class methods are defined inline, so the '{' should go after the "const" on the previous line. The constructors are fine because the almost-empty line nicely separates the initializer list and the function body there. @@ +116,5 @@ > + bool operator!=(const ComplexAddress &other) const; > +#endif > +}; > + > +class OtherOperand { Nit: add a brief comment explaining what "Other" means (not a ComplexAddress?) ::: js/src/jit/shared/Disassembler-x86-shared.cpp @@ +18,5 @@ > +static bool REX_X(uint8_t rex) { return (rex >> 1) & 0x1; } > +static bool REX_B(uint8_t rex) { return (rex >> 0) & 0x1; } > + > +static uint8_t MakeREXFlags(bool w, bool r, bool x, bool b) > +{ Nit: static uint8_t Make...(...) { Also below. @@ +119,5 @@ > +js::jit::Disassembler::DisassembleHeapAccess(uint8_t *ptr, HeapAccess *access) > +{ > + VexOperandType type = VEX_PS; > + uint32_t opcode = OP_HLT; > + //uint8_t vex_vvvv = 0; Nit: remove, also some others below.
Attachment #8553862 -
Flags: review?(jdemooij) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8553863 [details] [diff] [review] x86-disassembler-checking.patch Review of attachment 8553863 [details] [diff] [review]: ----------------------------------------------------------------- Cool.
Attachment #8553863 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Nits fixed. Thanks! remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/54b740e62970 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6586cc875f
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/54b740e62970 https://hg.mozilla.org/mozilla-central/rev/4b6586cc875f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•