On Wed, May 23, 2018 at 04:07:37PM +0100, Mark Rutland wrote:I will send the following patch once Dan's [1] has been applied upstream.
I think that either way, we have a potential problem if the compiler
generates a branch dependent on the result of validate_index_nospec().
In that case, we could end up with codegen approximating:
bool safe = false;
if (idx < bound) {
idx = array_index_nospec(idx, bound);
safe = true;
}
// this branch can be mispredicted
if (safe) {
foo = array[idx];
}
... and thus we lose the nospec protection.
I see GCC do this at -O0, but so far I haven't tricked it into doing
this at -O1 or above.
Regardless, I worry this is fragile -- GCC *can* generate code as per
the above, even if it's unlikely to.
I also suspect that compiler transformations mean that this might
already be the case for patterns like:
if (idx < bound) {
safe_idx = array_index_nospec(idx, bound)];
...
foo = array[safe_idx];
}
... if the compiler can transform that to something like:
if (idx < bound) {
idx = array_index_nospec(idx, bound);
}
// can be mispredicted
if (idx < bound) {
foo = array[idx];
}
... which I think a compiler might be capable of, depending on the rest
of the function body (e.g. if there's a common portion shared with the
else case).
I'll see if I can trigger that in a test case. :/
No luck so far, but I'll keeep fighting...
GCC will happily pull a common suffix after the branch, e.g.
if (cond) {
foo();
bar();
} else {
bar();
}
.. goes to:
if (cond)
foo()
bar();
... but I can't convince it to pull a common prefix before the branch.
Mark.