Hi Eric, thanks for your review! On Saturday 19 of September 2015 15:05:07 Eric Blake wrote: > On 09/19/2015 02:09 AM, Pavel Raiskup wrote: > > Hi Hiroyuki Sato, > > > > On Wednesday 02 of September 2015 16:00:34 Hiroyuki Sato wrote: > >> This configure.ac is extreme slow on libtool-2.4.6. > >> But It run smoothly on libtool-2.4.2. > >> https://github.com/groonga/groonga/blob/master/configure.ac > > > > thanks for reproducer! > > > > This _really_ looks like issue mentioned [1], though the thread is > > believed to be resolved (with existing small slowdown between > > libtool-2.4.5 and 2.4.2). Let me CC Robert whether this patch does not > > actually fix the "1 sec" slowdown of libtoolize. > > This looks very much like the same bug that gettext had: > http://git.savannah.gnu.org/cgit/gettext.git/commit/?id=d75090f2 > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764580
Indeed, I missed that report. > Your proposed solution doesn't seem quite right: > > > # Disable these macros. > > m4_undefine([m4_dnl]) > > - m4_undefine([m4_include]) > > m4_undefine([m4_m4exit]) > > m4_undefine([m4_m4wrap]) > > m4_undefine([m4_maketemp]) > > + # Rather do not use undefine here because people tend to define > > + # macros of the same name as file included in their bodies - which > > + # results in infinite recursion. > > + m4_define([m4_include], []) > > I'd recommend that you use the same fix as gettext, and define ALL of > these macros to an empty string, rather than special-casing m4_include > as the only one that does not get undefined. Agreed, m4_pushdef([trace_breaking_macro]) should not cause any harm. I fixed the patch (attached). Pavel
>From 5e8a4c5173f1aa0786e8eba15fb07bfe04b83862 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup <prais...@redhat.com> Date: Fri, 18 Sep 2015 23:17:07 +0200 Subject: [PATCH] libtoolize: fix infinite recursion in m4 Some projects use this construct in configure.ac: m4_define([version], m4_include([version]) pkg_version=version When the m4_include builtin is undefined (as was done in libtoolize and extract-trace scripts), the call to this 'version' macro gone to infinite recursion (until ENOMEM). So rather re-define all potentially dangerous macros by empty strings, suggested by Eric Blake. While we are on it, merge the macro-"blacklist" with similar list implemented in gettext, except the 'm4_esyscmd'. It's kept defined because we already trace AC_INIT macro for package version, while it is often specified by m4_esyscmd(git-version-gen). Similarly to m4_include, m4_esyscmd might be opt-in-blacklisted in future. References: http://lists.gnu.org/archive/html/libtool/2015-09/msg00000.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764580 * gl/build-aux/extract-trace (_G_mini): Redefine trace-breaking macros to empty strings rather than undefining those. Use 'dnl' for comments. * bootstrap: Likewise, sync with extract-trace. * NEWS: Document. * NO-THANKS: Mention Hiroyuki Sato. --- NEWS | 4 ++++ NO-THANKS | 1 + bootstrap | 44 ++++++++++++++++++++++++++++---------------- gl/build-aux/extract-trace | 44 ++++++++++++++++++++++++++++---------------- 4 files changed, 61 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index c5c9023..82dd037 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ NEWS - list of user-visible changes between releases of GNU Libtool * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes: + + - Fix significant slowdown of libtoolize for certain projects (regression + introduced in 2.4.3 release) caused by infinite m4 macro recursion. * Noteworthy changes in release 2.4.6 (2015-02-15) [stable] diff --git a/NO-THANKS b/NO-THANKS index 10de16b..51ec8eb 100644 --- a/NO-THANKS +++ b/NO-THANKS @@ -82,6 +82,7 @@ Donn Washburn n5...@comcast.net Erik van Pienbroek erik-...@vanpienbroek.nl Ethan Mallove ethan.mall...@sun.com Fred Cox sailorf...@yahoo.com +Hiroyuki Sato hiroys...@gmail.com Jakub Bogusz qbo...@pld-linux.org James Su james...@gmail.com Jay Krell jay.kr...@cornell.edu diff --git a/bootstrap b/bootstrap index 4596413..17fb169 100755 --- a/bootstrap +++ b/bootstrap @@ -2472,29 +2472,41 @@ func_extract_trace () # arguments to Autocof functions, but without following # 'm4_s?include' files. _G_mini=' - # Initialisation. + dnl Initialisation. m4_changequote([,]) m4_define([m4_copy], [m4_define([$2], m4_defn([$1]))]) m4_define([m4_rename], [m4_copy([$1], [$2])m4_undefine([$1])]) - # Disable these macros. - m4_undefine([m4_dnl]) - m4_undefine([m4_include]) - m4_undefine([m4_m4exit]) - m4_undefine([m4_m4wrap]) - m4_undefine([m4_maketemp]) - - # Copy and rename macros not handled by "m4 --prefix". + dnl Replace macros which may abort m4 with a no-op variant. + m4_pushdef([m4_assert]) + m4_pushdef([m4_exit]) + m4_pushdef([m4_fatal]) + m4_pushdef([m4_m4exit]) + + dnl Replace macros that might break stderr of m4. + m4_pushdef([m4_errprint]) + m4_pushdef([m4_errprintn]) + m4_pushdef([m4_include]) + m4_pushdef([m4_warn]) + + dnl Avoid side-effects of tracing by extract-trace. + m4_pushdef([m4_maketemp]) + m4_pushdef([m4_mkstemp]) + + dnl TODO: reasons for this + m4_pushdef([m4_dnl]) + m4_pushdef([m4_m4wrap]) + + dnl Copy and rename macros not handled by "m4 --prefix". m4_define([dnl], [m4_builtin([dnl])]) m4_copy([m4_define], [m4_defun]) m4_rename([m4_ifelse], [m4_if]) - m4_ifdef([m4_mkstemp], [m4_undefine([m4_mkstemp])]) m4_rename([m4_patsubst], [m4_bpatsubst]) m4_rename([m4_regexp], [m4_bregexp]) - # "m4sugar.mini" - useful m4-time macros for dynamic arguments. - # If we discover packages that need more m4 macros defined in - # order to bootstrap correctly, add them here: + dnl "m4sugar.mini" - useful m4-time macros for dynamic arguments. + dnl If we discover packages that need more m4 macros defined in + dnl order to bootstrap correctly, add them here: m4_define([m4_bmatch], [m4_if([$#], 0, [], [$#], 1, [], [$#], 2, [$2], [m4_if(m4_bregexp([$1], [$2]), -1, @@ -2505,11 +2517,11 @@ func_extract_trace () m4_define([m4_require], [$1]) m4_define([m4_shift3], [m4_shift(m4shift(m4shift($@)))]) - # "autoconf.mini" - things from autoconf macros we care about. + dnl "autoconf.mini" - things from autoconf macros we care about. m4_copy([m4_defun], [AC_DEFUN]) - # Dummy definitions for the macros we want to trace. - # AM_INIT_AUTOMAKE at least produces no trace without this. + dnl Dummy definitions for the macros we want to trace. + dnl AM_INIT_AUTOMAKE at least produces no trace without this. ' _G_save=$IFS diff --git a/gl/build-aux/extract-trace b/gl/build-aux/extract-trace index 315a32a..c6abd21 100755 --- a/gl/build-aux/extract-trace +++ b/gl/build-aux/extract-trace @@ -329,29 +329,41 @@ func_extract_trace () # arguments to Autocof functions, but without following # 'm4_s?include' files. _G_mini=' - # Initialisation. + dnl Initialisation. m4_changequote([,]) m4_define([m4_copy], [m4_define([$2], m4_defn([$1]))]) m4_define([m4_rename], [m4_copy([$1], [$2])m4_undefine([$1])]) - # Disable these macros. - m4_undefine([m4_dnl]) - m4_undefine([m4_include]) - m4_undefine([m4_m4exit]) - m4_undefine([m4_m4wrap]) - m4_undefine([m4_maketemp]) - - # Copy and rename macros not handled by "m4 --prefix". + dnl Replace macros which may abort m4 with a no-op variant. + m4_pushdef([m4_assert]) + m4_pushdef([m4_exit]) + m4_pushdef([m4_fatal]) + m4_pushdef([m4_m4exit]) + + dnl Replace macros that might break stderr of m4. + m4_pushdef([m4_errprint]) + m4_pushdef([m4_errprintn]) + m4_pushdef([m4_include]) + m4_pushdef([m4_warn]) + + dnl Avoid side-effects of tracing by extract-trace. + m4_pushdef([m4_maketemp]) + m4_pushdef([m4_mkstemp]) + + dnl TODO: reasons for this + m4_pushdef([m4_dnl]) + m4_pushdef([m4_m4wrap]) + + dnl Copy and rename macros not handled by "m4 --prefix". m4_define([dnl], [m4_builtin([dnl])]) m4_copy([m4_define], [m4_defun]) m4_rename([m4_ifelse], [m4_if]) - m4_ifdef([m4_mkstemp], [m4_undefine([m4_mkstemp])]) m4_rename([m4_patsubst], [m4_bpatsubst]) m4_rename([m4_regexp], [m4_bregexp]) - # "m4sugar.mini" - useful m4-time macros for dynamic arguments. - # If we discover packages that need more m4 macros defined in - # order to bootstrap correctly, add them here: + dnl "m4sugar.mini" - useful m4-time macros for dynamic arguments. + dnl If we discover packages that need more m4 macros defined in + dnl order to bootstrap correctly, add them here: m4_define([m4_bmatch], [m4_if([$#], 0, [], [$#], 1, [], [$#], 2, [$2], [m4_if(m4_bregexp([$1], [$2]), -1, @@ -362,11 +374,11 @@ func_extract_trace () m4_define([m4_require], [$1]) m4_define([m4_shift3], [m4_shift(m4shift(m4shift($@)))]) - # "autoconf.mini" - things from autoconf macros we care about. + dnl "autoconf.mini" - things from autoconf macros we care about. m4_copy([m4_defun], [AC_DEFUN]) - # Dummy definitions for the macros we want to trace. - # AM_INIT_AUTOMAKE at least produces no trace without this. + dnl Dummy definitions for the macros we want to trace. + dnl AM_INIT_AUTOMAKE at least produces no trace without this. ' _G_save=$IFS -- 2.5.0
