From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44241 invoked by alias); 20 Jul 2016 18:37:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 44229 invoked by uid 89); 20 Jul 2016 18:37:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=proved, reminder, reservation, decoded X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Jul 2016 18:36:55 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id E25DBBB4F4A33; Wed, 20 Jul 2016 19:36:37 +0100 (IST) Received: from [10.20.78.96] (10.20.78.96) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Wed, 20 Jul 2016 19:36:41 +0100 Date: Wed, 20 Jul 2016 18:37:00 -0000 From: "Maciej W. Rozycki" To: Tom Tromey CC: Yao Qi , "gdb-patches@sourceware.org" Subject: Re: [RFA 5/6] Remove unused variables In-Reply-To: <87furdnubd.fsf@tromey.com> Message-ID: References: <1465248812-23902-1-git-send-email-tom@tromey.com> <1465248812-23902-6-git-send-email-tom@tromey.com> <87wpl9t4l2.fsf@tromey.com> <87furdnubd.fsf@tromey.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2016-07/txt/msg00228.txt.bz2 On Wed, 13 Jul 2016, Tom Tromey wrote: > commit c097ec493b76209e6cf830ffca9b36fe2c2643fc > Author: Tom Tromey > Date: Mon Jun 6 14:18:30 2016 -0600 > > Remove unused variables > > This patch removes set-but-unused variables. This holds all the > removals I consider to be simple and relatively uncontroversial. I see you got to committing it actually now, before I got to the MIPS part (sorry about it), already proposed by Trevor Saunders previously BTW. > 2016-07-13 Tom Tromey > > * mips-tdep.c (micromips_scan_prologue): Remove "frame_addr". > (mips_o32_push_dummy_call): Remove "stack_used_p". TBH I disagree the changes to `micromips_scan_prologue' are uncontroversial -- it looks to me like the presence of `frame_addr', clearly copied over from `mips32_scan_prologue', is a sign of broken virtual frame pointer tracking code, which has not been completed for frames produced by microMIPS code. Obviously they need to follow the same rules WRT `alloca' calls as frames made with regular MIPS code, as there's nothing special defined in the ABI for microMIPS code. Of course one could argue that keeping broken code (though in a manner harmless to irrelevant cases) has little value, but at least it serves as a reminder to do something about it sometime. I'll see if I can add the missing piece regardless sometime, though regrettably I can't give it a high priority. Overall with recent and less so improvements to GCC's and other compilers' optimizers I think these heuristic unwinders have hardly any value nowadays, they seem unable to get through function prologues containing arbitrary instructions thrown there by the scheduler. This is very annoying in a common case where you interrupt a debuggee in the middle of a sleeping syscall, with no way to backtrace through stripped system shared libraries. What I think could work is combining PDR records with reduced heuristic unwinders, which in that case do not actually need to search for frame offsets as they're already provided in PDR records. So for non-leaf frames there's really nothing to do beyond interpreting these records, because by the time a nested function call has been made, the caller's frame has already been fully populated. For leaf frames it's a tad more complicated, because the records do not carry information as to where in the prologue individual slots are initialised (i.e. registers stored). But for that you do not have to teach the unwinder of the whole instruction set, it just needs to know these actually used for register saves, which there are a small number only. And the frame offset does not have to be decoded from the instruction stream either, as it's already in the PDR record. One could argue DWARF records are the modern and therefore natural choice for this, but typically they're gone with stripping unless special care is taken and records are removed selectively. I doubt such care is taken in the field on a regular basis -- i.e. people just run `strip' or maybe even `gcc -s', or use the `install-strip' Makefile target, which again just boils down to `strip' typically -- though I'd be happy to be proved wrong. PDR records are always kept OTOH and they are really tiny, so they don't consume much disk space either. Unlike DWARF records they're also usually present even in handcoded assembly, as they're set up with the `.frame', `.mask' and `.fmask' pseudo-ops, the former of which is actually required for MIPS PIC code to assemble at all, and PDR generation is on by default in GAS. The only drawback is some toolchain configurations may disable them explicitly with the `-mno-pdr' GAS option in the GCC driver. NB, on the contrary the changes to `mips_o32_push_dummy_call' look good to me as they stand, without reservation. FWIW, Maciej