Automated rollback of commit 6a6c931317.

PiperOrigin-RevId: 413362657
Change-Id: Ie75672101b2aba4183f9aa3e39679a99f309e155

sandboxed_api/BUILD.bazel

@@ -131,6 +131,7 @@ cc_library(
         "var_abstract.cc",
         "var_int.cc",
         "var_lenval.cc",
+        "var_pointable.cc",
     ],
     hdrs = [
         "proto_helper.h",
@@ -139,6 +140,7 @@ cc_library(
         "var_array.h",
         "var_int.h",
         "var_lenval.h",
+        "var_pointable.h",
         "var_proto.h",
         "var_ptr.h",
         "var_reg.h",

sandboxed_api/CMakeLists.txt

@@ -139,6 +139,8 @@ add_library(sapi_vars ${SAPI_LIB_TYPE}
   var_int.h
   var_lenval.cc
   var_lenval.h
+  var_pointable.cc
+  var_pointable.h
   var_proto.h
   var_ptr.h
   var_reg.h

sandboxed_api/var_abstract.cc

@@ -18,14 +18,11 @@
 
 #include <sys/uio.h>
 
-#include <memory>
-
 #include <glog/logging.h>
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/rpcchannel.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
@@ -35,36 +32,6 @@ Var::~Var() {
   }
 }
 
-void Var::PtrDeleter::operator()(Ptr* p) { delete p; }
-
-Ptr* Var::PtrNone() {
-  if (!ptr_none_) {
-    ptr_none_.reset(new Ptr(this, kSyncNone));
-  }
-  return ptr_none_.get();
-}
-
-Ptr* Var::PtrBoth() {
-  if (!ptr_both_) {
-    ptr_both_.reset(new Ptr(this, kSyncBoth));
-  }
-  return ptr_both_.get();
-}
-
-Ptr* Var::PtrBefore() {
-  if (!ptr_before_) {
-    ptr_before_.reset(new Ptr(this, kSyncBefore));
-  }
-  return ptr_before_.get();
-}
-
-Ptr* Var::PtrAfter() {
-  if (!ptr_after_) {
-    ptr_after_.reset(new Ptr(this, kSyncAfter));
-  }
-  return ptr_after_.get();
-}
-
 absl::Status Var::Allocate(RPCChannel* rpc_channel, bool automatic_free) {
   void* addr;
   SAPI_RETURN_IF_ERROR(rpc_channel->Allocate(GetSize(), &addr));

sandboxed_api/var_abstract.h

@@ -19,7 +19,6 @@
 #include <string>
 #include <type_traits>
 
-#include "absl/base/attributes.h"
 #include "absl/base/macros.h"
 #include "absl/status/status.h"
 #include "sandboxed_api/var_type.h"
@@ -37,29 +36,8 @@ namespace sapi::v {
 
 class Ptr;
 
-class ABSL_DEPRECATED(
-    "Use the Var::PtrXXX() family of methods instead") Pointable {
- public:
-  enum SyncType {
-    // Do not synchronize the underlying object after/before calls.
-    kSyncNone = 0x0,
-    // Synchronize the underlying object (send the data to the sandboxee)
-    // before the call takes place.
-    kSyncBefore = 0x1,
-    // Synchronize the underlying object (retrieve data from the sandboxee)
-    // after the call has finished.
-    kSyncAfter = 0x2,
-    // Synchronize the underlying object with the remote object, by sending the
-    // data to the sandboxee before the call, and retrieving it from the
-    // sandboxee after the call has finished.
-    kSyncBoth = kSyncBefore | kSyncAfter,
-  };
-
-  virtual ~Pointable() = default;
-};
-
 // An abstract class representing variables.
-class Var : public Pointable {
+class Var {
  public:
   Var(const Var&) = delete;
   Var& operator=(const Var&) = delete;
@@ -90,12 +68,6 @@ class Var : public Pointable {
  protected:
   Var() = default;
 
-  // Functions to get pointers with certain type of synchronization schemes.
-  Ptr* PtrNone();
-  Ptr* PtrBoth();
-  Ptr* PtrBefore();
-  Ptr* PtrAfter();
-
   // Set pointer to local storage class.
   void SetLocal(void* local) { local_ = local; }
 
@@ -124,19 +96,9 @@ class Var : public Pointable {
                                              pid_t pid);
 
  private:
-  // Needed so that we can use unique_ptr with incomplete type.
-  struct PtrDeleter {
-    void operator()(Ptr* p);
-  };
-
   // Invokes Allocate()/Free()/Transfer*Sandboxee().
   friend class ::sapi::Sandbox;
 
-  std::unique_ptr<Ptr, PtrDeleter> ptr_none_;
-  std::unique_ptr<Ptr, PtrDeleter> ptr_both_;
-  std::unique_ptr<Ptr, PtrDeleter> ptr_before_;
-  std::unique_ptr<Ptr, PtrDeleter> ptr_after_;
-
   // Pointer to local storage of the variable.
   void* local_ = nullptr;
   // Pointer to remote storage of the variable.

sandboxed_api/var_array.h

@@ -27,18 +27,15 @@
 #include "sandboxed_api/rpcchannel.h"
 #include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/var_abstract.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 // Class representing an array.
 template <class T>
-class Array : public Var {
+class Array : public Var, public Pointable {
  public:
-  using Var::PtrAfter;
-  using Var::PtrBefore;
-  using Var::PtrBoth;
-
   // The array is not owned by this object.
   Array(T* arr, size_t nelem)
       : arr_(arr),
@@ -99,6 +96,10 @@ class Array : public Var {
  private:
   friend class LenVal;
 
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
+
   // Resizes the internal storage.
   absl::Status EnsureOwnedLocalBuffer(size_t size) {
     if (size % sizeof(T)) {

sandboxed_api/var_int.h

@@ -18,21 +18,24 @@
 #include <memory>
 
 #include "sandboxed_api/sandbox2/comms.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_reg.h"
 
 namespace sapi::v {
 
-// Intermediate class for register sized variables so we don't have to implement
-// ptr() everywhere.
+// Intermediate class for register sized variables
+// so we don't have to implement ptr() everywhere.
 template <class T>
-class IntBase : public Reg<T> {
+class IntBase : public Reg<T>, public Pointable {
  public:
-  using Var::PtrAfter;
-  using Var::PtrBefore;
-  using Var::PtrBoth;
+  IntBase() : IntBase(static_cast<T>(0)) {}
+  explicit IntBase(T value) { this->SetValue(value); }
 
-  explicit IntBase(T value = {}) { this->SetValue(value); }
+ private:
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
 };
 
 using Bool = IntBase<bool>;

sandboxed_api/var_lenval.h

@@ -23,6 +23,7 @@
 #include "sandboxed_api/lenval_core.h"
 #include "sandboxed_api/var_abstract.h"
 #include "sandboxed_api/var_array.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_struct.h"
 
@@ -35,12 +36,8 @@ class Proto;
 // sandboxee which allows the bidirectional synchronization data structures with
 // changing lengths (e.g. protobuf structures). You probably want to directly
 // use protobufs as they are easier to handle.
-class LenVal : public Var {
+class LenVal : public Var, public Pointable {
  public:
-  using Var::PtrAfter;
-  using Var::PtrBefore;
-  using Var::PtrBoth;
-
   explicit LenVal(const char* data, uint64_t size)
       : array_(const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(data)),
                size),
@@ -57,6 +54,10 @@ class LenVal : public Var {
   std::string GetTypeString() const final { return "LengthValue"; }
   std::string ToString() const final { return "LenVal"; }
 
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
+
   absl::Status ResizeData(RPCChannel* rpc_channel, size_t size);
   size_t GetDataSize() const { return struct_.data().size; }
   uint8_t* GetData() const { return array_.GetData(); }
@@ -64,7 +65,6 @@ class LenVal : public Var {
 
  protected:
   size_t GetSize() const final { return 0; }
-
   absl::Status Allocate(RPCChannel* rpc_channel, bool automatic_free) override;
   absl::Status Free(RPCChannel* rpc_channel) override;
   absl::Status TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) override;

sandboxed_api/var_pointable.cc

@@ -0,0 +1,22 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "sandboxed_api/var_pointable.h"
+#include "sandboxed_api/var_ptr.h"
+
+namespace sapi::v {
+
+void Pointable::PtrDeleter::operator()(Ptr *p) { delete p; }
+
+}  // namespace sapi::v

sandboxed_api/var_pointable.h

@@ -0,0 +1,94 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SANDBOXED_API_VAR_POINTABLE_H_
+#define SANDBOXED_API_VAR_POINTABLE_H_
+
+#include <memory>
+
+#include "sandboxed_api/var_reg.h"
+
+namespace sapi::v {
+
+class Ptr;
+
+// Class that implements pointer support for different objects.
+class Pointable {
+ public:
+  enum SyncType {
+    // Do not synchronize the underlying object after/before calls.
+    kSyncNone = 0x0,
+    // Synchronize the underlying object (send the data to the sandboxee)
+    // before the call takes place.
+    kSyncBefore = 0x1,
+    // Synchronize the underlying object (retrieve data from the sandboxee)
+    // after the call has finished.
+    kSyncAfter = 0x2,
+    // Synchronize the underlying object with the remote object, by sending the
+    // data to the sandboxee before the call, and retrieving it from the
+    // sandboxee after the call has finished.
+    kSyncBoth = kSyncBefore | kSyncAfter,
+  };
+
+  virtual ~Pointable() = default;
+
+  // Functions to get pointers with certain type of synchronization scheme.
+  Ptr* PtrNone() {
+    if (ptr_none_ == nullptr) {
+      ptr_none_.reset(CreatePtr(kSyncNone));
+    }
+
+    return ptr_none_.get();
+  }
+
+  Ptr* PtrBoth() {
+    if (ptr_both_ == nullptr) {
+      ptr_both_.reset(CreatePtr(kSyncBoth));
+    }
+    return ptr_both_.get();
+  }
+
+  Ptr* PtrBefore() {
+    if (ptr_before_ == nullptr) {
+      ptr_before_.reset(CreatePtr(kSyncBefore));
+    }
+    return ptr_before_.get();
+  }
+
+  Ptr* PtrAfter() {
+    if (ptr_after_ == nullptr) {
+      ptr_after_.reset(CreatePtr(kSyncAfter));
+    }
+    return ptr_after_.get();
+  }
+
+ private:
+  // Needed so that we can use unique_ptr with incomplete type.
+  struct PtrDeleter {
+    void operator()(Ptr* p);
+  };
+
+  // Necessary to implement creation of Ptr in inheriting class as it is
+  // incomplete type here.
+  virtual Ptr* CreatePtr(SyncType sync_type) = 0;
+
+  std::unique_ptr<Ptr, PtrDeleter> ptr_none_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_both_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_before_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_after_;
+};
+
+}  // namespace sapi::v
+
+#endif  // SANDBOXED_API_VAR_POINTABLE_H_

sandboxed_api/var_proto.h

@@ -28,20 +28,17 @@
 #include "sandboxed_api/proto_helper.h"
 #include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/var_lenval.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 template <typename T>
-class Proto : public Var {
+class Proto : public Pointable, public Var {
  public:
   static_assert(std::is_base_of<google::protobuf::Message, T>::value,
                 "Template argument must be a proto message");
 
-  using Var::PtrAfter;
-  using Var::PtrBefore;
-  using Var::PtrBoth;
-
   ABSL_DEPRECATED("Use Proto<>::FromMessage() instead")
   explicit Proto(const T& proto)
       : wrapped_var_(SerializeProto(proto).value()) {}
@@ -106,6 +103,10 @@ class Proto : public Var {
 
   explicit Proto(std::vector<uint8_t> data) : wrapped_var_(std::move(data)) {}
 
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
+
   // The management of reading/writing the data to the sandboxee is handled by
   // the LenVal class.
   LenVal wrapped_var_;

sandboxed_api/var_ptr.h

@@ -19,14 +19,14 @@
 
 #include "absl/base/macros.h"
 #include "absl/strings/str_format.h"
-#include "sandboxed_api/var_abstract.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_reg.h"
 
 namespace sapi::v {
 
 // Class representing a pointer. Takes both Var* and regular pointers in the
 // initializers.
-class Ptr : public Reg<Var*> {
+class Ptr : public Reg<Var*>, public Pointable {
  public:
   Ptr() = delete;
 
@@ -66,6 +66,10 @@ class Ptr : public Reg<Var*> {
   }
 
  private:
+  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
+    return new Ptr(this, sync_type);
+  }
+
   // GetDataPtr() interface requires of us to return a pointer to the data
   // (variable) that can be copied. We cannot get pointer to pointer with
   // Var::GetRemote(), hence we cache it, and return pointer to it.

sandboxed_api/var_reg.h

@@ -29,6 +29,9 @@ namespace sapi::v {
 // type specifier in methods.
 class Callable : public Var {
  public:
+  Callable(const Callable&) = delete;
+  Callable& operator=(const Callable&) = delete;
+
   // Get pointer to the stored data.
   virtual const void* GetDataPtr() = 0;
 
@@ -79,8 +82,7 @@ class Reg : public Callable {
   std::string ToString() const override;
 
  protected:
-  // The stored value.
-  T value_;
+  T value_;  // The stored value.
 };
 
 template <typename T>

sandboxed_api/var_struct.h

@@ -19,18 +19,15 @@
 
 #include "absl/base/macros.h"
 #include "absl/strings/str_cat.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 // Class representing a structure.
 template <class T>
-class Struct : public Var {
+class Struct : public Var, public Pointable {
  public:
-  using Var::PtrAfter;
-  using Var::PtrBefore;
-  using Var::PtrBoth;
-
   // Forwarding constructor to initalize the struct_ field.
   template <typename... Args>
   explicit Struct(Args&&... args) : struct_(std::forward<Args>(args)...) {
@@ -51,6 +48,11 @@ class Struct : public Var {
   friend class LenVal;
 
   T struct_;
+
+ private:
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
 };
 
 }  // namespace sapi::v

sandboxed_api/var_void.h

@@ -23,10 +23,13 @@
 namespace sapi::v {
 
 // Good, old void.
-class Void : public Callable {
+class Void : public Callable, public Pointable {
  public:
   Void() = default;
 
+  Void(const Void&) = delete;
+  Void& operator=(const Void&) = delete;
+
   size_t GetSize() const final { return 0U; }
   Type GetType() const final { return Type::kVoid; }
   std::string GetTypeString() const final { return "Void"; }
@@ -34,6 +37,11 @@ class Void : public Callable {
 
   const void* GetDataPtr() override { return nullptr; }
   void SetDataFromPtr(const void* ptr, size_t max_sz) override {}
+
+ private:
+  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
+    return new Ptr(this, sync_type);
+  }
 };
 
 }  // namespace sapi::v

sandboxed_api/vars.h

@@ -18,6 +18,7 @@
 #include "sandboxed_api/var_array.h"
 #include "sandboxed_api/var_int.h"
 #include "sandboxed_api/var_lenval.h"
+#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_proto.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_struct.h"