Patchwork [review] Handle %I64d in format_pieces

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Nov. 15, 2019, 4:24 p.m.
Message ID <gerrit.1573835099000.If335c7c2fc8d01e629cd55182394a483334d79c7@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/35943/
State New
Headers show

Comments

Simon Marchi (Code Review) - Nov. 15, 2019, 4:24 p.m.
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(-)
Simon Marchi (Code Review) - Nov. 21, 2019, 9:40 p.m.
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.

Patch

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 */