From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11169 invoked by alias); 14 Jul 2009 14:59:41 -0000 Received: (qmail 11160 invoked by uid 22791); 14 Jul 2009 14:59:40 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtagate1.de.ibm.com (HELO mtagate1.de.ibm.com) (195.212.17.161) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Jul 2009 14:59:33 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate1.de.ibm.com (8.13.1/8.13.1) with ESMTP id n6EExTsV028794 for ; Tue, 14 Jul 2009 14:59:29 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n6EExTjb2576388 for ; Tue, 14 Jul 2009 16:59:29 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n6EExTeR004706 for ; Tue, 14 Jul 2009 16:59:29 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id n6EExSxC004676; Tue, 14 Jul 2009 16:59:28 +0200 Message-Id: <200907141459.n6EExSxC004676@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Tue, 14 Jul 2009 16:59:28 +0200 Subject: [rfc] Fix register cache invalidation To: gdb-patches@sourceware.org Date: Tue, 14 Jul 2009 20:22:00 -0000 From: "Ulrich Weigand" Cc: pedro@codesourcery.com MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2009-07/txt/msg00381.txt.bz2 Hello, when testing the Cell/B.E. combined debugger patches I ran into a situation where registers_changed was not called soon enough. A while ago I proposed a patch to simplify register invalidation (as part of a infrun cleanup series): http://sourceware.org/ml/gdb-patches/2008-12/msg00120.html This cleanup actually fixes the combined debugger problem as well. At the time, Pedro pointed out that parts of that patch weren't safe in the non-stop case. The version below omits those parts, and only changes code to simplify calling registers_changed and resetting waiton_ptid. Tested on powerpc64-linux with no regressions. Any comments? I'd like to commit this in a couple of days. Bye, Ulrich ChangeLog: * infrun.c (wait_for_inferior): Invalidate registers and overlay cache every time before calling target_wait. (handle_inferior_event): Make static. Always reset waiton_ptid. Never call registers_changed. (prepare_to_wait): Do not invaliate registers or overlay cache (moved to wait_for_inferior). Do not reset waiton_ptid (moved to handle_inferior_event). (fetch_inferior_event): Remove check for always-true condition. Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c +++ src/gdb/infrun.c @@ -1756,7 +1756,7 @@ struct execution_control_state static void init_execution_control_state (struct execution_control_state *ecs); -void handle_inferior_event (struct execution_control_state *ecs); +static void handle_inferior_event (struct execution_control_state *ecs); static void handle_step_into_function (struct gdbarch *gdbarch, struct execution_control_state *ecs); @@ -1993,23 +1993,22 @@ wait_for_inferior (int treat_exec_as_sig ecs = &ecss; memset (ecs, 0, sizeof (*ecs)); - overlay_cache_invalid = 1; - /* We'll update this if & when we switch to a new thread. */ previous_inferior_ptid = inferior_ptid; - /* We have to invalidate the registers BEFORE calling target_wait - because they can be loaded from the target while in target_wait. - This makes remote debugging a bit more efficient for those - targets that provide critical registers as part of their normal - status mechanism. */ - - registers_changed (); - while (1) { struct cleanup *old_chain; + /* We have to invalidate the registers BEFORE calling target_wait + because they can be loaded from the target while in target_wait. + This makes remote debugging a bit more efficient for those + targets that provide critical registers as part of their normal + status mechanism. */ + + overlay_cache_invalid = 1; + registers_changed (); + if (deprecated_target_wait_hook) ecs->ptid = deprecated_target_wait_hook (waiton_ptid, &ecs->ws, 0); else @@ -2063,14 +2062,8 @@ fetch_inferior_event (void *client_data) memset (ecs, 0, sizeof (*ecs)); - overlay_cache_invalid = 1; - - /* We can only rely on wait_for_more being correct before handling - the event in all-stop, but previous_inferior_ptid isn't used in - non-stop. */ - if (!ecs->wait_some_more) - /* We'll update this if & when we switch to a new thread. */ - previous_inferior_ptid = inferior_ptid; + /* We'll update this if & when we switch to a new thread. */ + previous_inferior_ptid = inferior_ptid; if (non_stop) /* In non-stop mode, the user/frontend should not notice a thread @@ -2085,6 +2078,7 @@ fetch_inferior_event (void *client_data) targets that provide critical registers as part of their normal status mechanism. */ + overlay_cache_invalid = 1; registers_changed (); if (deprecated_target_wait_hook) @@ -2375,7 +2369,7 @@ stepped_in_from (struct frame_info *fram by an event from the inferior, figure out what it means and take appropriate action. */ -void +static void handle_inferior_event (struct execution_control_state *ecs) { struct frame_info *frame; @@ -2444,8 +2438,6 @@ handle_inferior_event (struct execution_ case infwait_thread_hop_state: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: infwait_thread_hop_state\n"); - /* Cancel the waiton_ptid. */ - waiton_ptid = pid_to_ptid (-1); break; case infwait_normal_state: @@ -2476,7 +2468,9 @@ handle_inferior_event (struct execution_ default: internal_error (__FILE__, __LINE__, _("bad switch")); } + infwait_state = infwait_normal_state; + waiton_ptid = pid_to_ptid (-1); switch (ecs->ws.kind) { @@ -3001,7 +2995,6 @@ targets should add new threads to the th ecs->event_thread->stepping_over_breakpoint = 1; keep_going (ecs); - registers_changed (); return; } } @@ -3076,7 +3069,6 @@ targets should add new threads to the th /* Single step */ hw_step = maybe_software_singlestep (gdbarch, stop_pc); target_resume (ecs->ptid, hw_step, TARGET_SIGNAL_0); - registers_changed (); waiton_ptid = ecs->ptid; if (target_have_steppable_watchpoint) infwait_state = infwait_step_watch_state; @@ -4482,19 +4474,7 @@ prepare_to_wait (struct execution_contro { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: prepare_to_wait\n"); - if (infwait_state == infwait_normal_state) - { - overlay_cache_invalid = 1; - /* We have to invalidate the registers BEFORE calling - target_wait because they can be loaded from the target while - in target_wait. This makes remote debugging a bit more - efficient for those targets that provide critical registers - as part of their normal status mechanism. */ - - registers_changed (); - waiton_ptid = pid_to_ptid (-1); - } /* This is the old end of the while loop. Let everybody know we want to wait for the inferior some more and get called again soon. */ -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com