Re: [PATCH v2] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow

2014-11-21 Thread Chen Gang

Thank you very much for your work. I shall send patch v3 for it.

Thanks.

On 11/22/2014 05:57 AM, Jakub Jelinek wrote:
> On Sat, Nov 22, 2014 at 05:03:37AM +0800, Chen Gang wrote:
>> According to the next code, 'pretty_name' may need additional bytes more
>> than 16 (may have unlimited length for array type). There is an easy way
>> to fix it: use 'pretty_print' for 'pretty_name'.
>>
>> Let the code meet 2 white spaces alignment coding styles (originally,
>> some of code is 1 white sapce alignment).
>>
>> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
>>
>> 2014-11-22  Chen Gang  
>>
>> * ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for
>> 'pretty_name' to avoid memory overflow
> 
> Add a . at the end.
> 
>>while (deref_depth-- > 0)
>> -pretty_name[pos++] = '*';
>> -  pretty_name[pos++] = '\'';
>> -  pretty_name[pos] = '\0';
>> +pp_star(&pretty_name);
>> +  pp_quote(&pretty_name);
> 
> Formatting, missing space before (.  Happens many times in the patch.
> 
>>if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
>> -pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
>> -tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
>> +pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC,
>> +   tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
> 
> You don't know if TYPE_MAX_VALUE (dom) will fit into uhwi, and you are
> using signed printing anyway.
> You said that using pp_wide_int breaks too many tests, so perhaps
> do
>   if (tree_fits_uhwi_p (TYPE_MAX_VALUE (dom))
>   && tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1 != 0)
> pp_printf (..., HOST_WIDE_INT_PRINT_UNSIGNED,
>   else
> pp_wide_int (..., wi::to_widest (TYPE_MAX_VALUE (dom)) + 1);
> ?
> 

Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed


Re: [PATCH v2] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow

2014-11-21 Thread Jakub Jelinek
On Sat, Nov 22, 2014 at 05:03:37AM +0800, Chen Gang wrote:
> According to the next code, 'pretty_name' may need additional bytes more
> than 16 (may have unlimited length for array type). There is an easy way
> to fix it: use 'pretty_print' for 'pretty_name'.
> 
> Let the code meet 2 white spaces alignment coding styles (originally,
> some of code is 1 white sapce alignment).
> 
> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
> 
> 2014-11-22  Chen Gang  
> 
> * ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for
> 'pretty_name' to avoid memory overflow

Add a . at the end.

>while (deref_depth-- > 0)
> -pretty_name[pos++] = '*';
> -  pretty_name[pos++] = '\'';
> -  pretty_name[pos] = '\0';
> + pp_star(&pretty_name);
> +  pp_quote(&pretty_name);

Formatting, missing space before (.  Happens many times in the patch.

> if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
> - pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
> - tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
> + pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC,
> +tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);

You don't know if TYPE_MAX_VALUE (dom) will fit into uhwi, and you are
using signed printing anyway.
You said that using pp_wide_int breaks too many tests, so perhaps
do
  if (tree_fits_uhwi_p (TYPE_MAX_VALUE (dom))
  && tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1 != 0)
pp_printf (..., HOST_WIDE_INT_PRINT_UNSIGNED,
  else
pp_wide_int (..., wi::to_widest (TYPE_MAX_VALUE (dom)) + 1);
?

Jakub


[PATCH v2] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow

2014-11-21 Thread Chen Gang
According to the next code, 'pretty_name' may need additional bytes more
than 16 (may have unlimited length for array type). There is an easy way
to fix it: use 'pretty_print' for 'pretty_name'.

Let the code meet 2 white spaces alignment coding styles (originally,
some of code is 1 white sapce alignment).

It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.

2014-11-22  Chen Gang  

* ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for
'pretty_name' to avoid memory overflow
---
 gcc/ubsan.c | 57 +++--
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 41cf546..c03b000 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -336,7 +336,7 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style 
pstyle)
   tree dtype = ubsan_type_descriptor_type ();
   tree type2 = type;
   const char *tname = NULL;
-  char *pretty_name;
+  pretty_printer pretty_name;
   unsigned char deref_depth = 0;
   unsigned short tkind, tinfo;
 
@@ -375,54 +375,50 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style 
pstyle)
 /* We weren't able to determine the type name.  */
 tname = "";
 
-  /* Decorate the type name with '', '*', "struct", or "union".  */
-  pretty_name = (char *) alloca (strlen (tname) + 16 + deref_depth);
   if (pstyle == UBSAN_PRINT_POINTER)
 {
-  int pos = sprintf (pretty_name, "'%s%s%s%s%s%s%s",
-TYPE_VOLATILE (type2) ? "volatile " : "",
-TYPE_READONLY (type2) ? "const " : "",
-TYPE_RESTRICT (type2) ? "restrict " : "",
-TYPE_ATOMIC (type2) ? "_Atomic " : "",
-TREE_CODE (type2) == RECORD_TYPE
-? "struct "
-: TREE_CODE (type2) == UNION_TYPE
-  ? "union " : "", tname,
-deref_depth == 0 ? "" : " ");
+  pp_printf (&pretty_name, "'%s%s%s%s%s%s%s",
+TYPE_VOLATILE (type2) ? "volatile " : "",
+TYPE_READONLY (type2) ? "const " : "",
+TYPE_RESTRICT (type2) ? "restrict " : "",
+TYPE_ATOMIC (type2) ? "_Atomic " : "",
+TREE_CODE (type2) == RECORD_TYPE
+? "struct "
+: TREE_CODE (type2) == UNION_TYPE
+  ? "union " : "", tname,
+deref_depth == 0 ? "" : " ");
   while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
-  pretty_name[pos++] = '\'';
-  pretty_name[pos] = '\0';
+   pp_star(&pretty_name);
+  pp_quote(&pretty_name);
 }
   else if (pstyle == UBSAN_PRINT_ARRAY)
 {
   /* Pretty print the array dimensions.  */
   gcc_assert (TREE_CODE (type) == ARRAY_TYPE);
   tree t = type;
-  int pos = sprintf (pretty_name, "'%s ", tname);
+  pp_printf (&pretty_name, "'%s ", tname);
   while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
+   pp_star(&pretty_name);
   while (TREE_CODE (t) == ARRAY_TYPE)
{
- pretty_name[pos++] = '[';
+ pp_left_bracket(&pretty_name);
  tree dom = TYPE_DOMAIN (t);
  if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
-   pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
-   tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
+   pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC,
+  tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
  else
/* ??? We can't determine the variable name; print VLA unspec.  */
-   pretty_name[pos++] = '*';
- pretty_name[pos++] = ']';
+   pp_star(&pretty_name);
+ pp_right_bracket(&pretty_name);
  t = TREE_TYPE (t);
}
-  pretty_name[pos++] = '\'';
-  pretty_name[pos] = '\0';
+  pp_quote(&pretty_name);
 
- /* Save the tree with stripped types.  */
- type = t;
+  /* Save the tree with stripped types.  */
+  type = t;
 }
   else
-sprintf (pretty_name, "'%s'", tname);
+pp_printf (&pretty_name, "'%s'", tname);
 
   switch (TREE_CODE (type))
 {
@@ -459,8 +455,9 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style 
pstyle)
   DECL_IGNORED_P (decl) = 1;
   DECL_EXTERNAL (decl) = 0;
 
-  size_t len = strlen (pretty_name);
-  tree str = build_string (len + 1, pretty_name);
+  const char *tmp = pp_formatted_text(&pretty_name);
+  size_t len = strlen (tmp);
+  tree str = build_string (len + 1, tmp);
   TREE_TYPE (str) = build_array_type (char_type_node,
  build_index_type (size_int (len)));
   TREE_READONLY (str) = 1;
-- 
1.9.3