diff mbox

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

Message ID CABOHX+dgJ8vVnfnQq7o-+H2ksdWtg0HzeCrRDy8QBHkUE+vHjw@mail.gmail.com
State New
Headers show

Commit Message

Iain Buclaw April 15, 2017, 3:27 p.m. UTC
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.

---

Comments

Jeff Law April 25, 2017, 3:41 p.m. UTC | #1
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<ibuclaw@gdcproject.org>
> Date:   Sat Apr 15 12:02:10 2017 +0200
> 
>      libiberty/ChangeLog:
>      
>      2017-04-15  Iain Buclaw<ibuclaw@gdcproject.org>
>      
>      	* 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
diff mbox

Patch

commit 87041417fdf6911b5112c4c68b324577202fa2d0
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sat Apr 15 12:02:10 2017 +0200

    libiberty/ChangeLog:
    
    2017-04-15  Iain Buclaw  <ibuclaw@gdcproject.org>
    
    	* 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 <http://www.gnu.org/licenses/>.  */
 
-/* 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 <http://www.gnu.org/licenses/>.  */
 
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
-#else
-extern long strtol (const char *nptr, char **endptr, int base);
 #endif
 
 #include <demangle.h>
@@ -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, &len);
 
-  if (mangled == NULL || *mangled == '\0')
-    return NULL;
-
-  len = strtol (mangled, &endptr, 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, &endptr, 10);
+      long val;
 
-      if (endptr == NULL || val < 0)
+      mangled = dlang_number (mangled, &val);
+      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, &endptr, 10);
+      long val;
 
-      if (endptr == NULL || val < 0)
+      mangled = dlang_number (mangled, &val);
+      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 = strtol (mangled, &endptr, 10);
-
-  if (endptr == NULL || len < 0)
-    return NULL;
-
-  mangled = endptr;
-  if (*mangled != '_')
+  mangled = dlang_number (mangled, &len);
+  if (mangled == NULL || *mangled != '_')
     return NULL;
 
   mangled++;
@@ -1143,13 +1159,12 @@  dlang_parse_string (string *decl, const char *mangled)
 static const char *
 dlang_parse_arrayliteral (string *decl, const char *mangled)
 {
-  char *endptr;
-  long elements = strtol (mangled, &endptr, 10);
+  long elements;
 
-  if (endptr == NULL || elements < 0)
+  mangled = dlang_number (mangled, &elements);
+  if (mangled == NULL)
     return NULL;
 
-  mangled = endptr;
   string_append (decl, "[");
   while (elements--)
     {
@@ -1167,13 +1182,12 @@  dlang_parse_arrayliteral (string *decl, const char *mangled)
 static const char *
 dlang_parse_assocarray (string *decl, const char *mangled)
 {
-  char *endptr;
-  long elements = strtol (mangled, &endptr, 10);
+  long elements;
 
-  if (endptr == NULL || elements < 0)
+  mangled = dlang_number (mangled, &elements);
+  if (mangled == NULL)
     return NULL;
 
-  mangled = endptr;
   string_append (decl, "[");
   while (elements--)
     {
@@ -1194,13 +1208,12 @@  dlang_parse_assocarray (string *decl, const char *mangled)
 static const char *
 dlang_parse_structlit (string *decl, const char *mangled, const char *name)
 {
-  char *endptr;
-  long args = strtol (mangled, &endptr, 10);
+  long args;
 
-  if (endptr == NULL || args < 0)
+  mangled = dlang_number (mangled, &args);
+  if (mangled == NULL)
     return NULL;
 
-  mangled = endptr;
   if (name != NULL)
     string_append (decl, name);
 
@@ -1241,8 +1254,6 @@  dlang_value (string *decl, const char *mangled, const char *name, char type)
 
     case 'i':
       mangled++;
-      if (*mangled < '0' || *mangled > '9')
-	return NULL;
       mangled = dlang_parse_integer (decl, mangled, type);
       break;
 
@@ -1462,13 +1473,12 @@  dlang_parse_qualified (string *decl, const char *mangled,
 static const char *
 dlang_parse_tuple (string *decl, const char *mangled)
 {
-  char *endptr;
-  long elements = strtol (mangled, &endptr, 10);
+  long elements;
 
-  if (endptr == NULL || elements < 0)
+  mangled = dlang_number (mangled, &elements);
+  if (mangled == NULL)
     return NULL;
 
-  mangled = endptr;
   string_append (decl, "Tuple!(");
 
   while (elements--)
diff --git a/libiberty/testsuite/d-demangle-expected b/libiberty/testsuite/d-demangle-expected
index 7166152..9fc1551 100644
--- a/libiberty/testsuite/d-demangle-expected
+++ b/libiberty/testsuite/d-demangle-expected
@@ -964,6 +964,118 @@  _D5__T1aZv
 _D5__T1aZv
 #
 --format=dlang
+_D00
+_D00
+#
+--format=dlang
+_D9223372036854775817
+_D9223372036854775817
+#
+--format=dlang
+_D1az
+_D1az
+#
+--format=dlang
+_D1aN
+_D1aN
+#
+--format=dlang
+_D1aF
+_D1aF
+#
+--format=dlang
+_D1aM
+_D1aM
+#
+--format=dlang
+_D1aFZNz
+_D1aFZNz
+#
+--format=dlang
+_D1aFNzZv
+_D1aFNzZv
+#
+--format=dlang
+_D4testFDX
+_D4testFDX
+#
+--format=dlang
+_D5__T0aZv
+_D5__T0aZv
+#
+--format=dlang
+_D10__T4testYZv
+_D10__T4testYZv
+#
+--format=dlang
+_D4testFBaZv
+_D4testFBaZv
+#
+--format=dlang
+_D8__T4test
+_D8__T4test
+#
+--format=dlang
+_D10__T4testVi
+_D10__T4testVi
+#
+--format=dlang
+_D10__T4testVai
+_D10__T4testVai
+#
+--format=dlang
+_D10__T4testVbi
+_D10__T4testVbi
+#
+--format=dlang
+_D11__T4testS1a
+_D11__T4testS1a
+#
+--format=dlang
+_D12__T4testViiZv
+_D12__T4testViiZv
+#
+--format=dlang
+_D12__T4testViYZv
+_D12__T4testViYZv
+#
+--format=dlang
+_D12__T4testVrcZv
+_D12__T4testVrcZv
+#
+--format=dlang
+_D13__T4testVdeYZv
+_D13__T4testVdeYZv
+#
+--format=dlang
+_D13__T4testViSiZv
+_D13__T4testViSiZv
+#
+--format=dlang
+_D14__T4testVAiAiZv
+_D14__T4testVAiAiZv
+#
+--format=dlang
+_D14__T4testS123aZv
+_D14__T4testS123aZv
+#
+--format=dlang
+_D15__T4testVHiiAiZv
+_D15__T4testVHiiAiZv
+#
+--format=dlang
+_D15__T4testVfe0p1Zv
+_D15__T4testVfe0p1Zv
+#
+--format=dlang
+_D16__T4testVAyaa0aZv
+_D16__T4testVAyaa0aZv
+#
+--format=dlang
+_D18__T4testVAyaa1_YYZv
+_D18__T4testVAyaa1_YYZv
+#
+--format=dlang
 _D4test3fooAa
 test.foo
 #