Bug 176644 - WebAssembly: Wasm::IndexOrName has a raw pointer to Name
Summary: WebAssembly: Wasm::IndexOrName has a raw pointer to Name
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebAssembly (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: JF Bastien
URL:
Keywords: InRadar
Depends on: 177472
Blocks:
  Show dependency treegraph
 
Reported: 2017-09-08 21:20 PDT by Filip Pizlo
Modified: 2017-11-15 13:06 PST (History)
10 users (show)

See Also:


Attachments
patch (25.87 KB, patch)
2017-10-31 21:48 PDT, JF Bastien
no flags Details | Formatted Diff | Diff
patch (25.88 KB, patch)
2017-10-31 22:00 PDT, JF Bastien
msaboff: review+
msaboff: commit-queue-
Details | Formatted Diff | Diff
patch (25.79 KB, patch)
2017-10-31 22:44 PDT, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2017-09-08 21:20:43 PDT
Since StackFrame can be kept around for a while (see Exception.h), we need that reference to be protected somehow.
Comment 1 JF Bastien 2017-09-08 22:00:55 PDT
It can stick around longer than the WebAssembly module?
Comment 2 Filip Pizlo 2017-09-08 22:19:08 PDT
(In reply to JF Bastien from comment #1)
> It can stick around longer than the WebAssembly module?

Exception is an object. It can stick around for as long as the user likes.
Comment 3 JF Bastien 2017-10-31 21:48:21 PDT
Created attachment 325542 [details]
patch
Comment 4 Build Bot 2017-10-31 21:50:50 PDT
Attachment 325542 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmNameSection.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 JF Bastien 2017-10-31 22:00:46 PDT
Created attachment 325543 [details]
patch

Odd, looks like my compiler does return value optimization to move, but the bots do it to copy, and Ref hates that. Using RefPtr instead, since that's what I store to right after anyways.
Comment 6 Build Bot 2017-10-31 22:01:50 PDT
Attachment 325543 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmNameSection.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Michael Saboff 2017-10-31 22:15:38 PDT
Comment on attachment 325543 [details]
patch

r=me after build fixes.
Comment 8 JF Bastien 2017-10-31 22:44:49 PDT
Created attachment 325545 [details]
patch

Can't use include guards on Name / NameSection / IndexOrName because they're included in JSC Stack things, and the RefPtr needs to call their dtor.
Comment 9 Build Bot 2017-10-31 22:46:20 PDT
Attachment 325545 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmNameSection.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 WebKit Commit Bot 2017-10-31 23:16:03 PDT
Comment on attachment 325545 [details]
patch

Clearing flags on attachment: 325545

Committed r224272: <https://trac.webkit.org/changeset/224272>
Comment 11 WebKit Commit Bot 2017-10-31 23:16:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2017-11-15 13:06:41 PST
<rdar://problem/35568809>