Subject: Re: some code assumes sizeof(char *) == sizeof(int)
To: None <port-sparc64@netbsd.org, taya@sm.sony.co.jp, tech-pkg@netbsd.org>
From: None <eeh@netbsd.org>
List: tech-pkg
Date: 02/21/2001 17:17:57
	unwind_prot.[ch] of bash-2.04 assumes sizeof(char *) == sizeof(int).
	This cause problem on sparc64(LP64, big endian arch).

That's not nice....

	unwind_prot.h:
	/* Try to force correct alignment on machines where pointers and ints
	   differ in size. */
	typedef union {
	  char *s;
	  int i;
	} UWP;

	/* How to protect an integer. */
	#define unwind_protect_int(X) \
		do \
		  { \
		    UWP u; \
		    u.i = (X); \
		    unwind_protect_var (&(X), u.s, sizeof (int)); \
		  } \
		while (0)


	unwind_prot.c:
	void
	unwind_protect_var (var, value, size)
	     int *var;
	     char *value;
	     int size;
	{
	  SAVED_VAR *s = (SAVED_VAR *)xmalloc (sizeof (SAVED_VAR));

What's a `SAVED_VAR'?

	  s->variable = var;
	  if (size != sizeof (int))
	    {
		.
		.
		.
	  else
	    s->desired_setting = value;
		.
		.
		.


	static void
	restore_variable (sv)
	     SAVED_VAR *sv;
	{
	  if (sv->size != sizeof (int))
	    {
	      FASTCOPY ((char *)sv->desired_setting, (char *)sv->variable, sv->size);
	      free (sv->desired_setting);
	    }
	  else
	    *(sv->variable) = (int)sv->desired_setting;

What's the type of `sv->variable'?

	  free (sv);
	}

	---
	This means..
	1) if i call unwind_protect_int() as follows,

	   int   a = 0x12345678;
	   unwind_protect_int(a);

	acutual 'value' passed to unwind_protect_var (var, value, size) would
	be "0x12345678XXXXXXXX"(Xs are garbage) and this value is saved to
	sv->desired-settings.

	2) if i call restore_variable (sv), lower 32bit(0xXXXXXXXX) would be
	restored to the variable.
	This is a problem.

	--
	I think there are two solutions for this problem.
	1) change sizeof(int) -> sizeof(size_t)

	e.g.
	  if (size != sizeof (int))
	    {
		.
		.
		.
		|
		V

	  if (size != sizeof (size_t))
	    {

	and related stuffs(including unwind_protect_int() macro).

	This cause always alloc memory for each unwind_protect_int().
	This would be the performace problem.

	2)change macro for unwind_prot_int()

	#define unwind_protect_int(X) \
		unwind_protect_var(&(X), (char *)(X), sizeof(int))

	This is dirty hack. Many warning would be issued. But work faster and
	easier to fix.

	Which fix is better?

The correct fix is probably to change the macro to:

#define unwind_protect_int(X) \
        unwind_protect_var(&(X), (char *)(unsigned long)(X), sizeof(X))

(The `unsigned' is optional, but correctly converts 32-bit pointers
to 64-bit pointers.  But then we're probably not dealing with pointers
anyway.)

You want to use `sizeof(X)' rather than `sizeof(int)' so you can handle
both `int' and `long', since the borken code means that they probably 
did not distinguish the two.  

Also: `*(sv->variable) = (int)sv->desired_setting;' is dangerous in
this setting since it's not clear what the type of `sv->variable' is.
You may want to add code to handle the standard scalar, and possibly
floating point, types individually.  That way you won't need to 
malloc() something for variables smaller than 8 (or 16) bytes.

Eduardo