[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch generation.

[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch generation.

For Courgette-gen, label adjustment needs lists of abs32 & rel32 Label*
sorted by file offset. Let these lists be "Label Annotations".
Previously these were extracted during label adjustment from list of
instructions in AssemblyProgram, but now we wish to remove these stored
instructions.

This CL make AssemblyProgram store Label annotations. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced with visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
Review-Url: https://codereview.chromium.org/2793153003
Cr-Commit-Position: refs/heads/master@{#464536}
Committed: https://chromium.googlesource.com/chromium/src/+/c4155eb674ece85b214aaf4cb781510d7377d390

[huangs, 2017-04-04 20:13:27]

huangs@chromium.org changed reviewers:
+ chrisha@chromium.org

[huangs, 2017-04-04 20:13:27]

PTAL. Thanks!

[huangs, 2017-04-04 22:39:21]

The CQ bit was checked by huangs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 22:40:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 00:20:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 00:20:55]

Dry run: This issue passed the CQ dry run.

[huangs, 2017-04-10 16:54:52]

Ping for review. Thanks!

[huangs, 2017-04-13 15:53:55]

huangs@chromium.org changed reviewers:
+ wfh@chromium.org
- chrisha@chromium.org

[huangs, 2017-04-13 15:53:56]

Changed to wfh@, since chrisha@ is OOO.  PTAL. Thanks!

[Will Harris, 2017-04-13 17:14:05]

am I right a lot of code to support the -gen1a switch?

how often is this used, and does it have any tests?

https://codereview.chromium.org/2793153003/diff/1/courgette/assembly_program.h
File courgette/assembly_program.h (right):

https://codereview.chromium.org/2793153003/diff/1/courgette/assembly_program....
courgette/assembly_program.h:86: const std::vector<Label*>&
abs32_label_annotation() const {
seems these annotation variable names should all be plural, if they are a vector
of labels?

should be called abs32_label_annotations?

[huangs, 2017-04-13 18:02:06]

Re: -gen1a switch: It's likely just a debugging tool that doesn't get used in
production. I don't think there's any test. I'd prefer to keep it though.

PTAL. Thanks!

https://codereview.chromium.org/2793153003/diff/1/courgette/assembly_program.h
File courgette/assembly_program.h (right):

https://codereview.chromium.org/2793153003/diff/1/courgette/assembly_program....
courgette/assembly_program.h:86: const std::vector<Label*>&
abs32_label_annotation() const {
On 2017/04/13 17:14:05, Will Harris wrote:
> seems these annotation variable names should all be plural, if they are a
vector
> of labels?
> 
> should be called abs32_label_annotations?

Makes sense. Done!

[huangs, 2017-04-13 18:15:11]

Description was changed from

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs a sorted list of abs32 (and
rel32) Label* sorted by file offset (now called "Label Annotations").
Previously this was extracted during label adjustment from list of
instructions in AssemblyProgram, and we wish to remove these stored
instructions.

This CL stores Label annotations in AssemblyProgram. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotation_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced by visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
==========

to

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs a sorted list of abs32 (and
rel32) Label* sorted by file offset (now called "Label Annotations").
Previously this was extracted during label adjustment from list of
instructions in AssemblyProgram, and we wish to remove these stored
instructions.

This CL stores Label annotations in AssemblyProgram. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced by visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
==========

[Will Harris, 2017-04-13 18:51:05]

On 2017/04/13 18:02:06, huangs wrote:
> Re: -gen1a switch: It's likely just a debugging tool that doesn't get used in
> production. I don't think there's any test. I'd prefer to keep it though.

does it make sense to keep this code if we have no way of knowing it works...?
Is there any way of adding some tests for it?

[huangs, 2017-04-13 18:53:35]

Oh, I manually tested it, and verified that the code changes in
courgette_tool.cc are needed to produce correct output.

It's an issue of "who tests the testers?"

[Will Harris, 2017-04-13 19:16:22]

okay sure. lgtm :)

[huangs, 2017-04-13 19:29:49]

Thanks. Committing soon!

[huangs, 2017-04-13 19:30:12]

The CQ bit was checked by huangs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 19:30:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[huangs, 2017-04-13 20:37:18]

Description was changed from

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs a sorted list of abs32 (and
rel32) Label* sorted by file offset (now called "Label Annotations").
Previously this was extracted during label adjustment from list of
instructions in AssemblyProgram, and we wish to remove these stored
instructions.

This CL stores Label annotations in AssemblyProgram. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced by visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
==========

to

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs lists of abs32 & rel32 Label*
sorted by file offset. Let these lists be "Label Annotations".
Previously these were extracted during label adjustment from list of
instructions in AssemblyProgram, but now we wish to remove these stored
instructions.

This CL make AssemblyProgram store Label annotations. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced with visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
==========

[commit-bot: I haz the power, 2017-04-13 20:37:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 20:37:48]

Dry run: This issue passed the CQ dry run.

[huangs, 2017-04-13 20:38:07]

The CQ bit was checked by huangs@chromium.org

[commit-bot: I haz the power, 2017-04-13 20:38:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 20:47:15]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492115887208020,
"parent_rev": "1348142cf8663a4e48249e9c0497c14ed91aa0b5", "commit_rev":
"c4155eb674ece85b214aaf4cb781510d7377d390"}

[commit-bot: I haz the power, 2017-04-13 20:48:05]

Description was changed from

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs lists of abs32 & rel32 Label*
sorted by file offset. Let these lists be "Label Annotations".
Previously these were extracted during label adjustment from list of
instructions in AssemblyProgram, but now we wish to remove these stored
instructions.

This CL make AssemblyProgram store Label annotations. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced with visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980
==========

to

==========
[Courgette] Refactor: Store Label Annotation in AssemblyProgram for patch
generation.

For Courgette-gen, label adjustment needs lists of abs32 & rel32 Label*
sorted by file offset. Let these lists be "Label Annotations".
Previously these were extracted during label adjustment from list of
instructions in AssemblyProgram, but now we wish to remove these stored
instructions.

This CL make AssemblyProgram store Label annotations. These are computed
only when needed (Courgette-gen / -gen1a). Details:

- Add ParseDetecteExecutableWithAnnotation() alongside
  ParseDetecteExecutable(), to avoid affecting flows that don't require
  Label Annotations.
- AssemblyProgram: Add |*_label_annotations_| as storage vectors. These
  are optionally populated in InstructionStoreReceptor when
  Disassembler::Disassemble() gets called.
  - InstructionCountReceptor now put into use.
- Simplify Label adjustment: AssemblyProgram::HandleInstructionLabels()
  is replaced with visiting AssemblyProgram's Label Annotations.
- Subtle: GraphAdjuster now includes abs64. The class doesn't get used,
  but the change is logical anyway.

BUG=660980

Review-Url: https://codereview.chromium.org/2793153003
Cr-Commit-Position: refs/heads/master@{#464536}
Committed:
https://chromium.googlesource.com/chromium/src/+/c4155eb674ece85b214aaf4cb781...
==========

[commit-bot: I haz the power, 2017-04-13 20:48:06]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c4155eb674ece85b214aaf4cb781...