IonMonkey: Segmentation fault in ion::Loop::hoistInstructions




JavaScript Engine
7 years ago
7 years ago


(Reporter: adrake, Assigned: rpearl)


(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)



(2 attachments)



7 years ago
Crashes due to a call to MBasicBlock::remove on an already-unlinked instruction, probably because it has already been removed from its block.

Comment 1

7 years ago
Created attachment 551307 [details]
Test case

Now with 100% more test case.

Comment 2

7 years ago
Created attachment 551962 [details] [diff] [review]
patch v0

The problem in the bug is that when we remove instructions from the graph in any way, although we replace all uses of the instructions (if any), references to the instruction still exist in the use chain of every input to the instruction. For example, if we have x = add(y, z), and remove x, then the use-chains of y and z will still contain x, although it isn't in the graph.

There are a few ways of dealing with this. We could do a check when traversing use-chains as to whether the defs they reference still exist. We could put such a check in the iterator itself. Or we could update the use chains when we remove a def, which is what the attached patch does.

It is still an open question as to how we should deal with dead-code elimination--what to do if an instruction has no uses. Currently, we just leave the instruction, although GVN clears away instructions with no uses, and presumably lowering does too. 

We need to either do an explicit pass to remove dead instructions, or chase around use counts. It may be acceptable to only ever manipulate an instruction if it has uses (and otherwise remove it), and just put in such checks everywhere (or in the definition iterator), but this is insufficient to remove ALL dead code, since the only consumers of an instruction may be dead instructions themselves, and we can't chase down all the dead code without explicitly checking.
Assignee: general → rpearl
Attachment #551962 - Flags: review?(dvander)
Attachment #551962 - Flags: review?(dvander) → review+

Comment 3

7 years ago
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.