From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.3 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, NICE_REPLY_A,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 6BD821F4B4 for ; Sat, 9 Jan 2021 01:24:40 +0000 (UTC) Received: from localhost ([::1]:33328 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ky2zf-0000M2-LW for normalperson@yhbt.net; Fri, 08 Jan 2021 20:24:39 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:48990) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ky2zI-0000Lh-VQ for bug-gnulib@gnu.org; Fri, 08 Jan 2021 20:24:17 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]:49568) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ky2zI-0004iv-Lo; Fri, 08 Jan 2021 20:24:16 -0500 Received: from [2a02:8071:2c94:ea00::255b] (port=44770) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1ky2zI-0006Ho-44; Fri, 08 Jan 2021 20:24:16 -0500 Subject: Re: [PATCH 1/3] posix: Remove alloca usage on regex set_regs To: Adhemerval Zanella , libc-alpha@sourceware.org, Paul Eggert , bug-gnulib@gnu.org References: <20210106181707.1738066-1-adhemerval.zanella@linaro.org> From: Darshit Shah Message-ID: <4cfd0cb6-37b2-e7df-ec41-d175f62d963d@gnu.org> Date: Sat, 9 Jan 2021 02:24:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20210106181707.1738066-1-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" Hi, Since this patch has been applied, GNU Wget fails during the linking stage due to regmatch_list_free not being available: /usr/bin/ld: ../lib/libgnu.a(regex.o): in function `set_regs': /home/rincewind/Programming/wget/lib/regexec.c:1444: undefined reference to `regmatch_list_free' /usr/bin/ld: /home/rincewind/Programming/wget/lib/regexec.c:1421: undefined reference to `regmatch_list_free' /usr/bin/ld: /home/rincewind/Programming/wget/lib/regexec.c:1454: undefined reference to `regmatch_list_free' /usr/bin/ld: /home/rincewind/Programming/wget/lib/regexec.c:1444: undefined reference to `regmatch_list_free' /usr/bin/ld: /home/rincewind/Programming/wget/lib/regexec.c:1430: undefined reference to `regmatch_list_free' /usr/bin/ld: ../lib/libgnu.a(regex.o):/home/rincewind/Programming/wget/lib/regexec.c:1460: more undefined references to `regmatch_list_free' follow On 06.01.21 19:17, Adhemerval Zanella wrote: > It replaces the regmatch_t with a dynarray list. > > Checked on x86_64-linux-gnu. > --- > posix/regexec.c | 62 ++++++++++++++++++++++++------------------------- > 1 file changed, 31 insertions(+), 31 deletions(-) > > diff --git a/posix/regexec.c b/posix/regexec.c > index b083342f77..5e22f90842 100644 > --- a/posix/regexec.c > +++ b/posix/regexec.c > @@ -54,9 +54,6 @@ static Idx check_matching (re_match_context_t *mctx, bool fl_longest_match, > Idx *p_match_first); > static Idx check_halt_state_context (const re_match_context_t *mctx, > const re_dfastate_t *state, Idx idx); > -static void update_regs (const re_dfa_t *dfa, regmatch_t *pmatch, > - regmatch_t *prev_idx_match, Idx cur_node, > - Idx cur_idx, Idx nmatch); > static reg_errcode_t push_fail_stack (struct re_fail_stack_t *fs, > Idx str_idx, Idx dest_node, Idx nregs, > regmatch_t *regs, > @@ -1355,6 +1352,16 @@ pop_fail_stack (struct re_fail_stack_t *fs, Idx *pidx, Idx nregs, > return fs->stack[num].node; > } > > + > +#define DYNARRAY_STRUCT regmatch_list > +#define DYNARRAY_ELEMENT regmatch_t > +#define DYNARRAY_PREFIX regmatch_list_ > +#include > + > +static void update_regs (const re_dfa_t *dfa, regmatch_t *pmatch, > + struct regmatch_list *prev_idx_match, Idx cur_node, > + Idx cur_idx, Idx nmatch); > + > /* Set the positions where the subexpressions are starts/ends to registers > PMATCH. > Note: We assume that pmatch[0] is already set, and > @@ -1370,8 +1377,8 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > re_node_set eps_via_nodes; > struct re_fail_stack_t *fs; > struct re_fail_stack_t fs_body = { 0, 2, NULL }; > - regmatch_t *prev_idx_match; > - bool prev_idx_match_malloced = false; > + struct regmatch_list prev_idx_match; > + regmatch_list_init (&prev_idx_match); > > DEBUG_ASSERT (nmatch > 1); > DEBUG_ASSERT (mctx->state_log != NULL); > @@ -1388,23 +1395,18 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > cur_node = dfa->init_node; > re_node_set_init_empty (&eps_via_nodes); > > - if (__libc_use_alloca (nmatch * sizeof (regmatch_t))) > - prev_idx_match = (regmatch_t *) alloca (nmatch * sizeof (regmatch_t)); > - else > + if (!regmatch_list_resize (&prev_idx_match, nmatch)) > { > - prev_idx_match = re_malloc (regmatch_t, nmatch); > - if (prev_idx_match == NULL) > - { > - free_fail_stack_return (fs); > - return REG_ESPACE; > - } > - prev_idx_match_malloced = true; > + regmatch_list_free (&prev_idx_match); > + free_fail_stack_return (fs); > + return REG_ESPACE; > } > - memcpy (prev_idx_match, pmatch, sizeof (regmatch_t) * nmatch); > + for (size_t i = 0; i < nmatch; i++) > + *regmatch_list_at (&prev_idx_match, i) = pmatch[i]; > > for (idx = pmatch[0].rm_so; idx <= pmatch[0].rm_eo ;) > { > - update_regs (dfa, pmatch, prev_idx_match, cur_node, idx, nmatch); > + update_regs (dfa, pmatch, &prev_idx_match, cur_node, idx, nmatch); > > if (idx == pmatch[0].rm_eo && cur_node == mctx->last_node) > { > @@ -1417,8 +1419,7 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > if (reg_idx == nmatch) > { > re_node_set_free (&eps_via_nodes); > - if (prev_idx_match_malloced) > - re_free (prev_idx_match); > + regmatch_list_free (&prev_idx_match); > return free_fail_stack_return (fs); > } > cur_node = pop_fail_stack (fs, &idx, nmatch, pmatch, > @@ -1427,8 +1428,7 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > else > { > re_node_set_free (&eps_via_nodes); > - if (prev_idx_match_malloced) > - re_free (prev_idx_match); > + regmatch_list_free (&prev_idx_match); > return REG_NOERROR; > } > } > @@ -1442,8 +1442,7 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > if (__glibc_unlikely (cur_node == -2)) > { > re_node_set_free (&eps_via_nodes); > - if (prev_idx_match_malloced) > - re_free (prev_idx_match); > + regmatch_list_free (&prev_idx_match); > free_fail_stack_return (fs); > return REG_ESPACE; > } > @@ -1453,15 +1452,13 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, > else > { > re_node_set_free (&eps_via_nodes); > - if (prev_idx_match_malloced) > - re_free (prev_idx_match); > + regmatch_list_free (&prev_idx_match); > return REG_NOMATCH; > } > } > } > re_node_set_free (&eps_via_nodes); > - if (prev_idx_match_malloced) > - re_free (prev_idx_match); > + regmatch_list_free (&prev_idx_match); > return free_fail_stack_return (fs); > } > > @@ -1483,7 +1480,8 @@ free_fail_stack_return (struct re_fail_stack_t *fs) > > static void > update_regs (const re_dfa_t *dfa, regmatch_t *pmatch, > - regmatch_t *prev_idx_match, Idx cur_node, Idx cur_idx, Idx nmatch) > + struct regmatch_list *prev_idx_match, Idx cur_node, Idx cur_idx, > + Idx nmatch) > { > int type = dfa->nodes[cur_node].type; > if (type == OP_OPEN_SUBEXP) > @@ -1508,18 +1506,20 @@ update_regs (const re_dfa_t *dfa, regmatch_t *pmatch, > pmatch[reg_num].rm_eo = cur_idx; > /* This is a non-empty match or we are not inside an optional > subexpression. Accept this right away. */ > - memcpy (prev_idx_match, pmatch, sizeof (regmatch_t) * nmatch); > + for (size_t i = 0; i < nmatch; i++) > + *regmatch_list_at (prev_idx_match, i) = pmatch[i]; > } > else > { > if (dfa->nodes[cur_node].opt_subexp > - && prev_idx_match[reg_num].rm_so != -1) > + && regmatch_list_at (prev_idx_match, reg_num)->rm_so != -1) > /* We transited through an empty match for an optional > subexpression, like (a?)*, and this is not the subexp's > first match. Copy back the old content of the registers > so that matches of an inner subexpression are undone as > well, like in ((a?))*. */ > - memcpy (pmatch, prev_idx_match, sizeof (regmatch_t) * nmatch); > + memcpy (pmatch, regmatch_list_begin (prev_idx_match), > + sizeof (regmatch_t) * nmatch); > else > /* We completed a subexpression, but it may be part of > an optional one, so do not update PREV_IDX_MATCH. */ >