Type-punned pointers as lvalues

[Jerry James]

Fedora is in the midst of a mass rebuild of packages.  This is partly
to get all C/C++/... code recompiled with what will soon be called GCC
4.6.  The XEmacs package failed the build, due to a segfault in
temacs.  The segfault occurred in split_string_by_ichar_1, which was
passed a NULL pointer from split_external_path.  I noticed this in the
build messages:

fns.c: In function 'split_external_path':
fns.c:1090:3: warning: 'newpath' may be used uninitialized in this
function [-Wuninitialized]

and wondered what GCC was talking about, since newpath is initialized
by TO_INTERNAL_FORMAT ... isn't it?  Here's the code, for ease of
reference:

Lisp_Object
split_external_path (const Extbyte *path)
{
  Bytecount newlen;
  Ibyte *newpath;
  if (!path)
    return Qnil;

  TO_INTERNAL_FORMAT (C_STRING, path, ALLOCA, (newpath, newlen), Qfile_name);

  /* #### Does this make sense?  It certainly does for
     split_env_path(), but it looks dubious here.  Does any code
     depend on split_external_path("") returning nil instead of an empty
     string?  */
  if (!newlen)
    return Qnil;

  return split_string_by_ichar_1 (newpath, newlen, SEPCHAR, 0, 0);
}

After running that through the preprocessor, discarding extra
parentheses, and unfolding do ... while(0) constructs, that amounts to
this:

typedef union { char c; void * p; } *ANSI_ALIASING_voidp;

Lisp_Object
split_external_path (const Extbyte *path)
{
  Bytecount newlen;
  Ibyte *newpath;
  dfc_conversion_type dfc_simplified_source_type;
  dfc_conversion_type dfc_simplified_sink_type;
  dfc_conversion_data dfc_source;
  dfc_conversion_data dfc_sink;
  Lisp_Object dfc_codesys;
  void * dfc_sink_ret;

  if (!path)
    return Qnil;

  dfc_codesys = Qfile_name;
  dfc_source.data.ptr = path;
  dfc_source.data.len = dfc_external_data_len (path, dfc_codesys);
  dfc_simplified_source_type = DFC_TYPE_DATA;
  dfc_simplified_sink_type = DFC_TYPE_DATA;
  dfc_convert_to_internal_format (dfc_simplified_source_type, &dfc_source,
				  dfc_codesys, dfc_simplified_sink_type,
				  &dfc_sink);
  __temp_alloca_size__ = dfc_sink.data.len + 2;
  dfc_sink_ret = __temp_alloca_size__ > 262144
    ? xemacs_c_alloca (__temp_alloca_size__)
    : (need_to_check_c_alloca ? xemacs_c_alloca (0) : 0,
       __builtin_alloca (__temp_alloca_size__)));
  memcpy (dfc_sink_ret, dfc_sink.data.ptr, dfc_sink.data.len + 2);
  ((ANSI_ALIASING_voidp) &newpath)->p = dfc_sink_ret;
  newlen = dfc_sink.data.len;

  if (!newlen)
    return Qnil;

  return split_string_by_ichar_1 (newpath, newlen, ':');
}

The compiler warning above tells the story; GCC doesn't see an
assignment to ((ANSI_ALIASING_voidp) &newpath)->p as the same as
assigning to newpath, even with -fno-strict-aliasing.  In the GCC 4.6
manual, it says that assignments of the form:

*(fooptr *)&bar = foo_value;

have undefined semantics.  So it appears that we've been getting lucky
all these years, and our luck has now run out.  I think I remember
discussing this with Ben years ago when he first came up with the
ANSI_ALIASING macros.  Anyhow, weird casts like this as lvalues are
going to have to go if we want GCC 4.6 to be able to build XEmacs.
I'll try to start looking at what violence we need to do to our code
base to fix this issue.  Whee!
-- 
Jerry James
http://www.jamezone.org/