1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 11:42:21 +00:00

Temporarily revert separate stack for libast/regex (re: 1aa8f771)

Welcome to AT&T engineering practices in action: a fix in one thing
breaks a completely unreleated thing, but only in very specific
and inscrutable circumstances.

Commit ffe84ee7 introduced a regression test failure in types.sh:

test types begins at 2021-12-14+23:57:35
	types.sh[130]: z.r.s should be z.r.x
test types failed at 2021-12-14+23:57:35 with exit code 1 [ 86 tests 1 error ]
test types(C.UTF-8) begins at 2021-12-14+23:57:35
test types(C.UTF-8) passed at 2021-12-14+23:57:35 [ 86 tests 0 errors ]
test types(shcomp) begins at 2021-12-14+23:57:35
test types(shcomp) passed at 2021-12-14+23:57:35 [ 86 tests 0 errors ]

Only enough, I've *only* found this regression on the GitHub CI
runner. I've tried it on three different regular Linux systems and
it occurs on none of them, nor on macOS.

Another odd thing: it only fails on the first of those three test
runs. But my experiments show it fails very consistently.

Through a process of systematic elimination in a test branch, I've
found that the failure is triggered by the change to using a
separate stack in the regex code. All the other changes are fine.

Using a separate stack improves the robustness of the regex code,
but it apparently exposes some breakage in how the very dodgy
'typeset -T' code is handling the stack, which was being masked by
sharing a stack with it. Or at least that seems like the most
plausible explanation to me right now.

So, until that breakage can be traced and fixed, the regex code now
shares the main stack with everything else again for the time being.

_____
Just to record this: by adding a couple of debug lines:

  typeset -p z | sed 's/^/[DEBUG] /'
  printf '[DEBUG] %s\n' "${z.r.s}" "${z.r.x}"

the symptom reveals itself more clearly on the GitHub runner:

  test types begins at 2021-12-15+17:25:57
  [DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=''))
  [DEBUG]
  [DEBUG] foo
          types.sh[132]: z.r.s should be z.r.x
  test types failed at 2021-12-15+17:25:57 with exit code 1 [ 86 tests 1 error ]
  test types(C.UTF-8) begins at 2021-12-15+17:25:57
  [DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=foo))
  [DEBUG] foo
  [DEBUG] foo
  test types(C.UTF-8) passed at 2021-12-15+17:25:57 [ 86 tests 0 errors ]
  test types(shcomp) begins at 2021-12-15+17:25:57
  [DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=foo))
  [DEBUG] foo
  [DEBUG] foo
  test types(shcomp) passed at 2021-12-15+17:25:57 [ 86 tests 0 errors ]
This commit is contained in:
Martijn Dekker 2021-12-15 18:53:22 +01:00
parent 9166545aa3
commit 38aab428bb
3 changed files with 25 additions and 28 deletions

View file

@ -3279,8 +3279,6 @@ regcomp(regex_t* p, const char* pattern, regflags_t flags)
if (!(p->env = (Env_t*)alloc(disc, 0, sizeof(Env_t)))) if (!(p->env = (Env_t*)alloc(disc, 0, sizeof(Env_t))))
return fatal(disc, REG_ESPACE, pattern); return fatal(disc, REG_ESPACE, pattern);
memset(p->env, 0, sizeof(*p->env)); memset(p->env, 0, sizeof(*p->env));
if (!(p->env->mst = stkopen(STK_SMALL|STK_NULL)))
return fatal(disc, REG_ESPACE, pattern);
memset(&env, 0, sizeof(env)); memset(&env, 0, sizeof(env));
env.regex = p; env.regex = p;
env.flags = flags; env.flags = flags;

View file

@ -537,7 +537,6 @@ typedef struct reglib_s /* library private regex_t info */
Vector_t* bestpos; /* ditto for best match */ Vector_t* bestpos; /* ditto for best match */
regmatch_t* match; /* subexrs in current match */ regmatch_t* match; /* subexrs in current match */
regmatch_t* best; /* ditto in best match yet */ regmatch_t* best; /* ditto in best match yet */
Stk_t* mst; /* match stack */
Stk_pos_t stk; /* exec stack pos */ Stk_pos_t stk; /* exec stack pos */
size_t min; /* minimum match length */ size_t min; /* minimum match length */
size_t nsub; /* internal re_nsub */ size_t nsub; /* internal re_nsub */

View file

@ -236,8 +236,8 @@ typedef struct
} Match_frame_t; } Match_frame_t;
#define matchpush(e,x) ((x)->re.group.number?_matchpush(e,x):0) #define matchpush(e,x) ((x)->re.group.number?_matchpush(e,x):0)
#define matchcopy(e,x) do if ((x)->re.group.number) { Match_frame_t* fp = (void*)stkframe(e->mst)->data; memcpy(fp->match, fp->save, fp->size); } while (0) #define matchcopy(e,x) do if ((x)->re.group.number) { Match_frame_t* fp = (void*)stkframe(stkstd)->data; memcpy(fp->match, fp->save, fp->size); } while (0)
#define matchpop(e,x) do if ((x)->re.group.number) { Match_frame_t* fp = (void*)stkframe(e->mst)->data; memcpy(fp->match, fp->save, fp->size); stkpop(e->mst); } while (0) #define matchpop(e,x) do if ((x)->re.group.number) { Match_frame_t* fp = (void*)stkframe(stkstd)->data; memcpy(fp->match, fp->save, fp->size); stkpop(stkstd); } while (0)
#define pospop(e) (--(e)->pos->cur) #define pospop(e) (--(e)->pos->cur)
@ -256,7 +256,7 @@ _matchpush(Env_t* env, Rex_t* rex)
if (rex->re.group.number <= 0 || (num = rex->re.group.last - rex->re.group.number + 1) <= 0) if (rex->re.group.number <= 0 || (num = rex->re.group.last - rex->re.group.number + 1) <= 0)
num = 0; num = 0;
if (!(f = (Match_frame_t*)stkpush(env->mst, sizeof(Match_frame_t) + (num - 1) * sizeof(regmatch_t)))) if (!(f = (Match_frame_t*)stkpush(stkstd, sizeof(Match_frame_t) + (num - 1) * sizeof(regmatch_t))))
{ {
env->error = REG_ESPACE; env->error = REG_ESPACE;
return 1; return 1;
@ -964,7 +964,7 @@ DEBUG_TEST(0x0008,(sfprintf(sfstdout, "AHA#%04d 0x%04x parse %s `%-.*s'\n", __LI
e = env->end; e = env->end;
if (!(rex->flags & REG_MINIMAL)) if (!(rex->flags & REG_MINIMAL))
{ {
if (!(b = (unsigned char*)stkpush(env->mst, n))) if (!(b = (unsigned char*)stkpush(stkstd, n)))
{ {
env->error = REG_ESPACE; env->error = REG_ESPACE;
return BAD; return BAD;
@ -978,19 +978,19 @@ DEBUG_TEST(0x0008,(sfprintf(sfstdout, "AHA#%04d 0x%04x parse %s `%-.*s'\n", __LI
switch (follow(env, rex, cont, s)) switch (follow(env, rex, cont, s))
{ {
case BAD: case BAD:
stkpop(env->mst); stkpop(stkstd);
return BAD; return BAD;
case CUT: case CUT:
stkpop(env->mst); stkpop(stkstd);
return CUT; return CUT;
case BEST: case BEST:
stkpop(env->mst); stkpop(stkstd);
return BEST; return BEST;
case GOOD: case GOOD:
r = GOOD; r = GOOD;
break; break;
} }
stkpop(env->mst); stkpop(stkstd);
} }
else else
{ {
@ -1110,7 +1110,7 @@ DEBUG_TEST(0x0008,(sfprintf(sfstdout, "AHA#%04d 0x%04x parse %s `%-.*s'\n", __LI
} }
else else
{ {
if (!(b = (unsigned char*)stkpush(env->mst, n))) if (!(b = (unsigned char*)stkpush(stkstd, n)))
{ {
env->error = REG_ESPACE; env->error = REG_ESPACE;
return BAD; return BAD;
@ -1122,19 +1122,19 @@ DEBUG_TEST(0x0008,(sfprintf(sfstdout, "AHA#%04d 0x%04x parse %s `%-.*s'\n", __LI
switch (follow(env, rex, cont, s)) switch (follow(env, rex, cont, s))
{ {
case BAD: case BAD:
stkpop(env->mst); stkpop(stkstd);
return BAD; return BAD;
case CUT: case CUT:
stkpop(env->mst); stkpop(stkstd);
return CUT; return CUT;
case BEST: case BEST:
stkpop(env->mst); stkpop(stkstd);
return BEST; return BEST;
case GOOD: case GOOD:
r = GOOD; r = GOOD;
break; break;
} }
stkpop(env->mst); stkpop(stkstd);
} }
} }
else else
@ -1422,7 +1422,7 @@ DEBUG_TEST(0x0200,(sfprintf(sfstdout,"AHA#%04d 0x%04x parse %s=>%s `%-.*s'\n", _
n = ((i + 7) >> 3) + 1; n = ((i + 7) >> 3) + 1;
catcher.type = REX_NEG_CATCH; catcher.type = REX_NEG_CATCH;
catcher.re.neg_catch.beg = s; catcher.re.neg_catch.beg = s;
if (!(p = (unsigned char*)stkpush(env->mst, n))) if (!(p = (unsigned char*)stkpush(stkstd, n)))
return BAD; return BAD;
memset(catcher.re.neg_catch.index = p, 0, n); memset(catcher.re.neg_catch.index = p, 0, n);
catcher.next = rex->next; catcher.next = rex->next;
@ -1454,7 +1454,7 @@ DEBUG_TEST(0x0200,(sfprintf(sfstdout,"AHA#%04d 0x%04x parse %s=>%s `%-.*s'\n", _
break; break;
} }
} }
stkpop(env->mst); stkpop(stkstd);
return r; return r;
case REX_NEG_CATCH: case REX_NEG_CATCH:
bitset(rex->re.neg_catch.index, s - rex->re.neg_catch.beg); bitset(rex->re.neg_catch.index, s - rex->re.neg_catch.beg);
@ -1521,7 +1521,7 @@ DEBUG_TEST(0x0200,(sfprintf(sfstdout,"AHA#%04d 0x%04x parse %s=>%s `%-.*s'\n", _
} }
else else
{ {
if (!(b = (unsigned char*)stkpush(env->mst, n))) if (!(b = (unsigned char*)stkpush(stkstd, n)))
{ {
env->error = REG_ESPACE; env->error = REG_ESPACE;
return BAD; return BAD;
@ -1553,19 +1553,19 @@ DEBUG_TEST(0x0200,(sfprintf(sfstdout,"AHA#%04d 0x%04x parse %s=>%s `%-.*s'\n", _
switch (follow(env, rex, cont, s)) switch (follow(env, rex, cont, s))
{ {
case BAD: case BAD:
stkpop(env->mst); stkpop(stkstd);
return BAD; return BAD;
case BEST: case BEST:
stkpop(env->mst); stkpop(stkstd);
return BEST; return BEST;
case CUT: case CUT:
stkpop(env->mst); stkpop(stkstd);
return CUT; return CUT;
case GOOD: case GOOD:
r = GOOD; r = GOOD;
break; break;
} }
stkpop(env->mst); stkpop(stkstd);
} }
} }
else else
@ -1881,14 +1881,14 @@ regnexec_20120528(const regex_t* p, const char* s, size_t len, size_t nmatch, re
env->regex = p; env->regex = p;
env->beg = (unsigned char*)s; env->beg = (unsigned char*)s;
env->end = env->beg + len; env->end = env->beg + len;
stknew(stkstd, &env->stk);
env->flags &= ~REG_EXEC; env->flags &= ~REG_EXEC;
env->flags |= (flags & REG_EXEC); env->flags |= (flags & REG_EXEC);
advance = 0; advance = 0;
stknew(env->mst, &env->stk);
if (env->stack = env->hard || !(env->flags & REG_NOSUB) && nmatch) if (env->stack = env->hard || !(env->flags & REG_NOSUB) && nmatch)
{ {
n = env->nsub; n = env->nsub;
if (!(env->match = (regmatch_t*)stkpush(env->mst, 2 * (n + 1) * sizeof(regmatch_t))) || if (!(env->match = (regmatch_t*)stkpush(stkstd, 2 * (n + 1) * sizeof(regmatch_t))) ||
!env->pos && !(env->pos = vecopen(16, sizeof(Pos_t))) || !env->pos && !(env->pos = vecopen(16, sizeof(Pos_t))) ||
!env->bestpos && !(env->bestpos = vecopen(16, sizeof(Pos_t)))) !env->bestpos && !(env->bestpos = vecopen(16, sizeof(Pos_t))))
{ {
@ -2023,7 +2023,7 @@ regnexec_20120528(const regex_t* p, const char* s, size_t len, size_t nmatch, re
} }
k = 0; k = 0;
done: done:
stkold(env->mst, &env->stk); stkold(stkstd, &env->stk);
env->stk.base = 0; env->stk.base = 0;
if (k > REG_NOMATCH) if (k > REG_NOMATCH)
fatal(p->env->disc, k, NiL); fatal(p->env->disc, k, NiL);
@ -2052,8 +2052,8 @@ regfree(regex_t* p)
vecclose(env->pos); vecclose(env->pos);
if (env->bestpos) if (env->bestpos)
vecclose(env->bestpos); vecclose(env->bestpos);
if (env->mst) if (env->stk.base)
stkclose(env->mst); stkold(stkstd, &env->stk);
alloc(env->disc, env, 0); alloc(env->disc, env, 0);
} }
} }