Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 22 Dec 2013 10:47:21 +0000
From:      Andrew Turner <andrew@fubar.geek.nz>
To:        Howard Su <howard0su@gmail.com>
Cc:        freebsd-arm <freebsd-arm@freebsd.org>
Subject:   Re: [PATCH] Add stack unwind support for the functions in .ko
Message-ID:  <20131222104721.22b1bd45@bender.Home>
In-Reply-To: <CAAvnz_r1WsxC0Q%2BNOWhSs0Wp_Vs9mjpusZVrixsW1v%2BfwiXDpA@mail.gmail.com>
References:  <CAAvnz_pWZqw-HEB-GTJ_CkErjXrz3v7ozabMXE__vr=SfkBVSg@mail.gmail.com> <CAAvnz_p2vw9=HWbGPePKm_yDxM-bM1h6VaQBeJq9sYXPRb2-Lg@mail.gmail.com> <20131215173042.0dead636@bender.Home> <CAAvnz_qmn%2Ba%2B-aE4GkYw6TDOPzfST2j5WGvHqKX4WWVsVACSzQ@mail.gmail.com> <C6E93F79-BA1C-48E9-8689-775D88462921@bsdimp.com> <CAAvnz_r1WsxC0Q%2BNOWhSs0Wp_Vs9mjpusZVrixsW1v%2BfwiXDpA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 22 Dec 2013 17:50:31 +0800
Howard Su <howard0su@gmail.com> wrote:

> here is the new version which address the print registers out problem.

There are a number of style bugs in this patch: make sure you limit
lines to 80 characters; indentation should use tabs, followed by 4
spaces if you need to split a long line; braces should be at the end of
a line when used with an if, for or while statement.

You've added the A0 register. I don't see where it is defined by ARM as
a register name or synonym. It is also used to adjust which bit from a
bitmask we check. This would mean we read the wrong bit and may get a
register that was never updated. Because of this we will never read r0.

Andrew

> 
> On Monday, December 16, 2013, Warner Losh wrote:
> 
> >
> > On Dec 15, 2013, at 4:56 PM, Howard Su wrote:
> >
> > > On Monday, December 16, 2013, Andrew Turner wrote:
> > >
> > >> On Mon, 9 Dec 2013 22:44:14 +0800
> > >> Howard Su <howard0su@gmail.com <javascript:;> <javascript:;>>
> > >> wrote:
> > >>
> > >>> Here is a new version which solve the unreadable $a problem.
> > >>> (the fix is in ddb/db_main.c in the end of the patch.)
> > >>>
> > >>> I attached the diff for review.
> > >>
> > >> I can't comment on the MD parts of the code, but the ARM change
> > >> looks good. My only request is to add a kernel option to turn on
> > >> printing the registers in the stack trace as it has been useful
> > >> for tracking down bugs.
> > >>
> > > I read the MD code of MIPS. seems it faces the same situation
> > > like arm (register pass parameter). I will follow its pattern to
> > > print out
> > register.
> > > better than an option?
> >
> > I thought MIPS printed the args from those registers as args and
> > ARM was the odd man out...
> >
> > Warner
> >
> >
> 




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131222104721.22b1bd45>