Skip to content

Commit

Permalink
arm: Fix floating point arguments in arm
Browse files Browse the repository at this point in the history
The designated initializer in C makes implicit memset internally in arm
architecture.  Due to the memset, VFP registers are clobbered
unexpectedly, so this patch fixes the problem by replacing the
designated initializer to explicit 'mcount_memset4'.

In addition, the offset of VFP registers were incorrect because the
following commit pushs d1 register on top of the original d0.

  ffb69ce arm: Handle struct return type by keeping more regs on stack

So the offset is adjusted from -2 to -4 to cope with the change.

Source:
  #include <stdio.h>

  float float_add(float a, float b)
  {
          fprintf(stderr, "a = %f, b = %f\n", a, b);
          return a + b;
  }

  int main(int argc, char *argv[])
  {
          double c;

          c = float_add(-0.1, 0.2);
          fprintf(stderr, "c = %f\n", c);
          return c > 0;
  }

Before:
  $ uftrace -a -F main a.out
  a = 0.000000, b = 0.000000
  c = 0.000000
  # DURATION     TID     FUNCTION
              [ 25362] | main(1, 0x7ea25344) {
              [ 25362] |   float_add(0.000000, 0.000000) {
   503.228 us [ 25362] |     fprintf(&_IO_2_1_stderr_, "a = %f, b = %f\n") = 27;
   511.197 us [ 25362] |   } = 0.000000; /* float_add */
     9.687 us [ 25362] |   fprintf(&_IO_2_1_stderr_, "c = %f\n") = 13;
   531.977 us [ 25362] | } = 0; /* main */

After:
  $ uftrace -a -F main a.out
  a = -0.100000, b = 0.200000
  c = 0.100000
  # DURATION     TID     FUNCTION
              [ 25146] | main(1, 0x7edbb344) {
              [ 25146] |   float_add(-0.100000, 0.200000) {
   501.769 us [ 25146] |     fprintf(&_IO_2_1_stderr_, "a = %f, b = %f\n") = 28;
   509.321 us [ 25146] |   } = 0.100000; /* float_add */
    12.500 us [ 25146] |   fprintf(&_IO_2_1_stderr_, "c = %f\n") = 13;
   533.539 us [ 25146] | } = 1; /* main */

Fixed: #1088

Signed-off-by: Honggyu Kim <[email protected]>
  • Loading branch information
honggyukim authored and namhyung committed Mar 16, 2020
1 parent f78e721 commit 19e6f0d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 18 deletions.
6 changes: 4 additions & 2 deletions arch/arm/mcount-support.c
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,10 @@ void mcount_arch_get_retval(struct mcount_arg_context *ctx,
/* type of return value cannot be FLOAT, so check format instead */
#ifdef HAVE_ARM_HARDFP
if (spec->fmt == ARG_FMT_FLOAT && use_hard_float) {
/* d0 register (64 bit) was saved below the r0 */
memcpy(ctx->val.v, ctx->retval - 2, spec->size);
/* d0, d1 registers (64 bit) were saved below the r0 */
long *float_retval = ctx->retval - 4;

mcount_memcpy4(ctx->val.v, float_retval, spec->size);
}
else
#endif /* HAVE_ARM_HARDFP */
Expand Down
6 changes: 3 additions & 3 deletions libmcount/plthook.c
Original file line number Diff line number Diff line change
Expand Up @@ -732,16 +732,16 @@ static unsigned long __plthook_entry(unsigned long *ret_addr,
struct sym *sym;
struct mcount_thread_data *mtdp = NULL;
struct mcount_ret_stack *rstack;
struct uftrace_trigger tr = {
.flags = 0,
};
bool skip = false;
bool recursion = true;
enum filter_result filtered;
struct plthook_data *pd;
struct plthook_special_func *func;
unsigned long special_flag = 0;
unsigned long real_addr = 0;
struct uftrace_trigger tr;

mcount_memset4(&tr, 0, sizeof(tr));

// if necessary, implement it by architecture.
child_idx = mcount_arch_child_idx(child_idx);
Expand Down
24 changes: 13 additions & 11 deletions libmcount/record.c
Original file line number Diff line number Diff line change
Expand Up @@ -503,12 +503,13 @@ void save_argument(struct mcount_thread_data *mtdp,
{
void *argbuf = get_argbuf(mtdp, rstack);
unsigned size;
struct mcount_arg_context ctx = {
.regs = regs,
.stack_base = rstack->parent_loc,
.regions = &mtdp->mem_regions,
.arch = &mtdp->arch,
};
struct mcount_arg_context ctx;

mcount_memset4(&ctx, 0, sizeof(ctx));
ctx.regs = regs;
ctx.stack_base = rstack->parent_loc;
ctx.regions = &mtdp->mem_regions;
ctx.arch = &mtdp->arch;

size = save_to_argbuf(argbuf, args_spec, &ctx);
if (size == -1U) {
Expand All @@ -526,11 +527,12 @@ void save_retval(struct mcount_thread_data *mtdp,
struct list_head *args_spec = rstack->pargs;
void *argbuf = get_argbuf(mtdp, rstack);
unsigned size;
struct mcount_arg_context ctx = {
.retval = retval,
.regions = &mtdp->mem_regions,
.arch = &mtdp->arch,
};
struct mcount_arg_context ctx;

mcount_memset4(&ctx, 0, sizeof(ctx));
ctx.retval = retval;
ctx.regions = &mtdp->mem_regions;
ctx.arch = &mtdp->arch;

size = save_to_argbuf(argbuf, args_spec, &ctx);
if (size == -1U) {
Expand Down
2 changes: 0 additions & 2 deletions tests/t083_arg_float.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,5 @@ def runcmd(self):
# argument count follows the size of type
argopt = argopt.replace('float_mul@fparg1/64,fparg2/32',
'float_mul@fparg1/64,fparg3/32')
argopt = argopt.replace('float_div@fparg1,fparg2',
'float_div@fparg1,fparg3')

return '%s %s %s' % (TestBase.uftrace_cmd, argopt, 't-' + self.name)

0 comments on commit 19e6f0d

Please sign in to comment.