| Message ID | CAESRpQBH_-vymM7ugmvLG7BWptVpup9i7Sm9t2L71_aSTq==vA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
> 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
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.
> 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
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.
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.
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:
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.