Fix hex floating point lexing

Message ID 20200606201806.19353-1-tom@tromey.com
State New
Headers show
Series
  • Fix hex floating point lexing
Related show

Commit Message

Tom Tromey June 6, 2020, 8:18 p.m.
PR gdb/18318 notes that gdb will sometimes incorrectly handle hex
floating point input.  This turns out to be a bug in the C lexer; the
'p' was not being correctly recognized, and so the exponent was not
always passed to the floating point "from_string" method.

Tested by the buildbot "Fedora-x86_64-m64" builder.

gdb/ChangeLog
2020-06-06  Tom Tromey  <tom@tromey.com>

	PR gdb/18318:
	* c-exp.y (lex_one_token): Handle 'p' like 'e'.

gdb/testsuite/ChangeLog
2020-06-06  Tom Tromey  <tom@tromey.com>

	PR gdb/18318:
	* gdb.base/printcmds.exp (test_float_accepted): Add more hex
	floating point tests.
---
 gdb/ChangeLog                        |  5 +++++
 gdb/c-exp.y                          | 11 +++++++----
 gdb/testsuite/ChangeLog              |  6 ++++++
 gdb/testsuite/gdb.base/printcmds.exp | 27 +++++++++++++++++++--------
 4 files changed, 37 insertions(+), 12 deletions(-)

-- 
2.17.2

Comments

Simon Marchi June 10, 2020, 11:55 p.m. | #1
On 2020-06-06 4:18 p.m., Tom Tromey wrote:
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp

> index 066e7fce87b..05ababff78c 100644

> --- a/gdb/testsuite/gdb.base/printcmds.exp

> +++ b/gdb/testsuite/gdb.base/printcmds.exp

> @@ -127,14 +127,25 @@ proc test_float_accepted {} {

>      gdb_test "p 1.5l" " = 1.5"

>  

>      # Test hexadecimal floating point.

> -    set test "p 0x1.1"

> -    gdb_test_multiple $test $test {

> -	-re " = 1\\.0625\r\n$gdb_prompt $" {

> -	    pass $test

> -	}

> -	-re "Invalid number \"0x1\\.1\"\\.\r\n$gdb_prompt $" {

> -	    # Older glibc does not support hex float, newer does.

> -	    xfail $test

> +    foreach {num result} {

> +	0x1.1 1.0625

> +	0x1.8480000000000p+6 97.125

> +	0x1.8480000000000p6 97.125

> +	0x00.1p0 0.0625

> +	0x00.1p1 0.125

> +	0x00.1p-1 0.03125

> +    } {

> +	set test "p $num"

> +	set rxn [string_to_regexp $num]

> +	set rxr [string_to_regexp $result]


These two variables are not used.  I suppose that you wanted to use $rxr
in the regexp below.  Not sure about $rxn.

> +	gdb_test_multiple $test $test {

> +	    -re " = $result\r\n$gdb_prompt $" {

> +		pass $test

> +	    }

> +	    -re "Invalid number \".*\"\\.\r\n$gdb_prompt $" {

> +		# Older glibc does not support hex float, newer does.

> +		xfail $test

> +	    }


I am wondering if you could clean this up at the same time, replacing it with
a simple gdb_test.  I chased down the original thread for the patch that
introduced, here it is:

  https://sourceware.org/legacy-ml/gdb-patches/2010-08/msg00363.html

It doesn't explicitly give details about what "Older glibc" means, but I understand
that support for hex float was added in between these two releases:

  PASS CentOS-4.8 glibc-2.3.4-2.43.el4_8.3
  FAIL CentOS-5.5 glibc-2.5-49.el5_5.4

That's a long time ago (glibc 2.5 is from 2006), so I think we can consider that all
hosts today will support it (unless maybe if we build GDB against another libc than
glibc).

The host's glibc matters here because we use the host's scanf to parse the float string
when the host's float precision is good enough.

Simon
Tom Tromey June 11, 2020, 4:35 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


>> +	set rxn [string_to_regexp $num]

>> +	set rxr [string_to_regexp $result]


Simon> These two variables are not used.  I suppose that you wanted to use $rxr
Simon> in the regexp below.  Not sure about $rxn.

Ugh, oops.

Simon> I am wondering if you could clean this up at the same time, replacing it with
Simon> a simple gdb_test.

I did this.
I'm going to check it in now.

Tom

Patch

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index e7d0a0dc4ad..5ec84eb8ed7 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2748,7 +2748,7 @@  lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
     case '9':
       {
 	/* It's a number.  */
-	int got_dot = 0, got_e = 0, toktype;
+	int got_dot = 0, got_e = 0, got_p = 0, toktype;
 	const char *p = tokstart;
 	int hex = input_radix > 10;
 
@@ -2768,13 +2768,16 @@  lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 	    /* This test includes !hex because 'e' is a valid hex digit
 	       and thus does not indicate a floating point number when
 	       the radix is hex.  */
-	    if (!hex && !got_e && (*p == 'e' || *p == 'E'))
+	    if (!hex && !got_e && !got_p && (*p == 'e' || *p == 'E'))
 	      got_dot = got_e = 1;
+	    else if (!got_e && !got_p && (*p == 'p' || *p == 'P'))
+	      got_dot = got_p = 1;
 	    /* This test does not include !hex, because a '.' always indicates
 	       a decimal floating point number regardless of the radix.  */
 	    else if (!got_dot && *p == '.')
 	      got_dot = 1;
-	    else if (got_e && (p[-1] == 'e' || p[-1] == 'E')
+	    else if (((got_e && (p[-1] == 'e' || p[-1] == 'E'))
+		      || (got_p && (p[-1] == 'p' || p[-1] == 'P')))
 		     && (*p == '-' || *p == '+'))
 	      /* This is the sign of the exponent, not the end of the
 		 number.  */
@@ -2787,7 +2790,7 @@  lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 	      break;
 	  }
 	toktype = parse_number (par_state, tokstart, p - tokstart,
-				got_dot|got_e, &yylval);
+				got_dot | got_e | got_p, &yylval);
         if (toktype == ERROR)
 	  {
 	    char *err_copy = (char *) alloca (p - tokstart + 1);
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 066e7fce87b..05ababff78c 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -127,14 +127,25 @@  proc test_float_accepted {} {
     gdb_test "p 1.5l" " = 1.5"
 
     # Test hexadecimal floating point.
-    set test "p 0x1.1"
-    gdb_test_multiple $test $test {
-	-re " = 1\\.0625\r\n$gdb_prompt $" {
-	    pass $test
-	}
-	-re "Invalid number \"0x1\\.1\"\\.\r\n$gdb_prompt $" {
-	    # Older glibc does not support hex float, newer does.
-	    xfail $test
+    foreach {num result} {
+	0x1.1 1.0625
+	0x1.8480000000000p+6 97.125
+	0x1.8480000000000p6 97.125
+	0x00.1p0 0.0625
+	0x00.1p1 0.125
+	0x00.1p-1 0.03125
+    } {
+	set test "p $num"
+	set rxn [string_to_regexp $num]
+	set rxr [string_to_regexp $result]
+	gdb_test_multiple $test $test {
+	    -re " = $result\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	    -re "Invalid number \".*\"\\.\r\n$gdb_prompt $" {
+		# Older glibc does not support hex float, newer does.
+		xfail $test
+	    }
 	}
     }
 }