| Message ID | 5056833.bhCz5HFUgy@andrew-precision-3520 |
|---|---|
| State | New |
| Headers | show |
| Series | [fortan] PR87103 - [OOP] ICE in gfc_new_symbol() due to overlong symbol name | expand |
On Tue, 4 Sep 2018 at 18:43, Andrew Benson <abenson@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? thanks and cheers,
On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson <abenson@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 <aldot@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 <aldot@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; }
On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard Reutner-Fischer wrote: > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson <abenson@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 <aldot@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. This patch tests successfully on the current trunk for me.
This PR is still open - I re-tested the patch on the current trunk. The patch still applies with some line offsets (I've attached the updated patch) and regtests cleanly. It would be very helpful to me to get this patch committed if possible. Thanks, Andrew On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard Reutner-Fischer wrote: > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson <abenson@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 <aldot@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.
On Fri, 23 Aug 2019 17:17:37 -0700 Andrew Benson <abenson@carnegiescience.edu> wrote: > This PR is still open - I re-tested the patch on the current trunk. The patch > still applies with some line offsets (I've attached the updated patch) and > regtests cleanly. It would be very helpful to me to get this patch committed > if possible. I think Jerry ACKed the patch back then. I'll try to find the time to commit it maybe during one of the coming weekends unless someone else beats me to it.. Thanks for the reminder! Bernhard > > Thanks, > Andrew > > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard Reutner-Fischer > wrote: > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson <abenson@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 <aldot@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. > >
Thanks Bernhard. On Wednesday, August 28, 2019 9:00:36 PM PDT Bernhard Reutner-Fischer wrote: > On Fri, 23 Aug 2019 17:17:37 -0700 > > Andrew Benson <abenson@carnegiescience.edu> wrote: > > This PR is still open - I re-tested the patch on the current trunk. The > > patch still applies with some line offsets (I've attached the updated > > patch) and regtests cleanly. It would be very helpful to me to get this > > patch committed if possible. > > I think Jerry ACKed the patch back then. I'll try to find the time to > commit it maybe during one of the coming weekends unless someone else > beats me to it.. > > Thanks for the reminder! > Bernhard > > > Thanks, > > Andrew > > > > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard Reutner-Fischer > > > > wrote: > > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson > > > > > <abenson@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/for > > > > > tran > > > > > -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 <aldot@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.
I think this patch is still waiting to be applied. I checked that it applies against trunk (with line offsets) and reg tests cleanly and posted an updated version (diff'd against current trunk) at: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87103#c7 I'm happy to go ahead and commit this if Bernhard is ok with me doing so. -Andrew On Wednesday, August 28, 2019 9:00:36 PM PST Bernhard Reutner-Fischer wrote: > On Fri, 23 Aug 2019 17:17:37 -0700 > > Andrew Benson <abenson@carnegiescience.edu> wrote: > > This PR is still open - I re-tested the patch on the current trunk. The > > patch still applies with some line offsets (I've attached the updated > > patch) and regtests cleanly. It would be very helpful to me to get this > > patch committed if possible. > > I think Jerry ACKed the patch back then. I'll try to find the time to > commit it maybe during one of the coming weekends unless someone else > beats me to it.. > > Thanks for the reminder! > Bernhard > > > Thanks, > > Andrew > > > > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard Reutner-Fischer > > > > wrote: > > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle <jvdelisle@charter.net> wrote: > > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson > > > > > <abenson@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/for > > > > > tran > > > > > -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 <aldot@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.
On 29 January 2020 21:19:52 CET, Andrew Benson <abenson@carnegiescience.edu> wrote: >I think this patch is still waiting to be applied. I checked that it >applies >against trunk (with line offsets) and reg tests cleanly and posted an >updated >version (diff'd against current trunk) at: > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87103#c7 > >I'm happy to go ahead and commit this if Bernhard is ok with me doing >so. Please go ahead and push it. Many thanks in advance and sorry for the delay! thanks, > >-Andrew > >On Wednesday, August 28, 2019 9:00:36 PM PST Bernhard Reutner-Fischer >wrote: >> On Fri, 23 Aug 2019 17:17:37 -0700 >> >> Andrew Benson <abenson@carnegiescience.edu> wrote: >> > This PR is still open - I re-tested the patch on the current trunk. >The >> > patch still applies with some line offsets (I've attached the >updated >> > patch) and regtests cleanly. It would be very helpful to me to get >this >> > patch committed if possible. >> >> I think Jerry ACKed the patch back then. I'll try to find the time to >> commit it maybe during one of the coming weekends unless someone else >> beats me to it.. >> >> Thanks for the reminder! >> Bernhard >> >> > Thanks, >> > Andrew >> > >> > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard >Reutner-Fischer >> > >> > wrote: >> > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle ><jvdelisle@charter.net> >wrote: >> > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: >> > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson >> > > > > <abenson@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/for >> > > > > tran >> > > > > -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 <aldot@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.
Thanks Bernhard - this is now committed: https://gcc.gnu.org/g:004ac7b780308dc899e565b887c7def0a6e100f2 On Thursday, January 30, 2020 5:27:55 PM PST Bernhard Reutner-Fischer wrote: > On 29 January 2020 21:19:52 CET, Andrew Benson <abenson@carnegiescience.edu> wrote: > >I think this patch is still waiting to be applied. I checked that it > >applies > >against trunk (with line offsets) and reg tests cleanly and posted an > >updated > >version (diff'd against current trunk) at: > > > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87103#c7 > > > >I'm happy to go ahead and commit this if Bernhard is ok with me doing > >so. > > Please go ahead and push it. > Many thanks in advance and sorry for the delay! > thanks, > > >-Andrew > > > >On Wednesday, August 28, 2019 9:00:36 PM PST Bernhard Reutner-Fischer > > > >wrote: > >> On Fri, 23 Aug 2019 17:17:37 -0700 > >> > >> Andrew Benson <abenson@carnegiescience.edu> wrote: > >> > This PR is still open - I re-tested the patch on the current trunk. > > > >The > > > >> > patch still applies with some line offsets (I've attached the > > > >updated > > > >> > patch) and regtests cleanly. It would be very helpful to me to get > > > >this > > > >> > patch committed if possible. > >> > >> I think Jerry ACKed the patch back then. I'll try to find the time to > >> commit it maybe during one of the coming weekends unless someone else > >> beats me to it.. > >> > >> Thanks for the reminder! > >> Bernhard > >> > >> > Thanks, > >> > Andrew > >> > > >> > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard > > > >Reutner-Fischer > > > >> > wrote: > >> > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle > > > ><jvdelisle@charter.net> > > > >wrote: > >> > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote: > >> > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson > >> > > > > <abenson@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/for > > > >> > > > > tran > >> > > > > -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 <aldot@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.
Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 264085) +++ gcc/fortran/gfortran.h (working copy) @@ -54,7 +54,9 @@ not after. /* Major control parameters. */ -#define GFC_MAX_SYMBOL_LEN 63 /* Must be at least 63 for F2003. */ +/* Must be at least 63 for F2003, +1 for null terminator, + +12 for prefix "__tmp_class_". */ +#define GFC_MAX_SYMBOL_LEN 76 #define GFC_LETTERS 26 /* Number of letters in the alphabet. */ #define MAX_SUBRECORD_LENGTH 2147483639 /* 2**31-9 */