Who should handle the case when a plugin inserts too many instructions?

[alirazeen]

Suppose that before the scanner calls a plugin, the current block has at least MIN_FSPACE instructions. It is possible for the plugin to insert a large number of instructions, so much so that the instructions spill into the next block. When the plugin returns, arm_check_free_space will allocate a new block, insert a branch to the new block, and update the write pointer (write_p) to the new block. I suspect that there are a number of issues here:

1. There are no checks in the emit or the helper functions to ensure that the plugin does not insert too many instructions.

2. There isn't a way for the plugin developer to check what the limit is. That is, there isn't a field in mambo_context or a function that she can call to check if it's all right for her to insert an instruction.

3. arm_check_free_space assumes that write_p still refers to an address within the current block. But it might well be the case that write_p is now an address in a new block because the instructions inserted by the plugin caused a spillover to the next block. In other words, write_p >= data_p, never mind the write_p + size >= data_p check.

I encountered this issue and I was wondering what the "correct" thing to do is. Clearly I can try to optimize my plugin so that it doesn't insert so many instructions :) But aside from that, is there a more general fix?

A hacky fix that I have now is to call arm_check_free_space with a custom size argument before the plugin is called. This could be made more general by allowing the developer to specify to Mambo that her plugin needs at least MIN_PLUGIN_FSPACE in the current block. But I'm not sure if this might break something else in Mambo.

Any suggestions?

[alirazeen]

I suppose one could also ensure the the emit functions create a new basic block so that spillovers are avoided. It appears to me however that this would require a significant structural change to Mambo, given the nice isolation between the emitters and the scanners.

[lgeek]

Hi

The mid-term plan is to redesign basic block allocation to allow arbitrary sized basic blocks, similar to the current traces. That should avoid this issue altogether.

Currently, 72 bytes in ARM and 82 bytes in Thumb are guaranteed to be available when the pre/post instruction callbacks are called with writing enabled. From a performance perspective, I recommend using a call to a shared routine if the instrumentation can't be inlined in that space. If that's not possible and this is becoming a major annoyance / blocking issue for you (or any other user), I'm open to implementing allocation logic to the emit wrappers or an API equivalent of check_free_space() which plugins could use to ensure that at least X bytes are available in a contiguous block for writing when they actually intend to insert code. On the other hand, allowing plugins to specify that they always need a fixed size larger than MAMBO's MIN_FSPACE seems a bit coarse and would likely increase code cache fragmentation.

check_free_space() or its API equivalent would potentially be problematic to implement for sizes larger than a basic block (256 bytes) with the current code cache design.

Please note that writing beyond the end of a basic block (i.e. when write_p >= data_p) is unsafe because of stubs. When a stub is replaced with the actual translation, the next basic block in the code cache is likely to already be in use and would be overwritten.

Let me know if you can put up with using the workaround for a while. If not, I'll do a more detailed investigation of the available options.

[alirazeen]

Thanks, that make sense. I believe I can rely on the workaround, that shouldn't be a problem. The slightly greater issue for me is that from a plugin perspective, I can't tell when I'm writing beyond the end of a basic block. But again, I can hack up something to detect that and at least print an assert message.

[IgWod]

Closing due to inactivity. Please re-open if required.