New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847571 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Zucchini: Complete DEX support

Project Member Reported by ckitagawa@chromium.org, May 29 2018

Issue description

Within Zucchini support for DEX files is incomplete. This bug tracks the completion of support.

Full spec here:
https://source.android.com/devices/tech/dalvik/dex-format

There are three categories of references to finish support for:

Simple Types:
- Absolute offsets
- Indexes
- Code Items

List Types
- Lists of sublists

Compound List Types
- Lists of multiple sublists

The later two must be implicitly parsed as they are variable length. Ideally this will be three separate CLs in order to keep things manageable for reviewers.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b87359eb909433d97367382b5982baab5352124e

commit b87359eb909433d97367382b5982baab5352124e
Author: Calder Kitagawa <ckitagawa@chromium.org>
Date: Tue Jun 05 14:10:29 2018

[Zucchini]: Finish Simple DEX Type Support

This finishes support for simple DEX types including absolute offsets,
indexes into lists and code items. It is mostly just pattern matching
to add the new types to the already existing types.

The exception to pattern matching is cases with sentinel values. This
requires slight refactoring to existing code to account for
empty/sentinel fields which should be skipped.

Bug:  847571 
Change-Id: Ia83a9d9adee2967bfcc10644ea134063865929f9
Reviewed-on: https://chromium-review.googlesource.com/1076901
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564479}
[modify] https://crrev.com/b87359eb909433d97367382b5982baab5352124e/components/zucchini/disassembler_dex.cc
[modify] https://crrev.com/b87359eb909433d97367382b5982baab5352124e/components/zucchini/disassembler_dex.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55a60dd9ab731cb569e49ae3600bc42c716d4756

commit 55a60dd9ab731cb569e49ae3600bc42c716d4756
Author: Calder Kitagawa <ckitagawa@chromium.org>
Date: Mon Jun 11 13:54:24 2018

[Zucchini]: Support reference lists in DEX

Adds support for types which contain variable length lists of references
in DEX. These lists take the form: |NTTTTT|NTT|N|NTT|... where N is the
header containing the length and T is a reference body. There are three
types which utilize this format. AnnotationsDirectoryItem also uses a
variant of this format with multiple lists per item (to be added in a
separate CL).

Method:
We pre-cache the offsets of each T within the list using the parser and
iterate over it in the ReferenceReader. This is faster than implicitly
parsing each list and avoids having to handle all the accounting for
the number of lists, items per list, map size, etc. in the
ReferenceReader. The tradeoff is memory which varies by number of types
and annotations but could exceed 1 MB in a very large DEX file. This is
an acceptable cost for the time and simplicity gained. Explicitly
parsing beforehand is also safer as it delegates the validation of DEX
structure to the parser early before any references are read. This is
also inkeeping with the style of the other ReferenceReaders in the file.

Bug:  847571 
Change-Id: I853905b10ab7003e87895cc50c5ebf6b9fb4a424
Reviewed-on: https://chromium-review.googlesource.com/1087409
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565989}
[modify] https://crrev.com/55a60dd9ab731cb569e49ae3600bc42c716d4756/components/zucchini/disassembler_dex.cc
[modify] https://crrev.com/55a60dd9ab731cb569e49ae3600bc42c716d4756/components/zucchini/disassembler_dex.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/966646525b939a8fd52a993894bbb3822d9cb674

commit 966646525b939a8fd52a993894bbb3822d9cb674
Author: Calder Kitagawa <ckitagawa@chromium.org>
Date: Tue Jun 12 14:37:39 2018

[Zucchini]: Support DEX AnnotationsDirectoryItem

Adds support for AnnotationsDirectoryItem references. These take a
similar form to variable length reference lists; however, the header
for the list is not just the size of one list but rather three sublists
and also contains a reference to class annotations within the header.

The CL adds a parser for these items and a few types. The
ReferenceReader is reused from the CachedItemListReferenceReader. There
isn't a noticeable change in generation or apply time but the memory
footprint can potentially be much larger as each annotation can produce
on average a few offsets and there can be thousands of instances.

Bug:  847571 
Change-Id: I04afe4c6e35c66c0c9157ed3ac3e5bf338931f03
Reviewed-on: https://chromium-review.googlesource.com/1095645
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566420}
[modify] https://crrev.com/966646525b939a8fd52a993894bbb3822d9cb674/components/zucchini/disassembler_dex.cc
[modify] https://crrev.com/966646525b939a8fd52a993894bbb3822d9cb674/components/zucchini/disassembler_dex.h
[modify] https://crrev.com/966646525b939a8fd52a993894bbb3822d9cb674/components/zucchini/type_dex.h

Status: Fixed (was: Assigned)

Sign in to add a comment