Skip to content

Commit 13d5ca7

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 30f39e5 commit 13d5ca7

File tree

2 files changed

+29
-30
lines changed

2 files changed

+29
-30
lines changed

regcomp.c

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,24 @@ 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+
/* Any or all of these might be NULL.
1221+
1222+
There's no point in setting them to NULL after the free, since
1223+
pRExC_state is about to be released.
1224+
*/
1225+
SvREFCNT_dec(RExC_rx_sv);
1226+
Safefree(RExC_open_parens);
1227+
Safefree(RExC_close_parens);
1228+
Safefree(RExC_logical_to_parno);
1229+
Safefree(RExC_parno_to_logical);
1230+
1231+
Safefree(pRExC_state);
1232+
}
1233+
12161234
/*
12171235
* Perl_re_op_compile - the perl internal RE engine's function to compile a
12181236
* regular expression into internal code.
@@ -1294,8 +1312,6 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
12941312
bool recompile = 0;
12951313
bool runtime_code = 0;
12961314
scan_data_t data;
1297-
RExC_state_t RExC_state;
1298-
RExC_state_t * const pRExC_state = &RExC_state;
12991315
#ifdef TRIE_STUDY_OPT
13001316
/* search for "restudy" in this file for a detailed explanation */
13011317
int restudied = 0;
@@ -1307,7 +1323,11 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
13071323

13081324
DEBUG_r(if (!PL_colorset) reginitcolors());
13091325

1326+
RExC_state_t *pRExC_state = NULL;
1327+
Newxz(pRExC_state, 1, RExC_state_t);
13101328

1329+
SAVEDESTRUCTOR_X(release_RExC_state, pRExC_state);
1330+
13111331
pRExC_state->warn_text = NULL;
13121332
pRExC_state->unlexed_names = NULL;
13131333
pRExC_state->code_blocks = NULL;
@@ -1708,6 +1728,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
17081728

17091729
/* Clean up what we did in this parse */
17101730
SvREFCNT_dec_NN(RExC_rx_sv);
1731+
RExC_rx_sv = NULL;
17111732

17121733
goto redo_parse;
17131734
}
@@ -1783,12 +1804,12 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
17831804
/* search for "restudy" in this file for a detailed explanation */
17841805
if (!restudied) {
17851806
StructCopy(&zero_scan_data, &data, scan_data_t);
1786-
copyRExC_state = RExC_state;
1807+
copyRExC_state = *pRExC_state;
17871808
} else {
17881809
U32 seen=RExC_seen;
17891810
DEBUG_OPTIMISE_r(Perl_re_printf( aTHX_ "Restudying\n"));
17901811

1791-
RExC_state = copyRExC_state;
1812+
*pRExC_state = copyRExC_state;
17921813
if (seen & REG_TOP_LEVEL_BRANCHES_SEEN)
17931814
RExC_seen |= REG_TOP_LEVEL_BRANCHES_SEEN;
17941815
else
@@ -2307,22 +2328,10 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
23072328
regdump(RExC_rx);
23082329
});
23092330

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-
}
2331+
/* we're returning ownership of the SV to the caller, ensure the cleanup
2332+
* doesn't release it
2333+
*/
2334+
RExC_rx_sv = NULL;
23262335

23272336
#ifdef USE_ITHREADS
23282337
/* under ithreads the ?pat? PMf_USED flag on the pmop is simulated
@@ -9212,7 +9221,6 @@ S_output_posix_warnings(pTHX_ RExC_state_t *pRExC_state, AV* posix_warnings)
92129221
array is mortal, but is a
92139222
fail-safe */
92149223
(void) sv_2mortal(msg);
9215-
PREPARE_TO_DIE;
92169224
}
92179225
Perl_warner(aTHX_ packWARN(WARN_REGEXP), "%s", SvPVX(msg));
92189226
SvREFCNT_dec_NN(msg);

regcomp_internal.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,6 @@ static const scan_data_t zero_scan_data = {
894894
const char *ellipses = ""; \
895895
IV len = RExC_precomp_end - RExC_precomp; \
896896
\
897-
PREPARE_TO_DIE; \
898897
if (len > RegexLengthToShowInErrorMessages) { \
899898
/* chop 10 shorter than the max, to ensure meaning of "..." */ \
900899
len = RegexLengthToShowInErrorMessages - 10; \
@@ -927,7 +926,6 @@ static const scan_data_t zero_scan_data = {
927926
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL()
928927
*/
929928
#define vFAIL(m) STMT_START { \
930-
PREPARE_TO_DIE; \
931929
Simple_vFAIL(m); \
932930
} STMT_END
933931

@@ -943,7 +941,6 @@ static const scan_data_t zero_scan_data = {
943941
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL2().
944942
*/
945943
#define vFAIL2(m,a1) STMT_START { \
946-
PREPARE_TO_DIE; \
947944
Simple_vFAIL2(m, a1); \
948945
} STMT_END
949946

@@ -960,7 +957,6 @@ static const scan_data_t zero_scan_data = {
960957
* Calls SAVEDESTRUCTOR_X if needed, then Simple_vFAIL3().
961958
*/
962959
#define vFAIL3(m,a1,a2) STMT_START { \
963-
PREPARE_TO_DIE; \
964960
Simple_vFAIL3(m, a1, a2); \
965961
} STMT_END
966962

@@ -973,19 +969,16 @@ static const scan_data_t zero_scan_data = {
973969
} STMT_END
974970

975971
#define vFAIL4(m,a1,a2,a3) STMT_START { \
976-
PREPARE_TO_DIE; \
977972
Simple_vFAIL4(m, a1, a2, a3); \
978973
} STMT_END
979974

980975
/* A specialized version of vFAIL2 that works with UTF8f */
981976
#define vFAIL2utf8f(m, a1) STMT_START { \
982-
PREPARE_TO_DIE; \
983977
S_re_croak(aTHX_ UTF, m REPORT_LOCATION, a1, \
984978
REPORT_LOCATION_ARGS(RExC_parse)); \
985979
} STMT_END
986980

987981
#define vFAIL3utf8f(m, a1, a2) STMT_START { \
988-
PREPARE_TO_DIE; \
989982
S_re_croak(aTHX_ UTF, m REPORT_LOCATION, a1, a2, \
990983
REPORT_LOCATION_ARGS(RExC_parse)); \
991984
} STMT_END
@@ -1026,8 +1019,6 @@ static const scan_data_t zero_scan_data = {
10261019
__FILE__, __LINE__, loc); \
10271020
} \
10281021
if (TO_OUTPUT_WARNINGS(loc)) { \
1029-
if (ckDEAD(warns)) \
1030-
PREPARE_TO_DIE; \
10311022
code; \
10321023
UPDATE_WARNINGS_LOC(loc); \
10331024
} \

0 commit comments

Comments
 (0)