From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3792 invoked by alias); 18 Dec 2003 22:12:44 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 3779 invoked from network); 18 Dec 2003 22:12:43 -0000 Received: from unknown (HELO mail-out4.apple.com) (17.254.13.23) by sources.redhat.com with SMTP; 18 Dec 2003 22:12:43 -0000 Received: from mailgate3.apple.com (a17-128-100-68.apple.com [17.128.100.68]) by mail-out4.apple.com (8.12.10/8.12.9) with ESMTP id hBIMCh7t021536 for ; Thu, 18 Dec 2003 14:12:43 -0800 (PST) Received: from relay2.apple.com (relay2.apple.com) by mailgate3.apple.com (Content Technologies SMTPRS 4.3.6) with ESMTP id ; Thu, 18 Dec 2003 14:12:42 -0800 Received: from [17.201.22.21] (moleja.apple.com [17.201.22.21]) by relay2.apple.com (8.12.10/8.12.9) with ESMTP id hBIMCgc4016302; Thu, 18 Dec 2003 22:12:42 GMT In-Reply-To: <3FE0C778.8010606@redhat.com> References: <3FE0C778.8010606@redhat.com> Mime-Version: 1.0 (Apple Message framework v609) Content-Type: multipart/mixed; boundary=Apple-Mail-3--1010919228 Message-Id: <4C0916E8-31A7-11D8-BFBD-000393D457E2@apple.com> Cc: gdb-patches@sources.redhat.com From: Jason Molenda Subject: Re: [RFA]: Fix for do_mixed_source_and_assembly in disasm.c Date: Thu, 18 Dec 2003 22:12:00 -0000 To: Jeff Johnston X-SW-Source: 2003-12/txt/msg00439.txt.bz2 --Apple-Mail-3--1010919228 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed Content-length: 2136 Hi Jeff, On Dec 17, 2003, at 1:15 PM, Jeff Johnston wrote: > There are a few bugs in do_mixed_source_and_assembly() when dealing > with the ia64. I hit one of the two bugs you're fixing last week, namely the double call to close off the list/tuple. My first fix looked like yours, but I think a slight reworking of the loop is very desirable here. Right now this loop looks approximately like this: Set close_list to 1 (the list/tuple will be closed at the end of this loop) First entry of a new source line number: Start a "src_and_asm_line" tuple, print the source line. Start a "line_asm_insn" list where instructions will be emitted. If we're not at the end of the mle array, and the next mle entry's source line number is not greater than the current source line entry, DON'T close off the list (close_list = 0) Print the assembly instructions for the current address range. If close_list is 1, close the tuple/list. Which is a very convoluted way of writing a loop, and more importantly, this only works correctly for one or two assembly ranges for a single source line. If you have a third, on the 2nd iter the tuple/list are closed and then on the 3rd you close them again. Instead, this is more clear: First entry of a new source line number: Start a "src_and_asm_line" tuple, print the source line. Start a "line_asm_insn" list where instructions will be emitted. Print the assembly instructions for the current address range. If we're at the end of the mle array or the next entry in the array is a new source line, close off the list and tuple. I also took the opportunity to combine two conditional statements - I won't push hard for that part of the change, but the rest is a clear improvement IMHO. We only had this code path executed when we were using a compiler with a bug in it internally, so it's not too easy for me to reproduce/test this. I've tried to combine my patch and yours against the current FSF TOT. What do you think of my suggested additional change? Can you try it on the IA64 test case you have? Jason --Apple-Mail-3--1010919228 Content-Transfer-Encoding: 7bit Content-Type: text/plain; x-unix-mode=0644; name="pa.txt" Content-Disposition: attachment; filename=pa.txt Content-length: 1992 Index: disasm.c =================================================================== RCS file: /cvs/src/src/gdb/disasm.c,v retrieving revision 1.17 diff -u -p -r1.17 disasm.c --- disasm.c 24 Oct 2003 17:37:03 -0000 1.17 +++ disasm.c 18 Dec 2003 22:05:20 -0000 @@ -164,6 +164,8 @@ do_mixed_source_and_assembly (struct ui_ CORE_ADDR pc; int num_displayed = 0; struct cleanup *ui_out_chain; + struct cleanup *ui_out_tuple_chain = NULL; + struct cleanup *ui_out_list_chain = NULL; mle = (struct dis_line_entry *) alloca (nlines * sizeof (struct dis_line_entry)); @@ -221,10 +223,6 @@ do_mixed_source_and_assembly (struct ui_ for (i = 0; i < newlines; i++) { - struct cleanup *ui_out_tuple_chain = NULL; - struct cleanup *ui_out_list_chain = NULL; - int close_list = 1; - /* Print out everything from next_line to the current line. */ if (mle[i].line >= next_line) { @@ -275,22 +273,22 @@ do_mixed_source_and_assembly (struct ui_ next_line = mle[i].line + 1; ui_out_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "line_asm_insn"); - /* Don't close the list if the lines are not in order. */ - if (i < (newlines - 1) && mle[i + 1].line <= mle[i].line) - close_list = 0; } num_displayed += dump_insns (uiout, di, mle[i].start_pc, mle[i].end_pc, how_many, stb); - if (close_list) + + /* When we've reached the end of the mle array, or we've seen the last + assembly range for this source line, close out the list/tuple. */ + if (i == (newlines - 1) || mle[i + 1].line > mle[i].line) { do_cleanups (ui_out_list_chain); do_cleanups (ui_out_tuple_chain); + ui_out_tuple_chain = NULL; + ui_out_list_chain = NULL; ui_out_text (uiout, "\n"); - close_list = 0; } - if (how_many >= 0) - if (num_displayed >= how_many) + if (how_many >= 0 && num_displayed >= how_many) break; } do_cleanups (ui_out_chain); --Apple-Mail-3--1010919228--