| Age | Commit message (Collapse) | Author |
|---|
|
DEX Version 39 added:
* const-method-handle containing a method_handle@BBBB reference
* const-method-type containing a proto@BBBB reference
This CL
* Updates CodeToProtoId for const-method-type
* Adds CodeToMethodHandle and WriteMethodHandle
Fuzzed about 500k iterations locally and uploaded new samples to
the clusterfuzz bucket. 97% coverage.
Manually tested on hand-written dex files using smali as well as the
dexdump test corpus.
Bug: 1231885
Change-Id: Id8ab09ac8d3331902c5e6f92ac39ebd26d36e79b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3060660
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918948}
NOKEYCHECK=True
GitOrigin-RevId: d08c50abf7b49f3a5b97a03d5bb79bce9fdb7fad
|
|
DEX Version 38 added:
* CallSiteId & CallSite items
* MethodHandle items
* invoke-polymorphic containing meth@BBBB and proto@HHHH references
* invoke-custom containing a call_site@BBBB reference
This CL:
* Adds CallSiteIdToCallSite
* Adds MethodHandleTo{MethodId, FieldId}
* Adds CodeToProtoId16 for invoke-polymorphic
* Adds CodeToCallSiteId16 and WriteCallSiteId16 for invoke-custom
* Updates CodeToMethodId16 for invoke-polymorphic
Fuzzed about 1 million iterations locally and uploaded new samples to
the clusterfuzz bucket. 97% coverage.
Manually tested on hand-written dex files using smali as well as the
dexdump test corpus.
Bug: 1231885
Change-Id: Icd885be2cfd433d0befe689d16c4a1e99573ca6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3060745
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918119}
NOKEYCHECK=True
GitOrigin-RevId: 9cc600ef0b60ff1ec76683a2bfb98a6bdbb05d1e
|
|
This CL replaces
* 30 instances of DISALLOW_COPY_AND_ASSIGN(Foo),
* 1 instance of DISALLOW_IMPLICIT_CONSTRUCTORS(Foo),
in Zucchini with:
Foo() = delete; // DISALLOW_IMPLICIT_CONSTRUCTORS only.
Foo(const Foo&) = delete;
const Foo& operator=(const Foo&) = delete;
All base/macros.h includes are removed.
Bug: 1010217
Change-Id: I13b3d5ed04f04e5c0b209d59e70ac018c5f4938c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3093198
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#911751}
NOKEYCHECK=True
GitOrigin-RevId: ba0e1f56993c535faa59e2ca02c371bae2ebbb20
|
|
Bug: 242216
Change-Id: I4ef4609a62af06cf5e0bc519e761d8c87579bf2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3014801
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Owners-Override: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899554}
NOKEYCHECK=True
GitOrigin-RevId: f2279caeb3f716287cddf465d9ee9ecf52853de9
|
|
This reverts commit e91c91c3e6471923fd83dbce0a44f7317f07393c.
Reason for revert:
Note: It is reported that sheriffs cannot submit CL created by Findit
(crbug.com/1187426). A workaround in the mean time is to abandon this
CL and create another revert CL.
Findit (https://goo.gl/kROfz5) identified CL at revision 898977 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2U5MWM5MWMzZTY0NzE5MjNmZDgzZGJjZTBhNDRmNzMxN2YwNzM5M2MM
Sample Failed Build: https://ci.chromium.org/b/8842407444966732864
Sample Failed Step: compile
Original change's description:
> Remove some unnecessary #includes.
>
> According to
> https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html
> these were responsible for 1,460,113,428 bytes of input to the compiler,
> or roughly 0.58% of the input used to build Chrome.
>
> Bug: 242216
> Change-Id: I1dd0a5fd3fcceb2da9bcf3dbae40e18590faf145
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3009975
> Auto-Submit: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Owners-Override: danakj <danakj@chromium.org>
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#898977}
Change-Id: I54080e564838f77ce45de045f1487a6695f46647
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 242216
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010718
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Owners-Override: Maggie Cai <mxcai@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898979}
NOKEYCHECK=True
GitOrigin-RevId: 53dea79d16778f8fc1bfe54ac8f450e5dc146dac
|
|
According to
https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html
these were responsible for 1,460,113,428 bytes of input to the compiler,
or roughly 0.58% of the input used to build Chrome.
Bug: 242216
Change-Id: I1dd0a5fd3fcceb2da9bcf3dbae40e18590faf145
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3009975
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Owners-Override: danakj <danakj@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898977}
NOKEYCHECK=True
GitOrigin-RevId: e91c91c3e6471923fd83dbce0a44f7317f07393c
|
|
This replaces:
- base::Optional -> absl::optional
- include "base/optional.h"
->
include "third_party/abseil-cpp/absl/types/optional.h"
- base::nullopt -> absl::nullopt
- base::make_optional -> absl::make_optional
Bug: 1202909
Change-Id: If697b7bf69b199c1796f873eedca3359cdb48c64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2897151
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Owners-Override: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#883296}
NOKEYCHECK=True
GitOrigin-RevId: 1156b5f891de178171e71b9221a96bef1ced3d3b
|
|
This CL is similar to:
https://chromium-review.googlesource.com/1133688
BufferRegion::FitsIn() (and BufferViewBase::covers()) decides whether
a BufferRegion fits inside a buffer. A special case is whether an empty
region fits at the end of a buffer?
Previously this was considered to be a pathological case, so the result
is "false". However, this led to a DCHECK failure found by the DEX
fuzzer: a CodeItem with insns_size = 0 is checked against an empty
buffer.
It may seem straightforward to change the DCHECK to a handled failure.
However, the failing code (in CodeItemParser::GetCodeItemInsns())
occurs after CodeItem have been supposedly validated, so the DCHECK
is correctly placed! Two causes are:
(1) Technically insns_size should be > 0, as dictated by constraint A1
("The insns array mus tnot be empty") in Dalvik spec.
(2) The FitsIn() check is too stringent.
This CL focuses on relaxing (2). This makes checking slightly more
permissive elsewhere in code (patch_reader.cc and Win32 disassembler),
but this looks like the right thing to do.
As for (1), we plan to visit
https://source.android.com/devices/tech/dalvik/constraints
and implement more rigorous checks. So we simply add a TODO for now.
Bug: 863478
Change-Id: Iacbb2bb9bf26701db960192c7b727351ea5afdec
Reviewed-on: https://chromium-review.googlesource.com/1142517
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576482}
NOKEYCHECK=True
GitOrigin-RevId: 2b31de169e783260c9e2fbaea295b39ae808fbf9
|
|
This CL fixes a bug discovered by the Fuzzer, where DEX parsing
triggers CHECK() failure due to an attempt to read a 4-byte value that
straddles EOF.
Tracing (Zucchini-read) using the Fuzzer-provided DEX show that the
faulty read attempt is for ClassDefItem::static_values_off, which is
a field in a fixed-length item.
Our fix is to validate MapItem entries for fixed-length items. Details:
- Add GetItemBaseSize() to return an item size bound. For fixed-length
items, this is exactly the item size. Moreover, no 4-byte alignment
is needed, since the item sizes are already multiples of 4 bytes.
- In DisassemblerDex::ParseHeader(), verify each MapItem fits in the
image. For fixed-length items, this check enables safe read for
items whose index are within bound (and avoid similar bugs). For
variable-length items, this serves as a sanity check to quickly
reject obviously bad inputs. Subsequent parsing will perform a more
refined check. For unhandled items, sizes are assumed to be 1.
- Not checked: Whether MapItems ranges overlap. May do this in the
future (more refactoring will be needed).
Bug: 862566
Change-Id: If8efce122979fa1a36d1d445556d414eb499d273
Reviewed-on: https://chromium-review.googlesource.com/1133713
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574294}
NOKEYCHECK=True
GitOrigin-RevId: 1f63bf220bcb93560358fbe593ebccc4d64f4baa
|
|
This bug was found by the fuzzer. If a large int32 value is present for
a RelCode32 the result of mapping the location to its target results in
integer overflow or underflow as found by UBSAN.
In the particular example found by the fuzzer a value of 1292632068 is
read from the image. The result of |(1292632067 - 1) * kInstrUnitSize|,
where |kInstrUnitSize = 2| results in an overflow. This is only
possible for RelCode32 so we only need the fix there. The solution is
to check for overflow and if it occurs just to skip the reference. In
a regular DEX file these should be very rare if ever present.
I've tested the updated version on a subset of the corpus with no ill
effects.
Bug: 862095
Change-Id: Ifedeeaf1ae7e72a147421ecb917ec1751f4bb8d4
Reviewed-on: https://chromium-review.googlesource.com/1131225
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574160}
NOKEYCHECK=True
GitOrigin-RevId: c87921042aae8f5fa594f96b33635ef76cfd1028
|
|
When a bad input is created the target indices can point far out of
bounds. Based on the fuzzing this should be promoted to a runtime
LOG(ERROR). Once we have a method to fail a write gracefully this
should be updated to fail the write step.
Bug: 860857
Change-Id: Ie8e4eaf9a655a71e0a2bf3efe2efae52574813db
Reviewed-on: https://chromium-review.googlesource.com/1128813
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573393}
NOKEYCHECK=True
GitOrigin-RevId: 8b47a6add32d3f7e1dab7c929f1d7a978beb94cc
|
|
The MakeWriteRelCodeXX used base::checked_cast which results in a
crash. To fail gracefully we instead use base::CheckedNumeric.
The appearance of this bug results from the input files being bad
resulting in the output from gen also being bad which then propagates
to the apply step.
Bug: 860128
Change-Id: I38fc91b0a1495ce0c337d778fb51cfe56537e766
Reviewed-on: https://chromium-review.googlesource.com/1126178
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572648}
NOKEYCHECK=True
GitOrigin-RevId: 2d78c3c15a71b517d3ef226010f1cfc4b13e5ea1
|
|
Zucchini makes the assumption that a valid DEX file has code items.
However, this contraint was not applied to whether the DEX file
contained valid and parsable code. As a result when attempting to find
references for within these code items, which weren't successfully
parsed, Zucchini would crash.
The solution is to impose a requirement that at least one code item was
parsed to create a disassembler.
Found during fuzzing of DEX files in CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1117123
Change-Id: I76fcbb9267099a7fe3d6eb61c345ffbfaf772fff
Reviewed-on: https://chromium-review.googlesource.com/1118851
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571276}
NOKEYCHECK=True
GitOrigin-RevId: 2f1a0765a55cda93faa787cf7110db3b78f02a26
|
|
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}
NOKEYCHECK=True
GitOrigin-RevId: 966646525b939a8fd52a993894bbb3822d9cb674
|
|
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}
NOKEYCHECK=True
GitOrigin-RevId: 55a60dd9ab731cb569e49ae3600bc42c716d4756
|
|
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}
NOKEYCHECK=True
GitOrigin-RevId: b87359eb909433d97367382b5982baab5352124e
|
|
(Committing on behalf of etiennep@).
This CL adds DEX References read / write for 11 basic types. Details:
- Add InstructionParser to visit DEX instructions in the insns member
of a CodeItem, taking care to skip non-instruction "payloads".
- Add InstructionReferenceReader to visit CodeItem References found in
|[lo, hi)|, using provided callbacks to apply type filters and
extract targets.
- Add ItemReferenceReader to visit fixed-sized items to extract
Referenes from a "member variable of interest", using a callback
to extract targets.
- DisassemblerDex: Add MakeRead*() and MakeWrite*() to return a visitor
to read / write supported DEX References.
- Add DEX unit test to audit the error-prone look-up table from
DisassemblerDex::MakeReferenceGroups().
Change-Id: Ice12a867aab4fdcb4a152bb1946ea7515ad426ef
Reviewed-on: https://chromium-review.googlesource.com/994066
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549178}
NOKEYCHECK=True
GitOrigin-RevId: 7b249ec48aa0f1058496a1b7f095b0f126011bf6
|
|
Creates Disassembler that recognises and parses DEX format. For now, it doesn't
extract any type reference, so it is equivalent to DisassemblerNoOp. Extraction
of various types of reference will be added in a follow-up CL.
BufferView::covers_array() and unittests were also added.
Change-Id: I08756244e9af899cf0f40dabd2b0059e1749328e
Reviewed-on: https://chromium-review.googlesource.com/967603
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546807}
NOKEYCHECK=True
GitOrigin-RevId: d214e2cf9e23bf055f0e0655e9564761d50206ad
|
