Issue676

classification
Title 64-bit lint, correct use of integer types
Type defect Module core code 21.5
Severity crash Platform x86_64, other cpu
Keywords Nosy List mike.kupfer
explanation
process
These controls should only be changed by committers and tracker administrators.
Status new   Reason
Superseder  
Priority normal   Assigned To

Created on 2010-02-11.23:37:21 by mike.kupfer, last changed 2011-05-01.14:07:46 by stephen.

Files
File name Uploaded Type Edit Remove
lint.out.txt mike.kupfer, 2010-02-11.23:48:46 text/plain
Messages
msg2286 [hidden] ([hidden]) Date: 2011-05-01.14:07:46
  Message-ID: <1304258866.31.0.167685305187.issue676@xemacs.org>
This is very bad for Sun, I'm afraid.  I suspect the reason for
*thousands* of complaints is mostly wrap_record(struct *), which boils
down to

typedef EMACS_INT Lisp_Object;
#define wrap_record(ptr,ty) wrap_pointer_1(ptr)
#define wrap_pointer_1(ptr) ((Lisp_object) ptr)

on a non-error-check, non-union-type build.  But I don't see how that
can happen because there is a basic assumption (see lisp.h:452) that
sizeof(void*)==sizeof(EMACS_INT); that's how the appropriate typedef for
EMACS_INT is chosen, in fact.

Could you try running lint on a --with-union-type=yes configuration?
msg2097 [hidden] ([hidden]) Date: 2010-02-11.23:37:20
I ran a recent version of 21.5 through the Sun Studio lint with
-errchk=longptr64,signext.  This generated approximately 5K complaints
related to 64-bit safety.  Basically, any place the code assigns a
long to an int, or a pointer to an int, is broken for 64-bit code.
While I haven't confirmed any crashes from these errors, it's
theoretically possible (e.g., due to pointer corruption).

I'll attach the lint output in a separate message.  There are enough
warnings that it's not feasible to go through them all by hand.  I
recommend picking a few issues that affect key common functions or
data structures, fixing those, and rerunning lint.

Unfortunately, it doesn't look like gcc has any options for flagging
this sort of error.  If you don't have access to lint, splint appears
to catch at least some long/int errors.

Also, while I'm thinking about 64-bit issues, I want to mention an
issue that I haven't seen in XEmacs, but which I have seen in two
other packages.  Functions that return pointers must be declared as
such wherever they are used.  You can't let the compiler implicitly
type the function as returning an int and then cast the return value.
On 64-bit machines, the upper 32 bits of the pointer get thrown away.

When I mentioned the 64-bit issues on xemacs-beta, Stephen T. proposed
the following policy for use of integral types:

  AFAIK in XEmacs we should not be using int, long, or unsigned versions
  thereof, except for things like loop variables.  Instead, anything
  that is global, and especially anything visible to Lisp, should be
  using EMACS_INT (or, rarely, EMACS_UINT).
  
  AFAIK EMACS_INT is always efficient in runtime, although on 64-bit
  systems it may cause an increase in space usage compared to int.
  
  This doesn't apply to external interfaces, but on modern systems those
  should be using typedefs like size_t, not int or long.  And those
  external values should be cast or converted to EMACS_INT at the point
  of use of the external interface, wherever the data might be
  accessible by other parts of XEmacs.

There was some discussion of whether EMACS_INT should be further
typedef'd to indicate what is being counted; I don't know if that got
resolved.  See
http://list-archive.xemacs.org/pipermail/xemacs-beta/2010-February/018848.html
and
http://list-archive.xemacs.org/pipermail/xemacs-beta/2010-February/018845.html
for details.

Here are two examples, with the output from lint and some commentary
from me.

1. "abbrev.c", line 285: warning: assignment of 64-bit integer to
   32-bit integer

Line 285 is

  int oldmodiff = BUF_MODIFF (buf);

BUF_MODIFF is defined in buffer.h as

  #define BUF_MODIFF(buf) ((buf)->text->modiff)

This presumably refers to 

  struct buffer_text
  {
    ...
    long modiff;		/* This counts buffer-modification events

2. "alloc.c", line 1569: warning: passing 64-bit integer arg,
   expecting 32-bit integer: internal_array_hash(arg 2)

The code here is

  return HASH2 (XVECTOR_LENGTH (obj),
		internal_array_hash (XVECTOR_DATA (obj),
				     XVECTOR_LENGTH (obj),
				     depth + 1));

We can confirm that internal_array_hash() does indeed take an int as
its second argument:

  internal_array_hash (Lisp_Object *arr, int size, int depth)

XVECTOR_LENGTH is defined as

  #define vector_length(v) ((v)->size)
  #define XVECTOR_LENGTH(s) vector_length (XVECTOR (s))

The size of a vector is a long, as in

  struct Lisp_Vector
  {
    struct LCRECORD_HEADER header;
    long size;
    Lisp_Object contents[1];
  };
History
Date User Action Args
2011-05-01 14:07:46stephensetmessages: + msg2286
2010-02-11 23:48:48mike.kupfersetfiles: + lint.out.txt
2010-02-11 23:48:09mike.kupfersetfiles: - lint.out.to-file
2010-02-11 23:47:37mike.kupfersetfiles: + lint.out.to-file
2010-02-11 23:37:21mike.kupfercreate