From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11454 invoked by alias); 21 Apr 2014 16:12:39 -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 11444 invoked by uid 89); 21 Apr 2014 16:12:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 21 Apr 2014 16:12:35 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 5F5DA1160EC; Mon, 21 Apr 2014 12:12:33 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id dnKjFZKKHVyX; Mon, 21 Apr 2014 12:12:33 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 286801160BF; Mon, 21 Apr 2014 12:12:33 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 90437E09B7; Mon, 21 Apr 2014 09:12:32 -0700 (PDT) Date: Mon, 21 Apr 2014 16:12:00 -0000 From: Joel Brobecker To: Simon Marchi Cc: GDB Patches Subject: Re: [PATCH] Remove unused variable Message-ID: <20140421161232.GF4477@adacore.com> References: <53553E27.6060009@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53553E27.6060009@ericsson.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-04/txt/msg00411.txt.bz2 Simon, > should_resume is set to 1 at the beginning and never changed. > > The legal paperwork for people at Ericsson Montreal has been completed > last week, so I would be ready to open an account to be able to submit > patches. Great! > > gdb/ChangeLog: > > 2014-04-21 Simon Marchi > > * infrun.c (resume): Remove should_resume (unused). Unfortunately, your patch does much much much much much much much more than just removing "should_resume" :-). Can you please submit a patch that just removes that variable? > > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 31bb132..49fd58c 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -1771,13 +1771,13 @@ user_visible_resume_ptid (int step) > void > resume (int step, enum gdb_signal sig) > { > - int should_resume = 1; > struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0); > struct regcache *regcache = get_current_regcache (); > struct gdbarch *gdbarch = get_regcache_arch (regcache); > struct thread_info *tp = inferior_thread (); > CORE_ADDR pc = regcache_read_pc (regcache); > struct address_space *aspace = get_regcache_aspace (regcache); > + ptid_t resume_ptid; > > QUIT; > > @@ -1921,87 +1921,82 @@ a command like `return' or `jump' to continue execution.")); > insert_breakpoints (); > } > > - if (should_resume) > + /* If STEP is set, it's a request to use hardware stepping > + facilities. But in that case, we should never > + use singlestep breakpoint. */ > + gdb_assert (!(singlestep_breakpoints_inserted_p && step)); > + > + /* Decide the set of threads to ask the target to resume. Start > + by assuming everything will be resumed, than narrow the set > + by applying increasingly restricting conditions. */ > + resume_ptid = user_visible_resume_ptid (step); > + > + /* Maybe resume a single thread after all. */ > + if ((step || singlestep_breakpoints_inserted_p) > + && tp->control.trap_expected) > + { > + /* We're allowing a thread to run past a breakpoint it has > + hit, by single-stepping the thread with the breakpoint > + removed. In which case, we need to single-step only this > + thread, and keep others stopped, as they can miss this > + breakpoint if allowed to run. */ > + resume_ptid = inferior_ptid; > + } > + > + if (gdbarch_cannot_step_breakpoint (gdbarch)) > { > - ptid_t resume_ptid; > + /* Most targets can step a breakpoint instruction, thus > + executing it normally. But if this one cannot, just > + continue and we will hit it anyway. */ > + if (step && breakpoint_inserted_here_p (aspace, pc)) > + step = 0; > + } > > - /* If STEP is set, it's a request to use hardware stepping > - facilities. But in that case, we should never > - use singlestep breakpoint. */ > - gdb_assert (!(singlestep_breakpoints_inserted_p && step)); > + if (debug_displaced > + && use_displaced_stepping (gdbarch) > + && tp->control.trap_expected) > + { > + struct regcache *resume_regcache = get_thread_regcache (resume_ptid); > + struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache); > + CORE_ADDR actual_pc = regcache_read_pc (resume_regcache); > + gdb_byte buf[4]; > > - /* Decide the set of threads to ask the target to resume. Start > - by assuming everything will be resumed, than narrow the set > - by applying increasingly restricting conditions. */ > - resume_ptid = user_visible_resume_ptid (step); > + fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ", > + paddress (resume_gdbarch, actual_pc)); > + read_memory (actual_pc, buf, sizeof (buf)); > + displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf)); > + } > > - /* Maybe resume a single thread after all. */ > - if ((step || singlestep_breakpoints_inserted_p) > - && tp->control.trap_expected) > - { > - /* We're allowing a thread to run past a breakpoint it has > - hit, by single-stepping the thread with the breakpoint > - removed. In which case, we need to single-step only this > - thread, and keep others stopped, as they can miss this > - breakpoint if allowed to run. */ > - resume_ptid = inferior_ptid; > - } > + if (tp->control.may_range_step) > + { > + /* If we're resuming a thread with the PC out of the step > + range, then we're doing some nested/finer run control > + operation, like stepping the thread out of the dynamic > + linker or the displaced stepping scratch pad. We > + shouldn't have allowed a range step then. */ > + gdb_assert (pc_in_thread_step_range (pc, tp)); > + } > > - if (gdbarch_cannot_step_breakpoint (gdbarch)) > - { > - /* Most targets can step a breakpoint instruction, thus > - executing it normally. But if this one cannot, just > - continue and we will hit it anyway. */ > - if (step && breakpoint_inserted_here_p (aspace, pc)) > - step = 0; > - } > + /* Install inferior's terminal modes. */ > + target_terminal_inferior (); > > - if (debug_displaced > - && use_displaced_stepping (gdbarch) > - && tp->control.trap_expected) > - { > - struct regcache *resume_regcache = get_thread_regcache (resume_ptid); > - struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache); > - CORE_ADDR actual_pc = regcache_read_pc (resume_regcache); > - gdb_byte buf[4]; > - > - fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ", > - paddress (resume_gdbarch, actual_pc)); > - read_memory (actual_pc, buf, sizeof (buf)); > - displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf)); > - } > - > - if (tp->control.may_range_step) > - { > - /* If we're resuming a thread with the PC out of the step > - range, then we're doing some nested/finer run control > - operation, like stepping the thread out of the dynamic > - linker or the displaced stepping scratch pad. We > - shouldn't have allowed a range step then. */ > - gdb_assert (pc_in_thread_step_range (pc, tp)); > - } > + /* Avoid confusing the next resume, if the next stop/resume > + happens to apply to another thread. */ > + tp->suspend.stop_signal = GDB_SIGNAL_0; > > - /* Install inferior's terminal modes. */ > - target_terminal_inferior (); > - > - /* Avoid confusing the next resume, if the next stop/resume > - happens to apply to another thread. */ > - tp->suspend.stop_signal = GDB_SIGNAL_0; > - > - /* Advise target which signals may be handled silently. If we have > - removed breakpoints because we are stepping over one (which can > - happen only if we are not using displaced stepping), we need to > - receive all signals to avoid accidentally skipping a breakpoint > - during execution of a signal handler. */ > - if ((step || singlestep_breakpoints_inserted_p) > - && tp->control.trap_expected > - && !use_displaced_stepping (gdbarch)) > - target_pass_signals (0, NULL); > - else > - target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); > + /* Advise target which signals may be handled silently. If we have > + removed breakpoints because we are stepping over one (which can > + happen only if we are not using displaced stepping), we need to > + receive all signals to avoid accidentally skipping a breakpoint > + during execution of a signal handler. */ > + if ((step || singlestep_breakpoints_inserted_p) > + && tp->control.trap_expected > + && !use_displaced_stepping (gdbarch)) > + target_pass_signals (0, NULL); > + else > + target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); > > - target_resume (resume_ptid, step, sig); > - } > + target_resume (resume_ptid, step, sig); > > discard_cleanups (old_cleanups); > } -- Joel