Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(19)

Issue 2771753004: [Courgette] Refactor: Unify Disassembler::Disassemble() and instantiate AssemblyProgram there. (Closed)

Created:
3 years, 9 months ago by huangs
Modified:
3 years, 9 months ago
Reviewers:
Will Harris
CC:
chromium-reviews, wfh+watch_chromium.org, huangs+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Courgette] Refactor: Unify Disassembler::Disassemble() and instantiate AssemblyProgram there. This is part of AssemblyProgram reduction effort. Details: - Add Disassembler::Disassemble() and dedup separate PE/ELF versions. - Instantiate AssemblyProgram there; replaces old behavior where caller (program_detector.cc) instantiates and passes as out param. - Add virtual Disassembler::GetInstructionGenerator(). - Remove InstructionGenerator's AssemblyProgram* param; update tests. - Move InstructionReceptor and InstructionGenerator from AssemblyProgram to new file instruction_utils.h. - Rename ParseAbs32Relocs() to ExtractAbs32Locations(); make common. - Rename ParseRel32Relocs() to ExtractRel32Locations(); make common. - Make DisassemblerElf32::abs32_locations_ non-mutable. BUG=660980 Review-Url: https://codereview.chromium.org/2771753004 Cr-Commit-Position: refs/heads/master@{#459272} Committed: https://chromium.googlesource.com/chromium/src/+/257f9fb084fd9c3a660ef8d7fd2948cb3d3fd1fb

Patch Set 1 #

Total comments: 3

Patch Set 2 : Fix signed/unsigned comparison issue in test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+154 lines, -131 lines) Patch
M courgette/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M courgette/adjustment_method_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M courgette/assembly_program.h View 3 chunks +1 line, -51 lines 0 comments Download
M courgette/assembly_program.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M courgette/disassembler.h View 4 chunks +21 lines, -6 lines 0 comments Download
M courgette/disassembler.cc View 3 chunks +20 lines, -4 lines 0 comments Download
M courgette/disassembler_elf_32.h View 5 chunks +10 lines, -6 lines 0 comments Download
M courgette/disassembler_elf_32.cc View 4 chunks +8 lines, -24 lines 0 comments Download
M courgette/disassembler_elf_32_arm.cc View 1 chunk +1 line, -1 line 0 comments Download
M courgette/disassembler_elf_32_x86.cc View 1 chunk +1 line, -1 line 0 comments Download
M courgette/disassembler_elf_32_x86_unittest.cc View 1 2 chunks +4 lines, -3 lines 0 comments Download
M courgette/disassembler_win32.h View 3 chunks +5 lines, -2 lines 0 comments Download
M courgette/disassembler_win32.cc View 5 chunks +9 lines, -23 lines 0 comments Download
A courgette/instruction_utils.h View 1 chunk +66 lines, -0 lines 0 comments Download
M courgette/program_detector.cc View 2 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 14 (9 generated)
huangs
PTAL, thanks! https://codereview.chromium.org/2771753004/diff/1/courgette/disassembler_elf_32.h File courgette/disassembler_elf_32.h (right): https://codereview.chromium.org/2771753004/diff/1/courgette/disassembler_elf_32.h#newcode18 courgette/disassembler_elf_32.h:18: #include "courgette/instruction_utils.h" Including for InstructionGenerator (as side ...
3 years, 9 months ago (2017-03-23 18:51:24 UTC) #2
Will Harris
lgtm
3 years, 9 months ago (2017-03-23 19:00:37 UTC) #3
huangs
Thanks. Committing soon!
3 years, 9 months ago (2017-03-23 20:49:21 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771753004/20001
3 years, 9 months ago (2017-03-23 22:08:00 UTC) #11
commit-bot: I haz the power
3 years, 9 months ago (2017-03-23 23:19:04 UTC) #14
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/257f9fb084fd9c3a660ef8d7fd29...

Powered by Google App Engine
This is Rietveld 408576698