Last Comment Bug 703833 - error: invalid conversion from 'const size_t* {aka const long unsigned int*}' to 'const jsuword* {aka const unsigned int*}' on s390
: error: invalid conversion from 'const size_t* {aka const long unsigned int*}'...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: Other Linux
: -- normal (vote)
: mozilla15
Assigned To: general
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-18 22:51 PST by Mike Hommey [:glandium]
Modified: 2012-05-23 08:06 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Avoid invalid conversion from 'const size_t*' to 'const jsuword*' on s390 (1.21 KB, patch)
2011-11-19 00:45 PST, Mike Hommey [:glandium]
no flags Details | Diff | Review
Add an uintptr_t alternative to the jsval payload, and use it for MarkStackRangeConservatively (4.03 KB, patch)
2012-05-18 10:23 PDT, Mike Hommey [:glandium]
luke: review+
Details | Diff | Review

Description Mike Hommey [:glandium] 2011-11-18 22:51:47 PST
js/src/jsgc.cpp: In function 'void js::MarkStackRangeConservatively(JSTracer*, JS::Value*, JS::Value*)':
js/src/jsgc.cpp:891:48: error: invalid conversion from 'const size_t* {aka const long unsigned int*}' to 'const jsuword* {aka const unsigned int*}' [-fpermissive]
js/src/jsgc.cpp:892:44: error: invalid conversion from 'const size_t* {aka const long unsigned int*}' to 'const jsuword* {aka const unsigned int*}' [-fpermissive]
make[5]: *** [jsgc.o] Error 1

This happens because js::Value::payloadWord returns a size_t * which is then assigned to a jsuword *, which is an uintptr_t. On s390, uintptr_t and size_t are actually not the same type, most probably because s390 only has 31 bits for addressing.

A cast on these assignments should just work, but i wonder if we shouldn't use the same type everywhere instead.
Comment 1 Mike Hommey [:glandium] 2011-11-19 00:45:14 PST
Created attachment 575634 [details] [diff] [review]
Avoid invalid conversion from 'const size_t*' to 'const jsuword*' on s390

See comment 0
Comment 2 Luke Wagner [:luke] 2011-11-19 14:02:44 PST
Yeah, I think we should use all size_t.
Comment 3 Mike Hommey [:glandium] 2012-05-18 05:33:02 PDT
(In reply to Luke Wagner [:luke] from comment #2)
> Yeah, I think we should use all size_t.

Since we now switched to standard integer types, and jsuword was replaced with uintptr_t there, wouldn't it make sense to change payloadWord to return an uintptr_t (and change its name)?
Comment 4 Luke Wagner [:luke] 2012-05-18 08:48:04 PDT
SM uses size_t pretty much everywhere for a "word-sized unsigned int" and that is not in the process of changing, so I think we should stick with that unless there was a compelling reason.
Comment 5 Mike Hommey [:glandium] 2012-05-18 08:52:04 PDT
(In reply to Luke Wagner [:luke] from comment #4)
> SM uses size_t pretty much everywhere for a "word-sized unsigned int" and
> that is not in the process of changing, so I think we should stick with that
> unless there was a compelling reason.

There's plenty of uses of uintptr_t for GC-related things. And the error is happening in a GC-related thing. As I read it, it does make more sense to use an uintptr_t there, than other types. It also makes the change something like a three-liner, while changing to size_t opens a huge can of worms.
Comment 6 Luke Wagner [:luke] 2012-05-18 09:32:29 PDT
(In reply to Mike Hommey [:glandium] from comment #5)
> There's plenty of uses of uintptr_t for GC-related things.

Oh, so there are; I haven't looked at this code in a while.  uintptr_t makes sense then.
Comment 7 Mike Hommey [:glandium] 2012-05-18 10:23:58 PDT
Created attachment 625153 [details] [diff] [review]
Add an uintptr_t alternative to the jsval payload, and use it for MarkStackRangeConservatively
Comment 9 Ed Morley [:emorley] 2012-05-23 08:06:49 PDT
https://hg.mozilla.org/mozilla-central/rev/7a1ae92b4b20

Note You need to log in before you can comment on or make changes to this bug.