Migrate to NOTREACHED() in ipc/ and sandbox/

NOTREACHED() and NOTREACHED_IN_MIGRATION() are both CHECK-fatal now.
The former is [[noreturn]] so this CL also performs dead-code removal
after the NOTREACHED().

This CL does not attempt to do additional rewrites of any surrounding
code, like:

if (!foo) {
  NOTREACHED();
}

to CHECK(foo);

Those transforms take a non-trivial amount of time (and there are
thousands of instances). Cleanup can be left as an exercise for the
reader.

Bug: 40580068
Low-Coverage-Reason: OTHER Should-be-unreachable code
Change-Id: I4060bd0afb2d58343599ba985d336e305b7c0574
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5872433
Reviewed-by: Tom Sepez <[email protected]>
Reviewed-by: Matthew Denton <[email protected]>
Commit-Queue: Peter Boström <[email protected]>
Reviewed-by: Alex Gough <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1359630}

ipc/handle_win.cc

@@ -24,7 +24,7 @@ void ParamTraits<HandleWin>::Write(base::Pickle* m, const param_type& p) {
   scoped_refptr<IPC::internal::HandleAttachmentWin> attachment(
       new IPC::internal::HandleAttachmentWin(p.get_handle()));
   if (!m->WriteAttachment(std::move(attachment)))
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
 }
 
 // static

ipc/ipc_channel_common.cc

@@ -67,15 +67,15 @@ Channel::AssociatedInterfaceSupport* Channel::GetAssociatedInterfaceSupport() {
 }
 
 void Channel::Pause() {
-  NOTREACHED_IN_MIGRATION();
+  NOTREACHED();
 }
 
 void Channel::Unpause(bool flush) {
-  NOTREACHED_IN_MIGRATION();
+  NOTREACHED();
 }
 
 void Channel::Flush() {
-  NOTREACHED_IN_MIGRATION();
+  NOTREACHED();
 }
 
 void Channel::SetUrgentMessageObserver(UrgentMessageObserver* observer) {

ipc/ipc_channel_mojo.cc

@@ -92,7 +92,7 @@ class ThreadSafeChannelProxy : public mojo::ThreadSafeProxy {
       mojo::Message& message,
       std::unique_ptr<mojo::MessageReceiver> responder) override {
     // We don't bother supporting this because it's not used in practice.
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 
  private:

ipc/ipc_channel_mojo_unittest.cc

@@ -276,7 +276,7 @@ class ListenerThatBindsATestStructPasser : public IPC::Listener,
 
   void OnChannelConnected(int32_t peer_pid) override {}
 
-  void OnChannelError() override { NOTREACHED_IN_MIGRATION(); }
+  void OnChannelError() override { NOTREACHED(); }
 
   void OnAssociatedInterfaceRequest(
       const std::string& interface_name,
@@ -289,7 +289,7 @@ class ListenerThatBindsATestStructPasser : public IPC::Listener,
 
  private:
   // IPC::mojom::TestStructPasser:
-  void Pass(IPC::mojom::TestStructPtr) override { NOTREACHED_IN_MIGRATION(); }
+  void Pass(IPC::mojom::TestStructPtr) override { NOTREACHED(); }
 
   mojo::AssociatedReceiver<IPC::mojom::TestStructPasser> receiver_{this};
 };
@@ -314,7 +314,7 @@ class ListenerThatExpectsNoError : public IPC::Listener {
     std::move(connect_closure_).Run();
   }
 
-  void OnChannelError() override { NOTREACHED_IN_MIGRATION(); }
+  void OnChannelError() override { NOTREACHED(); }
 
  private:
   base::OnceClosure connect_closure_;
@@ -797,9 +797,7 @@ class ListenerWithSimpleProxyAssociatedInterface
     std::move(callback).Run(next_expected_value_);
   }
 
-  void RequestValue(RequestValueCallback callback) override {
-    NOTREACHED_IN_MIGRATION();
-  }
+  void RequestValue(RequestValueCallback callback) override { NOTREACHED(); }
 
   void RequestQuit(RequestQuitCallback callback) override {
     std::move(callback).Run();
@@ -1176,7 +1174,7 @@ class SimpleTestClientImpl : public IPC::mojom::SimpleTestClient,
   void BindSync(
       mojo::PendingAssociatedReceiver<IPC::mojom::SimpleTestClient> receiver,
       BindSyncCallback callback) override {
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 
   void GetReceiverWithQueuedSyncMessage(
@@ -1810,10 +1808,7 @@ class ListenerThatVerifiesPeerPid : public TestListenerBase {
     RunQuitClosure();
   }
 
-  bool OnMessageReceived(const IPC::Message& message) override {
-    NOTREACHED_IN_MIGRATION();
-    return true;
-  }
+  bool OnMessageReceived(const IPC::Message& message) override { NOTREACHED(); }
 };
 
 // The global PID is only used on systems that use the zygote. Hence, this

ipc/ipc_channel_nacl.cc

@@ -382,7 +382,7 @@ bool ChannelNacl::DidEmptyInputBuffers() {
 void ChannelNacl::HandleInternalMessage(const Message& msg) {
   // The trusted side IPC::Channel should handle the "hello" handshake; we
   // should not receive the "Hello" message.
-  NOTREACHED_IN_MIGRATION();
+  NOTREACHED();
 }
 
 // Channel's methods

ipc/ipc_channel_proxy.cc

@@ -291,7 +291,7 @@ void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) {
     }
   }
 
-  NOTREACHED_IN_MIGRATION() << "filter to be removed not found";
+  NOTREACHED() << "filter to be removed not found";
 }
 
 // Called on the listener's thread

ipc/ipc_message_attachment.cc

@@ -103,8 +103,7 @@ mojo::ScopedHandle MessageAttachment::TakeMojoHandle() {
     default:
       break;
   }
-  NOTREACHED_IN_MIGRATION();
-  return mojo::ScopedHandle();
+  NOTREACHED();
 }
 
 // static
@@ -155,8 +154,7 @@ scoped_refptr<MessageAttachment> MessageAttachment::CreateFromMojoHandle(
         platform_file, internal::HandleAttachmentWin::FROM_WIRE);
   }
 #endif
-  NOTREACHED_IN_MIGRATION();
-  return nullptr;
+  NOTREACHED();
 }
 
 }  // namespace IPC

ipc/ipc_message_macros.h

@@ -396,11 +396,6 @@
       }                                                                        \
       break;
 
-#define IPC_MESSAGE_UNHANDLED_ERROR()                     \
-  IPC_MESSAGE_UNHANDLED(NOTREACHED_IN_MIGRATION()         \
-                        << "Invalid message with type = " \
-                        << ipc_message__.type())
-
 #define IPC_END_MESSAGE_MAP() \
   } \
 }

ipc/ipc_message_pipe_reader.cc

@@ -53,7 +53,7 @@ class ThreadSafeProxy : public mojo::ThreadSafeProxy {
       mojo::Message& message,
       std::unique_ptr<mojo::MessageReceiver> responder) override {
     // We don't bother supporting this because it's not used in practice.
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 
  private:

ipc/ipc_message_templates.h

@@ -189,11 +189,7 @@ class MessageT<Meta, std::tuple<Ins...>, std::tuple<Outs...>>
     bool ok = ReadSendParam(msg, &send_params);
     Message* reply = SyncMessage::GenerateReply(msg);
     if (!ok) {
-      NOTREACHED_IN_MIGRATION()
-          << "Error deserializing message " << msg->type();
-      reply->set_reply_error();
-      sender->Send(reply);
-      return false;
+      NOTREACHED() << "Error deserializing message " << msg->type();
     }
 
     ReplyParam reply_params;
@@ -214,11 +210,7 @@ class MessageT<Meta, std::tuple<Ins...>, std::tuple<Outs...>>
     bool ok = ReadSendParam(msg, &send_params);
     Message* reply = SyncMessage::GenerateReply(msg);
     if (!ok) {
-      NOTREACHED_IN_MIGRATION()
-          << "Error deserializing message " << msg->type();
-      reply->set_reply_error();
-      obj->Send(reply);
-      return false;
+      NOTREACHED() << "Error deserializing message " << msg->type();
     }
 
     std::tuple<Message&> t = std::tie(*reply);
@@ -237,11 +229,7 @@ class MessageT<Meta, std::tuple<Ins...>, std::tuple<Outs...>>
     bool ok = ReadSendParam(msg, &send_params);
     Message* reply = SyncMessage::GenerateReply(msg);
     if (!ok) {
-      NOTREACHED_IN_MIGRATION()
-          << "Error deserializing message " << msg->type();
-      reply->set_reply_error();
-      obj->Send(reply);
-      return false;
+      NOTREACHED() << "Error deserializing message " << msg->type();
     }
 
     std::tuple<Message&> t = std::tie(*reply);

ipc/ipc_message_utils.cc

@@ -310,8 +310,7 @@ bool ReadValue(const base::Pickle* pickle,
       break;
     }
     default:
-      NOTREACHED_IN_MIGRATION();
-      return false;
+      NOTREACHED();
   }
 
   return true;
@@ -432,8 +431,7 @@ bool ParamTraits<double>::Read(const base::Pickle* m,
                                param_type* r) {
   const char *data;
   if (!iter->ReadBytes(&data, sizeof(*r))) {
-    NOTREACHED_IN_MIGRATION();
-    return false;
+    NOTREACHED();
   }
   memcpy(r, data, sizeof(param_type));
   return true;
@@ -565,10 +563,10 @@ void ParamTraits<base::FileDescriptor>::Write(base::Pickle* m,
   if (p.auto_close) {
     if (!m->WriteAttachment(
             new internal::PlatformFileAttachment(base::ScopedFD(p.fd))))
-      NOTREACHED_IN_MIGRATION();
+      NOTREACHED();
   } else {
     if (!m->WriteAttachment(new internal::PlatformFileAttachment(p.fd)))
-      NOTREACHED_IN_MIGRATION();
+      NOTREACHED();
   }
 }
 
@@ -620,7 +618,7 @@ void ParamTraits<base::ScopedFD>::Write(base::Pickle* m, const param_type& p) {
 
   if (!m->WriteAttachment(new internal::PlatformFileAttachment(
           std::move(const_cast<param_type&>(p))))) {
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 }
 
@@ -705,7 +703,7 @@ void ParamTraits<zx::vmo>::Write(base::Pickle* m, const param_type& p) {
 
   if (!m->WriteAttachment(new internal::HandleAttachmentFuchsia(
           std::move(const_cast<param_type&>(p))))) {
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 }
 
@@ -750,7 +748,7 @@ void ParamTraits<zx::channel>::Write(base::Pickle* m, const param_type& p) {
 
   if (!m->WriteAttachment(new internal::HandleAttachmentFuchsia(
           std::move(const_cast<param_type&>(p))))) {
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 }
 
@@ -1485,8 +1483,7 @@ bool ParamTraits<MSG>::Read(const base::Pickle* m,
   if (result && data_size == sizeof(MSG)) {
     memcpy(r, data, sizeof(MSG));
   } else {
-    result = false;
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 
   return result;

ipc/ipc_sync_channel_unittest.cc

@@ -147,7 +147,7 @@ class Worker : public Listener, public Sender {
   SyncChannel* channel() { return channel_.get(); }
   // Functions for derived classes to implement if they wish.
   virtual void Run() { }
-  virtual void OnAnswer(int* answer) { NOTREACHED_IN_MIGRATION(); }
+  virtual void OnAnswer(int* answer) { NOTREACHED(); }
   virtual void OnAnswerDelay(Message* reply_msg) {
     // The message handler map below can only take one entry for
     // SyncChannelTestMsg_AnswerToLife, so since some classes want
@@ -159,17 +159,15 @@ class Worker : public Listener, public Sender {
     SyncChannelTestMsg_AnswerToLife::WriteReplyParams(reply_msg, answer);
     Send(reply_msg);
   }
-  virtual void OnDouble(int in, int* out) { NOTREACHED_IN_MIGRATION(); }
+  virtual void OnDouble(int in, int* out) { NOTREACHED(); }
   virtual void OnDoubleDelay(int in, Message* reply_msg) {
     int result;
     OnDouble(in, &result);
     SyncChannelTestMsg_Double::WriteReplyParams(reply_msg, result);
     Send(reply_msg);
   }
 
-  virtual void OnNestedTestMsg(Message* reply_msg) {
-    NOTREACHED_IN_MIGRATION();
-  }
+  virtual void OnNestedTestMsg(Message* reply_msg) { NOTREACHED(); }
 
   virtual SyncChannel* CreateChannel() {
     std::unique_ptr<SyncChannel> channel = SyncChannel::Create(

ipc/ipc_sync_message.cc

@@ -92,8 +92,7 @@ bool SyncMessage::ReadSyncHeader(const Message& msg, SyncHeader* header) {
   base::PickleIterator iter(msg);
   bool result = iter.ReadInt(&header->message_id);
   if (!result) {
-    NOTREACHED_IN_MIGRATION();
-    return false;
+    NOTREACHED();
   }
 
   return true;

ipc/mach_port_mac.cc

@@ -16,7 +16,7 @@ namespace IPC {
 void ParamTraits<MachPortMac>::Write(base::Pickle* m, const param_type& p) {
   if (!m->WriteAttachment(
           new IPC::internal::MachPortAttachmentMac(p.get_mach_port()))) {
-    NOTREACHED_IN_MIGRATION();
+    NOTREACHED();
   }
 }
 

ipc/message_router.cc

@@ -14,15 +14,13 @@ MessageRouter::MessageRouter() = default;
 MessageRouter::~MessageRouter() = default;
 
 bool MessageRouter::OnControlMessageReceived(const IPC::Message& msg) {
-  NOTREACHED_IN_MIGRATION()
+  NOTREACHED()
       << "should override in subclass if you care about control messages";
-  return false;
 }
 
 bool MessageRouter::Send(IPC::Message* msg) {
-  NOTREACHED_IN_MIGRATION()
+  NOTREACHED()
       << "should override in subclass if you care about sending messages";
-  return false;
 }
 
 bool MessageRouter::AddRoute(int32_t routing_id, IPC::Listener* listener) {

ipc/native_handle_type_converters.cc

@@ -22,8 +22,7 @@ IPC::MessageAttachment::Type TypeConverter<
     case native::SerializedHandleType::FUCHSIA_HANDLE:
       return IPC::MessageAttachment::Type::FUCHSIA_HANDLE;
   }
-  NOTREACHED_IN_MIGRATION();
-  return IPC::MessageAttachment::Type::MOJO_HANDLE;
+  NOTREACHED();
 }
 
 // static
@@ -42,8 +41,7 @@ native::SerializedHandleType TypeConverter<
     case IPC::MessageAttachment::Type::FUCHSIA_HANDLE:
       return native::SerializedHandleType::FUCHSIA_HANDLE;
   }
-  NOTREACHED_IN_MIGRATION();
-  return native::SerializedHandleType::MOJO_HANDLE;
+  NOTREACHED();
 }
 
 }  // namespace mojo

sandbox/linux/integration_tests/namespace_unix_domain_socket_unittest.cc

@@ -227,7 +227,7 @@ SANDBOX_TEST(UnixDomainSocketTest, DoubleNamespace) {
         CHECK_EQ(pid, sender_pid);
         break;
       default:
-        NOTREACHED_IN_MIGRATION();
+        NOTREACHED();
     }
   }
 

sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc

@@ -335,8 +335,7 @@ ResultExpr RestrictKillTarget(pid_t target_pid, int sysno) {
     case __NR_tkill:
       return CrashSIGSYSKill();
     default:
-      NOTREACHED_IN_MIGRATION();
-      return CrashSIGSYS();
+      NOTREACHED();
   }
 }
 
@@ -384,8 +383,7 @@ ResultExpr RestrictSchedTarget(pid_t target_pid, int sysno) {
           .Default(RewriteSchedSIGSYS());
     }
     default:
-      NOTREACHED_IN_MIGRATION();
-      return CrashSIGSYS();
+      NOTREACHED();
   }
 }
 

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

@@ -146,8 +146,7 @@ bool SandboxBPF::SupportsSeccompSandbox(SeccompLevel level) {
     case SeccompLevel::MULTI_THREADED:
       return KernelSupportsSeccompTsync();
   }
-  NOTREACHED_IN_MIGRATION();
-  return false;
+  NOTREACHED();
 }
 
 bool SandboxBPF::StartSandbox(SeccompLevel seccomp_level, bool enable_ibpb) {

sandbox/linux/services/namespace_utils.cc

@@ -78,8 +78,7 @@ bool NamespaceUtils::KernelSupportsUnprivilegedNamespace(int type) {
       path = "/proc/self/ns/uts";
       break;
     default:
-      NOTREACHED_IN_MIGRATION();
-      return false;
+      NOTREACHED();
   }
 
   return base::PathExists(base::FilePath(path));