Skip to content

Commit 72efebc

Browse files
committed
safer cleanup when failing to compile regexps
Prior to this commit when producing a warning the regexp compiler would check if the warning category was marked as FATAL, and if it was it would add clean up to the save stack to release buffers used during compilation and to release the working REGEXP SV. This causes two type of problems: - if an error was already queued, Perl_ck_warner() returns even if the warning is fatal, this meant that the normal clean up code Perl_re_op_compile() would also run, resulting in a double free of the buffers. - without fatal warnings, if a $SIG{__WARN__} handler died, the buffers and the working REGEXP SV would leak. Avoid this by using SAVEDESTRUCTOR_X() to release the memory and optionally the SV at the end of scope. Fixes #21661
1 parent a28a6c1 commit 72efebc

File tree

2 files changed

+37
-30
lines changed

2 files changed

+37
-30
lines changed

regcomp.c

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,32 @@ S_set_regex_pv(pTHX_ RExC_state_t *pRExC_state, REGEXP *Rx)
12131213
SvCUR_set(Rx, p - RX_WRAPPED(Rx));
12141214
}
12151215

1216+
static void
1217+
release_RExC_state(pTHX_ void *vstate) {
1218+
RExC_state_t *pRExC_state = (RExC_state_t *)vstate;
1219+
1220+
if (RExC_rx_sv) \
1221+
SvREFCNT_dec(RExC_rx_sv); \
1222+
if (RExC_open_parens) {
1223+
Safefree(RExC_open_parens);
1224+
RExC_open_parens = NULL;
1225+
}
1226+
if (RExC_close_parens) {
1227+
Safefree(RExC_close_parens);
1228+
RExC_close_parens = NULL;
1229+
}
1230+
if (RExC_logical_to_parno) {
1231+
Safefree(RExC_logical_to_parno);
1232+
RExC_logical_to_parno = NULL;
1233+
}
1234+
if (RExC_parno_to_logical) {
1235+
Safefree(RExC_parno_to_logical);
1236+
RExC_parno_to_logical = NULL;
1237+
}
1238+
1239+
Safefree(pRExC_state);
1240+
}
1241+
12161242
/*
12171243
* Perl_re_op_compile - the perl internal RE engine's function to compile a
12181244
* regular expression into internal code.
@@ -1294,8 +1320,6 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
12941320
bool recompile = 0;
12951321
bool runtime_code = 0;
12961322
scan_data_t data;
1297-
RExC_state_t RExC_state;
1298-
RExC_state_t * const pRExC_state = &RExC_state;
12991323
#ifdef TRIE_STUDY_OPT
13001324
/* search for "restudy" in this file for a detailed explanation */
13011325
int restudied = 0;
@@ -1307,7 +1331,11 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
13071331

13081332
DEBUG_r(if (!PL_colorset) reginitcolors());
13091333

1334+
RExC_state_t *pRExC_state = NULL;
1335+
Newxz(pRExC_state, 1, RExC_state_t);
13101336

1337+
SAVEDESTRUCTOR_X(release_RExC_state, pRExC_state);
1338+
13111339
pRExC_state->warn_text = NULL;
13121340
pRExC_state->unlexed_names = NULL;
13131341
pRExC_state->code_blocks = NULL;
@@ -1708,6 +1736,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
17081736

17091737
/* Clean up what we did in this parse */
17101738
SvREFCNT_dec_NN(RExC_rx_sv);
1739+
RExC_rx_sv = NULL;
17111740

17121741
goto redo_parse;
17131742
}
@@ -1783,12 +1812,12 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
17831812
/* search for "restudy" in this file for a detailed explanation */
17841813
if (!restudied) {
17851814
StructCopy(&zero_scan_data, &data, scan_data_t);
1786-
copyRExC_state = RExC_state;
1815+
copyRExC_state = *pRExC_state;
17871816
} else {
17881817
U32 seen=RExC_seen;
17891818
DEBUG_OPTIMISE_r(Perl_re_printf( aTHX_ "Restudying\n"));
17901819

1791-
RExC_state = copyRExC_state;
1820+
*pRExC_state = copyRExC_state;
17921821
if (seen & REG_TOP_LEVEL_BRANCHES_SEEN)
17931822
RExC_seen |= REG_TOP_LEVEL_BRANCHES_SEEN;
17941823
else
@@ -2307,22 +2336,10 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
23072336
regdump(RExC_rx);
23082337
});
23092338

2310-
if (RExC_open_parens) {
2311-
Safefree(RExC_open_parens);
2312-
RExC_open_parens = NULL;
2313-
}
2314-
if (RExC_close_parens) {
2315-
Safefree(RExC_close_parens);
2316-
RExC_close_parens = NULL;
2317-
}
2318-
if (RExC_logical_to_parno) {
2319-
Safefree(RExC_logical_to_parno);
2320-
RExC_logical_to_parno = NULL;
2321-
}
2322-
if (RExC_parno_to_logical) {
2323-
Safefree(RExC_parno_to_logical);
2324-
RExC_parno_to_logical = NULL;
2325-
}
2339+
/* we're returning ownership of the SV to the caller, ensure the cleanup
2340+
* doesn't release it
2341+
*/
2342+
RExC_rx_sv = NULL;
23262343

23272344
#ifdef USE_ITHREADS
23282345
/* under ithreads the ?pat? PMf_USED flag on the pmop is simulated
@@ -9212,7 +9229,6 @@ S_output_posix_warnings(pTHX_ RExC_state_t *pRExC_state, AV* posix_warnings)
92129229
array is mortal, but is a
92139230
fail-safe */
92149231
(void) sv_2mortal(msg);
9215-
PREPARE_TO_DIE;
92169232
}
92179233
Perl_warner(aTHX_ packWARN(WARN_REGEXP), "%s", SvPVX(msg));
92189234
SvREFCNT_dec_NN(msg);

regcomp_internal.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,6 @@ static const scan_data_t zero_scan_data = {
886886
const char *ellipses = ""; \
887887
IV len = RExC_precomp_end - RExC_precomp; \
888888
\
889-
PREPARE_TO_DIE; \
890889
if (len > RegexLengthToShowInErrorMessages) { \
891890
/* chop 10 shorter than the max, to ensure meaning of "..." */ \
892891
len = RegexLengthToShowInErrorMessages - 10; \
@@ -919,7 +918,6 @@ static const scan_data_t zero_scan_data = {
919918
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL()
920919
*/
921920
#define vFAIL(m) STMT_START { \
922-
PREPARE_TO_DIE; \
923921
Simple_vFAIL(m); \
924922
} STMT_END
925923

@@ -935,7 +933,6 @@ static const scan_data_t zero_scan_data = {
935933
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL2().
936934
*/
937935
#define vFAIL2(m,a1) STMT_START { \
938-
PREPARE_TO_DIE; \
939936
Simple_vFAIL2(m, a1); \
940937
} STMT_END
941938

@@ -952,7 +949,6 @@ static const scan_data_t zero_scan_data = {
952949
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL3().
953950
*/
954951
#define vFAIL3(m,a1,a2) STMT_START { \
955-
PREPARE_TO_DIE; \
956952
Simple_vFAIL3(m, a1, a2); \
957953
} STMT_END
958954

@@ -965,19 +961,16 @@ static const scan_data_t zero_scan_data = {
965961
} STMT_END
966962

967963
#define vFAIL4(m,a1,a2,a3) STMT_START { \
968-
PREPARE_TO_DIE; \
969964
Simple_vFAIL4(m, a1, a2, a3); \
970965
} STMT_END
971966

972967
/* A specialized version of vFAIL2 that works with UTF8f */
973968
#define vFAIL2utf8f(m, a1) STMT_START { \
974-
PREPARE_TO_DIE; \
975969
S_re_croak(aTHX_ UTF, m REPORT_LOCATION, a1, \
976970
REPORT_LOCATION_ARGS(RExC_parse)); \
977971
} STMT_END
978972

979973
#define vFAIL3utf8f(m, a1, a2) STMT_START { \
980-
PREPARE_TO_DIE; \
981974
S_re_croak(aTHX_ UTF, m REPORT_LOCATION, a1, a2, \
982975
REPORT_LOCATION_ARGS(RExC_parse)); \
983976
} STMT_END
@@ -1018,8 +1011,6 @@ static const scan_data_t zero_scan_data = {
10181011
__FILE__, __LINE__, loc); \
10191012
} \
10201013
if (TO_OUTPUT_WARNINGS(loc)) { \
1021-
if (ckDEAD(warns)) \
1022-
PREPARE_TO_DIE; \
10231014
code; \
10241015
UPDATE_WARNINGS_LOC(loc); \
10251016
} \

0 commit comments

Comments
 (0)