PR 18669: Remove use of strtod in libiberty/d-demangle.c

Message ID CABOHX+ecW-F6sMWF-OMH+Ojxxm+=EpaY5b-OEWhU1yNoX1kTng@mail.gmail.com
State New, archived
Headers

Commit Message

Iain Buclaw Aug. 4, 2015, 2:24 p.m. UTC
  Also submitted to upstream GCC.

https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00192.html

Have tested this against the gdb dlang testsuite and observed no
failing tests as a result.

Regards
Iain.
  

Comments

Joel Brobecker Aug. 4, 2015, 5:07 p.m. UTC | #1
> Also submitted to upstream GCC.
> 
> https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00192.html
> 
> Have tested this against the gdb dlang testsuite and observed no
> failing tests as a result.
> 
> Regards
> Iain.

> 2015-08-04  Iain Buclaw  <ibuclaw@gdcproject.org>
> 
> 	* d-demangle.c (dlang_parse_real): Remove call to strtod.
> 	(strtod): Remove declaration.
> 	* testsuite/d-demangle-expected: Update float and complex literal
> 	tests to check correct hexadecimal demangling.

GCC is the master, so you can (in fact, should!) update the binutils-gdb
copy as soon as the patch is approved on the GCC side.

Thank you,
  
Iain Buclaw Aug. 4, 2015, 5:16 p.m. UTC | #2
On 4 August 2015 at 19:07, Joel Brobecker <brobecker@adacore.com> wrote:
>> Also submitted to upstream GCC.
>>
>> https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00192.html
>>
>> Have tested this against the gdb dlang testsuite and observed no
>> failing tests as a result.
>>
>> Regards
>> Iain.
>
>> 2015-08-04  Iain Buclaw  <ibuclaw@gdcproject.org>
>>
>>       * d-demangle.c (dlang_parse_real): Remove call to strtod.
>>       (strtod): Remove declaration.
>>       * testsuite/d-demangle-expected: Update float and complex literal
>>       tests to check correct hexadecimal demangling.
>
> GCC is the master, so you can (in fact, should!) update the binutils-gdb
> copy as soon as the patch is approved on the GCC side.
>
> Thank you,
> --
> Joel


By the way, would this also be a candidate for the 7.10 branch?
Assume that upstream GCC respond in a timely manner. :-)

Regards
Iain
  
Joel Brobecker Aug. 4, 2015, 5:24 p.m. UTC | #3
> By the way, would this also be a candidate for the 7.10 branch?
> Assume that upstream GCC respond in a timely manner. :-)

It looks to me like this would only affect D support, and
the patch seems innocent enough that, yes, it would be OK to push
to the branch as well.

We're still waiting for a couple of issues to be resolved
(at least). So you still have a bit of time. I've found that
libiberty changes are usually reviewed pretty quick, so hopefully
you'll be OK. Ping me when I start talking about creating the release
if necessary.
  
Iain Buclaw Aug. 5, 2015, 3:51 p.m. UTC | #4
On 4 August 2015 at 19:24, Joel Brobecker <brobecker@adacore.com> wrote:
>> By the way, would this also be a candidate for the 7.10 branch?
>> Assume that upstream GCC respond in a timely manner. :-)
>
> It looks to me like this would only affect D support, and
> the patch seems innocent enough that, yes, it would be OK to push
> to the branch as well.
>

I was just thinking about any build bots running Solaris 9 that would
report a regression in 7.10 because of this.  But I guess that we
don't have any...

Regards
Iain
  
Iain Buclaw Aug. 11, 2015, 6:48 a.m. UTC | #5
On 4 August 2015 at 19:24, Joel Brobecker <brobecker@adacore.com> wrote:
>> By the way, would this also be a candidate for the 7.10 branch?
>> Assume that upstream GCC respond in a timely manner. :-)
>
> It looks to me like this would only affect D support, and
> the patch seems innocent enough that, yes, it would be OK to push
> to the branch as well.
>
> We're still waiting for a couple of issues to be resolved
> (at least). So you still have a bit of time. I've found that
> libiberty changes are usually reviewed pretty quick, so hopefully
> you'll be OK. Ping me when I start talking about creating the release
> if necessary.
>

I'm just prepping it up to push into GCC.  Will also push into GDB
whilst I'm waiting for 'svn rebase' to complete.

Iain
  

Patch

2015-08-04  Iain Buclaw  <ibuclaw@gdcproject.org>

	* d-demangle.c (dlang_parse_real): Remove call to strtod.
	(strtod): Remove declaration.
	* testsuite/d-demangle-expected: Update float and complex literal
	tests to check correct hexadecimal demangling.


--- a/libiberty/d-demangle.c
+++ b/libiberty/d-demangle.c
@@ -28,7 +28,7 @@  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* This file exports one function; dlang_demangle.
 
-   This file imports strtol and strtod for decoding mangled literals.  */
+   This file imports strtol for decoding mangled literals.  */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -44,7 +44,6 @@  If not, see <http://www.gnu.org/licenses/>.  */
 #include <stdlib.h>
 #else
 extern long strtol (const char *nptr, char **endptr, int base);
-extern double strtod (const char *nptr, char **endptr);
 #endif
 
 #include <demangle.h>
@@ -970,8 +969,6 @@  dlang_parse_real (string *decl, const char *mangled)
 {
   char buffer[64];
   int len = 0;
-  double value;
-  char *endptr;
 
   /* Handle NAN and +-INF.  */
   if (strncmp (mangled, "NAN", 3) == 0)
@@ -1035,14 +1032,10 @@  dlang_parse_real (string *decl, const char *mangled)
       mangled++;
     }
 
-  /* Convert buffer from hexadecimal to floating-point.  */
+  /* Write out the demangled hexadecimal, rather than trying to
+     convert the buffer into a floating-point value.  */
   buffer[len] = '\0';
-  value = strtod (buffer, &endptr);
-
-  if (endptr == NULL || endptr != (buffer + len))
-    return NULL;
-
-  len = snprintf (buffer, sizeof(buffer), "%#g", value);
+  len = strlen (buffer);
   string_appendn (decl, buffer, len);
   return mangled;
 }
--- a/libiberty/testsuite/d-demangle-expected
+++ b/libiberty/testsuite/d-demangle-expected
@@ -719,19 +719,19 @@  demangle.test!('\U000186a0')
 #
 --format=dlang
 _D8demangle17__T4testVde0A8P6Zv
-demangle.test!(42.0000)
+demangle.test!(0x0.A8p6)
 #
 --format=dlang
 _D8demangle16__T4testVdeA8P2Zv
-demangle.test!(42.0000)
+demangle.test!(0xA.8p2)
 #
 --format=dlang
 _D8demangle18__T4testVdeN0A8P6Zv
-demangle.test!(-42.0000)
+demangle.test!(-0x0.A8p6)
 #
 --format=dlang
 _D8demangle31__T4testVde0F6E978D4FDF3B646P7Zv
-demangle.test!(123.456)
+demangle.test!(0x0.F6E978D4FDF3B646p7)
 #
 --format=dlang
 _D8demangle15__T4testVdeNANZv
@@ -747,27 +747,27 @@  demangle.test!(-Inf)
 #
 --format=dlang
 _D8demangle23__T4testVfe0FFFFFFP128Zv
-demangle.test!(3.40282e+38)
+demangle.test!(0x0.FFFFFFp128)
 #
 --format=dlang
 _D8demangle32__T4testVde0FFFFFFFFFFFFF8P1024Zv
-demangle.test!(1.79769e+308)
+demangle.test!(0x0.FFFFFFFFFFFFF8p1024)
 #
 --format=dlang
 _D8demangle19__T4testVfe08PN125Zv
-demangle.test!(1.17549e-38)
+demangle.test!(0x0.8p-125)
 #
 --format=dlang
 _D8demangle20__T4testVde08PN1021Zv
-demangle.test!(2.22507e-308)
+demangle.test!(0x0.8p-1021)
 #
 --format=dlang
 _D8demangle51__T4testVrc0C4CCCCCCCCCCCCCDP4c0B666666666666666P6Zv
-demangle.test!(12.3000+45.6000i)
+demangle.test!(0x0.C4CCCCCCCCCCCCCDp4+0x0.B666666666666666p6i)
 #
 --format=dlang
 _D8demangle52__T4testVrcN0C4CCCCCCCCCCCCCDP4c0B666666666666666P6Zv
-demangle.test!(-12.3000+45.6000i)
+demangle.test!(-0x0.C4CCCCCCCCCCCCCDp4+0x0.B666666666666666p6i)
 #
 --format=dlang
 _D8demangle22__T4testVG3ua3_616263Zv
@@ -787,7 +787,7 @@  demangle.test!([1, 2, 3, 4])
 #
 --format=dlang
 _D8demangle25__T4testVAdA2e08P1eN08P1Zv
-demangle.test!([1.00000, -1.00000])
+demangle.test!([0x0.8p1, -0x0.8p1])
 #
 --format=dlang
 _D8demangle23__T4testVHiiA2i1i2i3i4Zv