On 9/3/21 7:59 AM, Alex Bennée wrote:
Coverity doesn't know enough about how we have arranged our plugin TCG
ops to know we will always have incremented insn_idx before injecting
the callback. Let us assert it for the benefit of Coverity and protect
ourselves from accidentally breaking the assumption and triggering
harder to grok errors deeper in the code if we attempt a negative
indexed array lookup.
Fixes: Coverity 1459509
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
---
accel/tcg/plugin-gen.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 88e25c6df9..b38aa1bb36 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -820,10 +820,9 @@ static void pr_ops(void)
static void plugin_gen_inject(const struct qemu_plugin_tb *plugin_tb)
{
TCGOp *op;
- int insn_idx;
+ int insn_idx = -1;
pr_ops();
- insn_idx = -1;
QSIMPLEQ_FOREACH(op, &tcg_ctx->plugin_ops, plugin_link) {
enum plugin_gen_from from = op->args[0];
enum plugin_gen_cb type = op->args[1];
@@ -834,6 +833,7 @@ static void plugin_gen_inject(const struct qemu_plugin_tb
*plugin_tb)
type == PLUGIN_GEN_ENABLE_MEM_HELPER) {
insn_idx++;
}
+ g_assert(from == PLUGIN_GEN_FROM_TB || insn_idx >= 0);
plugin_inject_cb(plugin_tb, op, insn_idx);
Hmm. This is the single caller of plugin_inject_cb.
I think we could simplify all of this by inlining it, so that we can put these blocks into
their proper place within the switch.
Also, existing strageness in insn_idx being incremented for non-insns? Should it be named
something else? I haven't looked at how it's really used in the end.
r~