WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
28391
Incorrect casts in JavaScriptCore with gcc-3.4
https://bugs.webkit.org/show_bug.cgi?id=28391
Summary
Incorrect casts in JavaScriptCore with gcc-3.4
Balazs Kelemen
Reported
2009-08-17 08:44:45 PDT
I tried to compiling ToT on the Maemo VM. "gcc --version" says: sbox-arm-linux-gcc (GCC) 3.4.4 (release) (CodeSourcery ARM 2005q3-2) There are reinterpret_cast-s in the JIT infrastructure what casts void*-s to function pointers and vica versa. My compiler does not like this (I heard that the problem is typical in gcc-3) so I get such errors: "ISO C++ forbids casting between pointer-to-function and pointer-to-object".
Attachments
proposed patch
(6.89 KB, patch)
2009-08-17 08:50 PDT
,
Balazs Kelemen
eric
: review-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Balazs Kelemen
Comment 1
2009-08-17 08:50:37 PDT
Created
attachment 34969
[details]
proposed patch Using bitwise casts when reinterpret_cast leads to compile error.
Oliver Hunt
Comment 2
2009-08-18 11:42:03 PDT
I am tempted to r- this patch, as webkit requires gcc-4.x or higher to build so i'm surprised you can get beyond jsc ...
Eric Seidel (no email)
Comment 3
2009-08-21 10:57:00 PDT
Comment on
attachment 34969
[details]
proposed patch No your fault, but I went to look up WTF::bitwise_cast, only to find that the comment next to bitwise_cast is basically useless: /* * C++'s idea of a reinterpret_cast lacks sufficient cojones. */ template<typename TO, typename FROM> TO bitwise_cast(FROM from) { COMPILE_ASSERT(sizeof(TO) == sizeof(FROM), WTF_bitwise_cast_sizeof_casted_types_is_equal); union { FROM from; TO to; } u; u.from = from; return u.to; } And I would expect that bitwise_cast violates the c++ standard, since I've been told that only the last union member you write to is valid to read from, any other member is undefined. (I've also been told the GCC does define this behavior and that our use of unions here will work for GCC. I'm anything bug a C++ standards expert though.) I agree with oliver, GCC 3.x is not supported for WebKit. I don't think we should take one-off patches for it, since it's undoubtably going to break again.
Eric Seidel (no email)
Comment 4
2009-08-21 10:59:23 PDT
Looks like Gavin and Geoff wrote that comment. :)
http://trac.webkit.org/changeset/39958
Joe Mason
Comment 5
2009-08-21 11:31:00 PDT
This post seems to quote the relevant sections of the standard:
http://learningcppisfun.blogspot.com/2008/07/c-unions-can-have-functions.html
It concludes that it's legal to do this cast as long as both members are POD types or structs containing only POD types with the same layout. So whether this cast is legal or not depends on what FROM and TO are.
Gavin Barraclough
Comment 6
2009-08-21 15:04:11 PDT
My bad on the comment, I should make this more descriptive. That said, I reassert my statement about reinterpret_cast lacking cojones. :-) The real intention of the cast was to cast between POD types, (int64_t and double), so it sounds like this is safe, but I'll look into this more at some point. If this is not safe, maybe we should more explicitly limit this (maybe we can do a kind of compile assert with a template specialization to guard against bitwise_cast being used with pointers, if that helps), or we could remove the generic cast from WTF and just make this an explicit cast within JSImmediate to prevent further use. Or we can always implement this in inline asm, if absolutely necessary. (Oh, c++, why are you so dumb).
Balazs Kelemen
Comment 7
2009-08-22 05:39:22 PDT
I did not think I would start such an interesting conversation :) Anyway, my original wish (to be able to compile webkit with gcc-3.x) was not a valid conception, so I would close the bug. Should I close it, or you guys want to move on with the casting debate here? :-)
Eric Seidel (no email)
Comment 8
2009-08-22 08:26:19 PDT
Nah, we can close. Thank you for the patch though! And thanks to Gavin and Joe for following up with the C++ lesson. :) I certainly learned something!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug