Bug 24932 - WebKit compilation support in Solaris 10 with Sun Studio 12 (CC 5.9)
Summary: WebKit compilation support in Solaris 10 with Sun Studio 12 (CC 5.9)
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-30 06:04 PDT by Thiago Macieira
Modified: 2014-12-03 16:02 PST (History)
8 users (show)

See Also:


Attachments
[PATCH 01/16] Add a COMPILER(SUNCC) define. This will be useful in other contexts too. (1.44 KB, patch)
2009-03-30 06:09 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 02/16] Add support for aligned buffers without alignment macros (2.36 KB, patch)
2009-03-30 06:10 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 03/16] Fix oversize-buffer support for aligning. (1.81 KB, patch)
2009-03-30 06:11 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 04/16] Fix Sun CC 5.9 compiler error: the default constructor for HashTableIteratorAdapter isn't called. (5.80 KB, patch)
2009-03-30 06:13 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 05/16] Fix compilation with Sun CC 5.9: std::pair does not call types' constructors. (3.02 KB, patch)
2009-03-30 06:14 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 06/16] Fix compilation with Sun CC 5.9: std::pair constructor again. (2.59 KB, patch)
2009-03-30 06:15 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 07/16] Fix compilation with Sun CC 5.9: template parameters MUST match. (1.83 KB, patch)
2009-03-30 06:16 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 08/16] Fix compilation error on Solaris: mmap/munmap take/return a char*, not void*. (3.02 KB, patch)
2009-03-30 06:19 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 09/16] Fix compilation with Sun CC 5.9: casts like these are not allowed (2.38 KB, patch)
2009-03-30 06:20 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 10/16] Fix compilation with Sun CC 5.9: ambiguity in casting to bool. (1.45 KB, patch)
2009-03-30 06:22 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 11/16] Fix compilation with Sun CC 5.9: I have no clue why it thinks one of the operands is bool. (1005 bytes, patch)
2009-03-30 06:22 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 12/16] Remove comma at end of enum. Some compilers are more picky than others. (7.22 KB, patch)
2009-03-30 06:23 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 13/16] Workaround a Sun CC 5.9 compiler bug: when generating debug information (-g), it requires the Event class to be defined because it instantiates the PassRefPtr<Event> too soon (888 bytes, patch)
2009-03-30 06:23 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 14/16] Fix compilation with Sun CC 5.9: ambiguity in ?: (2.33 KB, patch)
2009-03-30 06:24 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 15/16] Fix compilation with Sun CC 5.9: ambiguity in ?: (12.89 KB, patch)
2009-03-30 06:24 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 16/16] Fix compilation with Sun CC 5.9: ambiguity with QString's operator+. (11.26 KB, patch)
2009-03-30 06:25 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
Add a COMPILER(SUNCC) define. This will be useful in other contexts too. (1.46 KB, patch)
2009-07-28 07:56 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 02/17] Add support for aligned buffers without alignment macros (2.28 KB, patch)
2009-07-28 07:58 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 03/17] Fix oversize-buffer support for aligning. (1.81 KB, patch)
2009-07-28 07:59 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 04/17] Fix compilation on Solaris 9: missing definition for time_t (1.91 KB, patch)
2009-07-28 08:02 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 05/17] Fix crash misaligned reads on Sparc processors. (1.43 KB, patch)
2009-07-28 08:05 PDT, Thiago Macieira
eric: review-
Details | Formatted Diff | Diff
[PATCH 06/17] Fix linking with SunCC 5.9: de-inline the operator new and delete in ParserArenaDeletable. (2.94 KB, patch)
2009-07-28 08:06 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 07/17] Fix compilation with Sun CC 5.9: template parameters MUST match. (1.83 KB, patch)
2009-07-28 08:12 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 08/17] Fix compilation error on Solaris: mmap/munmap take/return a char*, not void*. (2.49 KB, patch)
2009-07-28 08:13 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 09/17] Fix compilation with Sun CC 5.9: casts like these are not allowed (2.38 KB, patch)
2009-07-28 08:14 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 10/17] Fix compilation with Sun CC 5.9: I have no clue why it thinks one of the operands is bool. (1005 bytes, patch)
2009-07-28 08:15 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 11/17] Fix compilation with Sun CC 5.9: moving elements in a vector requires source not to be const (2.77 KB, patch)
2009-07-28 08:29 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 12/17] Remove comma at end of enum. Some compilers are more picky than others. (13.04 KB, patch)
2009-07-28 08:30 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 13/17] Fix compilation with Sun CC 5.9: types must match on ?: (3.21 KB, patch)
2009-07-28 08:33 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 14/17] Fix compilation with Sun CC 5.9: ambiguity in ?: (2.31 KB, patch)
2009-07-28 08:34 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 15/17] Fix compilation with Sun CC 5.9: ambiguity in ?: (17.98 KB, patch)
2009-07-28 08:34 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 16/17] Fix compilation with Sun CC 5.9: ambiguity with QString's operator+. (11.41 KB, patch)
2009-07-28 08:35 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff
[PATCH 17/17] Fix linking with Sun CC 5.9: function pointers for extern "C" are treated differently (2.26 KB, patch)
2009-07-28 08:36 PDT, Thiago Macieira
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago Macieira 2009-03-30 06:04:50 PDT
Currently WebKit does not compile with that compiler and its standard STL headers.

I spent some time fixing the issues, with some help from other developers in Nokia. The result is the set of 16 patches that I'm about to attach. The code still compiles with gcc after these patches.
Comment 1 Thiago Macieira 2009-03-30 06:09:04 PDT
Created attachment 29056 [details]
[PATCH 01/16] Add a COMPILER(SUNCC) define. This will be useful in other contexts too.



Add a COMPILER(SUNCC) define. This will be useful in other contexts too.

Also add support for 32-bit SPARC.
Comment 2 Thiago Macieira 2009-03-30 06:10:47 PDT
Created attachment 29057 [details]
[PATCH 02/16] Add support for aligned buffers without alignment macros

If we don't have alignment macros, we do what we can:
overcommit 64 bytes and find the proper 64-byte-aligned position in
the buffer.
Comment 3 Thiago Macieira 2009-03-30 06:11:45 PDT
Created attachment 29058 [details]
[PATCH 03/16] Fix oversize-buffer support for aligning.

Fix oversize-buffer support for aligning.

Since Vector initialises VectorBase with the value of inlineBuffer(), it does so before the m_inlineBuffer member has had a chance to initialise. This lead to dereferencing of uninitialised pointers and, as was expected, crashes.

[this is a fix to patch 2]
Comment 4 Thiago Macieira 2009-03-30 06:13:05 PDT
Created attachment 29059 [details]
[PATCH 04/16] Fix Sun CC 5.9 compiler error: the default constructor for HashTableIteratorAdapter isn't called.

Fix Sun CC 5.9 compiler error: the default constructor for HashTableIteratorAdapter isn't called.

"../JavaScriptCore/wtf/RefPtrHashMap.h", line 208: Error: Cannot use std::pair<WTF::HashTableIterator<WTF::RefPtr<JSC::UString::Rep>, std::pair<WTF::RefPtr<JSC::UString::Rep>, StaticValueEntry*>, WTF::PairFirstExtractor<std::pair<WTF::RefPtr<JSC::UString::Rep>, StaticValueEntry*>>, WTF::StrHash<WTF::R
efPtr<JSC::UString::Rep>>, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<JSC::UString::Rep>>, WTF::HashTraits<StaticValueEntry*>>, WTF::HashTraits<WTF::RefPtr<JSC::UString::Rep>>>, bool> to initialize std::pair<WTF::HashTableIteratorAdapter<WTF::HashTable<WTF::RefPtr<JSC::UString::Rep>, std::pair<WTF::RefPtr<JSC::UString::Rep>, StaticValueEntry*>, WTF::PairFirstExtractor<std::pair<WTF::RefPtr<JSC::UString::Rep>, StaticValueEntry*>>, WTF::StrHash<WTF::RefPtr<JSC::UString::Rep>>, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<JSC::UString::Rep>>, WTF::HashTraits<StaticValueEntry*>>, WTF::HashTraits<WTF::RefPtr<JSC::UString::Rep>>>, std::pair<WTF::RefPtr<JSC::UString::Rep>, StaticValueEntry*>>, bool>.
Comment 5 Thiago Macieira 2009-03-30 06:14:53 PDT
Created attachment 29060 [details]
[PATCH 05/16] Fix compilation with Sun CC 5.9: std::pair does not call types' constructors.



Fix compilation with Sun CC 5.9: std::pair does not call types' constructors.

Error: Could not find a match for WTF::HashMap<std::pair<WTF::RefPtr<JSC::UString::Rep>, unsigned>, JSC::Structure*, JSC::StructureTransitionTableHash, JSC::StructureTransitionTableHashTraits, WTF::HashTraits<JSC::Structure*>>::remove(std::pair<JSC::UString::Rep*, unsigned>) needed in JSC::Structure::~Structure().
Comment 6 Thiago Macieira 2009-03-30 06:15:56 PDT
Created attachment 29061 [details]
[PATCH 06/16] Fix compilation with Sun CC 5.9: std::pair constructor again.

Fix compilation with Sun CC 5.9: std::pair constructor again.

Error: Could not find a match for std::pair<unsigned, unsigned>::pair(const std::pair<unsigned short, unsigned short>) needed in WTF::Vector<std::pair<unsigned, unsigned>, 0>::append<std::pair<unsigned short, unsigned short>>(const std::pair<unsigned short, unsigned short>&).

Error: Could not find a match for std::pair<WebCore::KURL, WebCore::KURL>::pair(const std::pair<WebCore::String, WebCore::String>) needed in WTF::Vector<std::pair<WebCore::KURL, WebCore::KURL>, 0>::append<std::pair<WebCore::String, WebCore::String>>(const std::pair<WebCore::String, WebCore::String>&).
Comment 7 Thiago Macieira 2009-03-30 06:16:32 PDT
Created attachment 29062 [details]
[PATCH 07/16] Fix compilation with Sun CC 5.9: template parameters MUST match.

Fix compilation with Sun CC 5.9: template parameters MUST match.

We're passing 'char[]' to the template, but it expects 'const char *'.
Let's hope this hackish solution actually works.

The correct solution is to change the variable types.
Comment 8 Thiago Macieira 2009-03-30 06:19:27 PDT
Created attachment 29063 [details]
[PATCH 08/16] Fix compilation error on Solaris: mmap/munmap take/return a char*, not void*.

Fix compilation error on Solaris: mmap/munmap take/return a char*, not void*.

"../JavaScriptCore/interpreter/RegisterFile.h", line 128: Error: Using static_cast to convert from char* to JSC::Register* not allowed.

Error: Formal argument 1 of type char* in call to munmap(char*, unsigned) is being passed JSC::Register*.
Comment 9 Thiago Macieira 2009-03-30 06:20:21 PDT
Created attachment 29064 [details]
[PATCH 09/16] Fix compilation with Sun CC 5.9: casts like these are not allowed

Fix compilation with Sun CC 5.9: casts like these are not allowed

"../css/CSSGrammar.y", line 735: Error: Using static_cast to convert from WebCore::CSSParserString& to const WebCore::String& not allowed.
"../css/CSSGrammar.y", line 737: Error: Using static_cast to convert from WebCore::CSSParserString& to const WebCore::String& not allowed.
Comment 10 Thiago Macieira 2009-03-30 06:22:01 PDT
Created attachment 29065 [details]
[PATCH 10/16] Fix compilation with Sun CC 5.9: ambiguity in casting to bool.

Fix compilation with Sun CC 5.9: ambiguity in casting to bool.

Error: Overloading ambiguity between "JSC::ProtectedPtr<JSC::JSObject>::operator JSC::JSObject*() const" and "JSC::ProtectedPtr<JSC::JSObject>::operator bool() const".
Error: Overloading ambiguity between "JSC::ProtectedPtr<WebCore::JSDOMGlobalObject>::operator WebCore::JSDOMGlobalObject*() const" and "JSC::ProtectedPtr<WebCore::JSDOMGlobalObject>::operator bool() const".
Comment 11 Thiago Macieira 2009-03-30 06:22:32 PDT
Created attachment 29066 [details]
[PATCH 11/16] Fix compilation with Sun CC 5.9: I have no clue why it thinks one of the operands is bool.

Fix compilation with Sun CC 5.9: I have no clue why it thinks one of the operands is bool.

Error: Different types for "?:" (bool and WebCore::RenderStyle*).
Comment 12 Thiago Macieira 2009-03-30 06:23:09 PDT
Created attachment 29067 [details]
[PATCH 12/16] Remove comma at end of enum. Some compilers are more picky than others.
Comment 13 Thiago Macieira 2009-03-30 06:23:39 PDT
Created attachment 29068 [details]
[PATCH 13/16] Workaround a Sun CC 5.9 compiler bug: when generating debug information (-g), it requires the Event class to be defined because it instantiates the PassRefPtr<Event> too soon
Comment 14 Thiago Macieira 2009-03-30 06:24:23 PDT
Created attachment 29069 [details]
[PATCH 14/16] Fix compilation with Sun CC 5.9: ambiguity in ?:

Fix compilation with Sun CC 5.9: ambiguity in ?:

Error: Ambiguous "?:" expression, second operand of type "JSC::JSObject*" and third operand of type "JSC::JSValuePtr" can be converted to one another.
Error: Ambiguous "?:" expression, second operand of type "JSC::RuntimeObjectImp*" and third operand of type "JSC::JSValuePtr" can be converted to one another.
Comment 15 Thiago Macieira 2009-03-30 06:24:52 PDT
Created attachment 29070 [details]
[PATCH 15/16] Fix compilation with Sun CC 5.9: ambiguity in ?:

Fix compilation with Sun CC 5.9: ambiguity in ?:

Error: Ambiguous "?:" expression, second operand of type "WTF::PassRefPtr<WebCore::DocumentFragment>" and third operand of type "int" can be convertedto one another.
Error: Ambiguous "?:" expression, second operand of type "WTF::PassRefPtr<JSC::Label>" and third operand of type "int" can be converted to one another.
[and others similar]
Comment 16 Thiago Macieira 2009-03-30 06:25:24 PDT
Created attachment 29071 [details]
[PATCH 16/16] Fix compilation with Sun CC 5.9: ambiguity with QString's operator+.

Fix compilation with Sun CC 5.9: ambiguity with QString's operator+.

This is also a slight performance improvement since we're avoiding a temporary string.

Error: Overloading ambiguity between "WebCore::operator+(const WebCore::String&, const char*)" and "operator+(const QString&, const char*)".
[and others similar]
Comment 17 Thiago Macieira 2009-07-21 07:51:35 PDT
Patches are now obsolete due to trunk moving.

I have a new set of patches now.
Comment 18 Thiago Macieira 2009-07-28 07:56:54 PDT
Created attachment 33632 [details]
Add a COMPILER(SUNCC) define. This will be useful in other contexts too.

Also add support for 32-bit SPARC.
Comment 19 Thiago Macieira 2009-07-28 07:58:06 PDT
Created attachment 33633 [details]
[PATCH 02/17] Add support for aligned buffers without alignment macros

If we don't have alignment macros, we do what we can: overcommit 64
bytes and find the proper 64-byte-aligned position in the buffer.
Comment 20 Thiago Macieira 2009-07-28 07:59:07 PDT
Created attachment 33634 [details]
[PATCH 03/17] Fix oversize-buffer support for aligning.

Since Vector initialises VectorBase with the value of inlineBuffer(), it does so before the m_inlineBuffer member has had a chance to initialise. This lead to dereferencing of uninitialised pointers and, as was expected, crashes.
Comment 21 Thiago Macieira 2009-07-28 08:02:45 PDT
Created attachment 33635 [details]
[PATCH 04/17] Fix compilation on Solaris 9: missing definition for time_t

"platform/network/ResourceResponseBase.h", line 83: Error: time_t is not defined.
"page/Page.h", line 245: Error: No storage class or type for this declaration.
[etc]
Comment 22 Simon Hausmann 2009-07-28 08:04:24 PDT
Thiago, if you'd like to put the patches up for review please mark them with
"review?" in the patch details. I noticed the patches are missing a ChangeLog
entry. You can use the prepare-ChangeLog script to generate those entries (
http://webkit.org/coding/contributing.html ). They work also with git :)
Comment 23 Thiago Macieira 2009-07-28 08:05:08 PDT
Created attachment 33636 [details]
[PATCH 05/17] Fix crash misaligned reads on Sparc processors.

Sparc cannot do 32-bit reads on misaligned boundaries. The program
crashes if you try to do so.

t@1 (l@1) signal BUS (invalid address alignment) in WebCore::equal at 0xffffffff7e2719fc
0xffffffff7e2719fc: equal+0x002c:       ld       [%o1], %g1
(dbx) where -q 1
=>[1] WebCore::equal(WebCore::StringImpl*,const unsigned short*,unsigned)
(dbx) print -fx $o1
$o1 = 0x45d15a
Comment 24 Thiago Macieira 2009-07-28 08:06:49 PDT
Created attachment 33637 [details]
[PATCH 06/17] Fix linking with SunCC 5.9: de-inline the operator new and delete in ParserArenaDeletable.

If you mark functions as "inline", the compiler doesn't have to emit
out-of-line copies. What happens is that Nodes.h declares these
functions, but the inline bodies are in NodeConstructors.h.

ParserArena.cpp used these functions, but didn't include
NodeConstructor.h. I could have added the missing #include, but this
is error-prone, since you have to remember to do that.

Moving the bodies into Nodes.h was also not possible, because it
requires JSC::Parser to be defined and Parser.h needs to #include
"Nodes.h".

So the solution is to de-inline.
Comment 25 Thiago Macieira 2009-07-28 08:10:53 PDT
The original patches 4, 5 and 6 were required because we were using the default STL implementation of the Sun compiler, which is not standards compliant.

Since Qt 4.6 is stricter in which STLs it will accept, we need to change to a more compliant version. As such, the patches were no longer needed.

To keep the same numbering, I moved up 3 new changes to 4, 5 and 6.
Comment 26 Thiago Macieira 2009-07-28 08:12:49 PDT
Created attachment 33638 [details]
[PATCH 07/17] Fix compilation with Sun CC 5.9: template parameters MUST match.

We're passing 'char[]' to the template, but it expects 'const char *'.
Let's hope this hackish solution actually works.

The correct solution is to change the variable types.
Comment 27 Thiago Macieira 2009-07-28 08:13:45 PDT
Created attachment 33639 [details]
[PATCH 08/17] Fix compilation error on Solaris: mmap/munmap take/return a char*, not void*.
Comment 28 Thiago Macieira 2009-07-28 08:14:28 PDT
Created attachment 33640 [details]
[PATCH 09/17] Fix compilation with Sun CC 5.9: casts like these are not allowed

"../css/CSSGrammar.y", line 735: Error: Using static_cast to convert from WebCore::CSSParserString& to const WebCore::String& not allowed.
"../css/CSSGrammar.y", line 737: Error: Using static_cast to convert from WebCore::CSSParserString& to const WebCore::String& not allowed.
Comment 29 Thiago Macieira 2009-07-28 08:15:16 PDT
Created attachment 33641 [details]
[PATCH 10/17] Fix compilation with Sun CC 5.9: I have no clue why it thinks one of the operands is bool.

Error: Different types for "?:" (bool and WebCore::RenderStyle*).
Comment 30 Thiago Macieira 2009-07-28 08:29:21 PDT
Created attachment 33643 [details]
[PATCH 11/17] Fix compilation with Sun CC 5.9: moving elements in a vector requires source not to be const

I don't know why the compiler couldn't call src->~T() on a const T *src,
but fact is it couldn't.

In any case, since move is copying the source and deleting it, formally
the argument shouldn't be const anyway.
Comment 31 Thiago Macieira 2009-07-28 08:30:09 PDT
Created attachment 33644 [details]
[PATCH 12/17] Remove comma at end of enum. Some compilers are more picky than others.
Comment 32 Thiago Macieira 2009-07-28 08:33:01 PDT
Created attachment 33645 [details]
[PATCH 13/17] Fix compilation with Sun CC 5.9: types must match on ?:
Comment 33 Thiago Macieira 2009-07-28 08:34:00 PDT
Created attachment 33646 [details]
[PATCH 14/17] Fix compilation with Sun CC 5.9: ambiguity in ?:

Error: Ambiguous "?:" expression, second operand of type "JSC::JSObject*" and third operand of type "JSC::JSValuePtr" can be converted to one another.
Error: Ambiguous "?:" expression, second operand of type "JSC::RuntimeObjectImp*" and third operand of type "JSC::JSValuePtr" can be converted to one another.
Comment 34 Thiago Macieira 2009-07-28 08:34:39 PDT
Created attachment 33647 [details]
[PATCH 15/17] Fix compilation with Sun CC 5.9: ambiguity in ?:

Error: Ambiguous "?:" expression, second operand of type "WTF::PassRefPtr<WebCore::DocumentFragment>" and third operand of type "int" can be converted to one another.
Error: Ambiguous "?:" expression, second operand of type "WTF::PassRefPtr<JSC::Label>" and third operand of type "int" can be converted to one another.
[and others similar]
Comment 35 Thiago Macieira 2009-07-28 08:35:23 PDT
Created attachment 33648 [details]
[PATCH 16/17] Fix compilation with Sun CC 5.9: ambiguity with QString's operator+.

This is also a slight performance improvement since we're avoiding a temporary string.

Error: Overloading ambiguity between "WebCore::operator+(const WebCore::String&, const char*)" and "operator+(const QString&, const char*)".
[and others similar]
Comment 36 Thiago Macieira 2009-07-28 08:36:40 PDT
Created attachment 33649 [details]
[PATCH 17/17] Fix linking with Sun CC 5.9: function pointers for extern "C" are treated differently

The Sun CC compiler treats C functions and C++ functions differently,
as if they had a different calling sequence (they don't, but they
could). So if you declare a function in C++ having a function pointer
as a parameter, it's understood to be C++ even if it had previously
been declared as extern "C".

This could be a compiler error, though. In any case, the end result is
that WebKit fails to link because of an undefined reference to
NPN_PluginThreadAsyncCall.

"plugins/npapi.cpp", line 177: Warning (Anachronism): Formal argument 2 of type void(*)(void*) in call to WebCore::PluginMainThreadScheduler::scheduleCall(_NPP*, void(*)(void*), void*) is being passed extern "C" void(*)(void*).

There are more of these errors left in WebKit, but they are not
causing problems right now.
Comment 37 George Staikos 2009-07-28 08:39:59 PDT
Some of these look a bit ugly.  it might make sense to maintain them as an external patch set.
Comment 38 Thiago Macieira 2009-07-28 09:02:48 PDT
Patches 2, 3, 7, 10, and 16 are fixes to compiler limitations or bugs. Those are the only that I would agree to keeping in a separate branch. In special, patches 2 and 3 are required for some other compilers without alignof and aligned support (IBM xlC for example).

Patch 4 should be accepted because code using time_t should #include <sys/time.h>. It should not depend on that file coming via indirect include (like #include <memory>). I even wonder how WebKit compiles with gcc 4.4 and glibc 2.10 on Linux, which have done a great deal of work cleaning up indirect includes.

Patch 5 should be accepted because misaligned reads aren't allowed on this processor, and the change just makes sense.

Patch 6 should be accepted because it's a ticking timebomb for other compilers as well. The fact that the compiler did not inline in the current version doesn't mean future versions won't have the same problem. Even the same versions of the compiler could inline depending on how WebKit code changes in the future.

Patch 8 uses is a simple fix for where Solaris deviates from POSIX (http://www.opengroup.org/onlinepubs/9699919799/functions/mmap.html), but is innocuous since char* can be cast to void*.

Patch 9 seems to be just the compiler being stricter. It also makes sense because it appears to be what is intended anyway.

Patch 11 is formalism: the element being moved is deleted. It should not be passed as a const parameter.

Patch 12 is pedantism. The patch isn't strictly necessary, since WebKit compiles without, but other compilers are stricter than SunCC (for example, HP's aCC does not allow terminating commas). In fact, there are a couple of other places where I didn't apply the change (pcre and JSC's Opcode.h), but will need the fix if WebKit is to compile with aCC.

Patches 13, 14, and 15, as far as I know, are required per the standard: the second and third arguments of ?: must be of the same type.

Patch 17 is "anachronism": the fact that the calling sequence for C++ functions is the same as C is a coincidence, but is not required by the standard. It could be that this is just a bug that the compiler decided to decorate the function instead of exporting it as extern "C", but this should be technically used. In fact, you should never pass a static member function to a C callback.
Comment 39 George Staikos 2009-07-28 09:06:45 PDT
I think our views are relatively well-aligned.  I already r+ one of the patches anyway. :)
Comment 40 Eric Seidel (no email) 2009-07-31 21:28:11 PDT
Are these supposed to be up for review? 17 patches is a lot for one bug.
Comment 41 Eric Seidel (no email) 2009-07-31 21:28:56 PDT
Comment on attachment 33636 [details]
[PATCH 05/17] Fix crash misaligned reads on Sparc processors.

None of these have ChangeLogs, so they're not ready for review.  r-
Comment 42 Thiago Macieira 2009-08-03 02:37:46 PDT
Yes, they're all supposed to be reviewed.

I will add ChangeLogs and re-upload all patches again. I'll reorder them as well so that the simplest and most acceptable ones are first, whereas the more hackish are last. (see comment #38)

Also note I have some 10 more patches required for building with IBM xlC 7 on AIX. They depend on these patches, but they're not ready for review yet (the linker gets OOM-killed trying to build QtWebKit, so I don't know if it works or not). We'll need a machine with more than 1 GB of RAM to complete the process.
Comment 43 Pavel Heimlich (hajma) 2009-10-19 01:34:33 PDT
Hi,
I'm not sure if it's related to the patches or if it's a separate issue, however with the patches applied to qt 4.6.0-beta1 and building it with Sun Studio 12U1
I'm getting:
"../JavaScriptCore/wtf/Vector.h", line 83: Error: Taking address of the bound function WTF::AlignedBuffer<8, 0>::buffer().

line 83 contains this:
std::swap(a.buffer[i], b.buffer[i]);

(complete file after all patches is at ftp://hajma.cz/pub/kde-solaris/Vector.h)

I had to modify some of the patches (6 and 9 seem to be not applicable anymore. minor changes in 1,12,15,16) to make them working, the current modified set is at http://solaris.bionicmutton.org/hg/kde4-specs-440/file/0e7817c0d9ab/specs/patches/
Comment 44 Thiago Macieira 2009-10-19 01:53:18 PDT
Some of the patches were applied to the Qt source already and will be upstreamed (if they haven't been yet) due to the QtScript / JavaScriptCore merge.

Unfortunately, the WebKit tree has progressed and the patches have become stale. I don't have time to maintain this (probably will only have next Summer again). Hopefully the bulk of the changes will be taken care of by in JSC, leaving only smaller patches to be upstreamed.
Comment 45 Anders Carlsson 2014-12-03 16:02:45 PST
We're not going to do this.