Issue #2148 - Discard RegExpShared data tables when discarding regexp JIT code.

Based on Mozilla bug 1378736's part 2.
Should fix the double-free.
Follow-up for #2083
This commit is contained in:
Job Bautista 2023-03-08 19:23:13 +08:00 committed by roytam1
commit cdd91edd79
8 changed files with 64 additions and 47 deletions

View file

@ -63,10 +63,11 @@ using namespace js::jit;
* The tempN registers are free to use for computations.
*/
NativeRegExpMacroAssembler::NativeRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared,
JSRuntime* rt, Mode mode, int registers_to_save)
: RegExpMacroAssembler(cx, *alloc, shared, registers_to_save),
cx(cx), runtime(rt), mode_(mode)
NativeRegExpMacroAssembler::NativeRegExpMacroAssembler(LifoAlloc* alloc, JSRuntime* rt,
Mode mode, int registers_to_save,
RegExpShared::JitCodeTables& tables)
: RegExpMacroAssembler(*alloc, registers_to_save),
tables(tables), runtime(rt), mode_(mode)
{
// Find physical registers for each compiler register.
AllocatableGeneralRegisterSet regs(GeneralRegisterSet::All());
@ -886,11 +887,11 @@ NativeRegExpMacroAssembler::CheckCharacterNotInRange(char16_t from, char16_t to,
}
void
NativeRegExpMacroAssembler::CheckBitInTable(uint8_t* table, Label* on_bit_set)
NativeRegExpMacroAssembler::CheckBitInTable(RegExpShared::JitCodeTable table, Label* on_bit_set)
{
JitSpew(SPEW_PREFIX "CheckBitInTable");
masm.movePtr(ImmPtr(table), temp0);
masm.movePtr(ImmPtr(table.get()), temp0);
// kTableMask is currently 127, so we need to mask even if the input is
// Latin1. V8 has the same issue.
@ -901,6 +902,13 @@ NativeRegExpMacroAssembler::CheckBitInTable(uint8_t* table, Label* on_bit_set)
masm.load8ZeroExtend(BaseIndex(temp0, temp1, TimesOne), temp0);
masm.branchTest32(Assembler::NonZero, temp0, temp0, BranchOrBacktrack(on_bit_set));
// Transfer ownership of |table| to the |tables| Vector.
{
AutoEnterOOMUnsafeRegion oomUnsafe;
if (!tables.append(Move(table)))
oomUnsafe.crash("RegExp table append");
}
}
void

View file

@ -87,8 +87,8 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem
// Type of input string to generate code for.
enum Mode { ASCII = 1, CHAR16 = 2 };
NativeRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared,
JSRuntime* rt, Mode mode, int registers_to_save);
NativeRegExpMacroAssembler(LifoAlloc* alloc, JSRuntime* rt, Mode mode, int registers_to_save,
RegExpShared::JitCodeTables& tables);
// Inherited virtual methods.
RegExpCode GenerateCode(JSContext* cx, bool match_only);
@ -116,7 +116,7 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem
jit::Label* on_in_range);
void CheckCharacterNotInRange(char16_t from, char16_t to,
jit::Label* on_not_in_range);
void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set);
void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set);
void CheckPosition(int cp_offset, jit::Label* on_outside_input);
void JumpOrBacktrack(jit::Label* to);
bool CheckSpecialCharacterClass(char16_t type, jit::Label* on_no_match);
@ -173,8 +173,8 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem
private:
jit::MacroAssembler masm;
RegExpShared::JitCodeTables& tables;
JSContext* cx;
JSRuntime* runtime;
Mode mode_;
jit::Label entry_label_;

View file

@ -1262,7 +1262,7 @@ RegExpCode
irregexp::CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data,
HandleLinearString sample, bool is_global, bool ignore_case,
bool is_ascii, bool match_only, bool force_bytecode, bool sticky,
bool unicode)
bool unicode, RegExpShared::JitCodeTables& tables)
{
if ((data->capture_count + 1) * 2 - 1 > RegExpMacroAssembler::kMaxRegister) {
JS_ReportErrorASCII(cx, "regexp too big");
@ -1350,10 +1350,10 @@ irregexp::CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompile
: NativeRegExpMacroAssembler::CHAR16;
ctx.emplace(cx, (jit::TempAllocator*) nullptr);
native_assembler.emplace(cx, &alloc, shared, cx->runtime(), mode, (data->capture_count + 1) * 2);
native_assembler.emplace(&alloc, cx->runtime(), mode, (data->capture_count + 1) * 2, tables);
assembler = native_assembler.ptr();
} else {
interpreted_assembler.emplace(cx, &alloc, shared, (data->capture_count + 1) * 2);
interpreted_assembler.emplace(&alloc, (data->capture_count + 1) * 2);
assembler = interpreted_assembler.ptr();
}
@ -2040,21 +2040,21 @@ BoyerMooreLookahead::EmitSkipInstructions(RegExpMacroAssembler* masm)
return true;
}
uint8_t* boolean_skip_table;
RegExpShared::JitCodeTable boolean_skip_table;
{
AutoEnterOOMUnsafeRegion oomUnsafe;
boolean_skip_table = static_cast<uint8_t*>(js_malloc(kSize));
if (!boolean_skip_table || !masm->shared->addTable(boolean_skip_table))
boolean_skip_table.reset(static_cast<uint8_t*>(js_malloc(kSize)));
if (!boolean_skip_table)
oomUnsafe.crash("Table malloc");
}
int skip_distance = GetSkipTable(min_lookahead, max_lookahead, boolean_skip_table);
int skip_distance = GetSkipTable(min_lookahead, max_lookahead, boolean_skip_table.get());
MOZ_ASSERT(skip_distance != 0);
jit::Label cont, again;
masm->Bind(&again);
masm->LoadCurrentCharacter(max_lookahead, &cont, true);
masm->CheckBitInTable(boolean_skip_table, &cont);
masm->CheckBitInTable(Move(boolean_skip_table), &cont);
masm->AdvanceCurrentPosition(skip_distance);
masm->JumpOrBacktrack(&again);
masm->Bind(&cont);
@ -2832,18 +2832,18 @@ EmitUseLookupTable(RegExpMacroAssembler* masm,
}
// TODO(erikcorry): Cache these.
uint8_t* ba;
RegExpShared::JitCodeTable ba;
{
AutoEnterOOMUnsafeRegion oomUnsafe;
ba = static_cast<uint8_t*>(js_malloc(kSize));
if (!ba || !masm->shared->addTable(ba))
ba.reset(static_cast<uint8_t*>(js_malloc(kSize)));
if (!ba)
oomUnsafe.crash("Table malloc");
}
for (int i = 0; i < kSize; i++)
ba[i] = templ[i];
masm->CheckBitInTable(ba, on_bit_set);
masm->CheckBitInTable(Move(ba), on_bit_set);
if (on_bit_clear != fall_through)
masm->JumpOrBacktrack(on_bit_clear);
}

View file

@ -106,7 +106,7 @@ RegExpCode
CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data,
HandleLinearString sample, bool is_global, bool ignore_case,
bool is_ascii, bool match_only, bool force_bytecode, bool sticky,
bool unicode);
bool unicode, RegExpShared::JitCodeTables& tables);
// Note: this may return RegExpRunStatus_Error if an interrupt was requested
// while the code was executing.

View file

@ -96,10 +96,9 @@ irregexp::CaseInsensitiveCompareUCStrings(const char16_t* substring1,
const char16_t* substring2,
size_t byteLength);
InterpretedRegExpMacroAssembler::InterpretedRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc,
RegExpShared* shared,
InterpretedRegExpMacroAssembler::InterpretedRegExpMacroAssembler(LifoAlloc* alloc,
size_t numSavedRegisters)
: RegExpMacroAssembler(cx, *alloc, shared, numSavedRegisters),
: RegExpMacroAssembler(*alloc, numSavedRegisters),
pc_(0),
advance_current_start_(0),
advance_current_offset_(0),
@ -317,7 +316,8 @@ InterpretedRegExpMacroAssembler::CheckCharacterNotInRange(char16_t from, char16_
}
void
InterpretedRegExpMacroAssembler::CheckBitInTable(uint8_t* table, jit::Label* on_bit_set)
InterpretedRegExpMacroAssembler::CheckBitInTable(RegExpShared::JitCodeTable table,
jit::Label* on_bit_set)
{
static const int kBitsPerByte = 8;

View file

@ -40,14 +40,12 @@ namespace irregexp {
class MOZ_STACK_CLASS RegExpMacroAssembler
{
public:
RegExpMacroAssembler(JSContext* cx, LifoAlloc& alloc, RegExpShared* shared,
size_t numSavedRegisters)
RegExpMacroAssembler(LifoAlloc& alloc, size_t numSavedRegisters)
: slow_safe_compiler_(false),
global_mode_(NOT_GLOBAL),
alloc_(alloc),
num_registers_(numSavedRegisters),
num_saved_registers_(numSavedRegisters),
shared(cx, shared)
num_saved_registers_(numSavedRegisters)
{}
enum StackCheckFlag {
@ -137,7 +135,7 @@ class MOZ_STACK_CLASS RegExpMacroAssembler
// The current character (modulus the kTableSize) is looked up in the byte
// array, and if the found byte is non-zero, we jump to the on_bit_set label.
virtual void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set) = 0;
virtual void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set) = 0;
// Checks whether the given offset from the current position is before
// the end of the string. May overwrite the current character.
@ -213,9 +211,6 @@ class MOZ_STACK_CLASS RegExpMacroAssembler
if (num_registers_ <= reg)
num_registers_ = reg + 1;
}
public:
RootedRegExpShared shared;
};
template <typename CharT>
@ -230,8 +225,7 @@ CaseInsensitiveCompareUCStrings(const CharT* substring1, const CharT* substring2
class MOZ_STACK_CLASS InterpretedRegExpMacroAssembler final : public RegExpMacroAssembler
{
public:
InterpretedRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared,
size_t numSavedRegisters);
InterpretedRegExpMacroAssembler(LifoAlloc* alloc, size_t numSavedRegisters);
~InterpretedRegExpMacroAssembler();
// Inherited virtual methods.
@ -258,7 +252,7 @@ class MOZ_STACK_CLASS InterpretedRegExpMacroAssembler final : public RegExpMacro
jit::Label* on_in_range);
void CheckCharacterNotInRange(char16_t from, char16_t to,
jit::Label* on_not_in_range);
void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set);
void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set);
void JumpOrBacktrack(jit::Label* to);
void Fail();
void IfRegisterGE(int reg, int comparand, jit::Label* if_ge);

View file

@ -965,6 +965,9 @@ RegExpShared::discardJitCode()
{
for (auto& comp : compilationArray)
comp.jitCode = nullptr;
// We can also purge the tables used by JIT code.
tables.clearAndFree();
}
void
@ -972,8 +975,6 @@ RegExpShared::finalize(FreeOp* fop)
{
for (auto& comp : compilationArray)
js_free(comp.byteCode);
for (size_t i = 0; i < tables.length(); i++)
js_free(tables[i]);
tables.~JitCodeTables();
}
@ -1070,6 +1071,7 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa
}
}
JitCodeTables tables;
irregexp::RegExpCode code = irregexp::CompilePattern(cx, re, &data, input,
false /* global() */,
re->ignoreCase(),
@ -1077,7 +1079,8 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa
mode == MatchOnly,
force == ForceByteCode,
re->sticky(),
re->unicode());
re->unicode(),
tables);
if (code.empty())
return false;
@ -1085,10 +1088,20 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa
MOZ_ASSERT_IF(force == ForceByteCode, code.byteCode);
RegExpCompilation& compilation = re->compilation(mode, input->hasLatin1Chars());
if (code.jitCode)
if (code.jitCode) {
// First copy the tables. GC can purge the tables if the RegExpShared
// has no JIT code, so it's important to do this right before setting
// compilation.jitCode (to ensure no purging happens between adding the
// tables and setting the JIT code).
for (size_t i = 0; i < tables.length(); i++) {
if (!re->addTable(Move(tables[i])))
return false;
}
compilation.jitCode = code.jitCode;
else if (code.byteCode)
} else if (code.byteCode) {
MOZ_ASSERT(tables.empty(), "RegExpInterpreter does not use data tables");
compilation.byteCode = code.byteCode;
}
return true;
}
@ -1244,7 +1257,7 @@ RegExpShared::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
n += tables.sizeOfExcludingThis(mallocSizeOf);
for (size_t i = 0; i < tables.length(); i++)
n += mallocSizeOf(tables[i]);
n += mallocSizeOf(tables[i].get());
return n;
}

View file

@ -109,6 +109,9 @@ class RegExpShared : public gc::TenuredCell
ForceByteCode
};
using JitCodeTable = UniquePtr<uint8_t[], JS::FreePolicy>;
using JitCodeTables = Vector<JitCodeTable, 0, SystemAllocPolicy>;
private:
friend class RegExpCompartment;
friend class RegExpStatics;
@ -148,7 +151,6 @@ class RegExpShared : public gc::TenuredCell
}
// Tables referenced by JIT code.
using JitCodeTables = Vector<uint8_t*, 0, SystemAllocPolicy>;
JitCodeTables tables;
/* Internal functions. */
@ -181,8 +183,8 @@ class RegExpShared : public gc::TenuredCell
MatchPairs* matches, size_t* endIndex);
// Register a table with this RegExpShared, and take ownership.
bool addTable(uint8_t* table) {
return tables.append(table);
bool addTable(JitCodeTable table) {
return tables.append(Move(table));
}
/* Accessors */