From owner-svn-src-head@freebsd.org Sat Apr 28 00:35:19 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 92725FB5CB7; Sat, 28 Apr 2018 00:35:19 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id D720885D9E; Sat, 28 Apr 2018 00:35:18 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id CD27B1A2E6A; Sat, 28 Apr 2018 10:18:14 +1000 (AEST) Date: Sat, 28 Apr 2018 10:18:13 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin cc: Ian Lepore , Li-Wen Hsu , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r333010 - head/sys/mips/mips In-Reply-To: <25448250.A6B4XqlUaS@ralph.baldwin.cx> Message-ID: <20180428082729.F3982@besplex.bde.org> References: <201804251946.w3PJkdkH040243@repo.freebsd.org> <1524756570.57768.115.camel@freebsd.org> <25448250.A6B4XqlUaS@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=FNpr/6gs c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=pNZfrhq-Dav1uGoF_aIA:9 a=gEDmztwniwxxe4nX:21 a=CtCnc-sCAy8039-h:21 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Apr 2018 00:35:19 -0000 On Fri, 27 Apr 2018, John Baldwin wrote: > On Thursday, April 26, 2018 09:29:30 AM Ian Lepore wrote: >> On Thu, 2018-04-26 at 19:01 +0800, Li-Wen Hsu wrote: >>> On Thu, Apr 26, 2018 at 7:59 AM, Ian Lepore wrote: >>>> >>>> On Wed, 2018-04-25 at 19:46 +0000, Li-Wen Hsu wrote: >>>>> >>>>> Author: lwhsu (ports committer) >>>>> Date: Wed Apr 25 19:46:39 2018 >>>>> New Revision: 333010 >>>>> URL: https://svnweb.freebsd.org/changeset/base/333010 >>>>> >>>>> Log: >>>>> Fix mips32 build after r332951. >>>>> >>>>> Approved by: jhb >>>>> >>>>> Modified: >>>>> head/sys/mips/mips/pm_machdep.c >>>>> >>>>> Modified: head/sys/mips/mips/pm_machdep.c >>>>> ============================================================================== >>>>> --- head/sys/mips/mips/pm_machdep.c Wed Apr 25 18:59:29 2018 (r333009) >>>>> +++ head/sys/mips/mips/pm_machdep.c Wed Apr 25 19:46:39 2018 (r333010) >>>>> @@ -264,7 +264,7 @@ ptrace_single_step(struct thread *td) >>>>> va = locr0->pc + 4; >>>>> } >>>>> if (td->td_md.md_ss_addr) { >>>>> - printf("SS %s (%d): breakpoint already set at %lx (va %lx)\n", >>>>> + printf("SS %s (%d): breakpoint already set at %zx (va %zx)\n", >>>>> p->p_comm, p->p_pid, td->td_md.md_ss_addr, va); /* XXX */ >>>>> error = EFAULT; >>>>> goto out; >>>>> @@ -500,7 +500,7 @@ ptrace_clear_single_step(struct thread *td) >>>>> >>>>> if (error != 0) { >>>>> log(LOG_ERR, >>>>> - "SS %s %d: can't restore instruction at %lx: %x\n", >>>>> + "SS %s %d: can't restore instruction at %zx: %x\n", >>>>> p->p_comm, p->p_pid, td->td_md.md_ss_addr, >>>>> td->td_md.md_ss_instr); >>>>> } >>>>> >>>> This isn't right either. %z is for size_t values, both md_ss_addr and >>>> va are integers and a plain %x should be the right format. >>> But it will break mips64: >>> >>> cc1: warnings being treated as errors >>> /home/lwhsu/src/sys/mips/mips/pm_machdep.c: In function 'ptrace_single_step': >>> /home/lwhsu/src/sys/mips/mips/pm_machdep.c:268: warning: format '%x' >>> expects type 'unsigned int', but argument 4 has type 'uintptr_t' >>> [-Wformat] >>> /home/lwhsu/src/sys/mips/mips/pm_machdep.c:268: warning: format '%x' >>> expects type 'unsigned int', but argument 5 has type 'uintptr_t' >>> [-Wformat] >>> /home/lwhsu/src/sys/mips/mips/pm_machdep.c: In function >>> 'ptrace_clear_single_step': >>> /home/lwhsu/src/sys/mips/mips/pm_machdep.c:505: warning: format '%x' >>> expects type 'unsigned int', but argument 5 has type 'uintptr_t' >>> [-Wformat] >>> *** [pm_machdep.o] Error code 1 >>> >>> Another way is cast arguments to uintmax_t and use %jx. Will that be better? >> Oh, my bad, my source was out of date. Now I see that the types are not >> plain integers anymore. In that case, I think the only options are to >> cast to uintmax_t and use %jx, or cast to void* and use %p. > > The %z does happen to work in this case since size_t == uintptr_t on MIPS, > but I think %p is the best route since these really are pointers. I've > build-tested a patch to use %p and will commit it in a bit. Ugh, these really aren't (kernel data) pointers. They are user(?) instruction pointers. The correct integer type for representing (both kernel and user) instruction pointers is uintfptr_t. vm_offset_t is next best. vm_offset_t must be large enough to represent any virtual address. Both it and uintfptr_t were recently broken for at least kernel profiling purposes on i386. i386 still has a flat address space, so a bit is not needed to distinguish function addresses from data addresses, but kernel instruction addresses are now indistinguishable from user instruction addresses. ddb backtracing and pmc have problems with this too, but are used more and have some fixes. %p is only good for simple printing of pointers even for data pointers, since it doesn't allow controlling the format. E.g., it forces an 0x prefix and doesn't allow controlling the field width or padding (except possibly in the kernel where it has undocumented extensions). This gives bugs like wasting 2 columns for printing '0x' for wchan in db_ps (wchan is about the least useful kernel pointer to print by default, but that is another bug). ps(1) has better printing for kernel pointers, but still has lots of bogus like casting ki_wchan to long instead of to uintptr_t for printing it, but before that it is an ABI design bug for ki_wchan to be a kernel pointer instead of a good representation of a kernel pointer as an integer. ki_wchan ends up as being raw bits via a type pun. Bruce