FinalizeHash(hash) was trying to avoid UB in expression 1 << 32
by casting 1 to uintptr_t. This type however is not wide enough
on 32-bit platforms.
Instead just use explicit comparison hashbits < kBitsPerInt32 to
avoid overflow in left shift.
This bug went unnoticed for a while because it the only place
where we call FinalizeHash(hash) is in the snapshot profile
writer code and it only triggers when gen_snapshot is a
32-bit binary - which is only true on Windows, as Mac and Linux
seem to use simarm_x64 configuration instead.
This UB was explicitly affecting the code behavior because C++
compiler would either optimize out or change behavior of any
code that consumed value produced by FinalizeHash(hash).
Fixes https://github.com/flutter/flutter/issues/97764
TEST=vm/cc/DirectChainedHashMap
Change-Id: I39f2b09e7516c875b765e5a065d1c1331f89fa33
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250741
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Rename PointerKeyValueTrait<T>, which is used to create sets of pointers
to T instances where T <: Object and T has appropriate Hash and Equals
instance methods, to the more specific name PointerSetKeyValueTrait.
Create a PointerSet<T> alias for using this trait with
DirectChainedHashMap and use that alias in other code as a shorthand.
Remove PointerSetKeyValueTrait<const char> as a superclass of
CStringKeyValueTrait, as the only reuse from the former are the two
methods KeyOf and ValueOf which just return their argument, and having
this relationship is odd since const char is not a subtype of Object.
TEST=Renaming/refactoring, so existing tests.
Change-Id: I0274b16cb9fcb3939a28fb109fb8626c1ac8c0e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215761
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This is a reland of 5909fd111d
Does a large refactoring on the V8 snapshot profile writer
to clean things up, add more debugging support, and to fix
the problems that surfaced during the original landing.
Other changes:
Changes Serializer::CreateArtificialNodeIfNeeded() to create
artificial nodes for Code objects and immutable arrays.
Fixes CodeSerializationCluster::Trace() to only push needed parts of
discarded code objects, instead of tracing them like full code objects.
Adds test cases to v8_snapshot_profile_writer_test that exercise
the following situations (both separately and together):
* Non-symbolic stack traces are enabled and code and function objects
are dropped when not needed at runtime.
* Creation of the dispatch table is disabled.
TEST=vm/dart{,_2}/v8_snapshot_profile_writer_test
Original change's description:
> [vm] Fix V8 snapshot profile handling of the dispatch table.
>
> Fixes https://github.com/dart-lang/sdk/issues/45702.
>
> TEST=Tests listed in the issue above.
>
> Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
> Change-Id: Ibf5e3ccf3828c01f9dda47de360314dabe8cb8a9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195272
> Reviewed-by: Daco Harkes <dacoharkes@google.com>
> Commit-Queue: Tess Strickland <sstrickl@google.com>
Change-Id: I8e7030267fe190079a8f68d00fe20bf7170e5719
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-mac-release-simarm64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-obfuscate-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195513
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
In the codebase, we have several different interfaces for equality
instance methods on a type T:
- bool Equals(const T&) const;
- bool Equals(const T*) const;
- bool Equals(T*) const;
This CL standardizes on `bool Equals(const T&) const;`, and adjusts any
related functions to match.
TEST=Existing test suites, as this is an internal refactoring.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Change-Id: I2ebcf2b861732ce65c6f1c673b2df2a2c7f5dd4d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195071
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
In the codebase, we have several different interfaces for instance
methods that return a hash:
- uword Hash() const;
- intptr_t Hash() const;
- uint32_t Hash() const;
- intptr_t Hashcode() const;
This CL standardizes on `uword Hash() const` and adjusts any related
functions to match.
TEST=Existing test suites, as this is an internal refactoring.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Change-Id: If2cbce57f3fae0f0d24031b6e324f0323c965f41
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195067
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
To get this information, enable --trace-precompiler.
There are two version of the output: a human readable version
and a JSON version (preceded by 'JSON for function decisions:')
for easier tabulation. The JSON version is an array of objects,
where the fields of the object are as follows:
* name: a string, the function name
* kind: a string, the function kind
* retained: a boolean, whether the function was retained
* reasons: an array of strings, reasons for retaining
(only for retained objects)
TEST=vm/dart{,_2}/use_trace_precompiler_flag_test.dart
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: I9b006a49e1410a832950a1bd29a0abaabe9431f9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190000
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
BaseDirectChainedHashMap<KeyValueTrait, B, Allocator>::Iterator::Next()
was first checking if array_index_ is larger than the size of map's array,
and only then it was looking at remaining entries in the collision list.
So, when there is a collision in the last bucket, array_index_ was bumped
and the first entry in the list was returned. After that, Next() returned
NULL, skipping remaining entries in the collision list.
This caused incorrect stack state management in BytecodeFlowGraphBuilder
and BytecodeFlowGraphBuilder::DropUnusedValuesFromStack() was removing
too many entries from the stack, which caused crash in
BytecodeFlowGraphBuilder::BuildStoreIndexedTOS().
Issue: https://github.com/dart-lang/sdk/issues/38979
Change-Id: Ie073ca7014da5b04999b7984d508984b4c9743b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/122176
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
The Insert method runs in constant time, but inserting multiple
pairs with the same key results in all pairs being added to the
map. Remove only removes the most recent pair with a key equal
to its argument, so this means calling Remove after an Insert
does not guarantee that Lookup for a key will fail.
The new Update method, when used instead of Insert, guarantees
that the map contains at most one pair for a set of equal keys.
This means that Lookup is guaranteed to fail after a Remove
call, but the Update method requires walking the existing
chain of entries for the key's hash value.
(In practice, our code tends to call Lookup before Insert to
avoid adding pairs with equal keys more than once, but this
practice does mean that updating the value returned by Lookup
cannot be done without first removing the old entry. This new
method simplifies that workflow if used consistently.)
Change-Id: I2944472a7fc74c1de41cb25eb9469a7c7e491ceb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114069
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Fixes assumption about pointer equality of keys.
Adds comment to document current assumption that multiple pairs
with equivalent keys are not inserted.
Change-Id: Ieb17d1b357f903dcc09274ac648c1a97e7ee85d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113686
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This reverts commit 7bf5d87017.
The lock introduced by this change in MallocHooksState is held across
a fork(), which causes deadlock in the child when execvpe() fails and
tries to acquire the lock when freeing memory.
Review-Url: https://codereview.chromium.org/2647793005 .
This CL removes the use of STL map from freelist.cc by adding
MallocDirectChainedHashMap in hash_map.h and adding an iterator for
BaseDirectChainedHashMap there.
It also removes a use of STL map from hash_table.h that was dead code.
R=johnmccutchan@google.com
Review URL: https://codereview.chromium.org/2083103002 .
Original CL: http://codereview.chromium.org/10872035/
It enables elimination of redundant expressions across
basic blocks.
Currently used for smi checks and class checks, but will
be extended to other expressions in a separate CL.
Additionally, this fixes a bug in the register allocator where
a blocked register was illegally assigned to an unallocated live range.
It also fixes a Mac compiler issue with the constructor of
DirectChainedHashMap.
Review URL: https://chromiumcodereview.appspot.com//10871060
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@11302 260f80e4-7a28-3924-810f-c04153c831b5
This CL contains:
A new CheckClassComp instruction. Currently it is used only for instance loads:
(LoadInstanceFieldComp)
A pass LocalCSE that performs block-local common subexpression elimination. To identify
redundant expressions I use a hash map per basic block. Computations that do not have
side effects can participate in CSE. For now, I only enabled it for CheckClass.
Any computation that participates in CSE must implement the AttributesEqual function.
Other smaller fixes:
Places where we can pass the correct initial size for GrowableArrays
that have a known size. We should consider having a FixedLengthArray for this purpose.
Made the accessors ic_data() and set_ic_data() use a const ICData*.
Review URL: https://chromiumcodereview.appspot.com//10824349
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@10948 260f80e4-7a28-3924-810f-c04153c831b5
