From patchwork Mon Feb 19 03:35:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 25959 Received: (qmail 85566 invoked by alias); 19 Feb 2018 03:35:46 -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 85477 invoked by uid 89); 19 Feb 2018 03:35:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy= X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Feb 2018 03:35:44 +0000 X-ASG-Debug-ID: 1519011331-0c856e65d54cf6e60001-fS2M51 Received: from smtp.ebox.ca (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id C7B6ZtJ6hSilWN84 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 18 Feb 2018 22:35:31 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.lan (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) by smtp.ebox.ca (Postfix) with ESMTP id D1A17441B21; Sun, 18 Feb 2018 22:35:31 -0500 (EST) From: Simon Marchi X-Barracuda-Effective-Source-IP: 192-222-251-162.qc.cable.ebox.net[192.222.251.162] X-Barracuda-Apparent-Source-IP: 192.222.251.162 X-Barracuda-RBL-IP: 192.222.251.162 To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 1/2] Make parse_static_tracepoint_marker_definition work with multiple static tracepoint definitions Date: Sun, 18 Feb 2018 22:35:30 -0500 X-ASG-Orig-Subj: [PATCH 1/2] Make parse_static_tracepoint_marker_definition work with multiple static tracepoint definitions Message-Id: <20180219033531.19033-1-simon.marchi@polymtl.ca> X-Barracuda-Connect: smtp.electronicbox.net[96.127.255.82] X-Barracuda-Start-Time: 1519011331 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 5802 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.48071 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-IsSubscribed: yes Since I modify the parse_static_tracepoint_marker_definition function in the next patch, I wanted to write a unit test for it. Doing so showed that it doesn't handle multiple consecutive static tracepoint definitions separated by commas. However, the RSP documentation [1] states that servers may return multiple definitions, like: 1234:6d61726b657231:6578747261207374756666,abba:6d61726b657232: The problem is that the function uses strlen to compute the length of the last field (the extra field). If there are additional definitions in addition to the one we are currently parsing, the returned length will include those definitions, and we'll try to hex-decode past the extra field. This patch changes parse_static_tracepoint_marker_definition to consider the case where the current definition is followed by a comma and more definitions. It also adds the unit test that found the issue in the first place. I don't think this causes any backwards compatibility issues, because the previous code only handled single static tracepoint definitions, and the new code handles that correctly. gdb/ChangeLog: * tracepoint.c (parse_static_tracepoint_marker_definition): Consider case where the definition is followed by more definitions. * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add tracepoint-selftests.c. * unittests/tracepoint-selftests.c: New. [1] https://sourceware.org/gdb/onlinedocs/gdb/Tracepoint-Packets.html#qTfSTM --- gdb/Makefile.in | 1 + gdb/tracepoint.c | 14 ++++++-- gdb/unittests/tracepoint-selftests.c | 70 ++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 gdb/unittests/tracepoint-selftests.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 957654c9bd..5a2f4c8cac 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -427,6 +427,7 @@ SUBDIR_UNITTESTS_SRCS = \ unittests/scoped_fd-selftests.c \ unittests/scoped_mmap-selftests.c \ unittests/scoped_restore-selftests.c \ + unittests/tracepoint-selftests.c \ unittests/xml-utils-selftests.c SUBDIR_UNITTESTS_OBS = $(patsubst %.c,%.o,$(SUBDIR_UNITTESTS_SRCS)) diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index f34f103875..843041f739 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -3720,12 +3720,20 @@ parse_static_tracepoint_marker_definition (const char *line, const char **pp, p += 2 * end; p++; /* skip a colon */ - marker->extra = (char *) xmalloc (strlen (p) + 1); - end = hex2bin (p, (gdb_byte *) marker->extra, strlen (p) / 2); + /* This definition may be followed by another one, separated by a comma. */ + int hex_len; + endp = strchr (p, ','); + if (endp != nullptr) + hex_len = endp - p; + else + hex_len = strlen (p); + + marker->extra = (char *) xmalloc (hex_len / 2 + 1); + end = hex2bin (p, (gdb_byte *) marker->extra, hex_len / 2); marker->extra[end] = '\0'; if (pp) - *pp = p; + *pp = p + hex_len; } /* Release a static tracepoint marker's contents. Note that the diff --git a/gdb/unittests/tracepoint-selftests.c b/gdb/unittests/tracepoint-selftests.c new file mode 100644 index 0000000000..21ca3d0914 --- /dev/null +++ b/gdb/unittests/tracepoint-selftests.c @@ -0,0 +1,70 @@ +/* Self tests for tracepoint-related code for GDB, the GNU debugger. + + Copyright (C) 2018 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +#include "defs.h" +#include "selftest.h" +#include "tracepoint.h" + +namespace selftests { +namespace tracepoint_tests { + +static void +test_parse_static_tracepoint_marker_definition () +{ + static_tracepoint_marker marker; + const char def[] = ("1234:6d61726b657231:6578747261207374756666," + "abba:6d61726b657232:," + "cafe:6d61726b657233:6d6f72657374756666"); + const char *start = def; + const char *end; + + parse_static_tracepoint_marker_definition (start, &end, &marker); + + SELF_CHECK (marker.address == 0x1234); + SELF_CHECK (strcmp (marker.str_id, "marker1") == 0); + SELF_CHECK (strcmp (marker.extra, "extra stuff") == 0); + SELF_CHECK (end == strchr (start, ',')); + + start = end + 1; + parse_static_tracepoint_marker_definition (start, &end, &marker); + + SELF_CHECK (marker.address == 0xabba); + SELF_CHECK (strcmp (marker.str_id, "marker2") == 0); + SELF_CHECK (strcmp (marker.extra, "") == 0); + SELF_CHECK (end == strchr (start, ',')); + + start = end + 1; + parse_static_tracepoint_marker_definition (start, &end, &marker); + + SELF_CHECK (marker.address == 0xcafe); + SELF_CHECK (strcmp (marker.str_id, "marker3") == 0); + SELF_CHECK (strcmp (marker.extra, "morestuff") == 0); + SELF_CHECK (end == def + strlen (def)); +} + +} /* namespace tracepoint_tests */ +} /* namespace selftests */ + +void +_initialize_tracepoint_selftests () +{ + selftests::register_test + ("parse_static_tracepoint_marker_definition", + selftests::tracepoint_tests::test_parse_static_tracepoint_marker_definition); +}