From patchwork Thu May 30 19:53:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 32930 Received: (qmail 124428 invoked by alias); 30 May 2019 19:53:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124384 invoked by uid 89); 30 May 2019 19:53:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=3.1, corner X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 May 2019 19:53:38 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8E15585541 for ; Thu, 30 May 2019 19:53:37 +0000 (UTC) Received: from localhost.localdomain (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 289927A549 for ; Thu, 30 May 2019 19:53:36 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 03/24] Fix TID parser bug Date: Thu, 30 May 2019 20:53:12 +0100 Message-Id: <20190530195333.20448-4-palves@redhat.com> In-Reply-To: <20190530195333.20448-1-palves@redhat.com> References: <20190530195333.20448-1-palves@redhat.com> I noticed this inconsistency in the error messages below: (gdb) print --1 Left operand of assignment is not an lvalue. (gdb) thread apply 1 print --1 Thread 1 (Thread 0x7ffff7fb6740 (LWP 17805)): inverted range The "inverted range" error happens because get_number_trailer returns 0 to indicate error, but number_or_range_parser::get_number is not checking for that. I tried detected the error there, but that doesn't work because number_of_range_parser is used in places that _do_ want to legitimately handle 0. IMO we should fix get_number_trailer's interface or use something else when we want to parse 0 too. I've decided to fix it in a different way, similarly to how number_or_range_parser::finished was changed in commit 529c08b25ec7 ("Add helper functions parse_flags and parse_flags_qcs"). Seems like a good change, even if we tweaked number_or_range_parser::get_number, as it simplifies thread_apply_command and makes them consistent with number_or_range_parser::finished(). We now get the same error message in both cases: (gdb) print --1 Left operand of assignment is not an lvalue. (gdb) thread apply 1 print --1 Thread 1 (Thread 0x7ffff7fb6740 (LWP 17805)): Left operand of assignment is not an lvalue. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * thread.c (thread_apply_command): Adjust TID parsing. * tid-parse.c (tid_range_parser::finished): Ensure parsing end is detected before end of string. (tid_is_in_list): Error out if LIST is invalid. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.multi/tids.exp: Adjust expected output. Add "thread apply 1 foo --1" test. --- gdb/testsuite/gdb.multi/tids.exp | 16 ++++++++++++++-- gdb/thread.c | 15 ++++++--------- gdb/tid-parse.c | 10 +++++++++- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/gdb/testsuite/gdb.multi/tids.exp b/gdb/testsuite/gdb.multi/tids.exp index 617a1b0299c..3b0e1c1860a 100644 --- a/gdb/testsuite/gdb.multi/tids.exp +++ b/gdb/testsuite/gdb.multi/tids.exp @@ -350,8 +350,13 @@ with_test_prefix "two inferiors" { thr_apply_info_thr_error "${prefix}1-" "inverted range" thr_apply_info_thr_error "${prefix}2-1" "inverted range" thr_apply_info_thr_error "${prefix}2-\$one" "inverted range" - thr_apply_info_thr_error "${prefix}-1" "negative value" - thr_apply_info_thr_error "${prefix}-\$one" "negative value" + if {$prefix == ""} { + thr_apply_info_thr_error "${prefix}-1" "Invalid thread ID: -1" + thr_apply_info_thr_error "${prefix}-\$one" "Invalid thread ID: -\\\$one" + } else { + thr_apply_info_thr_error "${prefix}-1" "negative value" + thr_apply_info_thr_error "${prefix}-\$one" "negative value" + } thr_apply_info_thr_error "${prefix}\$minus_one" \ "negative value: ${prefix_re}\\\$minus_one" @@ -374,6 +379,13 @@ with_test_prefix "two inferiors" { gdb_test "thread apply 1.*" $output } + # Check that thread ID list parsing stops at the non-number token + # "foo" in a corner case where the "foo" is followed by hyphens. + # In this corner case, GDB used to skip past "foo", and then parse + # "--1" as a tid range for the current inferior. + gdb_test "thread apply 1 foo --1" \ + "Undefined command: \"foo\". Try \"help\"\\." + # Check that we do parse the inferior number and don't confuse it. gdb_test "info threads 3.1" \ "No threads match '3.1'\." diff --git a/gdb/thread.c b/gdb/thread.c index 9a6a7735950..a84dbf9fa1e 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -1560,7 +1560,6 @@ thread_apply_command (const char *tidlist, int from_tty) { qcs_flags flags; const char *cmd = NULL; - const char *cmd_or_flags; tid_range_parser parser; if (tidlist == NULL || *tidlist == '\000') @@ -1572,17 +1571,15 @@ thread_apply_command (const char *tidlist, int from_tty) int inf_num, thr_start, thr_end; if (!parser.get_tid_range (&inf_num, &thr_start, &thr_end)) - { - cmd = parser.cur_tok (); - break; - } + break; } - cmd_or_flags = cmd; - while (cmd != NULL && parse_flags_qcs ("thread apply", &cmd, &flags)) + cmd = parser.cur_tok (); + + while (parse_flags_qcs ("thread apply", &cmd, &flags)) ; - if (cmd == NULL) + if (*cmd == '\0') error (_("Please specify a command following the thread ID list")); if (tidlist == cmd || !isalpha (cmd[0])) @@ -1591,7 +1588,7 @@ thread_apply_command (const char *tidlist, int from_tty) scoped_restore_current_thread restore_thread; parser.init (tidlist, current_inferior ()->num); - while (!parser.finished () && parser.cur_tok () < cmd_or_flags) + while (!parser.finished ()) { struct thread_info *tp = NULL; struct inferior *inf; diff --git a/gdb/tid-parse.c b/gdb/tid-parse.c index 828362ea94b..07d7d2c3b2a 100644 --- a/gdb/tid-parse.c +++ b/gdb/tid-parse.c @@ -139,7 +139,13 @@ tid_range_parser::finished () const switch (m_state) { case STATE_INFERIOR: - return *m_cur_tok == '\0'; + /* Parsing is finished when at end of string or null string, + or we are not in a range and not in front of an integer, negative + integer, convenience var or negative convenience var. */ + return (*m_cur_tok == '\0' + || !(isdigit (*m_cur_tok) + || *m_cur_tok == '$' + || *m_cur_tok == '*')); case STATE_THREAD_RANGE: case STATE_STAR_RANGE: return m_range_parser.finished (); @@ -311,6 +317,8 @@ tid_is_in_list (const char *list, int default_inferior, return 1; tid_range_parser parser (list, default_inferior); + if (parser.finished ()) + invalid_thread_id_error (parser.cur_tok ()); while (!parser.finished ()) { int tmp_inf, tmp_thr_start, tmp_thr_end;