Bug 187233 – JavaScriptCore: Fix clang static analyzer warnings: Assigned value is garbage or undefined

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[David Kilzer (:ddkilzer)]

Fix "Assigned value is garbage or undefined" warnings in JavaScriptCore found by clang static analyzer deep analysis.

[Radar WebKit Bug Importer]

<rdar://problem/41698361>

[David Kilzer (:ddkilzer)]

Created attachment 344053 [details]
Patch v1

[Mark Lam]

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.

[David Kilzer (:ddkilzer)]

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 } };

[David Kilzer (:ddkilzer)]

Created attachment 344056 [details]
Patch v2 using '= default' ctors

[David Kilzer (:ddkilzer)]

Created attachment 344058 [details]
Patch v2 no union initializer to test WPE

[David Kilzer (:ddkilzer)]

(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.

[David Kilzer (:ddkilzer)]

(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?

[David Kilzer (:ddkilzer)]

(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.

[Michael Catanzaro]

(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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[David Kilzer (:ddkilzer)]

Committed r233410: <https://trac.webkit.org/changeset/233410>

[Adrian Perez]

(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.