Add a platform-specific syscall number iterator.

sandbox/linux/seccomp-bpf/syscall_iterator_unittest.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
+#include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
+#include "sandbox/linux/tests/unit_tests.h"
+
+using namespace playground2;
+
+namespace {
+
+SANDBOX_TEST(SyscallIterator, Monotonous) {
+  // We're assuming MIN_SYSCALL == 0 for all architectures.
+  // This is currently valid for Intel and ARM EABI.
+  SANDBOX_ASSERT(MIN_SYSCALL == 0u);

[jln (very slow on Chromium), 2012/10/13 02:44:22]

This assert is confusing, because I don't think anything here requires this to
be true. I'd just remove it.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+
+  for (int i = 0; i < 2; ++i) {
+    bool invalid_only = !i;   // Testing both |invalid_only| cases.
+    SyscallIterator iter(invalid_only);
+    uint32_t next = iter.Next();
+
+    if (!invalid_only) {
+      // MIN_SYSCALL should be the first valid syscall.
+      SANDBOX_ASSERT(next == MIN_SYSCALL);

[jln (very slow on Chromium), 2012/10/13 02:44:22]

I think 0 was correct there. The contract (syscall_iterator.h) says that you'll
iterate from 0 to MAXUINT32, this function tests that.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+    }
+    for (uint32_t last = next; !iter.Done(); last = next) {
+      next = iter.Next();
+      SANDBOX_ASSERT(last < next);
+    }
+    // The iterator should return 0xFFFFFFFFu as the last value.
+    SANDBOX_ASSERT(next == 0xFFFFFFFFu);
+  }
+}
+
+SANDBOX_TEST(SyscallIterator, PublicSyscallRange) {
+  // We're assuming MIN_SYSCALL == 0 for all architectures.
+  // This is currently valid for Intel and ARM EABI.
+  SANDBOX_ASSERT(MIN_SYSCALL == 0u);

[jln (very slow on Chromium), 2012/10/13 02:44:22]

Assertions are nice in a place where they help to read the code (i.e. assert the
invariant for instance). This one should probably be moved below as indicated in
the comment below.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+
+  SyscallIterator iter(false);
+  uint32_t next = iter.Next();
+
+  // The iterator should cover the public syscall range
+  // without skipping syscalls.
+  SANDBOX_ASSERT(next == MIN_SYSCALL);

[jln (very slow on Chromium), 2012/10/13 02:44:22]

It's not obvious why you'd assert that. You can assert next == 0, since this is
mandated in the interface.

But I think you may want to just add a comment, and probably regroup the two
asserts here to make it clear what the starting situation is and that you can
now iterate over public system calls.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+  for (uint32_t last = next; next < MAX_PUBLIC_SYSCALL + 1; last = next) {
+    SANDBOX_ASSERT((next = iter.Next()) == last + 1);
+  }
+  SANDBOX_ASSERT(next == MAX_PUBLIC_SYSCALL + 1);
+}
+
+#if defined(__arm__)
+SANDBOX_TEST(SyscallIterator, ARMPrivateSyscallRange) {
+  SyscallIterator iter(false);
+  uint32_t next = iter.Next();
+  while (next < MIN_PRIVATE_SYSCALL - 1) {

[jln (very slow on Chromium), 2012/10/13 02:44:22]

This is an implementation detail that we'll actually return MIN_PRIVATE_SYSCALL
- 1. Hopefully this will disappear soon with the "poking magic".

The test would be more clear if it just tested that we, in fact do cover the
private syscall range: from MIN_PRIVATE_SYSCALL to MAX_PRIVATE_SYSCALL.

That being said, we currently rely on that behavior, so perhaps the best course
of action is to document that +1 / -1 will be tested in syscall_iterator.h ?

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+    next = iter.Next();
+  }
+  // The iterator should cover the ARM private syscall range
+  // without skipping syscalls.
+  SANDBOX_ASSERT(next == MIN_PRIVATE_SYSCALL - 1);
+  for (uint32_t last = next; next < MAX_PRIVATE_SYSCALL + 1; last = next) {
+    SANDBOX_ASSERT((next = iter.Next()) == last + 1);
+  }
+  SANDBOX_ASSERT(next == MAX_PRIVATE_SYSCALL + 1);
+}
+
+SANDBOX_TEST(SyscallIterator, ARMHiddenSyscallRange) {
+  SyscallIterator iter(false);
+  uint32_t next = iter.Next();
+  while (next < MIN_GHOST_SYSCALL - 1) {

[jln (very slow on Chromium), 2012/10/13 02:44:22]

The same remark as above applies.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+    next = iter.Next();
+  }
+  // The iterator should cover the ARM hidden syscall range
+  // without skipping syscalls.
+  SANDBOX_ASSERT(next == MIN_GHOST_SYSCALL - 1);
+  for (uint32_t last = next; next < MAX_SYSCALL + 1; last = next) {
+    SANDBOX_ASSERT((next = iter.Next()) == last + 1);
+  }
+  SANDBOX_ASSERT(next == MAX_SYSCALL + 1);
+}
+#endif
+
+SANDBOX_TEST(SyscallIterator, Invalid) {
+  for (int i = 0; i < 2; ++i) {
+    bool invalid_only = !i;   // Testing both |invalid_only| cases.
+    SyscallIterator iter(invalid_only);
+    uint32_t next = iter.Next();
+
+    while (next < MAX_SYSCALL + 1) {
+      next = iter.Next();
+    }
+
+    SANDBOX_ASSERT(next == MAX_SYSCALL + 1);
+    while (next < 0x7FFFFFFFu) {
+      next = iter.Next();
+    }
+
+    // The iterator should return the signed/unsigned corner cases.
+    SANDBOX_ASSERT(next == 0x7FFFFFFFu);

[jln (very slow on Chromium), 2012/10/13 02:44:22]

I don't really like that this is testing implementation details that are not
specified in the interface, but that's fine.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+    next = iter.Next();
+    SANDBOX_ASSERT(next == 0x80000000u);
+    SANDBOX_ASSERT(!iter.Done());
+    next = iter.Next();
+    SANDBOX_ASSERT(iter.Done());
+    SANDBOX_ASSERT(next == 0xFFFFFFFFu);
+  }
+}
+
+SANDBOX_TEST(SyscallIterator, InvalidOnly) {
+  bool invalid_only = true;
+  SyscallIterator iter(invalid_only);
+  uint32_t next = iter.Next();
+  // First invalid syscall should be |MAX_PUBLIC_SYSCALL + 1|.

[jln (very slow on Chromium), 2012/10/13 02:44:22]

This is only true if 0 is a valid syscall, otherwise the first invalid syscall
would be 0.

So this time, you do need to assert that 0 is MIN_SYSCALL.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

Done.

+  SANDBOX_ASSERT(next == MAX_PUBLIC_SYSCALL + 1);
+
+#if defined(__arm__)
+  next = iter.Next();
+  // The iterator should skip until the last invalid syscall in this range.
+  SANDBOX_ASSERT(next == MIN_PRIVATE_SYSCALL - 1);
+  while (next <= MAX_PRIVATE_SYSCALL) {
+    next = iter.Next();

[jln (very slow on Chromium), 2012/10/13 02:44:22]

You're testing implementation details. In fact these details should change
(hopefully) soon as we can rid of the range testing.

I think the function was good as it was before. It really just tests that the
iterator only yields invalid syscalls.

Or, as before, you can document the +1/-1 behavior clearly in the interface.

[Jorge Lucangeli Obes, 2012/10/13 04:45:42]

The notion of an iterator and the wording in the interface suggest that
|invalid_only_| returns invalid syscalls in order, so we should test that. I'll
document the +/-1 behaviour in the interface.

+  }
+
+  next = iter.Next();
+  // The iterator should skip until the last invalid syscall in this range.
+  SANDBOX_ASSERT(next == MIN_GHOST_SYSCALL - 1);
+  while (next <= MAX_SYSCALL) {
+    next = iter.Next();
+  }
+  SANDBOX_ASSERT(next == MAX_SYSCALL + 1);
+#endif
+}
+
+}  // namespace
+