Summary

chicken.h wrongly assumes that __x86_64__ implies 64 bits

Metadata

Description

This bug was filed by a Debian maintainer against the Chicken 4.8.0 source:

 
 In chicken.h, the following line:
 
 # if defined (__alpha__) || defined(__ia64__) || defined(__x86_64__) || defined(__LP64__) || defined(__powerpc64__)
 
 contains both a zoology of systems, and the actual feature it is looking
 for.  The zoology is wrong: if the CPU is capable of 64 bit code, this does
 not mean that the architecture you're building for has 64 bit pointers.
 Because of compatibility with plenty of code that assumes sizeof(long) ==
 sizeof(void*), x32 has longs of only 32 bits.  What you want to check here,
 is whether longs are 64 bit.
 
 A fix:
 change that line to:
 
 # ifdef __LP64__
 
 This lets chicken build on both amd64 and x32.

Changes and comments

[2013-02-15 15:51:07 UTC] felix wrote:

Hm. This fix looks like its compiler-dependent: is __LP64__ defined everywhere on a 64-bit machine? And what platforms are meant with this well-meant suggestion? (I assume MING64 isn't which is the only one I'm aware of because only stupid OSs that are paranoid and pathologic about backwards-compatibility take that half-assed, dangerous and error-prone measure to maintain the illusion that you can simply recompile your 32-bit code on such a system and expect things to work).

In other words, what should we do?

[2013-02-17 06:50:33 UTC] iraikov wrote:

Since this was filed by a Debian developer, it may very well be that only Linux platforms are meant by this suggestion. And actually, I am able to compile Chicken 4.8.x on both 64-bit and 32-bit systems in Debian testing, so I need to ask for clarification on the specific platform and compiler version that is causing compilation errors.

Replying to felix: > Hm. This fix looks like its compiler-dependent: is __LP64__ defined everywhere on a 64-bit machine? And what platforms are meant with this well-meant suggestion? (I assume MING64 isn't which is the only one I'm aware of because only stupid OSs that are paranoid and pathologic about backwards-compatibility take that half-assed, dangerous and error-prone measure to maintain the illusion that you can simply recompile your 32-bit code on such a system and expect things to work). > > In other words, what should we do?

[2013-05-25 15:03:13 UTC] sjamaan wrote:

Ping Ivan. Any news on this?

[2013-06-15 23:50:14 UTC] felix changed status from new to closed

[2013-06-15 23:50:14 UTC] felix set resolution to wontfix

[2013-07-01 04:07:35 UTC] iraikov changed status from closed to reopened

[2013-07-01 04:07:35 UTC] iraikov removed resolution wontfix

[2013-07-01 04:07:35 UTC] iraikov wrote:

OK, we finally have an answer from the Debian developers:

Hi!

Somehow, I did not get the BTS mail, and looking in my mail server's logs, I see no matches. Hrm.

> Thank you for the suggestion, but for which platforms is this intended?

At least x32 with gcc and/or clang.

> Is __LP64__ always defined, or is it specific to compiler and platform?

Googling around, I see certain old proprietary compilers use _LP64 instead, if you care about them.

Windows doesn't have the __LP64__ define, but that's right as win64 doesn't use the LP64 model but LLP64: sizeof(long) == 4, while sizeof(void*) == 8. The current check in chicken is inconsistent: on mingw64 before this patch chicken.h will define SIXTY_FOUR but on MSVC it will not. I'm not sure whether you care about sizeof(long) or sizeof(void*) here, but it's obvious you want one or the other; this patch did not intend to make a change here but accidentally it makes mingw64 follow MSVC.

I think this covers all modern platforms then.

[2013-08-04 16:48:48 UTC] sjamaan wrote:

What, concretely, needs to happen next?

[2013-08-05 02:56:09 UTC] iraikov wrote:

Well, the Debian developer is suggesting to change this line in chicken.h:

  1. if defined (__alpha__) || defined(__ia64__) || defined(__x86_64__) || defined(__LP64__) || defined(__powerpc64__)

to

  1. ifdef __LP64__

If we agree that this is sensible, then we should make the change and test it on x32 with gcc, clang and the windows stuff.

Replying to sjamaan: > What, concretely, needs to happen next?

[2013-08-17 21:38:42 UTC] sjamaan wrote:

OK, thanks for clarifying. I've sent a patch to chicken-hackers, so hopefully we can close this soon.

[2013-08-24 00:41:36 UTC] sjamaan changed status from reopened to closed

[2013-08-24 00:41:36 UTC] sjamaan set resolution to fixed

[2013-08-24 00:41:36 UTC] sjamaan wrote:

This went in as 37cf50fe7f4dd2335fa330ab9538d245f1f58a06