Date: Wed, 14 Dec 2016 20:48:47 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: John Baldwin <jhb@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r310045 - head/sys/ddb Message-ID: <20161214201041.C4333@besplex.bde.org> In-Reply-To: <20161214140645.W3397@besplex.bde.org> References: <201612140018.uBE0ICrE004686@repo.freebsd.org> <2285301.DAKmd1GIbI@ralph.baldwin.cx> <20161214140645.W3397@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 14 Dec 2016, Bruce Evans wrote:
> I fixed printing of negative offsets from the frame pointer on i386,
> but this is currently broken on i386 and never worked on other arches.
> DB_LARGE_VALUE_MIN has the old i386 value on all arches, but this is
> now wrong for i386 and never worked on other arches. This is related
> to the current problem: I think offsets like -8 are looked up as
> symbols, and found to be at the largest kernel symbol plus some offset.
> DB_LARGE_VALUE_MIN is supposed to give essentially the address of this
> symbol. This address was very high on i386 (given by static allocation
> of some page table). Now the highest symbol is not far above KERNBASE,
> so is unusable for this purpose. To fix this, all arches should have
> a dummy symbol near the top of the address space. It must be found
> when an offset like -8 is looked up, and should be just a few hundred
> K below the top since negative offsets larger than a few hundred K
> are likely to be garbage.
The fix for amd64 and i386 was very easy, except it is missing comments
and doesn't clean up old comments:
X Index: amd64/amd64/locore.S
X ===================================================================
X --- amd64/amd64/locore.S (revision 309660)
X +++ amd64/amd64/locore.S (working copy)
X @@ -46,6 +46,8 @@
X .set dmapbase,DMAP_MIN_ADDRESS
X .set dmapend,DMAP_MAX_ADDRESS
X
X + .set db_small_value_min,-0x400000
X +
X .text
X /**********************************************************************
X *
X Index: i386/i386/locore.s
X ===================================================================
X --- i386/i386/locore.s (revision 309660)
X +++ i386/i386/locore.s (working copy)
X @@ -80,6 +80,8 @@
X .globl kernload
X .set kernload,KERNLOAD
X
X + .set db_small_value_min,-0x400000
X +
X /*
X * Globals
X */
Please fix it for other arches.
The magic number should be DB_SMALL_VALUE_MIN+-1 (actually, ddb should
use this symbol value directly). This value doesn't need to be much
more negative than the negative of the kernel stack size. The historical
value is just a convenient more negative value that old i386 kernels had.
The symbol must be fitted into the kernel address space without too much
conflict with symbols already there. If there are higher ones, then they
already affect db_printsym().
I have the following old workarounds for the bug:
X diff -u2 ddb/db_sym.c~ ddb/db_sym.c
X --- ddb/db_sym.c~ Wed Feb 25 21:05:51 2004
X +++ ddb/db_sym.c Wed Feb 25 21:11:09 2004
X @@ -303,5 +303,9 @@
X if (name == 0)
X value = off;
The (name == 0) case is apparently not executed enough to help.
X +#ifdef magic_large_symbols_unbroken
X if (value >= DB_SMALL_VALUE_MIN && value <= DB_SMALL_VALUE_MAX) {
X +#else
X + if (off >= DB_SMALL_VALUE_MIN && off <= DB_SMALL_VALUE_MAX) {
X +#endif
This forces the check to use the same variable as in the (name == 0) case.
The difference between checking value and off is subtle. IIRC, value is
for the symbol and off is the original value (not an offset at all), while
the offset is in d (off = value + d). We want to check 'value' and consider
it as garbage if it is this symbol and use only 'off' in this case.
DB_SMALL_VALUE_MIN is the address of this symbol +-1, where +-1 is to
try to see this symbol, but I think this doesn't work.
X db_printf("%+#lr", (long)off);
X return;
X diff -u2 i386/include/db_machdep.h~ i386/include/db_machdep.h
X --- i386/include/db_machdep.h~ Mon Nov 10 08:01:53 2003
X +++ i386/include/db_machdep.h Mon Nov 10 02:54:26 2003
X @@ -87,7 +87,9 @@
X * _APTmap = 0xffc00000. Accepting this is OK (unless db_maxoff is
X * set to >= 0x400000 - (max stack offset)).
X + * XXX _APTD and _APTmap went away. Now there are SMP_prvspace = 0xffc00000
X + * and lapic = 0xfffff000.
X */
X #define DB_SMALL_VALUE_MAX 0x7fffffff
X -#define DB_SMALL_VALUE_MIN (-0x400001)
X +#define DB_SMALL_VALUE_MIN (-0x3fffff)
X
X #endif /* !_MACHINE_DB_MACHDEP_H_ */
The amendment to the comment is also 15-20 years out of date. The adjustment
to DB_SMALL_VALUE_MIN is supposed to see SMP_prvspace better, but I doubt
that it works. Using the dummy symbol avoids problems with seeing or not
seeing the highest real kernel symbol, so the +-1 adjustment doesn't
matter.
The next most obvious bug in amd64 disassembly is that ddb doesn't
understand PC-relative offsets, so the addresses of global variables are
displayed unreadably as something like 0x6e631b(%rip).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161214201041.C4333>