Message ID | 20200122140818.84308-1-shahab.vahedi@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 66657 invoked by alias); 22 Jan 2020 14:08:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 66648 invoked by uid 89); 22 Jan 2020 14:08:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-lj1-f195.google.com Received: from mail-lj1-f195.google.com (HELO mail-lj1-f195.google.com) (209.85.208.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Jan 2020 14:08:33 +0000 Received: by mail-lj1-f195.google.com with SMTP id a13so6928164ljm.10 for <gdb-patches@sourceware.org>; Wed, 22 Jan 2020 06:08:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eivU/g0jP7rgA1kAglYGmPbnnJJoYb/H7/qyZP01+ck=; b=pNtx2uwyJ8oPfUGTbnieawIBjukbRYlf5v3as5VAmX9ERxnsoMpUY32gFxUPGOrSfQ O4BralSIafexxNE8C8D2P6L4TDRoIH3l2t3eQyETgRNrNxcjkvUk0Mt+PVacdYuAdLOg qfIslTvAC5ZJlHzsLkVcQeXuYorTmx+TWNyDKU6TYiLeFbGK3uFc1D4jjSLUAOSyxdDD rtjEifV4Fjqp/VVieNhFHbgTXIHhwm/KGA8v1RaOCJOsUQB9j37X2S2WqajTjQJ1NPZE zyJpay8VtvTS7PBxC0uKveKE/4n/So8TrEkDIatVLabjEuqRKlSMLuHvfKDwN+A14yI8 e1rA== Return-Path: <shahab.vahedi@gmail.com> Received: from archie.internal.synopsys.com ([2a03:1b20:6:f011::2d]) by smtp.gmail.com with ESMTPSA id f11sm8045762lfm.12.2020.01.22.06.08.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Jan 2020 06:08:30 -0800 (PST) From: Shahab Vahedi <shahab.vahedi@gmail.com> To: gdb-patches@sourceware.org Cc: Shahab Vahedi <shahab@synopsys.com>, Shahab Vahedi <shahab.vahedi@gmail.com>, Andrew Burgess <andrew.burgess@embecosm.com>, Tom Tromey <tom@tromey.com>, Claudiu Zissulescu <claziss@synopsys.com>, Francois Bedard <fbedard@synopsys.com> Subject: [PATCH] gdb: Catch exceptions if the source file is not found Date: Wed, 22 Jan 2020 15:08:18 +0100 Message-Id: <20200122140818.84308-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Shahab Vahedi
Jan. 22, 2020, 2:08 p.m. UTC
From: Shahab Vahedi <shahab@synopsys.com>
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 [2].
The patch from Pedro [1] 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]
https://sourceware.org/ml/gdb-patches/2020-01/msg00440.html
[2]
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
gdb/ChangeLog:
2020-01-22 Shahab Vahedi <shahab@synopsys.com>
* 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".
---
gdb/source-cache.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
Comments
* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-01-22 15:08:18 +0100]: > From: Shahab Vahedi <shahab@synopsys.com> > > 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 [2]. > > The patch from Pedro [1] 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. Would it be possible to create a version of this test (or similar) within gdb.tui/ so this fix would be protected in the future? Thanks, Andrew > > [1] > https://sourceware.org/ml/gdb-patches/2020-01/msg00440.html > > [2] > 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 > > gdb/ChangeLog: > 2020-01-22 Shahab Vahedi <shahab@synopsys.com> > > * 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". > --- > gdb/source-cache.c | 39 +++++++++++++++++++++------------------ > 1 file changed, 21 insertions(+), 18 deletions(-) > > diff --git a/gdb/source-cache.c b/gdb/source-cache.c > index 71277ecc9b3..9196e3a19e3 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<off_t> **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, > -- > 2.25.0 >
On Wed, Jan 22, 2020 at 03:49:01PM +0000, Andrew Burgess wrote: > > Would it be possible to create a version of this test (or similar) > within gdb.tui/ so this fix would be protected in the future? > This is reproducible on an x86_64 host/target as well. These are the necessary step that I have to automate by a test: $ cat > file1.c <<EOF extern int func2(int); int main() { int a = 4; return func2(a); } EOF $ cat > file2.c <<EOF int func2(int x) { x <<= 1; return x+5; } EOF $ gcc -g file1.c file2.c -o test $ mv file1.c file1.c.moved $ gdb -tui test.c (gdb) start (gdb) next (gdb) step at this point we should see func2 in source window. It is just a matter of time for me to write my _first_ test. If the changes are OK, I will submit the test in a separate patch if you don't mind. I just don't want this fix fall through the cracks as apparently it did once according to Tom. Cheers, Shahab
>>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes:
Shahab> It is just a matter of time for me to write my _first_ test.
Shahab> If the changes are OK, I will submit the test in a separate
Shahab> patch if you don't mind. I just don't want this fix fall
Shahab> through the cracks as apparently it did once according to
Shahab> Tom.
Is this needed for gdb 9? If so, then perhaps it can land on the branch
without a test. For trunk there's normally no particular rush to get
things in, and I think it would be good to have the test at the same
time.
Tom
Hi Tom, On Thu, Jan 23, 2020 at 11:14:16AM -0700, Tom Tromey wrote: > >>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes: > > Shahab> It is just a matter of time for me to write my _first_ test. > Shahab> If the changes are OK, I will submit the test in a separate > Shahab> patch if you don't mind. I just don't want this fix fall > Shahab> through the cracks as apparently it did once according to > Shahab> Tom. > > Is this needed for gdb 9? If so, then perhaps it can land on the branch This issue does exist even in GDB 8.3. Nevertheless, I have included the test into the latest patch [1]. That patch [1] has addressed the latest remarks [2] from Andrew and should be fine. A good way to check if the issue also happens for GDB 9 is by running the test in a GDB 9 branch. I don't have one ready now, but if you want I can give it a try. Cheers, Shahab [1] https://sourceware.org/ml/gdb-patches/2020-02/msg00127.html [2] https://sourceware.org/ml/gdb-patches/2020-02/msg00126.html > without a test. For trunk there's normally no particular rush to get > things in, and I think it would be good to have the test at the same > time. > > Tom
There is a newer patch (v4): https://sourceware.org/ml/gdb-patches/2020-02/msg00134.html
diff --git a/gdb/source-cache.c b/gdb/source-cache.c index 71277ecc9b3..9196e3a19e3 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<off_t> **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,