[PATCH] Remove unwanted spaces when looking up builtin types

[PATCH] Remove unwanted spaces when looking up builtin types

[Siva Chandra]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Currently, if we do something like gdb.lookup_type(" unsigned long "),
GDB errors out because of the leading and trailing spaces.  The
attached patch fixes this problem.

A practical situation where this problem is hit is when invoking
template methods. Its not uncommon to do things like this:

(gdb) p foo.bar< unsigned long >()

If "bar" happens to be an xmethod, then its implementation will
typically need to parse the name of the method ("bar< unsigned long >"
in the above example) to get the template argument and lookup the
type. GDB currently fails for such cases. One could of course
sanitize/fix such inputs in Python before calling lookup_type, but I
think it is better done on the GDB side as having white spaces is
valid syntax.

For non-builtin types, lookup_type goes through the symbol lookup
path. AFAIU, that path already ignores whitespaces.

gdb/ChangeLog:

        * language.c (language_lookup_primitive_type_by_name): Remove
        unwanted space in the type name before looking it up.

gdb/testsuite/ChangeLog:

        * gdb.python/py-type.exp: Add new tests.

[-- Attachment #2: unwanted_space_v1.txt --]
[-- Type: text/plain, Size: 2591 bytes --]

diff --git a/gdb/language.c b/gdb/language.c
index 034086d..abfed40 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -981,6 +981,17 @@ language_bool_type (const struct language_defn *la,
   return ld->arch_info[la->la_language].bool_type_default;
 }
 
+/* Return 1 if C is a whitespace character, 0 otherwise.  */
+
+static int
+whitespace_p (const char c)
+{
+  if (c == ' ' || c == '\n' || c == '\t')
+    return 1;
+  else
+    return 0;
+}
+
 struct type *
 language_lookup_primitive_type_by_name (const struct language_defn *la,
 					struct gdbarch *gdbarch,
@@ -989,14 +1000,44 @@ language_lookup_primitive_type_by_name (const struct language_defn *la,
   struct language_gdbarch *ld = gdbarch_data (gdbarch,
 					      language_gdbarch_data);
   struct type *const *p;
+  int len = strlen (name);
+  int i, j;
+  char *clean_name = (char *) xmalloc (sizeof (char) * len + 1);
+
+  /* Remove unwanted whitespace in the typename.  This could happen, for
+     example, happen if one does gdb.lookup_type(' unsigned long ') in
+     Python.  */
+  for (i = 0, j = 0; i < len; i++)
+    {
+      if (whitespace_p (name[i]))
+	{
+	  if (j == 0 || clean_name[j - 1] == ' ')
+	    continue;
+	}
+
+      if (whitespace_p (name[i]))
+	clean_name[j] = ' ';
+      else
+	clean_name[j] = name[i];
+
+      j++;
+    }
+  if (j > 0 && clean_name[j - 1] == ' ')
+    j--;
+  clean_name[j] = '\0';
 
   for (p = ld->arch_info[la->la_language].primitive_type_vector;
        (*p) != NULL;
        p++)
     {
-      if (strcmp (TYPE_NAME (*p), name) == 0)
-	return (*p);
+      if (strcmp (TYPE_NAME (*p), clean_name) == 0)
+	{
+	  xfree (clean_name);
+	  return (*p);
+	}
     }
+  xfree (clean_name);
+
   return (NULL);
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 6b61f48..c9388ed 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -264,3 +264,15 @@ with_test_prefix "lang_cpp" {
     test_template
     test_enums
 }
+
+# Tests to lookup builtin types
+gdb_test "python print gdb.lookup_type ('unsigned int')" "unsigned int" \
+    "lookup unsigned int"
+gdb_test "python print gdb.lookup_type (' unsigned long ')" "unsigned long" \
+    "lookup unsigned long"
+gdb_test "python print gdb.lookup_type (' unsigned  char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\n char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\tlong ')" "unsigned long" \
+    "lookup unsigned long"

Re: [PATCH] Remove unwanted spaces when looking up builtin types

[Siva Chandra]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On Wed, Oct 29, 2014 at 11:50 AM, Siva Chandra <sivachandra@google.com> wrote:
> Currently, if we do something like gdb.lookup_type(" unsigned long "),
> GDB errors out because of the leading and trailing spaces.  The
> attached patch fixes this problem.
>
> A practical situation where this problem is hit is when invoking
> template methods. Its not uncommon to do things like this:
>
> (gdb) p foo.bar< unsigned long >()
>
> If "bar" happens to be an xmethod, then its implementation will
> typically need to parse the name of the method ("bar< unsigned long >"
> in the above example) to get the template argument and lookup the
> type. GDB currently fails for such cases. One could of course
> sanitize/fix such inputs in Python before calling lookup_type, but I
> think it is better done on the GDB side as having white spaces is
> valid syntax.
>
> For non-builtin types, lookup_type goes through the symbol lookup
> path. AFAIU, that path already ignores whitespaces.

Ping.

gdb/ChangeLog:

2014-11-07  Siva Chandra Reddy  <sivachandra@google.com>

        * language.c (language_lookup_primitive_type_by_name): Remove
        unwanted space in the type name before looking it up.
        (whitespace_p): New function.

gdb/testsuite/ChangeLog:

2014-11-07  Siva Chandra Reddy  <sivachandra@google.com>

        * gdb.python/py-type.exp: Add new tests.

[-- Attachment #2: unwanted_space_v1.txt --]
[-- Type: text/plain, Size: 2591 bytes --]

diff --git a/gdb/language.c b/gdb/language.c
index 034086d..abfed40 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -981,6 +981,17 @@ language_bool_type (const struct language_defn *la,
   return ld->arch_info[la->la_language].bool_type_default;
 }
 
+/* Return 1 if C is a whitespace character, 0 otherwise.  */
+
+static int
+whitespace_p (const char c)
+{
+  if (c == ' ' || c == '\n' || c == '\t')
+    return 1;
+  else
+    return 0;
+}
+
 struct type *
 language_lookup_primitive_type_by_name (const struct language_defn *la,
 					struct gdbarch *gdbarch,
@@ -989,14 +1000,44 @@ language_lookup_primitive_type_by_name (const struct language_defn *la,
   struct language_gdbarch *ld = gdbarch_data (gdbarch,
 					      language_gdbarch_data);
   struct type *const *p;
+  int len = strlen (name);
+  int i, j;
+  char *clean_name = (char *) xmalloc (sizeof (char) * len + 1);
+
+  /* Remove unwanted whitespace in the typename.  This could happen, for
+     example, happen if one does gdb.lookup_type(' unsigned long ') in
+     Python.  */
+  for (i = 0, j = 0; i < len; i++)
+    {
+      if (whitespace_p (name[i]))
+	{
+	  if (j == 0 || clean_name[j - 1] == ' ')
+	    continue;
+	}
+
+      if (whitespace_p (name[i]))
+	clean_name[j] = ' ';
+      else
+	clean_name[j] = name[i];
+
+      j++;
+    }
+  if (j > 0 && clean_name[j - 1] == ' ')
+    j--;
+  clean_name[j] = '\0';
 
   for (p = ld->arch_info[la->la_language].primitive_type_vector;
        (*p) != NULL;
        p++)
     {
-      if (strcmp (TYPE_NAME (*p), name) == 0)
-	return (*p);
+      if (strcmp (TYPE_NAME (*p), clean_name) == 0)
+	{
+	  xfree (clean_name);
+	  return (*p);
+	}
     }
+  xfree (clean_name);
+
   return (NULL);
 }
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 6b61f48..c9388ed 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -264,3 +264,15 @@ with_test_prefix "lang_cpp" {
     test_template
     test_enums
 }
+
+# Tests to lookup builtin types
+gdb_test "python print gdb.lookup_type ('unsigned int')" "unsigned int" \
+    "lookup unsigned int"
+gdb_test "python print gdb.lookup_type (' unsigned long ')" "unsigned long" \
+    "lookup unsigned long"
+gdb_test "python print gdb.lookup_type (' unsigned  char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\n char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\tlong ')" "unsigned long" \
+    "lookup unsigned long"

Re: [PATCH] Remove unwanted spaces when looking up builtin types

[Siva Chandra]

On Fri, Nov 7, 2014 at 5:49 AM, Siva Chandra <sivachandra@google.com> wrote:
> On Wed, Oct 29, 2014 at 11:50 AM, Siva Chandra <sivachandra@google.com> wrote:
>> Currently, if we do something like gdb.lookup_type(" unsigned long "),
>> GDB errors out because of the leading and trailing spaces.  The
>> attached patch fixes this problem.
>>
>> A practical situation where this problem is hit is when invoking
>> template methods. Its not uncommon to do things like this:
>>
>> (gdb) p foo.bar< unsigned long >()
>>
>> If "bar" happens to be an xmethod, then its implementation will
>> typically need to parse the name of the method ("bar< unsigned long >"
>> in the above example) to get the template argument and lookup the
>> type. GDB currently fails for such cases. One could of course
>> sanitize/fix such inputs in Python before calling lookup_type, but I
>> think it is better done on the GDB side as having white spaces is
>> valid syntax.
>>
>> For non-builtin types, lookup_type goes through the symbol lookup
>> path. AFAIU, that path already ignores whitespaces.
>

Ping.
This is not super critical, but I see that an xmethod test fails after
I have upgraded to gcc-4.8.2 from gcc-4.6. The essential difference I
have found is that looking up a symbol for "  unsigned int  " works
when the test is compiled with gcc-4.6 and it does not when compiled
with 4.8.2. And, when it does not, the type is looked up via
"language_lookup_primitive_type_by_name" which errors out as it does
an exact string match.

[I have not yet looked into why there is a difference between gcc-4.6
and gcc-4.8.2 as I think that the proposed fix is not inappropriate
and does fix the problem.]

>
> gdb/ChangeLog:
>
> 2014-11-07  Siva Chandra Reddy  <sivachandra@google.com>
>
>         * language.c (language_lookup_primitive_type_by_name): Remove
>         unwanted space in the type name before looking it up.
>         (whitespace_p): New function.
>
> gdb/testsuite/ChangeLog:
>
> 2014-11-07  Siva Chandra Reddy  <sivachandra@google.com>
>
>         * gdb.python/py-type.exp: Add new tests.

Re: [PATCH] Remove unwanted spaces when looking up builtin types

[Siva Chandra]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

On Wed, Oct 29, 2014 at 11:50 AM, Siva Chandra <sivachandra@google.com> wrote:
> Currently, if we do something like gdb.lookup_type(" unsigned long "),
> GDB errors out because of the leading and trailing spaces.  The
> attached patch fixes this problem.
>
> A practical situation where this problem is hit is when invoking
> template methods. Its not uncommon to do things like this:
>
> (gdb) p foo.bar< unsigned long >()
>
> If "bar" happens to be an xmethod, then its implementation will
> typically need to parse the name of the method ("bar< unsigned long >"
> in the above example) to get the template argument and lookup the
> type. GDB currently fails for such cases. One could of course
> sanitize/fix such inputs in Python before calling lookup_type, but I
> think it is better done on the GDB side as having white spaces is
> valid syntax.

This is still relevant, but required a rebase over the recent changes
to language.c.

2015-01-06  Siva Chandra Reddy  <sivachandra@google.com>

gdb/ChangeLog:

        * language.c (language_lookup_primitive_type_1): Remove
        unwanted space in the type name before looking it up.
        (whitespace_p): New function.

gdb/testsuite/ChangeLog:

        * gdb.python/py-type.exp: Add new tests.

[-- Attachment #2: unwanted_space_v2.txt --]
[-- Type: text/plain, Size: 2415 bytes --]

diff --git a/gdb/language.c b/gdb/language.c
index 1e6b983..0c13219 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -987,6 +987,17 @@ language_bool_type (const struct language_defn *la,
   return ld->arch_info[la->la_language].bool_type_default;
 }
 
+/* Return 1 if C is a whitespace character, 0 otherwise.  */
+
+static int
+whitespace_p (const char c)
+{
+  if (c == ' ' || c == '\n' || c == '\t')
+    return 1;
+  else
+    return 0;
+}
+
 /* Helper function for primitive type lookup.  */
 
 static struct type **
@@ -994,13 +1005,39 @@ language_lookup_primitive_type_1 (const struct language_arch_info *lai,
 				  const char *name)
 {
   struct type **p;
+  int len = strlen (name);
+  int i, j;
+  char *clean_name = (char *) xmalloc (sizeof (char) * len + 1);
+
+  /* Remove unwanted whitespace in the typename.  This could happen, for
+     example, if one does gdb.lookup_type(' unsigned long ') in Python.  */
+  for (i = 0, j = 0; i < len; i++)
+    {
+      if (whitespace_p (name[i]))
+	{
+	  if (j == 0 || clean_name[j - 1] == ' ')
+	    continue;
+	}
+
+      if (whitespace_p (name[i]))
+	clean_name[j] = ' ';
+      else
+	clean_name[j] = name[i];
+
+      j++;
+    }
+  if (j > 0 && clean_name[j - 1] == ' ')
+    j--;
+  clean_name[j] = '\0';
 
   for (p = lai->primitive_type_vector; (*p) != NULL; p++)
     {
-      if (strcmp (TYPE_NAME (*p), name) == 0)
-	return p;
+      if (strcmp (TYPE_NAME (*p), clean_name) == 0)
+	break;
     }
-  return NULL;
+  xfree (clean_name);
+
+  return p;
 }
 
 /* See language.h.  */
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index c4c8d9f..0c45ec9 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -270,3 +270,15 @@ with_test_prefix "lang_cpp" {
     test_template
     test_enums
 }
+
+# Tests to lookup builtin types
+gdb_test "python print gdb.lookup_type ('unsigned int')" "unsigned int" \
+    "lookup unsigned int"
+gdb_test "python print gdb.lookup_type (' unsigned long ')" "unsigned long" \
+    "lookup unsigned long"
+gdb_test "python print gdb.lookup_type (' unsigned  char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\n char ')" "unsigned char" \
+    "lookup unsigned char"
+gdb_test "python print gdb.lookup_type (' unsigned\\tlong ')" "unsigned long" \
+    "lookup unsigned long"

Re: [PATCH] Remove unwanted spaces when looking up builtin types

[Joel Brobecker]

> +/* Return 1 if C is a whitespace character, 0 otherwise.  */
> +
> +static int
> +whitespace_p (const char c)
> +{
> +  if (c == ' ' || c == '\n' || c == '\t')
> +    return 1;
> +  else
> +    return 0;

Why not using isspace directly? Apologies if this was explained
before, but then, a comment would be helpful, and maybe a more
specific function name.  My first reaction to this kind of routine
was that this should be in one of our "utils" files, so as to allow
other parts of the code to use it as well.

-- 
Joel