bug in arm_push_arguments()

[Scott Bambrough <scottb at netwinder dot org>]

Hi guys,

I've been looking at this function as there are regressions in
gdb.base/callfuncs.  In particular I've been looking at this code.


     /* ANSI C code passes float arguments as integers, K&R code
         passes float arguments as doubles.  The .stabs record for 
         for ANSI prototype floating point arguments records the
         type as FP_INTEGER, while a K&R style (no prototype)
         .stabs records the type as FP_FLOAT.  In this latter case
         the compiler converts the float arguments to double before
         calling the function.  */
      if (TYPE_CODE_FLT == typecode && REGISTER_SIZE == len)
        {
          float f;
          double d;
          char * bufo = (char *) &d;
          char * bufd = (char *) &dbl_arg;

          len = sizeof (double);
          f = *(float *) val;
          SWAP_TARGET_AND_HOST (&f, sizeof (float));  /* adjust endianess */
          d = f;
          /* We must revert the longwords so they get loaded into the
             the right registers. */
          memcpy (bufd, bufo + len / 2, len / 2);
          SWAP_TARGET_AND_HOST (bufd, len / 2);  /* adjust endianess */
          memcpy (bufd + len / 2, bufo, len / 2);
          SWAP_TARGET_AND_HOST (bufd + len / 2, len / 2); /* adjust endianess */
          val = (char *) &dbl_arg;
        }

I added this particular piece of code to handle functions without prototypes
that pass floats as parameters, because the compiler treats them differently. 
Someone added code which is attempting the following (see my comments):

	  /* The value is in TARGET byte order.  Convert it to HOST byte 
	     order in preparation for conversion to a double.  */
          f = *(float *) val;
          SWAP_TARGET_AND_HOST (&f, sizeof (float));  /* adjust endianess */

	  /* Convert the float to a double in HOST byte order.  */
          d = f;

	  /* Convert the double to TARGET byte order and swap things to 
	     conform to the ARM floating point layout.  */
          memcpy (bufd, bufo + len / 2, len / 2);
          SWAP_TARGET_AND_HOST (bufd, len / 2);  /* adjust endianess */
          memcpy (bufd + len / 2, bufo, len / 2);
          SWAP_TARGET_AND_HOST (bufd + len / 2, len / 2); /* adjust endianess */
          val = (char *) &dbl_arg;

This is ok, but the code for the last step is unnecessarily complex.  I think
something similar to the following should suffice.

	  int tmp, *buf = &d;
	  SWAP_TARGET_AND_HOST (&d, sizeof (double));
	  tmp = buf[0];
	  buf[0] = buf[1];
	  buf[1] = tmp;

I think this will result in better code from the compiler.  There are a couple
of problems though with this code.  First what happens if sizeof (TARGET_DOUBLE)
!= sizeof (HOST_DOUBLE).  Second, this breaks in the native case, because the
double is already in the right format. This is why I have regressions I believe
(I have yet to test my theory).  Is there a clean way to solve these problems?

I also noticed this comment while investigating.  I disabled the code when I
rewrote the function.  I didn't know what it was for, and I wasn't sure it
should stay.  I left it for Stan and Jim to evaluate.  If it is necessary I
would document what it does and why it does it, and get rid of question.  In
particular why is setting the mode bit fine?  
#if 1
      /* I don't know why this code was disable. The only logical use
         for a function pointer is to call that function, so setting
         the mode bit is perfectly fine. FN */
      /* If the argument is a pointer to a function, and it is a Thumb
         function, set the low bit of the pointer.  */
      if (TYPE_CODE_PTR == typecode
          && NULL != target_type
          && TYPE_CODE_FUNC == TYPE_CODE (target_type))
        {
          CORE_ADDR regval = extract_address (val, len);
          if (arm_pc_is_thumb (regval))
            store_address (val, len, MAKE_THUMB_ADDR (regval));
        }
#endif

Scott
 
-- 
Scott Bambrough - Software Engineer
REBEL.COM    http://www.rebel.com
NetWinder    http://www.netwinder.org