Re: [PATCH 6/7] [D] libiberty: Fixes for decoding numbers.

2017-04-25 Thread Jeff Law 

On 04/15/2017 09:27 AM, Iain Buclaw wrote:

This fixes two main problems found in the use of strtol().  First that
it returns `0' if nothing is decoded, and none of the callers checked
whether nothing was consumed.  Second that it just returns  `LONG_MAX'
on overflow.  Rather than updating each individual call site, have
solved [1] by moving all uses into a new function that validates the
next character is a digit, and [2] by removing the import of `strtol'
and doing the decoding ourselves with the explicit check for overflow.

Added many coverage tests, many of which unearthed hidden segfaults.

---


06-d-demangle-dlang-number.patch


commit 87041417fdf6911b5112c4c68b324577202fa2d0
Author: Iain Buclaw
Date:   Sat Apr 15 12:02:10 2017 +0200

 libiberty/ChangeLog:
 
 2017-04-15  Iain Buclaw
 
 	* d-demangle.c (strtol): Remove declaration.

Updated all callers to use dlang_number.
(dlang_number): New function.
(dlang_value): Moved check for ISDIGIT into dlang_parse_integer.
* testsuite/d-demangle-expected: Add tests.


OK for the trunk.

jeff

[PATCH 6/7] [D] libiberty: Fixes for decoding numbers.

2017-04-15 Thread Iain Buclaw 
This fixes two main problems found in the use of strtol().  First that
it returns `0' if nothing is decoded, and none of the callers checked
whether nothing was consumed.  Second that it just returns  `LONG_MAX'
on overflow.  Rather than updating each individual call site, have
solved [1] by moving all uses into a new function that validates the
next character is a digit, and [2] by removing the import of `strtol'
and doing the decoding ourselves with the explicit check for overflow.

Added many coverage tests, many of which unearthed hidden segfaults.

---
commit 87041417fdf6911b5112c4c68b324577202fa2d0
Author: Iain Buclaw 
Date:   Sat Apr 15 12:02:10 2017 +0200

libiberty/ChangeLog:

2017-04-15  Iain Buclaw  

	* d-demangle.c (strtol): Remove declaration.
	Updated all callers to use dlang_number.
	(dlang_number): New function.
	(dlang_value): Moved check for ISDIGIT into dlang_parse_integer.
	* testsuite/d-demangle-expected: Add tests.

diff --git a/libiberty/d-demangle.c b/libiberty/d-demangle.c
index 7e9d5bf..84a7186 100644
--- a/libiberty/d-demangle.c
+++ b/libiberty/d-demangle.c
@@ -26,9 +26,7 @@ You should have received a copy of the GNU Library General Public
 License along with libiberty; see the file COPYING.LIB.
 If not, see .  */
 
-/* This file exports one function; dlang_demangle.
-
-   This file imports strtol for decoding mangled literals.  */
+/* This file exports one function; dlang_demangle.  */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -42,8 +40,6 @@ If not, see .  */
 
 #ifdef HAVE_STDLIB_H
 #include 
-#else
-extern long strtol (const char *nptr, char **endptr, int base);
 #endif
 
 #include 
@@ -197,6 +193,36 @@ static const char *dlang_parse_tuple (string *, const char *);
 static const char *dlang_parse_template (string *, const char *, long);
 
 
+/* Extract the number from MANGLED, and assign the result to RET.
+   Return the remaining string on success or NULL on failure.  */
+static const char *
+dlang_number (const char *mangled, long *ret)
+{
+  /* Return NULL if trying to extract something that isn't a digit.  */
+  if (mangled == NULL || !ISDIGIT (*mangled))
+return NULL;
+
+  *ret = 0;
+
+  while (ISDIGIT (*mangled))
+{
+  *ret *= 10;
+
+  /* If an overflow occured when multiplying by ten, the result
+	 will not be a multiple of ten.  */
+  if ((*ret % 10) != 0)
+	return NULL;
+
+  *ret += mangled[0] - '0';
+  mangled++;
+}
+
+  if (*mangled == '\0' || *ret < 0)
+return NULL;
+
+  return mangled;
+}
+
 /* Demangle the calling convention from MANGLED and append it to DECL.
Return the remaining string on success or NULL on failure.  */
 static const char *
@@ -709,15 +735,10 @@ static const char *
 dlang_identifier (string *decl, const char *mangled,
 		  enum dlang_symbol_kinds kind)
 {
-  char *endptr;
   long len;
+  const char *endptr = dlang_number (mangled, );
 
-  if (mangled == NULL || *mangled == '\0')
-return NULL;
-
-  len = strtol (mangled, , 10);
-
-  if (endptr == NULL || len <= 0)
+  if (endptr == NULL || len == 0)
 return NULL;
 
   /* In template parameter symbols, the first character of the mangled
@@ -726,7 +747,7 @@ dlang_identifier (string *decl, const char *mangled,
   if (kind == dlang_template_param)
 {
   long psize = len;
-  char *pend;
+  const char *pend;
   int saved = string_length (decl);
 
   /* Work backwards until a match is found.  */
@@ -871,10 +892,10 @@ dlang_parse_integer (string *decl, const char *mangled, char type)
   char value[10];
   int pos = 10;
   int width = 0;
-  char *endptr;
-  long val = strtol (mangled, , 10);
+  long val;
 
-  if (endptr == NULL || val < 0)
+  mangled = dlang_number (mangled, );
+  if (mangled == NULL)
 	return NULL;
 
   string_append (decl, "'");
@@ -923,19 +944,17 @@ dlang_parse_integer (string *decl, const char *mangled, char type)
 	  string_appendn (decl, &(value[pos]), 10 - pos);
 	}
   string_append (decl, "'");
-  mangled = endptr;
 }
   else if (type == 'b')
 {
   /* Parse boolean value.  */
-  char *endptr;
-  long val = strtol (mangled, , 10);
+  long val;
 
-  if (endptr == NULL || val < 0)
+  mangled = dlang_number (mangled, );
+  if (mangled == NULL)
 	return NULL;
 
   string_append (decl, val ? "true" : "false");
-  mangled = endptr;
 }
   else
 {
@@ -943,6 +962,9 @@ dlang_parse_integer (string *decl, const char *mangled, char type)
   const char *numptr = mangled;
   size_t num = 0;
 
+  if (! ISDIGIT (*mangled))
+	return NULL;
+
   while (ISDIGIT (*mangled))
 	{
 	  num++;
@@ -1070,17 +1092,11 @@ static const char *
 dlang_parse_string (string *decl, const char *mangled)
 {
   char type = *mangled;
-  char *endptr;
   long len;
 
   mangled++;
-  len =