Make parse_static_tracepoint_marker_definition work with multiple static tracepoint definitions

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
This commit is contained in:
Simon Marchi 2018-03-22 00:26:37 -04:00 committed by Simon Marchi
parent 92c00f4228
commit 62c222b6d9
4 changed files with 91 additions and 3 deletions

View File

@ -1,3 +1,12 @@
2018-03-22 Simon Marchi <simon.marchi@polymtl.ca>
* 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.
2018-03-21 Pedro Franco de Carvalho <pedromfc@linux.vnet.ibm.com>
* MAINTAINERS (Write After Approval): Add Pedro Franco de

View File

@ -430,6 +430,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/scoped_fd-selftests.c \
unittests/scoped_mmap-selftests.c \
unittests/scoped_restore-selftests.c \
unittests/tracepoint-selftests.c \
unittests/unpack-selftests.c \
unittests/utils-selftests.c \
unittests/xml-utils-selftests.c

View File

@ -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

View File

@ -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 <http://www.gnu.org/licenses/>. */
#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);
}