Bug 211353 - Fix static analyzer false positive in -[WebUndefined undefined]
Summary: Fix static analyzer false positive in -[WebUndefined undefined]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-03 10:39 PDT by David Kilzer (:ddkilzer)
Modified: 2020-05-03 20:43 PDT (History)
3 users (show)

See Also:

Attachments
Patch v1 (2.72 KB, patch)
2020-05-03 10:41 PDT, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Patch for landing (2.53 KB, patch)
2020-05-03 20:15 PDT, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2020-05-03 10:39:44 PDT 
Fix static analyzer false positive in -[WebUndefined undefined].

The clang static analyzer is confused by calling -allocWithZone: directly without an -init method.
Comment 1 David Kilzer (:ddkilzer) 2020-05-03 10:41:17 PDT 
Created attachment 398317 [details]
Patch v1
Comment 2 Darin Adler 2020-05-03 16:18:52 PDT 
Comment on attachment 398317 [details]
Patch v1

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

> Source/WebCore/ChangeLog:11
> +        - Use dispatch_once since iOS WK1 could initialize this on the
> +          WebThread or the main thread.

Are you certain this is needed? I am concerned, because there are many things that aren’t truly thread-safe that can go on either web thread of main thread. And this seems like a speculative fix.
Comment 3 David Kilzer (:ddkilzer) 2020-05-03 16:33:23 PDT 
Comment on attachment 398317 [details]
Patch v1

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

>> Source/WebCore/ChangeLog:11
>> +          WebThread or the main thread.
> 
> Are you certain this is needed? I am concerned, because there are many things that aren’t truly thread-safe that can go on either web thread of main thread. And this seems like a speculative fix.

This is speculative.  I won't add it then.
Comment 4 David Kilzer (:ddkilzer) 2020-05-03 20:15:48 PDT 
Created attachment 398344 [details]
Patch for landing
Comment 5 David Kilzer (:ddkilzer) 2020-05-03 20:16:49 PDT 
Comment on attachment 398344 [details]
Patch for landing

Marking cq+ since "Patch v1" passed all EWS queues, and this patch just reverted one change.
Comment 6 EWS 2020-05-03 20:42:06 PDT 
Committed r261070: <https://trac.webkit.org/changeset/261070>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398344 [details].
Comment 7 Radar WebKit Bug Importer 2020-05-03 20:43:11 PDT 
<rdar://problem/62828739>