Linux Sandbox: fix BPF compiler bug

Linux Sandbox: fix BPF compiler bug

The code responsible for detecting similar blocks and merging
them didn't check for the next blocks if the last instruction was
not a JMP or a RET.

The patch to fix this bug (in codegen.cc) is based on a patch by
jld@panix.com, attached to the bug report.

Additional unittests are from jln@chromium.org

BUG=351103
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260157

[jln (very slow on Chromium), 2014-03-27 18:31:39]

Markus: if you could take a look, that would be very helpful

Mdempsky: please, take a look!

[jln (very slow on Chromium), 2014-03-27 19:08:10]

+cc: Jed

[mdempsky, 2014-03-27 20:27:32]

lgtm

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen.cc (right):

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen.cc:447: if (BPF_CLASS(insns1.back()->code) !=
BPF_JMP &&
Are Instructions guaranteed to be non-empty?  I suspect in practice they are,
but seems like it might be worth also checking for !insns1.empty() before trying
to inspect insns1.back()?

(And if you do fix that, it might be worth adding a unit test that checks that
comparing empty blocks works as expected?)

[jln (very slow on Chromium), 2014-03-27 21:44:07]

Jorge, do you mind reviewing or stamping this?

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen.cc (right):

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen.cc:447: if (BPF_CLASS(insns1.back()->code) !=
BPF_JMP &&
On 2014/03/27 20:27:33, mdempsky wrote:
> Are Instructions guaranteed to be non-empty?  I suspect in practice they are,
> but seems like it might be worth also checking for !insns1.empty() before
trying
> to inspect insns1.back()?
> 
> (And if you do fix that, it might be worth adding a unit test that checks that
> comparing empty blocks works as expected?)

I've added a few assertions.

[mdempsky, 2014-03-27 21:46:38]

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen.cc (right):

https://codereview.chromium.org/215173002/diff/30001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen.cc:447: if (BPF_CLASS(insns1.back()->code) !=
BPF_JMP &&
On 2014/03/27 21:44:08, jln wrote:
> I've added a few assertions.

lgtm

[Jorge Lucangeli Obes, 2014-03-27 21:47:37]

lgtm

[jln (very slow on Chromium), 2014-03-28 12:40:46]

The CQ bit was checked by jln@chromium.org

[commit-bot: I haz the power, 2014-03-28 12:41:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/215173002/50001

[commit-bot: I haz the power, 2014-03-28 16:26:33]

Change committed as 260157