[webkit-reviews] review granted: [Bug 172768] [JSC] WTFGetBacktrace can return numberOfFrames == 0 in some architectures : [Attachment 313461] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 172768: [JSC] WTFGetBacktrace can return numberOfFrames == 0 in some
architectures
https://bugs.webkit.org/show_bug.cgi?id=172768

Attachment 313461: Patch

https://bugs.webkit.org/attachment.cgi?id=313461&action=review




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 313461
  --> https://bugs.webkit.org/attachment.cgi?id=313461
Patch

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

This change doesn’t make things worse, but this existing code does things
wrong. It’s not correct to allocate an object with placement new in memory that
was allocated with fastMalloc and then later delete that object with a call to
delete. Given how it’s allocated, the correct way to destroy that object is to
explicitly call the destructor and then use fastFree to release the memory. I’d
like to see captureStackTrace use std::unique_ptr or some other smart pointer
for the return value so callers aren’t likely to get that wrong.

> Source/WTF/wtf/StackTrace.cpp:62
> +	   delete trace;

As described above, this is incorrect, although the existing calling code
outside the function has the same problem. Correct code for this would be:

    trace->~StackTrace();
    fastFree(trace);