- User Since
- Apr 28 2017, 8:23 AM (37 w, 4 d)
Thu, Jan 11
Fixed comment spacing. Changed the comment slightly (words “I added this class to…” sound now like I did this, so, replaced it with passive voice).
Wed, Jan 10
There is the function GetDisasmToUse in InstructionLLVMC class that can return nullptr. But this case is not handled in any usage. I suppose that caller functions cannot be invoked if !DisassemblerLLVMC::IsValid(). But it still looks dangerous for me. May be GetDisasmToUse should assert if neither m_disasm_up nor m_alternate_disasm_up exists? And may return a reference then.
Added "_up" suffix to each unique_ptr, renamed MCDisasmToolset to MCDisasmInstance.
Tue, Jan 9
Added function Create that creates an instance of LLVMCDisassembler only if pass all constraints.
Moved LLVMCDisassembler declaration to .cpp file, renamed to MCDisasmToolset (is this name ok?).
Added const qualifier to some functions of the class.
I also had the courage to remove ‘_ap’ suffixes from unique pointers.
Thank you, Pavel.
Would you mind if I move LLVMCDisassembler declaration in .cpp also? It looks like perfect candidate for pimpl.
Tue, Dec 26
Nov 28 2017
Now I have commit after approval access and would land this revision. May I do it now?
Nov 22 2017
When I added instructions, I didn't care about properties like isBranch, isBarrier, etc., because didn't know its purpose. But it was found that debugger cannot step over a range of instructions correctly without this knowing, thus, I've added appropriate fields to instructions.
Made diff with full paths (studying to use arcanist)...
Things that didn't require review was committed separately.
Nov 16 2017
Build for ARC fails, fix TargetInfo for this target please.
Nov 15 2017
Still have a feeling that it is not enough to clarify for function users that "if (bytes_written != bytes_to_write)" is not always criteria of error, as well as "if (error.Success())" doesn't mean that whole area was written to memory...
Same for comment in the header file.
Nov 14 2017
I have not commit access, that is why I've decided to group all typos in one patch...
Removed clang-format changes.
Sorry for wrong formatting, I've removed it.
Nov 13 2017
Nov 8 2017
Removed accidentally added file.
Use type of instruction for its fields instead of 'auto'.
I agree that matching the style in other backends is very important, but it is hard to be consistent with last, because it was written long before even c++11 was released...
Oct 12 2017
Corrupted it by merging...
Thanks for comment, Pete!
Oct 6 2017
Forgot to change decoder method name.
Sep 22 2017
Applied fixes from 2nd comment.
Added tests, fixed ARCDisassembler.cpp according suggestions.
Sep 19 2017
Hi Pete, thank you for review!
Sep 18 2017
This obvious solution works well for me and seems safe.
Jul 3 2017
Jun 30 2017
Jun 27 2017
Saying about clear intent, it would be even much more better if class name doesn't start with DWARF ;)
Jun 26 2017
Works well for me. Thank you a lot for bringing it up to scratch!
Jun 7 2017
Removed non-printable characters from DWARFCallFrameInfo::GetFDEIndex and redundant version checks from UnwindTable::Initialize.
Jun 5 2017
Jun 2 2017
Added support of DWARF <= 5. For unsupported versions GetUnwindPlan will return false and next behavior should be like without debug_frame at all. Checked it on binaries with v.2 and v.4 on x86-linux target, but have not a chance to run tests for arm-android and other. I'd appreciate it if someone, who has adjusted environment, would check it.
May 25 2017
Yes, give those binaries, please.
May 24 2017
May 23 2017
It can make sense to add
assert(eReturnStatusStarted != result);
after cmd_obj->Execute() invocation at CommandInterpreter::HandleCommand, do you think?