From patchwork Thu Jun 7 22:44:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 27701 Received: (qmail 93438 invoked by alias); 7 Jun 2018 22:45:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 93360 invoked by uid 89); 7 Jun 2018 22:45:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hit, Reading, end_catch, TRY X-HELO: gateway33.websitewelcome.com Received: from gateway33.websitewelcome.com (HELO gateway33.websitewelcome.com) (192.185.146.97) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 Jun 2018 22:45:02 +0000 Received: from cm16.websitewelcome.com (cm16.websitewelcome.com [100.42.49.19]) by gateway33.websitewelcome.com (Postfix) with ESMTP id C33BF18478 for ; Thu, 7 Jun 2018 17:45:00 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id R3eOfbdiVaSeyR3eOfKxs2; Thu, 07 Jun 2018 17:45:00 -0500 X-Authority-Reason: nr=8 Received: from 75-166-19-45.hlrn.qwest.net ([75.166.19.45]:34646 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1fR3eO-001b6i-HQ; Thu, 07 Jun 2018 17:45:00 -0500 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA] Remove cleanups from record-full.c Date: Thu, 7 Jun 2018 16:44:57 -0600 Message-Id: <20180607224457.6520-1-tom@tromey.com> X-BWhitelist: no X-Source-L: No X-Exim-ID: 1fR3eO-001b6i-HQ X-Source-Sender: 75-166-19-45.hlrn.qwest.net (bapiya.Home) [75.166.19.45]:34646 X-Source-Auth: tom+tromey.com X-Email-Count: 4 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes This removes cleanups from record-full.c. In this case, the cleanups were only ever run when an exception was thrown. So, I replaced these with try/catch, rather than introduce a new specialized RAII type. Tested by the buildbot. gdb/ChangeLog 2018-06-07 Tom Tromey * record-full.c (record_full_arch_list_cleanups): Remove. (record_full_message): Use try/catch. (record_full_wait_cleanups): Remove. (record_full_wait_1): Use try/catch. (record_full_restore): Likewise. --- gdb/ChangeLog | 8 + gdb/record-full.c | 551 ++++++++++++++++++++++++++++-------------------------- 2 files changed, 290 insertions(+), 269 deletions(-) diff --git a/gdb/record-full.c b/gdb/record-full.c index 87c77e06ea9..c0c8cfe9a0e 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -696,12 +696,6 @@ record_full_check_insn_num (void) } } -static void -record_full_arch_list_cleanups (void *ignore) -{ - record_full_list_release (record_full_arch_list_tail); -} - /* Before inferior step (when GDB record the running message, inferior only can step), GDB will call this function to record the values to record_full_list. This function will call gdbarch_process_record to @@ -713,60 +707,66 @@ record_full_message (struct regcache *regcache, enum gdb_signal signal) { int ret; struct gdbarch *gdbarch = regcache->arch (); - struct cleanup *old_cleanups - = make_cleanup (record_full_arch_list_cleanups, 0); - record_full_arch_list_head = NULL; - record_full_arch_list_tail = NULL; + TRY + { + record_full_arch_list_head = NULL; + record_full_arch_list_tail = NULL; - /* Check record_full_insn_num. */ - record_full_check_insn_num (); + /* Check record_full_insn_num. */ + record_full_check_insn_num (); - /* If gdb sends a signal value to target_resume, - save it in the 'end' field of the previous instruction. + /* If gdb sends a signal value to target_resume, + save it in the 'end' field of the previous instruction. - Maybe process record should record what really happened, - rather than what gdb pretends has happened. + Maybe process record should record what really happened, + rather than what gdb pretends has happened. - So if Linux delivered the signal to the child process during - the record mode, we will record it and deliver it again in - the replay mode. + So if Linux delivered the signal to the child process during + the record mode, we will record it and deliver it again in + the replay mode. - If user says "ignore this signal" during the record mode, then - it will be ignored again during the replay mode (no matter if - the user says something different, like "deliver this signal" - during the replay mode). + If user says "ignore this signal" during the record mode, then + it will be ignored again during the replay mode (no matter if + the user says something different, like "deliver this signal" + during the replay mode). - User should understand that nothing he does during the replay - mode will change the behavior of the child. If he tries, - then that is a user error. + User should understand that nothing he does during the replay + mode will change the behavior of the child. If he tries, + then that is a user error. - But we should still deliver the signal to gdb during the replay, - if we delivered it during the recording. Therefore we should - record the signal during record_full_wait, not - record_full_resume. */ - if (record_full_list != &record_full_first) /* FIXME better way to check */ + But we should still deliver the signal to gdb during the replay, + if we delivered it during the recording. Therefore we should + record the signal during record_full_wait, not + record_full_resume. */ + if (record_full_list != &record_full_first) /* FIXME better way + to check */ + { + gdb_assert (record_full_list->type == record_full_end); + record_full_list->u.end.sigval = signal; + } + + if (signal == GDB_SIGNAL_0 + || !gdbarch_process_record_signal_p (gdbarch)) + ret = gdbarch_process_record (gdbarch, + regcache, + regcache_read_pc (regcache)); + else + ret = gdbarch_process_record_signal (gdbarch, + regcache, + signal); + + if (ret > 0) + error (_("Process record: inferior program stopped.")); + if (ret < 0) + error (_("Process record: failed to record execution log.")); + } + CATCH (ex, RETURN_MASK_ALL) { - gdb_assert (record_full_list->type == record_full_end); - record_full_list->u.end.sigval = signal; + record_full_list_release (record_full_arch_list_tail); + throw_exception (ex); } - - if (signal == GDB_SIGNAL_0 - || !gdbarch_process_record_signal_p (gdbarch)) - ret = gdbarch_process_record (gdbarch, - regcache, - regcache_read_pc (regcache)); - else - ret = gdbarch_process_record_signal (gdbarch, - regcache, - signal); - - if (ret > 0) - error (_("Process record: inferior program stopped.")); - if (ret < 0) - error (_("Process record: failed to record execution log.")); - - discard_cleanups (old_cleanups); + END_CATCH record_full_list->next = record_full_arch_list_head; record_full_arch_list_head->prev = record_full_list; @@ -1140,18 +1140,6 @@ record_full_sig_handler (int signo) record_full_get_sig = 1; } -static void -record_full_wait_cleanups (void *ignore) -{ - if (execution_direction == EXEC_REVERSE) - { - if (record_full_list->next) - record_full_list = record_full_list->next; - } - else - record_full_list = record_full_list->prev; -} - /* "wait" target method for process record target. In record mode, the target is always run in singlestep mode @@ -1310,144 +1298,163 @@ record_full_wait_1 (struct target_ops *ops, const struct address_space *aspace = regcache->aspace (); int continue_flag = 1; int first_record_full_end = 1; - struct cleanup *old_cleanups - = make_cleanup (record_full_wait_cleanups, 0); - CORE_ADDR tmp_pc; - - record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON; - status->kind = TARGET_WAITKIND_STOPPED; - /* Check breakpoint when forward execute. */ - if (execution_direction == EXEC_FORWARD) + TRY { - tmp_pc = regcache_read_pc (regcache); - if (record_check_stopped_by_breakpoint (aspace, tmp_pc, - &record_full_stop_reason)) - { - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - "Process record: break at %s.\n", - paddress (gdbarch, tmp_pc)); - goto replay_out; - } - } - - /* If GDB is in terminal_inferior mode, it will not get the signal. - And in GDB replay mode, GDB doesn't need to be in terminal_inferior - mode, because inferior will not executed. - Then set it to terminal_ours to make GDB get the signal. */ - target_terminal::ours (); + CORE_ADDR tmp_pc; - /* In EXEC_FORWARD mode, record_full_list points to the tail of prev - instruction. */ - if (execution_direction == EXEC_FORWARD && record_full_list->next) - record_full_list = record_full_list->next; + record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON; + status->kind = TARGET_WAITKIND_STOPPED; - /* Loop over the record_full_list, looking for the next place to - stop. */ - do - { - /* Check for beginning and end of log. */ - if (execution_direction == EXEC_REVERSE - && record_full_list == &record_full_first) + /* Check breakpoint when forward execute. */ + if (execution_direction == EXEC_FORWARD) { - /* Hit beginning of record log in reverse. */ - status->kind = TARGET_WAITKIND_NO_HISTORY; - break; - } - if (execution_direction != EXEC_REVERSE && !record_full_list->next) - { - /* Hit end of record log going forward. */ - status->kind = TARGET_WAITKIND_NO_HISTORY; - break; + tmp_pc = regcache_read_pc (regcache); + if (record_check_stopped_by_breakpoint (aspace, tmp_pc, + &record_full_stop_reason)) + { + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + "Process record: break at %s.\n", + paddress (gdbarch, tmp_pc)); + goto replay_out; + } } - record_full_exec_insn (regcache, gdbarch, record_full_list); - - if (record_full_list->type == record_full_end) + /* If GDB is in terminal_inferior mode, it will not get the + signal. And in GDB replay mode, GDB doesn't need to be + in terminal_inferior mode, because inferior will not + executed. Then set it to terminal_ours to make GDB get + the signal. */ + target_terminal::ours (); + + /* In EXEC_FORWARD mode, record_full_list points to the tail of prev + instruction. */ + if (execution_direction == EXEC_FORWARD && record_full_list->next) + record_full_list = record_full_list->next; + + /* Loop over the record_full_list, looking for the next place to + stop. */ + do { - if (record_debug > 1) - fprintf_unfiltered (gdb_stdlog, - "Process record: record_full_end %s to " - "inferior.\n", - host_address_to_string (record_full_list)); - - if (first_record_full_end && execution_direction == EXEC_REVERSE) + /* Check for beginning and end of log. */ + if (execution_direction == EXEC_REVERSE + && record_full_list == &record_full_first) { - /* When reverse excute, the first record_full_end is the - part of current instruction. */ - first_record_full_end = 0; + /* Hit beginning of record log in reverse. */ + status->kind = TARGET_WAITKIND_NO_HISTORY; + break; } - else + if (execution_direction != EXEC_REVERSE + && !record_full_list->next) { - /* In EXEC_REVERSE mode, this is the record_full_end of prev - instruction. - In EXEC_FORWARD mode, this is the record_full_end of - current instruction. */ - /* step */ - if (record_full_resume_step) + /* Hit end of record log going forward. */ + status->kind = TARGET_WAITKIND_NO_HISTORY; + break; + } + + record_full_exec_insn (regcache, gdbarch, record_full_list); + + if (record_full_list->type == record_full_end) + { + if (record_debug > 1) + fprintf_unfiltered + (gdb_stdlog, + "Process record: record_full_end %s to " + "inferior.\n", + host_address_to_string (record_full_list)); + + if (first_record_full_end + && execution_direction == EXEC_REVERSE) { - if (record_debug > 1) - fprintf_unfiltered (gdb_stdlog, - "Process record: step.\n"); - continue_flag = 0; + /* When reverse excute, the first + record_full_end is the part of current + instruction. */ + first_record_full_end = 0; } - - /* check breakpoint */ - tmp_pc = regcache_read_pc (regcache); - if (record_check_stopped_by_breakpoint (aspace, tmp_pc, - &record_full_stop_reason)) + else { - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - "Process record: break " - "at %s.\n", - paddress (gdbarch, tmp_pc)); + /* In EXEC_REVERSE mode, this is the + record_full_end of prev instruction. In + EXEC_FORWARD mode, this is the + record_full_end of current instruction. */ + /* step */ + if (record_full_resume_step) + { + if (record_debug > 1) + fprintf_unfiltered (gdb_stdlog, + "Process record: step.\n"); + continue_flag = 0; + } - continue_flag = 0; - } + /* check breakpoint */ + tmp_pc = regcache_read_pc (regcache); + if (record_check_stopped_by_breakpoint + (aspace, tmp_pc, &record_full_stop_reason)) + { + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + "Process record: break " + "at %s.\n", + paddress (gdbarch, tmp_pc)); - if (record_full_stop_reason == TARGET_STOPPED_BY_WATCHPOINT) - { - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - "Process record: hit hw " - "watchpoint.\n"); - continue_flag = 0; + continue_flag = 0; + } + + if (record_full_stop_reason + == TARGET_STOPPED_BY_WATCHPOINT) + { + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + "Process record: hit hw " + "watchpoint.\n"); + continue_flag = 0; + } + /* Check target signal */ + if (record_full_list->u.end.sigval != GDB_SIGNAL_0) + /* FIXME: better way to check */ + continue_flag = 0; } - /* Check target signal */ - if (record_full_list->u.end.sigval != GDB_SIGNAL_0) - /* FIXME: better way to check */ - continue_flag = 0; } - } - if (continue_flag) - { - if (execution_direction == EXEC_REVERSE) + if (continue_flag) { - if (record_full_list->prev) - record_full_list = record_full_list->prev; - } - else - { - if (record_full_list->next) - record_full_list = record_full_list->next; + if (execution_direction == EXEC_REVERSE) + { + if (record_full_list->prev) + record_full_list = record_full_list->prev; + } + else + { + if (record_full_list->next) + record_full_list = record_full_list->next; + } } } + while (continue_flag); + + replay_out: + if (record_full_get_sig) + status->value.sig = GDB_SIGNAL_INT; + else if (record_full_list->u.end.sigval != GDB_SIGNAL_0) + /* FIXME: better way to check */ + status->value.sig = record_full_list->u.end.sigval; + else + status->value.sig = GDB_SIGNAL_TRAP; } - while (continue_flag); - -replay_out: - if (record_full_get_sig) - status->value.sig = GDB_SIGNAL_INT; - else if (record_full_list->u.end.sigval != GDB_SIGNAL_0) - /* FIXME: better way to check */ - status->value.sig = record_full_list->u.end.sigval; - else - status->value.sig = GDB_SIGNAL_TRAP; + CATCH (ex, RETURN_MASK_ALL) + { + if (execution_direction == EXEC_REVERSE) + { + if (record_full_list->next) + record_full_list = record_full_list->next; + } + else + record_full_list = record_full_list->prev; - discard_cleanups (old_cleanups); + throw_exception (ex); + } + END_CATCH } signal (SIGINT, handle_sigint); @@ -2337,7 +2344,6 @@ static void record_full_restore (void) { uint32_t magic; - struct cleanup *old_cleanups; struct record_full_entry *rec; asection *osec; uint32_t osec_size; @@ -2382,108 +2388,115 @@ record_full_restore (void) record_full_arch_list_head = NULL; record_full_arch_list_tail = NULL; record_full_insn_num = 0; - old_cleanups = make_cleanup (record_full_arch_list_cleanups, 0); - regcache = get_current_regcache (); - while (1) + TRY { - uint8_t rectype; - uint32_t regnum, len, signal, count; - uint64_t addr; + regcache = get_current_regcache (); - /* We are finished when offset reaches osec_size. */ - if (bfd_offset >= osec_size) - break; - bfdcore_read (core_bfd, osec, &rectype, sizeof (rectype), &bfd_offset); + while (1) + { + uint8_t rectype; + uint32_t regnum, len, signal, count; + uint64_t addr; - switch (rectype) - { - case record_full_reg: /* reg */ - /* Get register number to regnum. */ - bfdcore_read (core_bfd, osec, ®num, - sizeof (regnum), &bfd_offset); - regnum = netorder32 (regnum); + /* We are finished when offset reaches osec_size. */ + if (bfd_offset >= osec_size) + break; + bfdcore_read (core_bfd, osec, &rectype, sizeof (rectype), &bfd_offset); - rec = record_full_reg_alloc (regcache, regnum); + switch (rectype) + { + case record_full_reg: /* reg */ + /* Get register number to regnum. */ + bfdcore_read (core_bfd, osec, ®num, + sizeof (regnum), &bfd_offset); + regnum = netorder32 (regnum); - /* Get val. */ - bfdcore_read (core_bfd, osec, record_full_get_loc (rec), - rec->u.reg.len, &bfd_offset); + rec = record_full_reg_alloc (regcache, regnum); - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - " Reading register %d (1 " - "plus %lu plus %d bytes)\n", - rec->u.reg.num, - (unsigned long) sizeof (regnum), - rec->u.reg.len); - break; - - case record_full_mem: /* mem */ - /* Get len. */ - bfdcore_read (core_bfd, osec, &len, - sizeof (len), &bfd_offset); - len = netorder32 (len); - - /* Get addr. */ - bfdcore_read (core_bfd, osec, &addr, - sizeof (addr), &bfd_offset); - addr = netorder64 (addr); - - rec = record_full_mem_alloc (addr, len); - - /* Get val. */ - bfdcore_read (core_bfd, osec, record_full_get_loc (rec), - rec->u.mem.len, &bfd_offset); + /* Get val. */ + bfdcore_read (core_bfd, osec, record_full_get_loc (rec), + rec->u.reg.len, &bfd_offset); - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - " Reading memory %s (1 plus " - "%lu plus %lu plus %d bytes)\n", - paddress (get_current_arch (), - rec->u.mem.addr), - (unsigned long) sizeof (addr), - (unsigned long) sizeof (len), - rec->u.mem.len); - break; - - case record_full_end: /* end */ - rec = record_full_end_alloc (); - record_full_insn_num ++; - - /* Get signal value. */ - bfdcore_read (core_bfd, osec, &signal, - sizeof (signal), &bfd_offset); - signal = netorder32 (signal); - rec->u.end.sigval = (enum gdb_signal) signal; - - /* Get insn count. */ - bfdcore_read (core_bfd, osec, &count, - sizeof (count), &bfd_offset); - count = netorder32 (count); - rec->u.end.insn_num = count; - record_full_insn_count = count + 1; - if (record_debug) - fprintf_unfiltered (gdb_stdlog, - " Reading record_full_end (1 + " - "%lu + %lu bytes), offset == %s\n", - (unsigned long) sizeof (signal), - (unsigned long) sizeof (count), - paddress (get_current_arch (), - bfd_offset)); - break; - - default: - error (_("Bad entry type in core file %s."), - bfd_get_filename (core_bfd)); - break; - } + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + " Reading register %d (1 " + "plus %lu plus %d bytes)\n", + rec->u.reg.num, + (unsigned long) sizeof (regnum), + rec->u.reg.len); + break; - /* Add rec to record arch list. */ - record_full_arch_list_add (rec); - } + case record_full_mem: /* mem */ + /* Get len. */ + bfdcore_read (core_bfd, osec, &len, + sizeof (len), &bfd_offset); + len = netorder32 (len); + + /* Get addr. */ + bfdcore_read (core_bfd, osec, &addr, + sizeof (addr), &bfd_offset); + addr = netorder64 (addr); + + rec = record_full_mem_alloc (addr, len); + + /* Get val. */ + bfdcore_read (core_bfd, osec, record_full_get_loc (rec), + rec->u.mem.len, &bfd_offset); + + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + " Reading memory %s (1 plus " + "%lu plus %lu plus %d bytes)\n", + paddress (get_current_arch (), + rec->u.mem.addr), + (unsigned long) sizeof (addr), + (unsigned long) sizeof (len), + rec->u.mem.len); + break; - discard_cleanups (old_cleanups); + case record_full_end: /* end */ + rec = record_full_end_alloc (); + record_full_insn_num ++; + + /* Get signal value. */ + bfdcore_read (core_bfd, osec, &signal, + sizeof (signal), &bfd_offset); + signal = netorder32 (signal); + rec->u.end.sigval = (enum gdb_signal) signal; + + /* Get insn count. */ + bfdcore_read (core_bfd, osec, &count, + sizeof (count), &bfd_offset); + count = netorder32 (count); + rec->u.end.insn_num = count; + record_full_insn_count = count + 1; + if (record_debug) + fprintf_unfiltered (gdb_stdlog, + " Reading record_full_end (1 + " + "%lu + %lu bytes), offset == %s\n", + (unsigned long) sizeof (signal), + (unsigned long) sizeof (count), + paddress (get_current_arch (), + bfd_offset)); + break; + + default: + error (_("Bad entry type in core file %s."), + bfd_get_filename (core_bfd)); + break; + } + + /* Add rec to record arch list. */ + record_full_arch_list_add (rec); + } + } + CATCH (ex, RETURN_MASK_ALL) + { + record_full_list_release (record_full_arch_list_tail); + throw_exception (ex); + } + END_CATCH /* Add record_full_arch_list_head to the end of record list. */ record_full_first.next = record_full_arch_list_head;