WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
183545
PerProcess<> should be safe by default
https://bugs.webkit.org/show_bug.cgi?id=183545
Summary
PerProcess<> should be safe by default
Filip Pizlo
Reported
2018-03-10 13:19:47 PST
Patch forthcoming.
Attachments
maybe this will work
(14.08 KB, patch)
2018-03-10 13:20 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(16.37 KB, patch)
2018-03-10 15:24 PST
,
Filip Pizlo
ysuzuki
: review+
Details
Formatted Diff
Diff
patch for landing
(16.92 KB, patch)
2018-03-11 10:18 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2018-03-10 13:20:25 PST
Created
attachment 335519
[details]
maybe this will work
Filip Pizlo
Comment 2
2018-03-10 15:24:03 PST
Created
attachment 335520
[details]
the patch
EWS Watchlist
Comment 3
2018-03-10 15:27:15 PST
Attachment 335520
[details]
did not pass style-queue: ERROR: Source/bmalloc/bmalloc/PerProcess.h:64: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/PerProcess.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 4
2018-03-11 01:34:22 PST
Comment on
attachment 335520
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=335520&action=review
r=me with a micro nit.
> Source/bmalloc/bmalloc/PerProcess.cpp:49 > +static unsigned stringHash(const char* string) > +{ > + unsigned result = 5381; > + while (char c = *string++) > + result = result * 33 + c; > + return result; > +}
Nits: you can make this `constexpr` function, move it to a header, and calculate hash value at compile time.
Filip Pizlo
Comment 5
2018-03-11 10:15:15 PDT
(In reply to Yusuke Suzuki from
comment #4
)
> Comment on
attachment 335520
[details]
> the patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=335520&action=review
> > r=me with a micro nit. > > > Source/bmalloc/bmalloc/PerProcess.cpp:49 > > +static unsigned stringHash(const char* string) > > +{ > > + unsigned result = 5381; > > + while (char c = *string++) > > + result = result * 33 + c; > > + return result; > > +} > > Nits: you can make this `constexpr` function, move it to a header, and > calculate hash value at compile time.
Really, the compiler will execute a constexpr loop?
Filip Pizlo
Comment 6
2018-03-11 10:18:50 PDT
Created
attachment 335540
[details]
patch for landing With Yusuke's suggested change.
EWS Watchlist
Comment 7
2018-03-11 10:20:44 PDT
Attachment 335540
[details]
did not pass style-queue: ERROR: Source/bmalloc/bmalloc/PerProcess.h:72: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/PerProcess.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 8
2018-03-11 10:21:23 PDT
(In reply to Filip Pizlo from
comment #5
)
> > Nits: you can make this `constexpr` function, move it to a header, and > > calculate hash value at compile time. > > Really, the compiler will execute a constexpr loop?
Yeah, C++14 relaxed constexpr is super powerful. You can check our wtf/text/StringHasher.h's computeLiteralHash for example.
Filip Pizlo
Comment 9
2018-03-11 10:46:22 PDT
Landed in
https://trac.webkit.org/changeset/229516/webkit
Radar WebKit Bug Importer
Comment 10
2018-03-12 17:24:43 PDT
<
rdar://problem/38397867
>
Joseph Pecoraro
Comment 11
2020-02-05 11:40:08 PST
***
Bug 182496
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug