PR pretty-print/67567 do not pass NULL as a string

PR pretty-print/67567 do not pass NULL as a string

[Manuel López-Ibáñez]

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

Fortran passes NULL where a non-null string is expected by the pretty-printer,
which causes a sanitizer warning. This could have been found earlier by using
gcc_checking_assert. Even if the assertion is false, the result is just an
incomplete diagnostic, thus it seems more user-friendly to assert only when
checking. I do not have any idea how to properly fix the Fortran bug, thus this
patch simply works-around it.

Bootstrapped & regtested on x86_64-linux-gnu.

OK?

gcc/fortran/ChangeLog:

2015-09-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR pretty-print/67567
    * resolve.c (resolve_fl_procedure): Work-around when iface->module
    == NULL.

gcc/ChangeLog:

2015-09-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR pretty-print/67567
    * pretty-print.c (pp_string): Add gcc_checking_assert.
    * pretty-print.h (output_buffer_append_r): Likewise.

[-- Attachment #2: fix-pr67567.diff --]
[-- Type: text/plain, Size: 2975 bytes --]

Index: gcc/pretty-print.c
===================================================================
--- gcc/pretty-print.c	(revision 227762)
+++ gcc/pretty-print.c	(working copy)
@@ -903,11 +903,12 @@ pp_character (pretty_printer *pp, int c)
 /* Append a STRING to the output area of PRETTY-PRINTER; the STRING may
    be line-wrapped if in appropriate mode.  */
 void
 pp_string (pretty_printer *pp, const char *str)
 {
-  pp_maybe_wrap_text (pp, str, str + (str ? strlen (str) : 0));
+  gcc_checking_assert (str);
+  pp_maybe_wrap_text (pp, str, str + strlen (str));
 }
 
 /* Maybe print out a whitespace if needed.  */
 
 void
Index: gcc/pretty-print.h
===================================================================
--- gcc/pretty-print.h	(revision 227762)
+++ gcc/pretty-print.h	(working copy)
@@ -137,10 +137,11 @@ output_buffer_formatted_text (output_buf
 /* Append to the output buffer a string specified by its
    STARTing character and LENGTH.  */
 static inline void
 output_buffer_append_r (output_buffer *buff, const char *start, int length)
 {
+  gcc_checking_assert (start);
   obstack_grow (buff->obstack, start, length);
   buff->line_length += length;
 }
 
 /*  Return a pointer to the last character emitted in the
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 227762)
+++ gcc/fortran/resolve.c	(working copy)
@@ -11738,11 +11738,14 @@ resolve_fl_procedure (gfc_symbol *sym, i
       /* Check the procedure characteristics.  */
       if (sym->attr.pure != iface->attr.pure)
 	{
 	  gfc_error ("Mismatch in PURE attribute between MODULE "
 		     "PROCEDURE at %L and its interface in %s",
-		     &sym->declared_at, iface->module);
+		     &sym->declared_at, 
+		     /* FIXME: PR fortran/67567: iface->module should
+			not be NULL !  */
+		     iface->module ? iface->module : "");
 	  return false;
 	}
 
       if (sym->attr.elemental != iface->attr.elemental)
 	{
@@ -11754,20 +11757,26 @@ resolve_fl_procedure (gfc_symbol *sym, i
 
       if (sym->attr.recursive != iface->attr.recursive)
 	{
 	  gfc_error ("Mismatch in RECURSIVE attribute between MODULE "
 		     "PROCEDURE at %L and its interface in %s",
-		     &sym->declared_at, iface->module);
+		     &sym->declared_at, 
+		     /* FIXME: PR fortran/67567: iface->module should
+			not be NULL !  */
+		     iface->module ? iface->module : "");
 	  return false;
 	}
 
       /* Check the result characteristics.  */
       if (!gfc_check_result_characteristics (sym, iface, errmsg, 200))
 	{
 	  gfc_error ("%s between the MODULE PROCEDURE declaration "
 		     "in module %s and the declaration at %L in "
-		     "SUBMODULE %s", errmsg, iface->module,
+		     "SUBMODULE %s", errmsg, 
+		     /* FIXME: PR fortran/67567: iface->module should
+			not be NULL !  */
+		     iface->module ? iface->module : "",
 		     &sym->declared_at, sym->ns->proc_name->name);
 	  return false;
 	}
 
 check_formal:

Re: PR pretty-print/67567 do not pass NULL as a string

[Manuel López-Ibáñez]

PING: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01219.html

On 16 September 2015 at 19:45, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> Fortran passes NULL where a non-null string is expected by the pretty-printer,
> which causes a sanitizer warning. This could have been found earlier by using
> gcc_checking_assert. Even if the assertion is false, the result is just an
> incomplete diagnostic, thus it seems more user-friendly to assert only when
> checking. I do not have any idea how to properly fix the Fortran bug, thus this
> patch simply works-around it.
>
> Bootstrapped & regtested on x86_64-linux-gnu.
>
> OK?
>
> gcc/fortran/ChangeLog:
>
> 2015-09-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     PR pretty-print/67567
>     * resolve.c (resolve_fl_procedure): Work-around when iface->module
>     == NULL.
>
> gcc/ChangeLog:
>
> 2015-09-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     PR pretty-print/67567
>     * pretty-print.c (pp_string): Add gcc_checking_assert.
>     * pretty-print.h (output_buffer_append_r): Likewise.

Re: PR pretty-print/67567 do not pass NULL as a string

[FX]

> PING: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01219.html

Given that this comes from submodules, which were recently introduced by Paul, I hoped he could comment. Paul?

FX

Re: PR pretty-print/67567 do not pass NULL as a string

[Manuel López-Ibáñez]

On 20 September 2015 at 21:14, FX <fxcoudert@gmail.com> wrote:
>> PING: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01219.html
>
> Given that this comes from submodules, which were recently introduced by Paul, I hoped he could comment. Paul?

If you can fix the Fortran part, then that would be nice, but it
should not hold up my patch since my patch changes nothing in the
output of Fortran. It just allows catching this type of errors when
checking is enabled.

Cheers,

Manuel.

Re: PR pretty-print/67567 do not pass NULL as a string

[FX]

> If you can fix the Fortran part, then that would be nice, but it
> should not hold up my patch since my patch changes nothing in the
> output of Fortran. It just allows catching this type of errors when
> checking is enabled.

The patch includes a Fortran part, and if we can get it fixed when this is still fresh (submodules were just implemented few weeks ago), then it’s better!
If Paul doesn’t reply in 2 days, then OK to commit.

FX

Re: PR pretty-print/67567 do not pass NULL as a string

[Manuel López-Ibáñez]

On 20 September 2015 at 23:05, FX <fxcoudert@gmail.com> wrote:
>> If you can fix the Fortran part, then that would be nice, but it
>> should not hold up my patch since my patch changes nothing in the
>> output of Fortran. It just allows catching this type of errors when
>> checking is enabled.
>
> The patch includes a Fortran part, and if we can get it fixed when this is still fresh (submodules were just implemented few weeks ago), then it’s better!
> If Paul doesn’t reply in 2 days, then OK to commit.

Thanks. I still need approval for the diagnostics part, which should
go in any case.

Cheers,

Manuel.

Re: PR pretty-print/67567 do not pass NULL as a string

[Dodji Seketeli]

Manuel López-Ibáñez <lopezibanez@gmail.com> a écrit:

> 2015-09-15  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     PR pretty-print/67567
>     * resolve.c (resolve_fl_procedure): Work-around when iface->module
>     == NULL.

This is OK, thanks.

-- 
		Dodji