* [PATCH] Fix -trace-save crash when argument is missing @ 2016-10-14 20:26 Simon Marchi 2016-10-17 20:09 ` Yao Qi 0 siblings, 1 reply; 8+ messages in thread From: Simon Marchi @ 2016-10-14 20:26 UTC (permalink / raw) To: gdb-patches; +Cc: Simon Marchi -trace-save doesn't check whether an argument is passed, leading to a segfault if you pass nothing. I added a small test, which only tests the error conditions of -trace-save. gdb/ChangeLog: * mi/mi-main.c (mi_cmd_trace_save): Check if argument is present before using it. gdb/testsuite/ChangeLog: * gdb.trace/mi-trace-save.exp: New file. --- gdb/mi/mi-main.c | 4 +++ gdb/testsuite/gdb.trace/mi-trace-save.exp | 41 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 gdb/testsuite/gdb.trace/mi-trace-save.exp diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 7cd9706..479d3a4 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2699,6 +2699,10 @@ mi_cmd_trace_save (char *command, char **argv, int argc) break; } } + + if (oind >= argc) + error (_("Argument required (file in which to save trace data)")); + filename = argv[oind]; if (generate_ctf) diff --git a/gdb/testsuite/gdb.trace/mi-trace-save.exp b/gdb/testsuite/gdb.trace/mi-trace-save.exp new file mode 100644 index 0000000..52ad9f1 --- /dev/null +++ b/gdb/testsuite/gdb.trace/mi-trace-save.exp @@ -0,0 +1,41 @@ +# Copyright 2016 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 <http://www.gnu.org/licenses/>. + +# The -trace-save command is already tested in other files (e.g. +# mi-trace-frame-collected.exp), so this file currently only tests the error +# cases of -trace-save. + +load_lib mi-support.exp + +mi_gdb_start + +# Test without the required "filename" argument. + +proc test_trace_save_missing_arg { } { + set err_re [string_to_regexp "^error,msg=\"Argument required (file in which to save trace data)\""] + + mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" +} + +# Test with an unrecognized option. + +proc test_trace_save_wrong_opt { } { + set err_re [string_to_regexp "^error,msg=\"-trace-save: Unknown option ``hey''\""] + + mi_gdb_test "-trace-save -hey" "$err_re" "-trace-save with wrong option" +} + +test_trace_save_missing_arg +test_trace_save_wrong_opt -- 2.10.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-14 20:26 [PATCH] Fix -trace-save crash when argument is missing Simon Marchi @ 2016-10-17 20:09 ` Yao Qi 2016-10-17 20:37 ` Simon Marchi 0 siblings, 1 reply; 8+ messages in thread From: Yao Qi @ 2016-10-17 20:09 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches HI Simon, Patch is good to me. A nit below, On Fri, Oct 14, 2016 at 4:25 PM, Simon Marchi <simon.marchi@ericsson.com> wrote: > + > + if (oind >= argc) Nit: if (argc - oind != 1) > + error (_("Argument required (file in which to save trace data)")); > + > filename = argv[oind]; > -- Yao (齐尧) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 20:09 ` Yao Qi @ 2016-10-17 20:37 ` Simon Marchi 2016-10-17 20:50 ` Simon Marchi 0 siblings, 1 reply; 8+ messages in thread From: Simon Marchi @ 2016-10-17 20:37 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 16-10-17 04:08 PM, Yao Qi wrote: > HI Simon, > Patch is good to me. A nit below, > > On Fri, Oct 14, 2016 at 4:25 PM, Simon Marchi <simon.marchi@ericsson.com> wrote: >> + >> + if (oind >= argc) > > Nit: if (argc - oind != 1) > >> + error (_("Argument required (file in which to save trace data)")); >> + Good idea, otherwise giving too many arguments still works, when it shouldn't. I'll change the error message to: "Exactly one argument required ..." Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 20:37 ` Simon Marchi @ 2016-10-17 20:50 ` Simon Marchi 2016-10-17 20:56 ` Simon Marchi 2016-10-17 20:59 ` Pedro Alves 0 siblings, 2 replies; 8+ messages in thread From: Simon Marchi @ 2016-10-17 20:50 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 16-10-17 04:36 PM, Simon Marchi wrote: > On 16-10-17 04:08 PM, Yao Qi wrote: >> HI Simon, >> Patch is good to me. A nit below, >> >> On Fri, Oct 14, 2016 at 4:25 PM, Simon Marchi <simon.marchi@ericsson.com> wrote: >>> + >>> + if (oind >= argc) >> >> Nit: if (argc - oind != 1) >> >>> + error (_("Argument required (file in which to save trace data)")); >>> + > > Good idea, otherwise giving too many arguments still works, when it > shouldn't. I'll change the error message to: > > "Exactly one argument required ..." > > Thanks. Pushed. Here is the final version: From 5bad3170301060ee0801a739ffc213abae664973 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@ericsson.com> Date: Mon, 17 Oct 2016 16:47:36 -0400 Subject: [PATCH] Fix -trace-save crash when argument is missing -trace-save doesn't check whether an argument is passed, leading to a segfault if you pass nothing. I added a small test, which only tests the error conditions of -trace-save. gdb/ChangeLog: * mi/mi-main.c (mi_cmd_trace_save): Check if argument is present before using it. gdb/testsuite/ChangeLog: * gdb.trace/mi-trace-save.exp: New file. --- gdb/ChangeLog | 5 ++++ gdb/mi/mi-main.c | 5 ++++ gdb/testsuite/ChangeLog | 4 +++ gdb/testsuite/gdb.trace/mi-trace-save.exp | 42 +++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 gdb/testsuite/gdb.trace/mi-trace-save.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4ebbf8d..4f706fd 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2016-10-17 Simon Marchi <simon.marchi@ericsson.com> + + * mi/mi-main.c (mi_cmd_trace_save): Check if argument is present + before using it. + 2016-10-17 Pedro Alves <palves@redhat.com> * charset.h (class wchar_iterator) [PHONY_ICONV] <m_desc>: Use diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 7cd9706..81e82ed 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2699,6 +2699,11 @@ mi_cmd_trace_save (char *command, char **argv, int argc) break; } } + + if (argc - oind != 1) + error (_("Exactly one argument required " + "(file in which to save trace data)")); + filename = argv[oind]; if (generate_ctf) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index ac2d05d..f0c8415 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2016-10-17 Simon Marchi <simon.marchi@ericsson.com> + + * gdb.trace/mi-trace-save.exp: New file. + 2016-10-13 Yao Qi <yao.qi@linaro.org> * gdb.base/code_elim.exp (get_var_address): Remove. diff --git a/gdb/testsuite/gdb.trace/mi-trace-save.exp b/gdb/testsuite/gdb.trace/mi-trace-save.exp new file mode 100644 index 0000000..96bb70d --- /dev/null +++ b/gdb/testsuite/gdb.trace/mi-trace-save.exp @@ -0,0 +1,42 @@ +# Copyright 2016 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 <http://www.gnu.org/licenses/>. + +# The -trace-save command is already tested in other files (e.g. +# mi-trace-frame-collected.exp), so this file currently only tests the error +# cases of -trace-save. + +load_lib mi-support.exp + +mi_gdb_start + +# Test without the required "filename" argument. + +proc test_trace_save_wrong_num_args { } { + set err_re [string_to_regexp "^error,msg=\"Exactly one argument required (file in which to save trace data)\""] + + mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" + mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with missing argument" +} + +# Test with an unrecognized option. + +proc test_trace_save_wrong_opt { } { + set err_re [string_to_regexp "^error,msg=\"-trace-save: Unknown option ``hey''\""] + + mi_gdb_test "-trace-save -hey" "$err_re" "-trace-save with wrong option" +} + +test_trace_save_wrong_num_args +test_trace_save_wrong_opt -- 2.10.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 20:50 ` Simon Marchi @ 2016-10-17 20:56 ` Simon Marchi 2016-10-17 20:59 ` Pedro Alves 1 sibling, 0 replies; 8+ messages in thread From: Simon Marchi @ 2016-10-17 20:56 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 16-10-17 04:49 PM, Simon Marchi wrote: > Pushed. Here is the final version: ... Oops, I had forgotten to update a comment. I pushed this quick fix: From 3ccdb4324b0dc9fa46ee7cad9b370f8c7c370c3b Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@ericsson.com> Date: Mon, 17 Oct 2016 16:54:24 -0400 Subject: [PATCH] Fix comment in mi-trace-save.exp This fixes a comment I forgot to update in the previous patch. gdb/testsuite/ChangeLog: * gdb.trace/mi-trace-save.exp (test_trace_save_wrong_num_args): Update comment. --- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.trace/mi-trace-save.exp | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f0c8415..9d35283 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2016-10-17 Simon Marchi <simon.marchi@ericsson.com> + * gdb.trace/mi-trace-save.exp (test_trace_save_wrong_num_args): + Update comment. + +2016-10-17 Simon Marchi <simon.marchi@ericsson.com> + * gdb.trace/mi-trace-save.exp: New file. 2016-10-13 Yao Qi <yao.qi@linaro.org> diff --git a/gdb/testsuite/gdb.trace/mi-trace-save.exp b/gdb/testsuite/gdb.trace/mi-trace-save.exp index 96bb70d..edb3a59 100644 --- a/gdb/testsuite/gdb.trace/mi-trace-save.exp +++ b/gdb/testsuite/gdb.trace/mi-trace-save.exp @@ -21,7 +21,7 @@ load_lib mi-support.exp mi_gdb_start -# Test without the required "filename" argument. +# Test with the wrong number of arguments. proc test_trace_save_wrong_num_args { } { set err_re [string_to_regexp "^error,msg=\"Exactly one argument required (file in which to save trace data)\""] -- 2.10.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 20:50 ` Simon Marchi 2016-10-17 20:56 ` Simon Marchi @ 2016-10-17 20:59 ` Pedro Alves 2016-10-17 21:07 ` Simon Marchi 1 sibling, 1 reply; 8+ messages in thread From: Pedro Alves @ 2016-10-17 20:59 UTC (permalink / raw) To: Simon Marchi, Yao Qi; +Cc: gdb-patches On 10/17/2016 09:49 PM, Simon Marchi wrote: > + mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" > + mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with missing argument" Duplicate test messages? -- Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 20:59 ` Pedro Alves @ 2016-10-17 21:07 ` Simon Marchi 2016-10-17 21:08 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Simon Marchi @ 2016-10-17 21:07 UTC (permalink / raw) To: Pedro Alves, Yao Qi; +Cc: gdb-patches On 16-10-17 04:59 PM, Pedro Alves wrote: > On 10/17/2016 09:49 PM, Simon Marchi wrote: >> + mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" >> + mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with missing argument" > > Duplicate test messages? > Ahh damnit! Just when I thought it was an obvious change :). Thanks for pointing it out. From e42b25a0407fbbf3529815f69bd56a61b1821295 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@ericsson.com> Date: Mon, 17 Oct 2016 17:05:46 -0400 Subject: [PATCH] Fix duplicate test message in mi-trace-save.exp gdb/testsuite/ChangeLog: * gdb.trace/mi-trace-save.exp (test_trace_save_wrong_num_args): Change test message. --- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.trace/mi-trace-save.exp | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 9d35283..4ecf0b7 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,6 +1,11 @@ 2016-10-17 Simon Marchi <simon.marchi@ericsson.com> * gdb.trace/mi-trace-save.exp (test_trace_save_wrong_num_args): + Change test message. + +2016-10-17 Simon Marchi <simon.marchi@ericsson.com> + + * gdb.trace/mi-trace-save.exp (test_trace_save_wrong_num_args): Update comment. 2016-10-17 Simon Marchi <simon.marchi@ericsson.com> diff --git a/gdb/testsuite/gdb.trace/mi-trace-save.exp b/gdb/testsuite/gdb.trace/mi-trace-save.exp index edb3a59..156ec54 100644 --- a/gdb/testsuite/gdb.trace/mi-trace-save.exp +++ b/gdb/testsuite/gdb.trace/mi-trace-save.exp @@ -27,7 +27,7 @@ proc test_trace_save_wrong_num_args { } { set err_re [string_to_regexp "^error,msg=\"Exactly one argument required (file in which to save trace data)\""] mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" - mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with missing argument" + mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with too many arguments" } # Test with an unrecognized option. -- 2.10.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix -trace-save crash when argument is missing 2016-10-17 21:07 ` Simon Marchi @ 2016-10-17 21:08 ` Pedro Alves 0 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-10-17 21:08 UTC (permalink / raw) To: Simon Marchi, Yao Qi; +Cc: gdb-patches On 10/17/2016 10:06 PM, Simon Marchi wrote: > On 16-10-17 04:59 PM, Pedro Alves wrote: >> On 10/17/2016 09:49 PM, Simon Marchi wrote: >>> + mi_gdb_test "-trace-save" "$err_re" "-trace-save with missing argument" >>> + mi_gdb_test "-trace-save a b" "$err_re" "-trace-save with missing argument" >> >> Duplicate test messages? >> > > Ahh damnit! Just when I thought it was an obvious change :). Thanks for pointing it out. This one looks obvious to me. :-) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-17 21:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-14 20:26 [PATCH] Fix -trace-save crash when argument is missing Simon Marchi 2016-10-17 20:09 ` Yao Qi 2016-10-17 20:37 ` Simon Marchi 2016-10-17 20:50 ` Simon Marchi 2016-10-17 20:56 ` Simon Marchi 2016-10-17 20:59 ` Pedro Alves 2016-10-17 21:07 ` Simon Marchi 2016-10-17 21:08 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox