* [obv] Code cleanup: Move print_command_1 expr variable scope @ 2015-03-26 17:46 Jan Kratochvil 2015-03-27 19:15 ` Simon Marchi 0 siblings, 1 reply; 4+ messages in thread From: Jan Kratochvil @ 2015-03-26 17:46 UTC (permalink / raw) To: gdb-patches [-- Attachment #0: Type: message/rfc822, Size: 1567 bytes --] From: Jan Kratochvil <jan.kratochvil@redhat.com> Subject: [PATCH] Code cleanup: Move print_command_1 expr variable scope Date: Thu, 26 Mar 2015 18:44:38 +0100 gdb/ChangeLog 2015-03-26 Jan Kratochvil <jan.kratochvil@redhat.com> Code cleanup. * printcmd.c (print_command_1): Move expr variable scope. --- gdb/ChangeLog | 5 +++++ gdb/printcmd.c | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 55a8383..94ef5c8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,6 +1,11 @@ 2015-03-26 Jan Kratochvil <jan.kratochvil@redhat.com> Code cleanup. + * printcmd.c (print_command_1): Move expr variable scope. + +2015-03-26 Jan Kratochvil <jan.kratochvil@redhat.com> + + Code cleanup. * printcmd.c (validate_format): Make the parameter cmdname const. 2015-03-26 Don Breazeal <donb@codesourcery.com> diff --git a/gdb/printcmd.c b/gdb/printcmd.c index deb501a..a1451f8 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -946,7 +946,6 @@ validate_format (struct format_data fmt, const char *cmdname) static void print_command_1 (const char *exp, int voidprint) { - struct expression *expr; struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); char format = 0; struct value *val; @@ -969,6 +968,8 @@ print_command_1 (const char *exp, int voidprint) if (exp && *exp) { + struct expression *expr; + expr = parse_expression (exp); make_cleanup (free_current_contents, &expr); val = evaluate_expression (expr); -- 2.3.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [obv] Code cleanup: Move print_command_1 expr variable scope 2015-03-26 17:46 [obv] Code cleanup: Move print_command_1 expr variable scope Jan Kratochvil @ 2015-03-27 19:15 ` Simon Marchi 2015-03-27 19:22 ` Revert: " Jan Kratochvil 0 siblings, 1 reply; 4+ messages in thread From: Simon Marchi @ 2015-03-27 19:15 UTC (permalink / raw) To: Jan Kratochvil, gdb-patches Hi Jan, I think this patch is wrong. Starting with that commit (f30d5c7), some tests (e.g. mi-break.exp) started to fail for me, because of gdb segfaulting. Backtrace here: http://paste.ubuntu.com/10690836/ The address of expr is passed to the cleanup. When the cleanup is ran, expr is no longer in scope, so what is at that address is probably not safe to use anymore. That's my guess. Simon ^ permalink raw reply [flat|nested] 4+ messages in thread
* Revert: [obv] Code cleanup: Move print_command_1 expr variable scope 2015-03-27 19:15 ` Simon Marchi @ 2015-03-27 19:22 ` Jan Kratochvil 2015-03-27 19:24 ` Simon Marchi 0 siblings, 1 reply; 4+ messages in thread From: Jan Kratochvil @ 2015-03-27 19:22 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 618 bytes --] Hi Simon, On Fri, 27 Mar 2015 20:15:13 +0100, Simon Marchi wrote: > I think this patch is wrong. Starting with that commit (f30d5c7), > some tests (e.g. mi-break.exp) started to fail for me, because > of gdb segfaulting. Backtrace here: http://paste.ubuntu.com/10690836/ > > The address of expr is passed to the cleanup. When the cleanup is ran, > expr is no longer in scope, so what is at that address is probably not > safe to use anymore. That's my guess. yes, you are sure right, I have reverted it now. Sorry I made that commit somehow automatically, not expecting it may have any side effects. Thanks, Jan [-- Attachment #2: Type: message/rfc822, Size: 1990 bytes --] From: Jan Kratochvil <jan.kratochvil@redhat.com> Subject: [PATCH] Revert: Code cleanup: Move print_command_1 expr variable scope Date: Fri, 27 Mar 2015 20:19:37 +0100 Simon Marchi: I think this patch is wrong. Starting with that commit (f30d5c7), some tests (e.g. mi-break.exp) started to fail for me, because of gdb segfaulting. The address of expr is passed to the cleanup. When the cleanup is ran, expr is no longer in scope, so what is at that address is probably not safe to use anymore. That's my guess. gdb/ChangeLog 2015-03-27 Jan Kratochvil <jan.kratochvil@redhat.com> Revert: 2015-03-26 Jan Kratochvil <jan.kratochvil@redhat.com> Code cleanup. * printcmd.c (print_command_1): Move expr variable scope. --- gdb/ChangeLog | 7 +++++++ gdb/printcmd.c | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 72940b0..6c6b94e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2015-03-27 Jan Kratochvil <jan.kratochvil@redhat.com> + + Revert: + 2015-03-26 Jan Kratochvil <jan.kratochvil@redhat.com> + Code cleanup. + * printcmd.c (print_command_1): Move expr variable scope. + 2015-03-27 Joel Brobecker <brobecker@adacore.com> * dtrace-probe.c (dtrace_process_dof_probe): Initialize expr to NULL. diff --git a/gdb/printcmd.c b/gdb/printcmd.c index a1451f8..deb501a 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -946,6 +946,7 @@ validate_format (struct format_data fmt, const char *cmdname) static void print_command_1 (const char *exp, int voidprint) { + struct expression *expr; struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); char format = 0; struct value *val; @@ -968,8 +969,6 @@ print_command_1 (const char *exp, int voidprint) if (exp && *exp) { - struct expression *expr; - expr = parse_expression (exp); make_cleanup (free_current_contents, &expr); val = evaluate_expression (expr); -- 2.1.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Revert: [obv] Code cleanup: Move print_command_1 expr variable scope 2015-03-27 19:22 ` Revert: " Jan Kratochvil @ 2015-03-27 19:24 ` Simon Marchi 0 siblings, 0 replies; 4+ messages in thread From: Simon Marchi @ 2015-03-27 19:24 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches On 15-03-27 03:22 PM, Jan Kratochvil wrote: > Hi Simon, > > On Fri, 27 Mar 2015 20:15:13 +0100, Simon Marchi wrote: >> I think this patch is wrong. Starting with that commit (f30d5c7), >> some tests (e.g. mi-break.exp) started to fail for me, because >> of gdb segfaulting. Backtrace here: http://paste.ubuntu.com/10690836/ >> >> The address of expr is passed to the cleanup. When the cleanup is ran, >> expr is no longer in scope, so what is at that address is probably not >> safe to use anymore. That's my guess. > > yes, you are sure right, I have reverted it now. > > Sorry I made that commit somehow automatically, not expecting it may have any > side effects. I would have done the same. I must say I was very surprised when git bisect told me that was the culprit. Very very sneaky bug... > Thanks, > Jan > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-27 19:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-26 17:46 [obv] Code cleanup: Move print_command_1 expr variable scope Jan Kratochvil 2015-03-27 19:15 ` Simon Marchi 2015-03-27 19:22 ` Revert: " Jan Kratochvil 2015-03-27 19:24 ` Simon Marchi
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).