Bug 152923

Summary: Fixed compilation of JavaScriptCore with GCC 4.8 on 32-bit platforms
Product: WebKit Reporter: Konstantin Tokarev <annulen>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, jbriance, keith_miller, mark.lam, mcatanzaro, msaboff, ossy, saam
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Konstantin Tokarev
Reported 2016-01-08 13:26:51 PST
This small patch restores compilation of JSC with GCC 4.8 on 32-bit platforms.
Attachments
Patch (1.42 KB, patch)
2016-01-08 13:28 PST, Konstantin Tokarev
no flags
Konstantin Tokarev
Comment 1 2016-01-08 13:28:20 PST
Michael Saboff
Comment 2 2016-01-08 14:01:25 PST
What is the error that GCC 4.8 reports? Aggregate initialization is a c++11 feature and should be in GCC 4.8.
Konstantin Tokarev
Comment 3 2016-01-08 14:28:26 PST
I hit this GCC bug which is fixed in 4.9.0: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50025 Exact message is: In file included from ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:33:0: ../../Source/JavaScriptCore/jit/CallFrameShuffler.h: In member function ‘void JSC::CallFrameShuffler::assumeCalleeIsCell()’: ../../Source/JavaScriptCore/jit/CallFrameShuffler.h:144:90: error: invalid initialization of non-const reference of type ‘JSC::CachedRecovery&’ from an rvalue of type ‘<brace-enclosed initializer list>’ CachedRecovery& calleeCachedRecovery { *getNew(VirtualRegister(JSStack::Callee)) };
Alex Christensen
Comment 4 2016-01-08 17:24:42 PST
This seems like it is completely unrelated to whether it is being compiled on a 32-bit platform.
Csaba Osztrogonác
Comment 5 2016-01-08 23:41:16 PST
(In reply to comment #4) > This seems like it is completely unrelated to whether it is being compiled > on a 32-bit platform. It is in JSVALUE32_64 block.
Michael Catanzaro
Comment 6 2016-01-09 10:45:55 PST
Frankly, if a one-line change is all that's needed to support a particular compiler, I would just take it. For openSUSE I got the latest stable WebKitGTK+ building and running fine with GCC 4.8 using -DENABLE_INDEXED_DATABASE=OFF and -DENABLE_DATABASE_PROCESS=OFF, in order to provide them the option of upgrading. (It built fine without running into this issue.) We will need to be more conservative with increasing our GCC requirement in the future.
Konstantin Tokarev
Comment 7 2016-01-09 10:49:44 PST
Many vendor-supplied cross-toolchains don't have GCC 4.9 yet, but provide 4.8.x. And many other Linux distributions, e.g. Ubuntu 14.04 LTS, provide GCC 4.8 in their repos, but not newer versions.
Csaba Osztrogonác
Comment 8 2016-01-11 05:41:05 PST
Just a note, once we would like to use B3 as FTL JIT backend, GCC 5.2 will be the minimum required version. See bug151624 for details.
Konstantin Tokarev
Comment 9 2016-01-11 05:46:29 PST
That's sad, I hoped I will be able to fix FTL compilation with 4.8 in local branch. However, right now I'm mostly interested in supporting GCC 4.8 for targeting MIPS with vendor-supplied toolchain, and it has no B3 yet.
Alex Christensen
Comment 10 2016-01-11 12:14:46 PST
(In reply to comment #5) > (In reply to comment #4) > > This seems like it is completely unrelated to whether it is being compiled > > on a 32-bit platform. > > It is in JSVALUE32_64 block. Oh, duh. (In reply to comment #6) > Frankly, if a one-line change is all that's needed to support a particular compiler, I would just take it. I agree. I see no harm in accepting this change right now.
WebKit Commit Bot
Comment 11 2016-01-11 13:04:36 PST
Comment on attachment 268578 [details] Patch Clearing flags on attachment: 268578 Committed r194861: <http://trac.webkit.org/changeset/194861>
WebKit Commit Bot
Comment 12 2016-01-11 13:04:39 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.