* [PATCH] Partially revert my UB fix in record_line
@ 2020-04-09 1:35 Bernd Edlinger
2020-04-09 9:33 ` Andrew Burgess
0 siblings, 1 reply; 2+ messages in thread
From: Bernd Edlinger @ 2020-04-09 1:35 UTC (permalink / raw)
To: gdb-patches, Andrew Burgess, Tom Tromey
This reverts the following commit partially:
commit 64dc2d4bd24ff7119c913fff91184414f09b8042
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Thu Mar 12 11:52:34 2020 +0100
Fix an undefined behavior in record_line
Additionally do not completely remove symbols
at the same PC than the end marker, instead
make them non-is-stmt breakpoints.
We keep the undefined behavoir fix,
but have to restore the original behavior
regarding deletion of the line entries.
2020-04-09 Bernd Edlinger <bernd.edlinger@hotmail.de>
revert partially:
2020-04-01 Bernd Edlinger <bernd.edlinger@hotmail.de>
* buildsym.c (record_line): Fix undefined behavior and preserve
lines at eof.
---
gdb/buildsym.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index fe07103..c08c476 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -691,29 +691,28 @@ struct blockvector *
* sizeof (struct linetable_entry))));
}
- /* 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. */
- if (line == 0 && subfile->line_vector->nitems > 0)
+ /* 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. */
+ if (line == 0)
{
- e = subfile->line_vector->item + subfile->line_vector->nitems;
- do
+ while (subfile->line_vector->nitems > 0)
{
- e--;
- if (e->pc != pc || e->line == 0)
+ e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
+ if (e->pc != pc)
break;
- e->is_stmt = 0;
+ subfile->line_vector->nitems--;
}
- while (e > subfile->line_vector->item);
}
e = subfile->line_vector->item + subfile->line_vector->nitems++;
--
1.9.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Partially revert my UB fix in record_line
2020-04-09 1:35 [PATCH] Partially revert my UB fix in record_line Bernd Edlinger
@ 2020-04-09 9:33 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2020-04-09 9:33 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gdb-patches, Tom Tromey
* Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-04-09 03:35:17 +0200]:
> This reverts the following commit partially:
>
> commit 64dc2d4bd24ff7119c913fff91184414f09b8042
> Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Thu Mar 12 11:52:34 2020 +0100
>
> Fix an undefined behavior in record_line
>
> Additionally do not completely remove symbols
> at the same PC than the end marker, instead
> make them non-is-stmt breakpoints.
>
> We keep the undefined behavoir fix,
> but have to restore the original behavior
> regarding deletion of the line entries.
>
> 2020-04-09 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> revert partially:
> 2020-04-01 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * buildsym.c (record_line): Fix undefined behavior and preserve
> lines at eof.
Looks good. Approved.
Thanks,
Andrew
> ---
> gdb/buildsym.c | 37 ++++++++++++++++++-------------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index fe07103..c08c476 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -691,29 +691,28 @@ struct blockvector *
> * sizeof (struct linetable_entry))));
> }
>
> - /* 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. */
> - if (line == 0 && subfile->line_vector->nitems > 0)
> + /* 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. */
> + if (line == 0)
> {
> - e = subfile->line_vector->item + subfile->line_vector->nitems;
> - do
> + while (subfile->line_vector->nitems > 0)
> {
> - e--;
> - if (e->pc != pc || e->line == 0)
> + e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> + if (e->pc != pc)
> break;
> - e->is_stmt = 0;
> + subfile->line_vector->nitems--;
> }
> - while (e > subfile->line_vector->item);
> }
>
> e = subfile->line_vector->item + subfile->line_vector->nitems++;
> --
> 1.9.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-09 9:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 1:35 [PATCH] Partially revert my UB fix in record_line Bernd Edlinger
2020-04-09 9:33 ` Andrew Burgess
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).