From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH v3 2/2] Fix an undefined behavior in record_line
Date: Sun, 5 Apr 2020 00:03:27 +0100 [thread overview]
Message-ID: <20200404230327.GD3917@embecosm.com> (raw)
In-Reply-To: <AM6PR03MB51704951966AB6320C920CABE4CC0@AM6PR03MB5170.eurprd03.prod.outlook.com>
* Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-03-27 04:50:29 +0100]:
> Additionally do not completely remove symbols
> at the same PC than the end marker, instead
> make them non-is-stmt breakpoints.
>
> 2020-03-27 Bernd Edlinger <bernd.edlinger@hotmail.de>
> * buildsym.c (record_line): Fix undefined behavior and preserve
> lines at eof.
> ---
> gdb/buildsym.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 2d1e441..46c5bb1 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -705,27 +705,29 @@ struct blockvector *
> * sizeof (struct linetable_entry))));
> }
>
> - /* Normally, we treat lines as unsorted. But the end of sequence
> - marker is special. We sort line markers at the same PC by line
> - number, so end of sequence markers (which have line == 0) appear
> - first. This is right if the marker ends the previous function,
> - and there is no padding before the next function. But it is
> - wrong if the previous line was empty and we are now marking a
> - switch to a different subfile. We must leave the end of sequence
> - marker at the end of this group of lines, not sort the empty line
> - to after the marker. The easiest way to accomplish this is to
> - delete any empty lines from our table, if they are followed by
> - end of sequence markers. All we lose is the ability to set
> - breakpoints at some lines which contain no instructions
> - anyway. */
> + /* The end of sequence marker is special. We need to reset the
> + is_stmt flag on previous lines at the same PC, otherwise these
> + lines may cause problems since they might be at the same address
> + as the following function. For instance suppose a function calls
> + abort there is no reason to emit a ret after that point (no joke).
> + So the label may be at the same address where the following
> + function begins. A similar problem appears if a label is at the
> + same address where an inline function ends we cannot reliably tell
> + if this is considered part of the inline function or the calling
> + program or even the next inline function, so stack traces may
> + give surprising results. Expect gdb.cp/step-and-next-inline.exp
> + to fail if these lines are not modified here. */
Out of interest I tried reverting this patch and don't see any
failures in gdb.cp/step-and-next-inline.exp. Could you expand on
which tests specifically you expect to see fail, and maybe which
version of GCC you're using? I'm on 9.3.1. It'll be Monday before I
can try my other machine which has a wider selection of compiler
versions.
I also don't understand what part of the previous behaviour was
undefined, could you help me to understand please.
Thanks,
Andrew
> if (line == 0 && subfile->line_vector->nitems > 0)
> {
> - e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> - while (subfile->line_vector->nitems > 0 && e->pc == pc)
> + e = subfile->line_vector->item + subfile->line_vector->nitems;
> + do
> {
> e--;
> - subfile->line_vector->nitems--;
> + if (e->pc != pc || e->line == 0)
> + break;
> + e->is_stmt = 0;
> }
> + while (e > subfile->line_vector->item);
> }
>
> e = subfile->line_vector->item + subfile->line_vector->nitems++;
> --
> 1.9.1
next prev parent reply other threads:[~2020-04-04 23:03 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 3:50 Bernd Edlinger
2020-04-01 16:23 ` Tom Tromey
2020-04-01 16:52 ` Bernd Edlinger
2020-04-01 18:40 ` Bernd Edlinger
2020-04-01 18:53 ` Tom Tromey
2020-04-01 19:01 ` Bernd Edlinger
2020-04-03 22:53 ` Luis Machado
2020-04-04 4:21 ` Bernd Edlinger
2020-04-04 7:06 ` Bernd Edlinger
2020-04-04 13:56 ` Luis Machado
2020-04-04 16:06 ` Bernd Edlinger
2020-04-04 16:22 ` Luis Machado
2020-04-04 16:34 ` Bernd Edlinger
2020-04-04 22:55 ` Andrew Burgess
2020-04-05 0:12 ` Bernd Edlinger
2020-04-04 23:03 ` Andrew Burgess [this message]
2020-04-06 17:44 ` Andrew Burgess
2020-04-06 18:48 ` Bernd Edlinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200404230327.GD3917@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=bernd.edlinger@hotmail.de \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).