From owner-svn-src-all@freebsd.org Wed Dec 14 10:11:41 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F2B2EC77193; Wed, 14 Dec 2016 10:11:40 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 890DC1592; Wed, 14 Dec 2016 10:11:39 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-106-153-191.carlnfd1.nsw.optusnet.com.au [122.106.153.191]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 7969C429B4C; Wed, 14 Dec 2016 20:48:48 +1100 (AEDT) Date: Wed, 14 Dec 2016 20:48:47 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bruce Evans cc: John Baldwin , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r310045 - head/sys/ddb In-Reply-To: <20161214140645.W3397@besplex.bde.org> Message-ID: <20161214201041.C4333@besplex.bde.org> References: <201612140018.uBE0ICrE004686@repo.freebsd.org> <2285301.DAKmd1GIbI@ralph.baldwin.cx> <20161214140645.W3397@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=cZeiljLM c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=kXIzg0_sEkvfXePip-IA:9 a=LgnZfvd9FYkpsdvo:21 a=suFtSTZC6S9h3vpZ:21 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Dec 2016 10:11:41 -0000 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