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

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: glandium, Unassigned)

Tracking

Trunk
mozilla15
Other
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 575634 [details] [diff] [review]
Avoid invalid conversion from 'const size_t*' to 'const jsuword*' on s390

See comment 0
Attachment #575634 - Flags: review?(luke)

Comment 2

6 years ago
Yeah, I think we should use all size_t.

Updated

6 years ago
Attachment #575634 - Flags: review?(luke)
(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

5 years ago
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.
(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

5 years ago
(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.
Created attachment 625153 [details] [diff] [review]
Add an uintptr_t alternative to the jsval payload, and use it for MarkStackRangeConservatively
Attachment #625153 - Flags: review?(luke)
Attachment #575634 - Attachment is obsolete: true

Updated

5 years ago
Attachment #625153 - Flags: review?(luke) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1ae92b4b20
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/7a1ae92b4b20
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.