Bug 187233 - JavaScriptCore: Fix clang static analyzer warnings: Assigned value is garbage or undefined
Summary: JavaScriptCore: Fix clang static analyzer warnings: Assigned value is garbage...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-01 10:15 PDT by David Kilzer (:ddkilzer)
Modified: 2018-07-02 12:30 PDT (History)
12 users (show)

See Also:


Attachments
Patch v1 (4.84 KB, patch)
2018-07-01 10:18 PDT, David Kilzer (:ddkilzer)
mark.lam: review+
Details | Formatted Diff | Diff
Patch v2 using '= default' ctors (4.86 KB, patch)
2018-07-01 12:39 PDT, David Kilzer (:ddkilzer)
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Patch v2 no union initializer to test WPE (4.61 KB, patch)
2018-07-01 12:41 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.81 MB, application/zip)
2018-07-01 14:46 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.