703833 - error: invalid conversion from 'const size_t* {aka const long unsigned int*}' to 'const jsuword* {aka const unsigned int*}' on s390

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mike Hommey [:glandium]]

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]]

Attachment: Avoid invalid conversion from 'const size_t*' to 'const jsuword*' on s390

See comment 0

Comment 2

[Luke Wagner [:luke]]

Yeah, I think we should use all size_t.

Comment 3

[Mike Hommey [:glandium]]

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

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]]

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

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

Attachment: Add an uintptr_t alternative to the jsval payload, and use it for MarkStackRangeConservatively

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1ae92b4b20

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a1ae92b4b20