Bug 143910 - Add debugging tools to test if a given pointer is a valid object and in the heap.
Summary: Add debugging tools to test if a given pointer is a valid object and in the h...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-17 17:31 PDT by Mark Lam
Modified: 2015-04-20 14:33 PDT (History)
9 users (show)

See Also:


Attachments
the patch. (6.42 KB, patch)
2015-04-17 17:44 PDT, Mark Lam
ggaren: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (596.25 KB, application/zip)
2015-04-17 18:32 PDT, Build Bot
no flags Details
patch for landing: applied Geoff's feedback (11.39 KB, patch)
2015-04-20 13:02 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2015-04-17 17:31:22 PDT
When doing debugging from lldb, sometimes, it is useful to be able to tell if a purported JSObject is really a valid object in the heap or not.  We can add the following utility functions to help:
    isValidObject(heap, candidate) - returns true if the candidate is a "live" object in the heap.
    isInHeap(heap, candidate) - returns true if the candidate is the heap's Object space or Storage space.
    isInObjectSpace(heap, candidate) - returns true if the candidate is the heap's Object space.
    isInStorageSpace(heap, candidate) - returns true if the candidate is the heap's Storage space.
Comment 1 Mark Lam 2015-04-17 17:44:48 PDT
Created attachment 251076 [details]
the patch.
Comment 2 Build Bot 2015-04-17 18:32:42 PDT
Comment on attachment 251076 [details]
the patch.

Attachment 251076 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5998144017924096

New failing tests:
compositing/scrolling/touch-scroll-to-clip.html
Comment 3 Build Bot 2015-04-17 18:32:45 PDT
Created attachment 251077 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 4 Mark Lam 2015-04-18 00:08:17 PDT
(In reply to comment #3)
> Created attachment 251077 [details]
> Archive of layout-test-results from ews103 for mac-mavericks
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-ews.
> Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

The test failures are due to a pre-existing flaky test.  The code in this patch is not even called.  Hence, it cannot possibly cause the test failures.
Comment 5 Geoffrey Garen 2015-04-20 11:17:06 PDT
Comment on attachment 251076 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=251076&action=review

> Source/JavaScriptCore/tools/JSDollarVMPrototype.cpp:165
> +        if (!cell->isObject())
> +            return;

No need to do this check.

> Source/JavaScriptCore/tools/JSDollarVMPrototype.h:77
> +JS_EXPORT_PRIVATE bool currentThreadOwnsJSLock(ExecState*);
> +JS_EXPORT_PRIVATE void gc(ExecState*);
> +JS_EXPORT_PRIVATE void edenGC(ExecState*);
> +JS_EXPORT_PRIVATE bool isInHeap(Heap*, void*);
> +JS_EXPORT_PRIVATE bool isInObjectSpace(Heap*, void*);
> +JS_EXPORT_PRIVATE bool isInStorageSpace(Heap*, void*);
> +JS_EXPORT_PRIVATE bool isValidObject(Heap*, JSObject*);
> +JS_EXPORT_PRIVATE bool isValidCodeBlock(ExecState*, CodeBlock*);
> +JS_EXPORT_PRIVATE CodeBlock* codeBlockForFrame(CallFrame* topCallFrame, unsigned frameNumber);
> +JS_EXPORT_PRIVATE void printCallFrame(CallFrame*);
> +JS_EXPORT_PRIVATE void printStack(CallFrame* topCallFrame);
> +JS_EXPORT_PRIVATE void printValue(JSValue);

Now that these are in a header, can you namespace them better? Can you make them statics in the VM object, or, if that's not compatible with the debugger, at least give them a "vm" prefix?
Comment 6 Mark Lam 2015-04-20 13:02:58 PDT
Created attachment 251184 [details]
patch for landing: applied Geoff's feedback

Per my offline discussion with Geoff, we'll replace isValidObject() with isValidCell() (which is more useful).

I'll also be moving the lldb callable functions into the JSDollarVMPrototype class, not the VM class.
Comment 7 Mark Lam 2015-04-20 14:33:09 PDT
Landed in r183027: <http://trac.webkit.org/r183027>.