From owner-svn-src-all@freebsd.org Thu Dec 15 06:07:44 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 B9C03C8032E; Thu, 15 Dec 2016 06:07:44 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 833601D9F; Thu, 15 Dec 2016 06:07:43 +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 mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 82CD9104910A; Thu, 15 Dec 2016 16:32:11 +1100 (AEDT) Date: Thu, 15 Dec 2016 16:32:10 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin cc: 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: <1943330.5ZCxhTZVMq@ralph.baldwin.cx> Message-ID: <20161215160138.C1437@besplex.bde.org> References: <201612140018.uBE0ICrE004686@repo.freebsd.org> <2285301.DAKmd1GIbI@ralph.baldwin.cx> <20161214140645.W3397@besplex.bde.org> <1943330.5ZCxhTZVMq@ralph.baldwin.cx> 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=K7JSJ2eI c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=avmG3HoKv_aMk1NEc9cA:9 a=yonnxYksHrBdyqm4:21 a=iummpsby-ga9SU4A: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: Thu, 15 Dec 2016 06:07:44 -0000 On Wed, 14 Dec 2016, John Baldwin wrote: > On Wednesday, December 14, 2016 07:24:46 PM Bruce Evans wrote: >> ... >> I don't see how initializing to 'val' can help. It still gets corrupted >> to unsigned, and I would expect it to make the problem much worse because >> the truncation gives an even smaller value. E.g., if val is any multiple >> of 2**32, truncating it gives 0. Without the truncation, 'val' is a good >> upper bound for the offset. > > I should have mentioned this change in the commit. Other symbol resolution > APIs in the kernel follow the convention that the raw address is returned > as the offset ('diff') if no matching symbol is found. This change made > db_search_symbol() consistent with that. > >> Perhaps the problem is more with the initialization of newdiff. It is >> initialized to the corrupted value in diff. I don't know the details of >> the API, but if X_db_search_symbol() uses this as an input parameter to >> limit the search, it might be happier with the smaller corrupted value >> than the larger one. X_db_search_symbol() seems to have working types >> and values, though still too much hard-coding of types. >> >> Your casts to uintmax_t have no effect. The types are already unsigned, >> and the corrupted values cannot be fixed by later casts. > > So I got myself into a bit of a mess and the hints are in my commit message > (but obscurely). I am working within a branch and one of the things I had > done in this branch was to fix numerous warnings compiling DDB with a MIPS > n32 kernel. n32 is quite special in that pointers are 32-bits while registers > are 64-bits, so db_addr_t is a uint32_t, and db_expr_t was a int64_t. I had > "fixed" newdiff and diff to both be db_expr_t to match the type that > X_db_search_symbol() returns and that is how I got into trouble as that > changed 'diff' to be signed instead of unsigned. I will revert the commit That is a good fix if you commit it and keep the casts to an unsigned type. > from HEAD (though perhaps re-commit the 'val' part of initializing diff if > making that API consistent is a good thing to do). I guess you fixed the intptr_t vs db_expr_t mismatches. Did you look at fixing the printf formats? I think ddb casts args to longs too perfectly to match buggy long formats, so no errors should have been detected at compile time. I don't like casting to intptr_t on all arches to support 1 strange one where this is needed, but as usual casting to the widest type is less ugly than using PRI*. sys/ddb has only 26 lines of casts to long, 9 to unsigned long and 2 to long long, so fixing them all won't be too painful. Bruce