Commit 2c93ce12 authored by Kevin Modzelewski's avatar Kevin Modzelewski Committed by Kevin Modzelewski

Don't crash in a couple rewriter cases

- if we try guarding after a mutation
- if we use all of our scratch space

Now, just set a "failed" flag internally and which prevents committing.

The motivation for the first part is trying to get rewrite calls to tp_getattro;
if the rewrite is from getattr then it will succeed, but if it comes from
callattr then we will want to do some more guards after the tp_getattro.  We
could try to pass that state around, but for now just use the 'failed' approach.
parent b5d994be
...@@ -726,6 +726,8 @@ void Rewriter::_call(RewriterVar* result, bool can_call_into_python, void* func_ ...@@ -726,6 +726,8 @@ void Rewriter::_call(RewriterVar* result, bool can_call_into_python, void* func_
{ {
// this forces the register allocator to spill this register: // this forces the register allocator to spill this register:
assembler::Register r2 = allocReg(l); assembler::Register r2 = allocReg(l);
if (failed)
return;
assert(r == r2); assert(r == r2);
assert(vars_by_location.count(l) == 0); assert(vars_by_location.count(l) == 0);
} }
...@@ -886,6 +888,11 @@ void Rewriter::commit() { ...@@ -886,6 +888,11 @@ void Rewriter::commit() {
assert(!finished); assert(!finished);
initPhaseEmitting(); initPhaseEmitting();
if (failed) {
this->abort();
return;
}
static StatCounter ic_rewrites_aborted_assemblyfail("ic_rewrites_aborted_assemblyfail"); static StatCounter ic_rewrites_aborted_assemblyfail("ic_rewrites_aborted_assemblyfail");
auto on_assemblyfail = [&]() { auto on_assemblyfail = [&]() {
...@@ -936,6 +943,11 @@ void Rewriter::commit() { ...@@ -936,6 +943,11 @@ void Rewriter::commit() {
for (int i = 0; i < actions.size(); i++) { for (int i = 0; i < actions.size(); i++) {
actions[i].action(); actions[i].action();
if (failed) {
this->abort();
return;
}
assertConsistent(); assertConsistent();
if (i == last_guard_action) { if (i == last_guard_action) {
on_done_guarding(); on_done_guarding();
...@@ -1116,7 +1128,9 @@ Location Rewriter::allocScratch() { ...@@ -1116,7 +1128,9 @@ Location Rewriter::allocScratch() {
return l; return l;
} }
} }
RELEASE_ASSERT(0, "Using all %d bytes of scratch!", scratch_size);
failed = true;
return Location(Location::None, 0);
} }
RewriterVar* Rewriter::add(RewriterVar* a, int64_t b, Location dest) { RewriterVar* Rewriter::add(RewriterVar* a, int64_t b, Location dest) {
...@@ -1301,6 +1315,9 @@ void Rewriter::spillRegister(assembler::Register reg, Location preserve) { ...@@ -1301,6 +1315,9 @@ void Rewriter::spillRegister(assembler::Register reg, Location preserve) {
} }
Location scratch = allocScratch(); Location scratch = allocScratch();
if (failed)
return;
assembler::Indirect mem = indirectFor(scratch); assembler::Indirect mem = indirectFor(scratch);
assembler->mov(reg, mem); assembler->mov(reg, mem);
addLocationToVar(var, scratch); addLocationToVar(var, scratch);
...@@ -1366,7 +1383,7 @@ assembler::Register Rewriter::allocReg(Location dest, Location otherThan) { ...@@ -1366,7 +1383,7 @@ assembler::Register Rewriter::allocReg(Location dest, Location otherThan) {
spillRegister(reg); spillRegister(reg);
} }
assert(vars_by_location.count(reg) == 0); assert(failed || vars_by_location.count(reg) == 0);
return reg; return reg;
} else { } else {
RELEASE_ASSERT(0, "%d", dest.type); RELEASE_ASSERT(0, "%d", dest.type);
...@@ -1499,6 +1516,7 @@ Rewriter::Rewriter(ICSlotRewrite* rewrite, int num_args, const std::vector<int>& ...@@ -1499,6 +1516,7 @@ Rewriter::Rewriter(ICSlotRewrite* rewrite, int num_args, const std::vector<int>&
assembler(rewrite->getAssembler()), assembler(rewrite->getAssembler()),
const_loader(this), const_loader(this),
return_location(rewrite->returnRegister()), return_location(rewrite->returnRegister()),
failed(false),
added_changing_action(false), added_changing_action(false),
marked_inside_ic(false), marked_inside_ic(false),
last_guard_action(-1), last_guard_action(-1),
......
...@@ -332,6 +332,7 @@ private: ...@@ -332,6 +332,7 @@ private:
const Location return_location; const Location return_location;
bool failed; // if we tried to generate an invalid rewrite.
bool finished; // committed or aborted bool finished; // committed or aborted
#ifndef NDEBUG #ifndef NDEBUG
int start_vars; int start_vars;
...@@ -366,6 +367,10 @@ private: ...@@ -366,6 +367,10 @@ private:
if (type == ActionType::MUTATION) { if (type == ActionType::MUTATION) {
added_changing_action = true; added_changing_action = true;
} else if (type == ActionType::GUARD) { } else if (type == ActionType::GUARD) {
if (added_changing_action) {
failed = true;
return;
}
assert(!added_changing_action); assert(!added_changing_action);
last_guard_action = (int)actions.size(); last_guard_action = (int)actions.size();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment