76217f5708
19 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Alex Kodat
|
76217f5708 |
[cpu-profiler] Ensure sampled thread has Isolate lock under Windows
While the sampler checked if the sampled thread had the Isolate locked (if locks are being used) under Linux, the check was not done under Windows (or Fuchsia) which meant that in a multi-threading application under Windows, thread locking was not checked making it prone to seg faults and the like as the profiler would be using isolate->js_entry_sp to determine the stack to walk but isolate->js_entry_sp is the stack pointer for the thread that currently has the Isolate lock so, if the sampled thread does not have the lock, the sampler woud be iterating over the wrong stack, one that might actually be actively changing on another thread. The fix was to move the lock check into CpuSampler and Ticker (--prof) so all OSes would do the correct check. The basic concept is that on all operating systems a CpuProfiler, and so its corresponding CpuCampler, the profiler is tied to a thread. This is not based on first principles or anything, it's simply the way it works in V8, though it is a useful conceit as it makes visualization and interpretation of profile data much easier. To collect a sample on a thread associated with a profiler the thread must be stopped for obvious reasons -- walking the stack of a running thread is a formula for disaster. The mechanism for stopping a thread is OS-specific and is done in sample.cc. There are currently three basic approaches, one for Linux/Unix variants, one for Windows and one for Fuchsia. The approaches vary as to which thread actually collects the sample -- under Linux the sample is actually collected on the (interrupted) sampled thread whereas under Fuchsia/Windows it's on a separate thread. However, in a multi-threaded environment (where Locker is used), it's not sufficient for the sampled thread to be stopped. Because the stack walk involves looking in the Isolate heap, no other thread can be messing with the heap while the sample is collected. The only ways to ensure this would be to either stop all threads whenever collecting a sample, or to ensure that the thread being sampled holds the Isolate lock so prevents other threads from messing with the heap. While there might be something to be said for the "stop all threads" approach, the current approach in V8 is to only stop the sampled thread so, if in a multi-threaded environment, the profiler must check if the thread being sampled holds the Isolate lock. Since this check must be done, independent of which thread the sample is being collected on (since it varies from OS to OS), the approach is to save the thread id of the thread to be profiled/sampled when the CpuSampler is instantiated (on all OSes it is instantiated on the sampled thread) and then check that thread id against the Isolate lock holder thread id before collecting a sample. If it matches, we know sample.cc has stop the sampled thread, one way or another, and we know that no other thread can mess with the heap (since the stopped thread holds the Isolate lock) so it's safe to walk the stack and collect data from the heap so the sample can be taken. It it doesn't match, we can't safely collect the sample so we don't. Bug: v8:10850 Change-Id: Iba6cabcd3e11a19c261c004103e37e806934dc6f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411343 Reviewed-by: Peter Marshall <petermarshall@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#69952} |
||
Z Nguyen-Huu
|
7b8cce7724 |
[v8windbg] Cast resource as ExternalStringResourceBase*
Cast resource field in ExternalString as v8: :String::ExternalStringResourceBase* would give us more info. Change-Id: Iae97b477f400f58365e2381b7230d2226d490aa7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2388742 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#69734} |
||
Peter Marshall
|
3243506267 |
Revert "[cpu-profiler] Ensure sampled thread has Isolate lock under Windows"
This reverts commit
|
||
|
Alex Kodat
|
dfb3f7daa5 |
[cpu-profiler] Ensure sampled thread has Isolate lock under Windows
While the sampler checked if the sampled thread had the Isolate locked (if locks are being used) under Linux, the check was not done under Windows (or Fuchsia) which meant that in a multi-threading application under Windows, thread locking was not checked making it prone to seg faults and the like as the profiler would be extracting info from a heap in motion. The fix was to move the lock check into CpuSampler and Ticker (--prof) so all OSes would do the correct check. The basic concept is that on all operating systems a CpuProfiler, and so its corresponding CpuCampler, the profiler is tied to a thread. This is not based on first principles or anything, it's simply the way it works in V8, though it is a useful conceit as it makes visualization and interpretation of profile data much easier. To collect a sample on a thread associated with a profiler the thread must be stopped for obvious reasons -- walking the stack of a running thread is a formula for disaster. The mechanism for stopping a thread is OS-specific and is done in sample.cc. There are currently three basic approaches, one for Linux/Unix variants, one for Windows and one for Fuchsia. The approaches vary as to which thread actually collects the sample -- under Linux the sample is actually collected on the (interrupted) sampled thread whereas under Fuchsia/Windows it's on a separate thread. However, in a multi-threaded environment (where Locker is used), it's not sufficient for the sampled thread to be stopped. Because the stack walk involves looking in the Isolate heap, no other thread can be messing with the heap while the sample is collected. The only ways to ensure this would be to either stop all threads whenever collecting a sample, or to ensure that the thread being sampled holds the Isolate lock so prevents other threads from messing with the heap. While there might be something to be said for the "stop all threads" approach, the current approach in V8 is to only stop the sampled thread so, if in a multi-threaded environment, the profiler must check if the thread being sampled holds the Isolate lock. Since this check must be done, independent of which thread the sample is being collected on (since it varies from OS to OS), the approach is to save the thread id of the thread to be profiled/sampled when the CpuSampler is instantiated (on all OSes it is instantiated on the sampled thread) and then check that thread id against the Isolate lock holder thread id before collecting a sample. If it matches, we know sample.cc has stop the sampled thread, one way or another, and we know that no other thread can mess with the heap (since the stopped thread holds the Isolate lock) so it's safe to walk the stack and collect data from the heap so the sample can be taken. It it doesn't match, we can't safely collect the sample so we don't. Bug: v8:10850 Change-Id: Iab2493130b9328430d7e5f5d3cf90ad6d10b1892 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2377108 Reviewed-by: Peter Marshall <petermarshall@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#69623} |
||
Leszek Swirski
|
4d1f17e4d6 |
[sandbox] Access ExternalString ResourceData via bottlenecks
Bug: v8:10391 Change-Id: I4e86394c53d02eab797c2daad2ccfde6acb83bf0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2151350 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Cr-Commit-Position: refs/heads/master@{#67619} |
||
Seth Brenith
|
8b1a5681de |
[tools] Fix v8windbg behavior on Map's bit_field2
Bill kindly pointed out to me that v8windbg was not handling bit_field2 correctly. The issue was that the constexpr type for ElementsKind was, somewhat unsurprisingly, "ElementsKind", but v8windbg expected a fully- qualified type name like "v8::internal::ElementsKind". This change addresses the problem in two ways: 1. Update v8windbg's type resolution logic to resolve type names as if they were used in the v8::internal namespace. This makes it more consistent with how those type names are used in other generated Torque code, reducing surprises and the number of times we have to write `v8::internal::` in .tq files. 2. Add compile-time verification that any constexpr type name used as a string in class-debug-readers-tq.cc can also resolve as a type name. Bug: v8:9376 Change-Id: I349cd6ab586fd8345a1fa8bfc3989bb8e6376ab8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2063769 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#66633} |
||
Tobias Tebbi
|
4f4d73f225 |
[torque] Generate GC object visitors for Torque classes
In the process: * Augment C++-generated Torque classes with SizeFor methods to calculate size of instances. * Add a new "@generateBodyDescriptor" annotation that causes Torque to generate C++ BodyDescriptors code that can be used to visit objects compatible with existing V8 mechanisms, e.g. GC * Fully automate C++ macro machinery so that adding non-extern Torque class doesn't require any C++ changes, including ensuring generation of instance types and proper boilerplate for validators and printers. * Make handling of @export a true annotation, allowing the modifier to be used on class declarations. * Add functionality such that classes with the @export annotation are available to be used from C++. Field accessors for exported classes are public and factory methods are generated to create instances of the objects from C++. * Change the Torque compiler such that Non-exported classes implicitly have the @generateBodyDescriptor annotation added and causes both verifiers and printers to be generated. * Switch non-extern Torque classes from using existing Struct-based machinery to being first-class classes that support more existing Torque class features. Change-Id: Ic60e60c2c6bd7acd57f949bce086898ad14a3b03 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2007490 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#66621} |
||
|
Seth Brenith
|
534482b35b |
[tools] Show contents of cached external strings from crash dumps
This change adds support for the postmortem inspection library to show the content of cached external strings if that content is available. It also fixes a minor annoyance where strings with unavailable data would show up as "...". Now, if fetching the very first character fails, we omit the literal value from the output. Bug: v8:9376 Change-Id: Id694a774c231ab3467fb59b1c149284729acfb20 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1987922 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#65961} |
||
|
Seth Brenith
|
dcb828b46f |
[tools] Add in-object properties to debug_helper
Until now, the in-object properties on JSObject have been invisible to tools using the postmortem debugging library. With this change, those tools will get enough information to show a flat list of property values. This is still less powerful than the runtime printers, which can show the corresponding key for each value, but it's a big step up from manually inspecting memory. This change basically requires a reimplementation of Map::GetInObjectProperties for postmortem debugging. I'm not enthusiastic about duplicating this logic, but it's pretty small and I don't see any good alternatives. As a drive-by cleanup, I moved some inline string literals into a batch of constexpr char arrays. Bug: v8:9376 Change-Id: Ia24c05f6e823086babaa07882d0d320ab9a225db Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1930174 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#65183} |
||
|
Seth Brenith
|
91e6421ccb |
[torque] Use generated instance types, part 1
This change begins making use of the fact that Torque now knows about
the relationship between classes and instance types, to replace a few
repetitive lists:
- Instance type checkers (single and range), defined in
src/objects/instance-type.h
- Verification dispatch in src/diagnostics/objects-debug.cc
- Printer dispatch in src/diagnostics/objects-printer.cc
- Postmortem object type detection in
tools/debug_helper/get-object-properties.cc
Torque is updated to generate four macro lists for the instance types,
representing all of the classes separated in two dimensions: classes
that correspond to a single instance type versus those that have a
range, and classes that are fully defined in Torque (with fields and
methods inside '{}') versus those that are only declared. The latter
distinction is useful because fully-defined classes are guaranteed to
correspond to real C++ classes, whereas only-declared classes are not.
A few other changes were required to make the lists above work:
- Renamed IsFiller to IsFreeSpaceOrFiller to better reflect what it does
and avoid conflicts with the new macro-generated IsFiller method. This
is the part I'm most worried about: I think the new name is an
improvement for clarity and consistency, but I could imagine someone
typing IsFiller out of habit and introducing a bug. If we'd prefer to
keep the name IsFiller, my other idea is to rename FreeSpace to
VariableSizeFiller and Filler to FixedSizeFiller.
- Made Tuple3 extend from Struct, not Tuple2, because IsTuple2 is
expected to check for only TUPLE2_TYPE and not include TUPLE3_TYPE.
- Normalized the dispatched behavior for BigIntBase and HeapNumber.
- Added a few new object printers.
Bug: v8:7793
Change-Id: I5462bb105f8a314baa59bd6ab6ab6215df6f313c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860314
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64597}
|
||
|
Seth Brenith
|
61815a22bd |
Use consistent capitalization rules for instance types
In preparation for allowing Torque to generate the list of instance types, I'd like to make the rules a bit more consistent for how instance types are spelled. This CL is my proposal for a system where every non-String instance type name is exactly equal to calling CapifyStringWithUnderscores on the corresponding class name and appending "_TYPE". This change is almost all find&replace; the only manual changes are in: - src/objects/instance-type.h - src/torque/utils.cc - tools/gen-postmortem-metadata.py This change is in response to the review comment https://chromium-review.googlesource.com/c/v8/v8/+/1757094/25/src/builtins/base.tq#132 Change-Id: Ife3857292669f54931708e934398b2684e60bea5 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1814888 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Frank Tang <ftang@chromium.org> Cr-Commit-Position: refs/heads/master@{#64199} |
||
|
Seth Brenith
|
1d3c4975be |
[tools] Use instance types of known Maps in v8_debug_helper
If we can read an object's Map pointer but not any data from the Map itself, we may still be able to accurately describe the object's type if the Map pointer matches one of the known Maps from the snapshot. GetObjectProperties uses that data in one of two ways: - If it is sure that the Map pointer matches a known Map, then it uses the type from that Map and continues as if it read the type normally. - If the Map pointer is at the right offset within a heap page to match a known Map, but the caller didn't provide the addresses of the first pages in Map space or read-only space, then the type of that Map is just a guess and gets returned in a separate array. This gives the caller the opportunity to present guessed types to the user, and perhaps call again using the guessed type as the type hint. Bug: v8:9376 Change-Id: I187f67b77e76699863a14534a9d635b79f654124 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1787986 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Michael Achenbach <machenbach@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#63908} |
||
Clemens Hammacher
|
859b2d77c6 |
Replace base::make_unique by std::make_unique
Since we switched to C++14 now, we can use {std::make_unique} instead
of our own {base::make_unique} from {template-utils.h}.
R=mstarzinger@chromium.org, yangguo@chromium.org
Bug: v8:9687
No-Try: true
Change-Id: I660eb30038bbb079cee93c7861cd87ccd134f01b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789300
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63642}
|
||
|
Seth Brenith
|
0a31d508f1 |
[tools] Teach v8_debug_helper where heap spaces start in ptr-compr mode
v8_debug_helper attempts to flag known object pointers when it can recognize them, even if the memory pointed to is not available in the crash dump. In ptr-compr builds, the first pages of the map space, read-only space, and old space are always at the same offsets within the heap reservation region, so we can more easily detect known objects. Bug: v8:9376 Change-Id: I04e0d2357143d753f575f556e94f8fd42ce9d811 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1783729 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#63624} |
||
|
Seth Brenith
|
2ccca6c5ac |
[tools][torque] Include string values in GetObjectProperties responses
This change provides a quick way to see string contents in postmortem debugging sessions, without digging through a (possibly very large, in the case of ConsString) tree of properties. As well as being convenient for inspecting String objects, this functionality will also be necessary for displaying property names on JSReceiver objects. In order to support custom behaviors for specific classes, this change extends the existing generated debug reader classes with a visitor pattern. Bug: v8:9376 Change-Id: I70eab9ea4e74ca0fab39bf5998d6a602716a4202 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1771939 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Cr-Commit-Position: refs/heads/master@{#63485} |
||
|
Seth Brenith
|
1a815e44b5 |
[tools][torque]Improve postmortem API behavior on strings
This change adds the indexed field for the characters in the definition of sequential string types, and introduces support for recognizing the various specific string types in v8_debug_helper. In an attempt to avoid duplicating info about string instance types, it also refactors String::Get so that StringShape (a simple class usable by postmortem tools) can dispatch using a class that defines behaviors for each concrete type. Bug: v8:9376 Change-Id: Id0653040f6decddc004c73f8fe93d2187828c2c6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1735795 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Michael Stanton <mvstanton@chromium.org> Cr-Commit-Position: refs/heads/master@{#63352} |
||
|
Seth Brenith
|
0921e8f28b |
Reland "Add postmortem debugging helper library"
This is a reland of
|
||
Zhi An Ng
|
6747e3a186 |
Revert "Add postmortem debugging helper library"
This reverts commit
|
||
|
Seth Brenith
|
517ab73fd7 |
Add postmortem debugging helper library
This change begins to implement the functionality described in https://docs.google.com/document/d/1evHnb1uLlSbvHAAsmOXyc25x3uh1DjgNa8u1RHvwVhk/edit# for investigating V8 state in crash dumps. This change adds a new library, v8_debug_helper, for providing platform- agnostic assistance with postmortem debugging. This library can be used by extensions built for debuggers such as WinDbg or lldb. Its public API is described by debug-helper.h; currently the only method it exposes is GetObjectProperties, but we'd like to add more functionality over time. The API surface is restricted to plain C-style structs and pointers, so that it's easy to link from a debugger extension built with a different toolchain. This change also adds a new cctest file to exercise some basic interaction with the new library. The API function GetObjectProperties takes an object pointer (which could be compressed, or weak, or a SMI), and returns a string description of the object and a list of properties the object contains. For now, the list of properties is entirely based on Torque object definitions, but we expect to add custom properties in future updates so that it can be easier to make sense of complex data structures such as dictionaries. GetObjectProperties does several things that are intended to generate somewhat useful results even in cases where memory may be corrupt or unavailable: - The caller may optionally provide a type string which will be used if the memory for the object's Map is inaccessible. - All object pointers are compared against the list of known objects generated by mkgrokdump. The caller may optionally provide the pointers for the first pages of various heap spaces, to avoid spurious matches. If those pointers are not provided, then any matches are prefixed with "maybe" in the resulting description string, such as "maybe UndefinedValue (0x4288000341 <Oddball>)". Bug: v8:9376 Change-Id: Iebf3cc2dea3133c7811bcefcdf38d9458b02fded Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1628012 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Michael Stanton <mvstanton@chromium.org> Cr-Commit-Position: refs/heads/master@{#62882} |