From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4811 invoked by alias); 6 Feb 2020 16:42:26 -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 4803 invoked by uid 89); 6 Feb 2020 16:42:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Feb 2020 16:42:23 +0000 Received: by mail-wr1-f67.google.com with SMTP id a6so7970677wrx.12 for ; Thu, 06 Feb 2020 08:42:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=U5/CpB9+wgjgsKjIiayyPlppDMUIxM5juscrfvGrzQY=; b=Tz2MSmBD7IwgDTU86nYhul6BLzSInkqYDLHPsuSEAAjDkYODVcWgdTTIgl4oZPkCIt 65nucxRvIF2lCfDl4hKHs43O3v0DfL16GFJiwBCxrqlYy5w9R8bcMDrOKWtTYj4afk72 s9WClnbw5QqpkiU6kiCDXDE/uMWUX+vu052B0yDU2qc5xdfgaB+3QySZBxBJCu/4u56a 0MxP0OcsJvqMDzUzrrjbGgrsSVfpJVM0gF0mOFi4g7hUoabrSMob235voszij8jnu+JN 7E+kfIuB1VWreFJOB3V8mWF35w5gDOG2RedZ+xfO+uityVIrGSXxyqAHscOkGq/ZroXA LONQ== Return-Path: Received: from localhost (host86-191-239-73.range86-191.btcentralplus.com. [86.191.239.73]) by smtp.gmail.com with ESMTPSA id n3sm67935wmc.27.2020.02.06.08.42.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 06 Feb 2020 08:42:20 -0800 (PST) Date: Thu, 06 Feb 2020 16:42:00 -0000 From: Andrew Burgess To: Shahab Vahedi Cc: gdb-patches@sourceware.org, Shahab Vahedi , Tom Tromey , Claudiu Zissulescu , Francois Bedard Subject: Re: [PATCH v4] gdb: Catch exceptions if the source file is not found Message-ID: <20200206164219.GG4020@embecosm.com> References: <20200122140818.84308-1-shahab.vahedi@gmail.com> <20200206152427.1859681-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200206152427.1859681-1-shahab.vahedi@gmail.com> X-Fortune: I feel like a wet parking meter on Darvon! X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00136.txt.bz2 * Shahab Vahedi [2020-02-06 16:24:27 +0100]: > From: Shahab Vahedi > > The source_cache::ensure method may throw an exception through > the invocation of source_cache::get_plain_source_lines. This > happens when the source file is not found. The expected behaviour > of "ensure" is only returning "true" or "false" according to the > documentation in the header file. > > So far, if gdb is in source layout and a file is missing, you see > some outputs like below: > > ,---------------------------------------------. > | test.c file is loaded in the source window. | > | | > | int main() | > | ... | > |---------------------------------------------| > | Remote debugging using :1234 | > | __start () at /path/to/crt0.S:141 | > | /path/to/crt0.S: No such file or directory. | > | (gdb) p/x $pc | > | $1 = 0x124 | > | (gdb) n | > | /path/to/crt0.S: No such file or directory. | > | (gdb) p/x $pc | > | $2 = 0x128 | > | (gdb) [pressing arrow-down key] | > | (gdb) terminate called after throwing an | > | instance of 'gdb_exception_error' | > `---------------------------------------------' > Other issues have been encountered as well [1]. > > The patch from Pedro [2] which is about preventing exceptions > from crossing the "readline" mitigates the situation by not > causing gdb crash, but still there are lots of errors printed: > > ,---------------------------------------------. > | test.c file is loaded in the source window. | > | | > | int main() | > | ... | > |---------------------------------------------| > | Remote debugging using :1234 | > | __start () at /path/to/crt0.S:141 | > | /path/to/crt0.S: No such file or directory. | > | (gdb) [pressing arrow-down key] | > | /path/to/crt0.S: No such file or directory. | > | (gdb) [pressing arrow-down key] | > | /path/to/crt0.S: No such file or directory. | > | (gdb) [pressing arrow-up key] | > | /path/to/crt0.S: No such file or directory. | > `---------------------------------------------' > > With the changes of this patch, the behavior is like: > ,---------------------------------------------. > | initially, source window is empty because | > | crt0.S is not found and according to the | > | program counter that is the piece of code | > | being executed. | > | | > | later, when we break at main (see commands | > | below), this window will be filled with the | > | the contents of test.c file. | > |---------------------------------------------| > | Remote debugging using :1234 | > | __start () at /path/to/crt0.S:141 | > | (gdb) p/x $pc | > | $1 = 0x124 | > | (gdb) n | > | (gdb) p/x $pc | > | $2 = 0x128 | > | (gdb) b main | > | Breakpoint 1 at 0x334: file test.c, line 8. | > | (gdb) cont | > | Continuing. | > | Breakpoint 1, main () at hello.c:8 | > | (gdb) n | > | (gdb) | > `---------------------------------------------' > > There is no crash and the error message is completely > gone. Maybe it is good practice that the error is > shown inside the source window. > > I tested this change against gdb.base/list-missing-source.exp > and there was no regression. > > [1] > It has also been observed in the past that the register > values are not transferred from qemu's gdb stub, see: > https://github.com/foss-for-synopsys-dwc-arc-processors/toolchain/issues/226 > > [2] > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2f267673f0fdee9287e6d404ecd4f2d29da0d2f2 > > gdb/ChangeLog: > > * source-cache.c (source_cache::ensure): Surround > get_plain_source_lines with a try/catch. > (source_cache::get_line_charpos): Get rid of try/catch > and only check for the return value of "ensure". > * tui/tui-source.c (tui_source_window::set_contents): > Simplify "nlines" calculation. > > gdb/testsuite/ChangeLog: > > * gdb.tui/tui-missing-src.exp: Add the "missing source > file" test for the TUI. LGTM. I see you have write access now, so feel free to go ahead and push this. Thanks, Andrew > --- > gdb/source-cache.c | 39 ++++----- > gdb/testsuite/gdb.tui/tui-missing-src.exp | 97 +++++++++++++++++++++++ > gdb/tui/tui-source.c | 2 +- > 3 files changed, 119 insertions(+), 19 deletions(-) > create mode 100644 gdb/testsuite/gdb.tui/tui-missing-src.exp > > diff --git a/gdb/source-cache.c b/gdb/source-cache.c > index 71277ecc9b..9196e3a19e 100644 > --- a/gdb/source-cache.c > +++ b/gdb/source-cache.c > @@ -176,7 +176,16 @@ source_cache::ensure (struct symtab *s) > } > } > > - std::string contents = get_plain_source_lines (s, fullname); > + std::string contents; > + try > + { > + contents = get_plain_source_lines (s, fullname); > + } > + catch (const gdb_exception_error &e) > + { > + /* If 's' is not found, an exception is thrown. */ > + return false; > + } > > if (source_styling && gdb_stdout->can_emit_style_escape ()) > { > @@ -241,26 +250,20 @@ bool > source_cache::get_line_charpos (struct symtab *s, > const std::vector **offsets) > { > - try > - { > - std::string fullname = symtab_to_fullname (s); > - > - auto iter = m_offset_cache.find (fullname); > - if (iter == m_offset_cache.end ()) > - { > - ensure (s); > - iter = m_offset_cache.find (fullname); > - /* cache_source_text ensured this was entered. */ > - gdb_assert (iter != m_offset_cache.end ()); > - } > + std::string fullname = symtab_to_fullname (s); > > - *offsets = &iter->second; > - return true; > - } > - catch (const gdb_exception_error &e) > + auto iter = m_offset_cache.find (fullname); > + if (iter == m_offset_cache.end ()) > { > - return false; > + if (!ensure (s)) > + return false; > + iter = m_offset_cache.find (fullname); > + /* cache_source_text ensured this was entered. */ > + gdb_assert (iter != m_offset_cache.end ()); > } > + > + *offsets = &iter->second; > + return true; > } > > /* A helper function that extracts the desired source lines from TEXT, > diff --git a/gdb/testsuite/gdb.tui/tui-missing-src.exp b/gdb/testsuite/gdb.tui/tui-missing-src.exp > new file mode 100644 > index 0000000000..2d9a851bec > --- /dev/null > +++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp > @@ -0,0 +1,97 @@ > +# Copyright 2020 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# This test checks if gdb can handle missing source files gracefully. > +# Testing steps are: > +# 1. Have a main() in main.c that calls an external function f2(). > +# 2. Have f2() implemented in f2.c. > +# 3. Build the two files into one executable. > +# 4. Remove main.c. > +# 5. Open the executable inside gdb while having gdb in source layout. > +# No source is found for the moment. > +# 6. After a little bit of playing, we enter f2() and now the source > +# layout must show the contents of f2.c. > +# 7. Going back to main() shall result in no contents again. > + > +load_lib "tuiterm.exp" > + > +standard_testfile > + > +set mainfile [standard_output_file main.c] > +set f2file [standard_output_file f2.c] > +set srcfiles [list $mainfile $f2file] > + > +# Step 1: Write the main.c file into the output directory. > +# This file will be removed after compilation. > +set fd [open "$mainfile" w] > +puts $fd { > +extern int f2(int); > +int > +main () > +{ > + int a = 4; > + a = f2(a); > + return a - a; > +} > +} > +close $fd > + > +# Step 2: Write the f2.c file into the output directory. > +set fd [open "$f2file" w] > +puts $fd { > +int > +f2 (int x) > +{ > + x <<= 1; > + return x+5; > +} > +} > +close $fd > + > +# Step 3: Compile the source files. > +if { [gdb_compile "${srcfiles}" "${binfile}" \ > + executable {debug additional_flags=-O0}] != "" } { > + untested "failed to compile" > + return -1 > +} > + > +# Step 4: Remove the main.c file. > +file delete $mainfile > + > +# Step 5: Load the executable into GDB. > +# There shall be no source content. > +Term::clean_restart 24 80 $testfile > +if {![Term::enter_tui]} { > + unsupported "TUI not supported" > +} > +# There must exist a source layout with the size 80x15 and > +# there should be nothing in it. > +Term::check_box_contents "check source box is empty" \ > + 0 0 80 15 "No Source Available" > + > +# Step 6: Go to main and after one next, enter f2(). > +Term::command "set pagination off" > +Term::command "start" > +Term::command "next" > +Term::command "step" > +Term::check_contents "checking if inside f2 ()" "f2 \\(x=4\\)" > +Term::check_box_contents "f2.c must be displayed in source window" \ > + 0 0 80 15 "return x\\+5" > + > +# Step 7: Back in main > +Term::command "finish" > +Term::check_box_contents "check source box is empty after return" \ > + 0 0 80 15 "No Source Available" > +Term::check_contents "Back in main" "Value returned is .* 13" > diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c > index 912eaa4544..3c7a8e1000 100644 > --- a/gdb/tui/tui-source.c > +++ b/gdb/tui/tui-source.c > @@ -55,7 +55,7 @@ tui_source_window::set_contents (struct gdbarch *arch, > line_width = width - TUI_EXECINFO_SIZE - 1; > /* Take hilite (window border) into account, when > calculating the number of lines. */ > - nlines = (line_no + (height - 2)) - line_no; > + nlines = height - 2; > > std::string srclines; > const std::vector *offsets; > -- > 2.25.0 >