* [review] Handle %I64d in format_pieces
@ 2019-11-15 16:25 Tom Tromey (Code Review)
2019-11-21 21:40 ` Tom Tromey (Code Review)
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-15 16:25 UTC (permalink / raw)
To: gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/656
......................................................................
Handle %I64d in format_pieces
We found a bug internally where gdb would crash while disassembling a
certain instruction. This was tracked down to the handling of %I64d
in format_pieces.
format_pieces will convert %ll to %I64d on mingw -- so format_pieces
should also handle parsing this format. In this patch, I've made the
parsing unconditional, since I think it is harmless to accept extra
formats. I've also taken the opportunity to convert the length
modifier test to a "switch".
Tested internally using our failing test case.
gdb/ChangeLog
2019-11-15 Tom Tromey <tromey@adacore.com>
* gdbsupport/format.c (format_pieces): Parse %I64d.
* unittests/format_pieces-selftests.c (test_windows_formats): New
function.
(run_tests): Call it.
Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
---
M gdb/ChangeLog
M gdb/gdbsupport/format.c
M gdb/unittests/format_pieces-selftests.c
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 134c883..d7598ec 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-15 Tom Tromey <tromey@adacore.com>
+
+ * gdbsupport/format.c (format_pieces): Parse %I64d.
+ * unittests/format_pieces-selftests.c (test_windows_formats): New
+ function.
+ (run_tests): Call it.
+
2019-11-14 Christian Biesinger <cbiesinger@google.com>
* README (`configure' options): Update.
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 2e2d90a..67daa6d 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -126,6 +126,7 @@
int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
+ bool seen_i64 = false;
/* Skip over "%%", it will become part of a literal piece. */
if (*f == '%')
@@ -195,13 +196,13 @@
}
/* The next part of a format specifier is a length modifier. */
- if (*f == 'h')
+ switch (*f)
{
+ case 'h':
seen_h = 1;
f++;
- }
- else if (*f == 'l')
- {
+ break;
+ case 'l':
f++;
lcount++;
if (*f == 'l')
@@ -209,21 +210,18 @@
f++;
lcount++;
}
- }
- else if (*f == 'L')
- {
+ break;
+ case 'L':
seen_big_l = 1;
f++;
- }
- /* Decimal32 modifier. */
- else if (*f == 'H')
- {
+ break;
+ case 'H':
+ /* Decimal32 modifier. */
seen_big_h = 1;
f++;
- }
- /* Decimal64 and Decimal128 modifiers. */
- else if (*f == 'D')
- {
+ break;
+ case 'D':
+ /* Decimal64 and Decimal128 modifiers. */
f++;
/* Check for a Decimal128. */
@@ -234,13 +232,24 @@
}
else
seen_big_d = 1;
- }
- /* For size_t or ssize_t. */
- else if (*f == 'z')
- {
+ break;
+ case 'z':
+ /* For size_t or ssize_t. */
seen_size_t = 1;
f++;
- }
+ break;
+ case 'I':
+ /* Support the Windows '%I64' extension, because an
+ earlier call to format_pieces might have converted %lld
+ to %I64d. */
+ if (f[1] == '6' && f[2] == '4')
+ {
+ f += 3;
+ lcount = 2;
+ seen_i64 = true;
+ }
+ break;
+ }
switch (*f)
{
@@ -353,7 +362,7 @@
sub_start = current_substring;
- if (lcount > 1 && USE_PRINTF_I64)
+ if (lcount > 1 && !seen_i64 && USE_PRINTF_I64)
{
/* Windows' printf does support long long, but not the usual way.
Convert %lld to %I64d. */
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index 3971201..d7e97d4 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -120,12 +120,23 @@
}
static void
+test_windows_formats ()
+{
+ check ("rc%I64d",
+ {
+ format_piece ("rc", literal_piece, 0),
+ format_piece ("%I64d", long_long_arg, 0),
+ });
+}
+
+static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
test_format_int_sizes ();
+ test_windows_formats ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
Gerrit-Change-Number: 656
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newchange
^ permalink raw reply [flat|nested] 4+ messages in thread
* [review] Handle %I64d in format_pieces
2019-11-15 16:25 [review] Handle %I64d in format_pieces Tom Tromey (Code Review)
@ 2019-11-21 21:40 ` Tom Tromey (Code Review)
2019-11-21 21:44 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-21 21:44 ` Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-21 21:40 UTC (permalink / raw)
To: gdb-patches
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/656
......................................................................
Patch Set 1:
I'm checking this one in now.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
Gerrit-Change-Number: 656
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Thu, 21 Nov 2019 21:40:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pushed] Handle %I64d in format_pieces
2019-11-15 16:25 [review] Handle %I64d in format_pieces Tom Tromey (Code Review)
2019-11-21 21:40 ` Tom Tromey (Code Review)
@ 2019-11-21 21:44 ` Sourceware to Gerrit sync (Code Review)
2019-11-21 21:44 ` Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-21 21:44 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
The original change was created by Tom Tromey.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/656
......................................................................
Handle %I64d in format_pieces
We found a bug internally where gdb would crash while disassembling a
certain instruction. This was tracked down to the handling of %I64d
in format_pieces.
format_pieces will convert %ll to %I64d on mingw -- so format_pieces
should also handle parsing this format. In this patch, I've made the
parsing unconditional, since I think it is harmless to accept extra
formats. I've also taken the opportunity to convert the length
modifier test to a "switch".
Tested internally using our failing test case.
gdb/ChangeLog
2019-11-21 Tom Tromey <tromey@adacore.com>
* gdbsupport/format.c (format_pieces): Parse %I64d.
* unittests/format_pieces-selftests.c (test_windows_formats): New
function.
(run_tests): Call it.
Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
---
M gdb/ChangeLog
M gdb/gdbsupport/format.c
M gdb/unittests/format_pieces-selftests.c
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0249048..0c81de4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-21 Tom Tromey <tromey@adacore.com>
+
+ * gdbsupport/format.c (format_pieces): Parse %I64d.
+ * unittests/format_pieces-selftests.c (test_windows_formats): New
+ function.
+ (run_tests): Call it.
+
2019-11-21 Peeter Joot <peeter.joot@lzlabs.com>
Byte reverse display of variables with DW_END_big, DW_END_little
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 2e2d90a..67daa6d 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -126,6 +126,7 @@
int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
+ bool seen_i64 = false;
/* Skip over "%%", it will become part of a literal piece. */
if (*f == '%')
@@ -195,13 +196,13 @@
}
/* The next part of a format specifier is a length modifier. */
- if (*f == 'h')
+ switch (*f)
{
+ case 'h':
seen_h = 1;
f++;
- }
- else if (*f == 'l')
- {
+ break;
+ case 'l':
f++;
lcount++;
if (*f == 'l')
@@ -209,21 +210,18 @@
f++;
lcount++;
}
- }
- else if (*f == 'L')
- {
+ break;
+ case 'L':
seen_big_l = 1;
f++;
- }
- /* Decimal32 modifier. */
- else if (*f == 'H')
- {
+ break;
+ case 'H':
+ /* Decimal32 modifier. */
seen_big_h = 1;
f++;
- }
- /* Decimal64 and Decimal128 modifiers. */
- else if (*f == 'D')
- {
+ break;
+ case 'D':
+ /* Decimal64 and Decimal128 modifiers. */
f++;
/* Check for a Decimal128. */
@@ -234,13 +232,24 @@
}
else
seen_big_d = 1;
- }
- /* For size_t or ssize_t. */
- else if (*f == 'z')
- {
+ break;
+ case 'z':
+ /* For size_t or ssize_t. */
seen_size_t = 1;
f++;
- }
+ break;
+ case 'I':
+ /* Support the Windows '%I64' extension, because an
+ earlier call to format_pieces might have converted %lld
+ to %I64d. */
+ if (f[1] == '6' && f[2] == '4')
+ {
+ f += 3;
+ lcount = 2;
+ seen_i64 = true;
+ }
+ break;
+ }
switch (*f)
{
@@ -353,7 +362,7 @@
sub_start = current_substring;
- if (lcount > 1 && USE_PRINTF_I64)
+ if (lcount > 1 && !seen_i64 && USE_PRINTF_I64)
{
/* Windows' printf does support long long, but not the usual way.
Convert %lld to %I64d. */
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index 3971201..d7e97d4 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -120,12 +120,23 @@
}
static void
+test_windows_formats ()
+{
+ check ("rc%I64d",
+ {
+ format_piece ("rc", literal_piece, 0),
+ format_piece ("%I64d", long_long_arg, 0),
+ });
+}
+
+static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
test_format_int_sizes ();
+ test_windows_formats ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
Gerrit-Change-Number: 656
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pushed] Handle %I64d in format_pieces
2019-11-15 16:25 [review] Handle %I64d in format_pieces Tom Tromey (Code Review)
2019-11-21 21:40 ` Tom Tromey (Code Review)
2019-11-21 21:44 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-21 21:44 ` Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-21 21:44 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/656
......................................................................
Handle %I64d in format_pieces
We found a bug internally where gdb would crash while disassembling a
certain instruction. This was tracked down to the handling of %I64d
in format_pieces.
format_pieces will convert %ll to %I64d on mingw -- so format_pieces
should also handle parsing this format. In this patch, I've made the
parsing unconditional, since I think it is harmless to accept extra
formats. I've also taken the opportunity to convert the length
modifier test to a "switch".
Tested internally using our failing test case.
gdb/ChangeLog
2019-11-21 Tom Tromey <tromey@adacore.com>
* gdbsupport/format.c (format_pieces): Parse %I64d.
* unittests/format_pieces-selftests.c (test_windows_formats): New
function.
(run_tests): Call it.
Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
---
M gdb/ChangeLog
M gdb/gdbsupport/format.c
M gdb/unittests/format_pieces-selftests.c
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0249048..0c81de4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-21 Tom Tromey <tromey@adacore.com>
+
+ * gdbsupport/format.c (format_pieces): Parse %I64d.
+ * unittests/format_pieces-selftests.c (test_windows_formats): New
+ function.
+ (run_tests): Call it.
+
2019-11-21 Peeter Joot <peeter.joot@lzlabs.com>
Byte reverse display of variables with DW_END_big, DW_END_little
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 2e2d90a..67daa6d 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -126,6 +126,7 @@
int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
+ bool seen_i64 = false;
/* Skip over "%%", it will become part of a literal piece. */
if (*f == '%')
@@ -195,13 +196,13 @@
}
/* The next part of a format specifier is a length modifier. */
- if (*f == 'h')
+ switch (*f)
{
+ case 'h':
seen_h = 1;
f++;
- }
- else if (*f == 'l')
- {
+ break;
+ case 'l':
f++;
lcount++;
if (*f == 'l')
@@ -209,21 +210,18 @@
f++;
lcount++;
}
- }
- else if (*f == 'L')
- {
+ break;
+ case 'L':
seen_big_l = 1;
f++;
- }
- /* Decimal32 modifier. */
- else if (*f == 'H')
- {
+ break;
+ case 'H':
+ /* Decimal32 modifier. */
seen_big_h = 1;
f++;
- }
- /* Decimal64 and Decimal128 modifiers. */
- else if (*f == 'D')
- {
+ break;
+ case 'D':
+ /* Decimal64 and Decimal128 modifiers. */
f++;
/* Check for a Decimal128. */
@@ -234,13 +232,24 @@
}
else
seen_big_d = 1;
- }
- /* For size_t or ssize_t. */
- else if (*f == 'z')
- {
+ break;
+ case 'z':
+ /* For size_t or ssize_t. */
seen_size_t = 1;
f++;
- }
+ break;
+ case 'I':
+ /* Support the Windows '%I64' extension, because an
+ earlier call to format_pieces might have converted %lld
+ to %I64d. */
+ if (f[1] == '6' && f[2] == '4')
+ {
+ f += 3;
+ lcount = 2;
+ seen_i64 = true;
+ }
+ break;
+ }
switch (*f)
{
@@ -353,7 +362,7 @@
sub_start = current_substring;
- if (lcount > 1 && USE_PRINTF_I64)
+ if (lcount > 1 && !seen_i64 && USE_PRINTF_I64)
{
/* Windows' printf does support long long, but not the usual way.
Convert %lld to %I64d. */
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index 3971201..d7e97d4 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -120,12 +120,23 @@
}
static void
+test_windows_formats ()
+{
+ check ("rc%I64d",
+ {
+ format_piece ("rc", literal_piece, 0),
+ format_piece ("%I64d", long_long_arg, 0),
+ });
+}
+
+static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
test_format_int_sizes ();
+ test_windows_formats ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If335c7c2fc8d01e629cd55182394a483334d79c7
Gerrit-Change-Number: 656
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-21 21:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 16:25 [review] Handle %I64d in format_pieces Tom Tromey (Code Review)
2019-11-21 21:40 ` Tom Tromey (Code Review)
2019-11-21 21:44 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-21 21:44 ` Sourceware to Gerrit sync (Code Review)
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).