Re: [commit] Fix OpenBSD/i386 and OpenBSD/amd64 kernel trapframe unwinders

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Thu, 22 Dec 2005 09:20:13 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Thu, Dec 22, 2005 at 03:09:21PM +0100, Mark Kettenis wrote:
> > +
> > +  find_pc_partial_function (func, &name, NULL, NULL);
> > +  if (name && strncmp(name, "Xintr", 5) == 0)
> > +    addr = sp + 8;		/* It's an interrupt frame.  */
> > +  else
> > +    addr = sp;
> > +
> 
> Is this series of patches really a good idea?
> 
> I realize there's no way to configure GDB at runtime to do this sort of
> thing, and I accept that that's a serious shortcoming that we
> eventually need to fix.  But you're adding bits to the OpenBSD target
> that only make sense in terms of the naming conventions of a single
> OpenBSD program (which happens to be the kernel); I've resisted doing
> this in the past both for The Server Then Known As XFree86 and for the
> Linux kernel.  Why should GDB know the names of functions in a single
> one of the infinitely many things it might be debugging?
>
> On a less metaphysical note, I'd worry about any other program with a
> function named Xintr.

I've struggled with this for a while.  Apart from adding DWARF2 CFI to
the kernel, there's no real alternative to matching function names.
And DWARF2 doesn't allow me to terminate the backtrace at the user to
kernel transition.  Simply matching the function names is of course
unacceptable, so I had to do something a bit more clever.  If you look
at the sniffer for the unwinder, you see the following check:

  cs = frame_unwind_register_unsigned (next_frame, AMD64_CS_REGNUM);
  if ((cs & I386_SEL_RPL) == I386_SEL_UPL)
    return 0;

This checks the Requested Protection Level, which is stored in the
lower two bits of %cs.  When executing in the kernel, the RPL is 0,
for userland its 3.  So we can reliably check whether we're in the
kernel or in userland.  And I only check the magic function names when
the current frame is a kernel frame.  I guess I should add a comment
in the code about that.

The nice thing about this approach is that you don't need any magic
knobs to turn things like this on.  I guess you can do something
similar for Linux; that's why I added the I386_SEL_XXX defines to
i386-tdep.h.

> On a nitpicking note, you missed a space before a parenthesis above :-)

Duh!  I'll fix that later today.

P.S. The new Xorg X11R6.9/X11R7.0 doesn't need any special patches
anymore since they now use dlopen(3) to load modules.