Fedora utrace changes break PPC GDB

Fedora utrace changes break PPC GDB

[Ulrich Weigand]

Hello,

I've been testing GDB on a PowerPC system running a Fedora 7 kernel,
and there are some regressions that appear to have been introduced
by the ptrace-on-utrace emulation layer that is now included in
Fedora.

Specifically, there are the following two problems (which also seem
to still be present in a current Fedora development kernel):

- PTRACE_SET_DEBUGREG ABI change

  In mainline kernels, PTRACE_SET_DEBUGREG takes an immediate 
  argument (like e.g. PTRACE_POKEUSR).  However, the Fedora code has:

        case PTRACE_GET_DEBUGREG:
        case PTRACE_SET_DEBUGREG:
                return ptrace_onereg_access(child, engine,
                                            utrace_native_view(current), 3,
                                            addr, (unsigned long __user *)data,
                                            *request == PTRACE_SET_DEBUGREG);

  which assumes the "data" argument is a *pointer* to the data,
  instead of the value itself.

  This breaks hardware watchpoint support completely.

- vrsave register access 

  The mainline kernel uses the following function to implement
  the PTRACE_GETVRREGS routine:

        /* copy AltiVec registers VR[0] .. VR[31] */
        regsize = 32 * sizeof(vector128);
        if (copy_to_user(data, task->thread.vr, regsize))
                return -EFAULT;
        data += (regsize / sizeof(unsigned long));

        /* copy VSCR */
        regsize = 1 * sizeof(vector128);
        if (copy_to_user(data, &task->thread.vscr, regsize))
                return -EFAULT;
        data += (regsize / sizeof(unsigned long));

        /* copy VRSAVE */
        if (put_user(task->thread.vrsave, (u32 __user *)data))
                return -EFAULT;

  Note that vscr and vrsave are provided in distinct fields of
  the thread struct:

        /* Complete AltiVec register set */
        vector128       vr[32] __attribute((aligned(16)));
        /* AltiVec status */
        vector128       vscr __attribute((aligned(16)));
        unsigned long   vrsave;
        int             used_vr;        /* set if process has used altivec */


  However, the utrace emulation layer in Fedora does

        case PTRACE_GETVRREGS:
                return ptrace_whole_regset(child, engine, data, 2, 0);
        case PTRACE_SETVRREGS:
                return ptrace_whole_regset(child, engine, data, 2, 1);

  using the following "regset" and access routines

#ifdef CONFIG_ALTIVEC
        {
                .n = 33*4+1, .size = sizeof(u32), .align = sizeof(u32),
                .active = vrregs_active, .get = vrregs_get, .set = vrregs_set
        },
#endif

static int
vrregs_get(struct task_struct *target,
           const struct utrace_regset *regset,
           unsigned int pos, unsigned int count,
           void *kbuf, void __user *ubuf)
{
        BUILD_BUG_ON(offsetof(struct thread_struct, vscr)
                     != offsetof(struct thread_struct, vr[32]));
        BUILD_BUG_ON(offsetof(struct thread_struct, vscr) + sizeof(vector128)
                     != offsetof(struct thread_struct, vrsave));

        flush_altivec_to_thread(target);

        return utrace_regset_copyout(&pos, &count, &kbuf, &ubuf,
                                     &target->thread.vr, 0, -1);
}

   This has the effect of simply accessing 33*4+1 "u32" values starting
   at &target->thread.vr

   Now, given the declaration above, the vr field is only 32 * 16 bytes
   long, so this is not really legal C in any case.  Even so, access to
   the vscr field happens to actually work correctly, as it immediately
   following vr (and there happens to be no padding).

   However, vrsave is accessed incorrectly on a 64-bit kernel: its value
   resides in the low 32 bits of the "unsigned long", and the copyout
   accesses the high 32 bits ...   (The BUILD_BUG_ON doesn't help.)

   This has the effect of GDB always reading vrsave as 0, even after
   attempting to set it to some other value.  (Every restart of the
   target process will apparently cause the upper 32 bit to be reset.)


Any suggestions how to get this fixed?

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Fedora utrace changes break PPC GDB

[Roland McGrath]

Hi, Ulrich.  Thanks for bringing these problems to my attention.
I have fixed them in "upstream" utrace (http://redhat.com/~roland/utrace/)
and those changes will appear in Fedora update kernels before too long.
I've also added regression tests for them to the ptrace-tests suite
(http://sourceware.org/systemtap/wiki/utrace/tests).

These are not gdb issues at all, just Fedora kernel bugs.  (Any
regression in ptrace ABI behavior of this sort from vanilla Linux kernels
is an unintended bug.)  The normal method is to file Fedora kernel bugs
via https://bugzilla.redhat.com/.  Since you know it's an issue with the
utrace code, the feedback is always appreciated if you want post to the
utrace-devel@redhat.com mailing list about any such issue.


Thanks,
Roland

Re: Fedora utrace changes break PPC GDB

[Ulrich Weigand]

Hi Roland,

> Hi, Ulrich.  Thanks for bringing these problems to my attention.
> I have fixed them in "upstream" utrace (http://redhat.com/~roland/utrace/)
> and those changes will appear in Fedora update kernels before too long.
> I've also added regression tests for them to the ptrace-tests suite
> (http://sourceware.org/systemtap/wiki/utrace/tests).

Thanks for the quick fix!  I'll verify that it works for me as soon as
the patches are included in Fedora.  (I'm assuming it is too late for
them to get into Fedora 8 at this point, right?)

> These are not gdb issues at all, just Fedora kernel bugs.  (Any
> regression in ptrace ABI behavior of this sort from vanilla Linux kernels
> is an unintended bug.)  The normal method is to file Fedora kernel bugs
> via https://bugzilla.redhat.com/.  Since you know it's an issue with the
> utrace code, the feedback is always appreciated if you want post to the
> utrace-devel@redhat.com mailing list about any such issue.

Thanks, I'll do that if I find further utrace-related problems.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Fedora utrace changes break PPC GDB

[Roland McGrath]

> Thanks for the quick fix!  I'll verify that it works for me as soon as
> the patches are included in Fedora.  (I'm assuming it is too late for
> them to get into Fedora 8 at this point, right?)

Yeah, that ship sailed a few days ago.  But there is kernel built already
(koji.fedoraproject.org) that will be an f8-updates kernel soon, I'd guess
within a couple of days.

> > These are not gdb issues at all, just Fedora kernel bugs.  (Any
> > regression in ptrace ABI behavior of this sort from vanilla Linux kernels
> > is an unintended bug.)  The normal method is to file Fedora kernel bugs
> > via https://bugzilla.redhat.com/.  Since you know it's an issue with the
> > utrace code, the feedback is always appreciated if you want post to the
> > utrace-devel@redhat.com mailing list about any such issue.
> 
> Thanks, I'll do that if I find further utrace-related problems.

Much appreciated.


Thanks,
Roland

Re: Fedora utrace changes break PPC GDB

[Ulrich Weigand]

Roland McGrath wrote:
> > Thanks for the quick fix!  I'll verify that it works for me as soon as
> > the patches are included in Fedora.  (I'm assuming it is too late for
> > them to get into Fedora 8 at this point, right?)
> 
> Yeah, that ship sailed a few days ago.  But there is kernel built already
> (koji.fedoraproject.org) that will be an f8-updates kernel soon, I'd guess
> within a couple of days.

Thanks again!  I've now verified that the ptrace interface works as expected
again with kernel-2.6.23.1-48.fc8.  Unfortunately I ran in a number of separate
issues that caused watchpoints to remain broken on my test system:

- A GDB PPC regression that came in recently.
  This is fixed by http://sourceware.org/ml/gdb-patches/2007-11/msg00202.html

- A Cell/B.E. specific kernel bug (the DABRX register is not enabled).
  I've got a hack to work around this, and am currently working with the
  Cell kernel team to get a proper fix upstream.

With those two problems fixed in addition to the utrace fix, the watchpoint
related test cases in the GDB test suite pass again.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com