Bug 187233

Summary: JavaScriptCore: Fix clang static analyzer warnings: Assigned value is garbage or undefined
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: JavaScriptCoreAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, clopez, ews-watchlist, fpizlo, keith_miller, magomez, mark.lam, mcatanzaro, msaboff, rego, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=187260
Attachments:
Description Flags
Patch v1
mark.lam: review+
Patch v2 using '= default' ctors
ews-watchlist: commit-queue-
Patch v2 no union initializer to test WPE
none
Archive of layout-test-results from ews206 for win-future none

Description David Kilzer (:ddkilzer) 2018-07-01 10:15:27 PDT
Fix "Assigned value is garbage or undefined" warnings in JavaScriptCore found by clang static analyzer deep analysis.
Comment 1 Radar WebKit Bug Importer 2018-07-01 10:15:50 PDT
<rdar://problem/41698361>
Comment 2 David Kilzer (:ddkilzer) 2018-07-01 10:18:18 PDT
Created attachment 344053 [details]
Patch v1
Comment 3 Mark Lam 2018-07-01 10:40:16 PDT
Comment on attachment 344053 [details]
Patch v1

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

> Source/JavaScriptCore/parser/ParserTokens.h:194
> +    JSTextPosition() { }

You can say this instead: JSTextPosition() = default;

> Source/JavaScriptCore/parser/ParserTokens.h:248
> +    JSTokenLocation() { }

Ditto.
Comment 4 David Kilzer (:ddkilzer) 2018-07-01 12:36:31 PDT
Huh.  This patch appears to have killed the g++ compiler for wpe:

g++-6: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.

The most unusual thing I did was to create struct member initialization for the JSTokenData union in struct JSToken:

    JSTokenData m_data { { nullptr, nullptr, false } };
Comment 5 David Kilzer (:ddkilzer) 2018-07-01 12:39:12 PDT
Created attachment 344056 [details]
Patch v2 using '= default' ctors
Comment 6 David Kilzer (:ddkilzer) 2018-07-01 12:41:45 PDT
Created attachment 344058 [details]
Patch v2 no union initializer to test WPE
Comment 7 David Kilzer (:ddkilzer) 2018-07-01 12:50:08 PDT
(In reply to David Kilzer (:ddkilzer) from comment #2)
> Created attachment 344053 [details]
> Patch v1

Hello Igalia folks!  This patch appears to have caused a g++ internal compiler error (ICE):

g++-6: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.

https://webkit-queues.webkit.org/patch/344053/wpe-ews

You might want to collect pre-compiled source and the command-line invocation and submit a bug to the g++ project, or update g++ on the bot if there is a new stable version that's been qualified.
Comment 8 David Kilzer (:ddkilzer) 2018-07-01 12:50:56 PDT
(In reply to David Kilzer (:ddkilzer) from comment #7)
> (In reply to David Kilzer (:ddkilzer) from comment #2)
> > Created attachment 344053 [details]
> > Patch v1
> 
> Hello Igalia folks!  This patch appears to have caused a g++ internal
> compiler error (ICE):
> 
> g++-6: internal compiler error: Killed (program cc1plus)
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.
> 
> https://webkit-queues.webkit.org/patch/344053/wpe-ews
> 
> You might want to collect pre-compiled source and the command-line
> invocation and submit a bug to the g++ project, or update g++ on the bot if
> there is a new stable version that's been qualified.

Or maybe one of the bots is just running out of resources (disk space or memory) when compiling?
Comment 9 David Kilzer (:ddkilzer) 2018-07-01 12:52:51 PDT
(In reply to David Kilzer (:ddkilzer) from comment #8)
> (In reply to David Kilzer (:ddkilzer) from comment #7)
> > (In reply to David Kilzer (:ddkilzer) from comment #2)
> > > Created attachment 344053 [details]
> > > Patch v1
> > 
> > Hello Igalia folks!  This patch appears to have caused a g++ internal
> > compiler error (ICE):
> > 
> > g++-6: internal compiler error: Killed (program cc1plus)
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.
> > 
> > https://webkit-queues.webkit.org/patch/344053/wpe-ews
> > 
> > You might want to collect pre-compiled source and the command-line
> > invocation and submit a bug to the g++ project, or update g++ on the bot if
> > there is a new stable version that's been qualified.
> 
> Or maybe one of the bots is just running out of resources (disk space or
> memory) when compiling?

Looks like bot `aperez-wpe-gcc6-ews` fails, but bot `igalia-wpe-ews works.

Will wait to land this until Monday so you have a chance to update or disable this bot until it's fixed.
Comment 10 Michael Catanzaro 2018-07-01 13:28:42 PDT
(In reply to David Kilzer (:ddkilzer) from comment #8)
> Or maybe one of the bots is just running out of resources (disk space or
> memory) when compiling?

GCC prints:

g++-6: internal compiler error: Killed (program cc1plus)

It's a ludicrously misleading error message, but the "Killed" indicates that cc1plus received the signal SIGKILL. That's almost certainly coming from the kernel out-of-memory killer. I'm sure Adrian will take a look tomorrow; in the meantime, you can safely ignore this EWS.
Comment 11 EWS Watchlist 2018-07-01 14:46:29 PDT
Comment on attachment 344056 [details]
Patch v2 using '= default' ctors

Attachment 344056 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8406137

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html
http/tests/preload/onload_event.html
http/tests/security/video-poster-cross-origin-crash2.html
Comment 12 EWS Watchlist 2018-07-01 14:46:40 PDT
Created attachment 344062 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 13 David Kilzer (:ddkilzer) 2018-07-01 15:38:59 PDT
Committed r233410: <https://trac.webkit.org/changeset/233410>
Comment 14 Adrian Perez 2018-07-02 09:47:31 PDT
(In reply to Michael Catanzaro from comment #10)
> (In reply to David Kilzer (:ddkilzer) from comment #8)
> > Or maybe one of the bots is just running out of resources (disk space or
> > memory) when compiling?
> 
> GCC prints:
> 
> g++-6: internal compiler error: Killed (program cc1plus)
> 
> It's a ludicrously misleading error message, but the "Killed" indicates that
> cc1plus received the signal SIGKILL. That's almost certainly coming from the
> kernel out-of-memory killer. I'm sure Adrian will take a look tomorrow; in
> the meantime, you can safely ignore this EWS.

Ah, yes... classic OOM. Until now I had not seen the bot being killed
due to lack of memory. Usually this happens when there are multiple
linker processes running in parallel. This would be solved by using
job pools, see bug #187254 for more details.

That being said, it could be anyway a good thing to limit the number
of concurrent processes in my EWS... Right now “ninja -j64” is being
used because the kernel reports 64 cores for this machine due to SMT
(that is: HyperThreading), but there are 32 physical cores, and after
doing some testing it turns out that using more than “ninja -j42” does
not result in faster builds in this particular box. I'll look into
limiting this, should be easy as the EWS is containerized.