Opened 12 years ago

Closed 12 years ago

Last modified 2 years ago

#8985 closed patch (fixed)

Pointer casts on LLP64 platforms lose precision

Reported by: dhewg Owned by:
Priority: normal Component: Port: Win64
Keywords: Cc:


With mingw-w64 (gcc version 4.4.0 20090308 (experimental) (GCC)) a few of these errors occur:

backends/platform/sdl/graphics.cpp:1000: error: cast from ‘const byte*’ to ‘long int’ loses precision

64bit MS platforms use the LLP64 data model, so a "long" is only 32bit wide. There're a few instances where pointers get casted to long (to check for alignment), and that results in the above mentioned error.

While that can be worked around with -fpermissive, it's not really a nice solution, so I'll attach a patch that changes those casts from long to size_t. It compiled just fine afterwards (all engines enabled). Maybe it would be better idea to provide an common inline function to check for the alignment of a pointer.

Btw: It's working too ;) I don't have a win64 box, but Hkz just played BS/PSX on Vista64 with that binary.

Ticket imported from: #2685623. Ticket imported from: patches/1090.

Attachments (1)

no-fpermissive.diff (4.2 KB ) - added by dhewg 12 years ago.

Download all attachments as: .zip

Change History (8)

by dhewg, 12 years ago

Attachment: no-fpermissive.diff added

comment:1 by fingolfin, 12 years ago

We should fix this, but I am not quite happy with the current patch: * size_t may not be sufficient to store a full pointer on some systems either. In stdint.h, there are type intptr_t and uintptr_t for that purpose, but stdint.h is not available everywhere, now is inttypes.h. Maybe we should provide our own PtrAsInt type or so, in scummsys.h ?

* Many of the changes affect code which just checks pointer alignment. For that it really shouldn't matter if the pointer is cast to a byte or a long long, only the 1-2 lowest bits are of interest. But if this causes errors or warnings, maybe we should wrap this into a macro / inline func: IS_PTR_ALIGNED(ptr) or so (OK, maybe with a second param which tells how many of the lowest bits shall be zero: 1, 2, 3...

* The OFFS style macros compute offsets, for that there is off_t (but also not quite portable); again, since this is about offsets within structs, an uint16 should be sufficient in most cases... But I guess casting to size_t should be OK, too.

comment:2 by sev-, 12 years ago

Yes, rolling out our own PrtAsInt makes sense. Same is for IS_PTR_ALIGNED()

comment:3 by lordhoto, 12 years ago

I just patched all of the source places, I found, to use the newly introduced IS_ALIGNED and fixed some other bad casts and an offset calculation related cast in the sky engine, now mingw64 compiles ScummVM just fine (check out buildbot for that).

I do not know if we really need to keep this one open now, thus I just set it to pending for now. Feel free to revert that if you have any problems with it.

comment:4 by lordhoto, 12 years ago

Resolution: fixed
Status: newpending

comment:5 by SF/sf-robot, 12 years ago

This Tracker item was closed automatically by the system. It was previously set to a Pending status, and the original submitter did not respond within 14 days (the time period specified by the administrator of this Tracker).

comment:6 by SF/sf-robot, 12 years ago

Status: pendingclosed

comment:7 by digitall, 2 years ago

Component: Port: Win64
Note: See TracTickets for help on using tickets.