On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdeli...@charter.net> wrote:
>
> On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote:
> > On Tue, 4 Sep 2018 at 18:43, Andrew Benson <aben...@carnegiescience.edu>
> > wrote:
> >>
> >> As suggested by Janus, PR87103 is easily fixed by the attached patch which
> >> increases GFC_MAX_SYMBOL_LEN to 76 (sufficient to hold the maximum allowed
> >> F08
> >> symbol length of 63, plus a null terminator, plus the "__tmp_class_"
> >> prefix).
> >
> > This is so much wrong.
> > Note that this will be fixed properly by the changes contained in the
> > https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/aldot/fortran-fe-stringpool
> > branch.
> > There we keep the GFC_MAX_SYMBOL_LEN at 63 proper but use an internal
> > buffer double that size which in turn is sufficient to hold all
> > compiler-generated identifiers.
> > See gfc_get_string() even in current TOT.
> >
> > Maybe we should bite the bullet and start to merge the stringpool
> > changes now instead of this hack?
>
> It all makes sense to me, please proceed. (my 2 cents worth)
Ok so i will reread the fortran-fe-stringpool series and submit it
here for review.
Let's return to the issue at hand for a moment, though.
I tested the attached alternate fix on top of the
fortran-fe-stringpool branch where it fixes PR87103.
Maybe somebody has spare cycles to test it on top of current trunk?
thanks,
[PATCH,FORTRAN] PR87103: Remove max symbol length check from gfc_new_symbol
gfc_match_name does check for too long names already. Since
gfc_new_symbol is also called for symbols with internal names containing
compiler-generated prefixes, these internal names can easily exceed the
max_identifier_length mandated by the standard.
gcc/fortran/ChangeLog
2018-09-04 Bernhard Reutner-Fischer <al...@gcc.gnu.org>
PR fortran/87103
* expr.c (gfc_check_conformance): Check vsnprintf for truncation.
* iresolve.c (gfc_get_string): Likewise.
* symbol.c (gfc_new_symbol): Remove check for maximum symbol
name length. Remove redundant 0 setting of new calloc()ed
gfc_symbol.
Remove max symbol length check from gfc_new_symbol
gfc_match_name does check for too long names already. Since
gfc_new_symbol is also called for symbols with internal names containing
compiler-generated prefixes, these internal names can easily exceed the
max_identifier_length mandated by the standard.
gcc/fortran/ChangeLog
2018-09-04 Bernhard Reutner-Fischer <al...@gcc.gnu.org>
PR fortran/87103
* expr.c (gfc_check_conformance): Check vsnprintf for truncation.
* iresolve.c (gfc_get_string): Likewise.
* symbol.c (gfc_new_symbol): Remove check for maximum symbol
name length. Remove redundant 0 setting of new calloc()ed
gfc_symbol.
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index c5bf822cd24..6b5671390ec 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -3225,8 +3225,10 @@ gfc_check_conformance (gfc_expr *op1, gfc_expr *op2, const char *optype_msgid, .
return true;
va_start (argp, optype_msgid);
- vsnprintf (buffer, 240, optype_msgid, argp);
+ d = vsnprintf (buffer, sizeof (buffer), optype_msgid, argp);
va_end (argp);
+ if (d < 1 || d >= (int) sizeof (buffer)) /* Reject truncation. */
+ gfc_internal_error ("optype_msgid overflow: %d", d);
if (op1->rank != op2->rank)
{
diff --git a/gcc/fortran/iresolve.c b/gcc/fortran/iresolve.c
index 61663fec7e5..d7bd0545173 100644
--- a/gcc/fortran/iresolve.c
+++ b/gcc/fortran/iresolve.c
@@ -60,9 +60,12 @@ gfc_get_string (const char *format, ...)
}
else
{
+ int ret;
va_start (ap, format);
- vsnprintf (temp_name, sizeof (temp_name), format, ap);
+ ret = vsnprintf (temp_name, sizeof (temp_name), format, ap);
va_end (ap);
+ if (ret < 1 || ret >= (int) sizeof (temp_name)) /* Reject truncation. */
+ gfc_internal_error ("identifier overflow: %d", ret);
temp_name[sizeof (temp_name) - 1] = 0;
str = temp_name;
}
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index cde34c67482..fc3354f0457 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -3142,25 +3142,9 @@ gfc_new_symbol (const char *name, gfc_namespace *ns)
gfc_clear_ts (&p->ts);
gfc_clear_attr (&p->attr);
p->ns = ns;
-
p->declared_at = gfc_current_locus;
-
- if (strlen (name) > GFC_MAX_SYMBOL_LEN)
- gfc_internal_error ("new_symbol(): Symbol name too long");
-
p->name = gfc_get_string ("%s", name);
- /* Make sure flags for symbol being C bound are clear initially. */
- p->attr.is_bind_c = 0;
- p->attr.is_iso_c = 0;
-
- /* Clear the ptrs we may need. */
- p->common_block = NULL;
- p->f2k_derived = NULL;
- p->assoc = NULL;
- p->dt_next = NULL;
- p->fn_result_spec = 0;
-
return p;
}
