diff --git a/fml/file_unittest.cc b/fml/file_unittest.cc index f25f4f1966dd1..c0081383273a8 100644 --- a/fml/file_unittest.cc +++ b/fml/file_unittest.cc @@ -269,6 +269,36 @@ TEST(FileTest, EmptyMappingTest) { ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents")); } +TEST(FileTest, MappingDontNeedSafeTest) { + fml::ScopedTemporaryDirectory dir; + + { + auto file = fml::OpenFile(dir.fd(), "my_contents", true, + fml::FilePermission::kReadWrite); + WriteStringToFile(file, "some content"); + } + + { + auto file = fml::OpenFile(dir.fd(), "my_contents", false, + fml::FilePermission::kRead); + fml::FileMapping mapping(file); + ASSERT_TRUE(mapping.IsValid()); + ASSERT_EQ(mapping.GetMutableMapping(), nullptr); + ASSERT_TRUE(mapping.IsDontNeedSafe()); + } + + { + auto file = fml::OpenFile(dir.fd(), "my_contents", false, + fml::FilePermission::kReadWrite); + fml::FileMapping mapping(file, {fml::FileMapping::Protection::kRead, + fml::FileMapping::Protection::kWrite}); + ASSERT_TRUE(mapping.IsValid()); + ASSERT_NE(mapping.GetMutableMapping(), nullptr); + ASSERT_FALSE(mapping.IsDontNeedSafe()); + } + ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents")); +} + TEST(FileTest, FileTestsWork) { fml::ScopedTemporaryDirectory dir; ASSERT_TRUE(dir.fd().is_valid()); diff --git a/fml/mapping.cc b/fml/mapping.cc index 254ccafc441ea..1b3f1e65d13bd 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -81,11 +81,19 @@ const uint8_t* DataMapping::GetMapping() const { return data_.data(); } +bool DataMapping::IsDontNeedSafe() const { + return false; +} + // NonOwnedMapping NonOwnedMapping::NonOwnedMapping(const uint8_t* data, size_t size, - const ReleaseProc& release_proc) - : data_(data), size_(size), release_proc_(release_proc) {} + const ReleaseProc& release_proc, + bool dontneed_safe) + : data_(data), + size_(size), + release_proc_(release_proc), + dontneed_safe_(dontneed_safe) {} NonOwnedMapping::~NonOwnedMapping() { if (release_proc_) { @@ -101,6 +109,10 @@ const uint8_t* NonOwnedMapping::GetMapping() const { return data_; } +bool NonOwnedMapping::IsDontNeedSafe() const { + return dontneed_safe_; +} + // MallocMapping MallocMapping::MallocMapping() : data_(nullptr), size_(0) {} @@ -134,6 +146,10 @@ const uint8_t* MallocMapping::GetMapping() const { return data_; } +bool MallocMapping::IsDontNeedSafe() const { + return false; +} + uint8_t* MallocMapping::Release() { uint8_t* result = data_; data_ = nullptr; @@ -174,4 +190,8 @@ const uint8_t* SymbolMapping::GetMapping() const { return mapping_; } +bool SymbolMapping::IsDontNeedSafe() const { + return true; +} + } // namespace fml diff --git a/fml/mapping.h b/fml/mapping.h index cd1be8e47d73d..741199126ba40 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -28,6 +28,10 @@ class Mapping { virtual const uint8_t* GetMapping() const = 0; + // Whether calling madvise(DONTNEED) on the mapping is non-destructive. + // Generally true for file-mapped memory and false for anonymous memory. + virtual bool IsDontNeedSafe() const = 0; + private: FML_DISALLOW_COPY_AND_ASSIGN(Mapping); }; @@ -65,6 +69,9 @@ class FileMapping final : public Mapping { // |Mapping| const uint8_t* GetMapping() const override; + // |Mapping| + bool IsDontNeedSafe() const override; + uint8_t* GetMutableMapping(); bool IsValid() const; @@ -96,6 +103,9 @@ class DataMapping final : public Mapping { // |Mapping| const uint8_t* GetMapping() const override; + // |Mapping| + bool IsDontNeedSafe() const override; + private: std::vector data_; @@ -107,7 +117,8 @@ class NonOwnedMapping final : public Mapping { using ReleaseProc = std::function; NonOwnedMapping(const uint8_t* data, size_t size, - const ReleaseProc& release_proc = nullptr); + const ReleaseProc& release_proc = nullptr, + bool dontneed_safe = false); ~NonOwnedMapping() override; @@ -117,10 +128,14 @@ class NonOwnedMapping final : public Mapping { // |Mapping| const uint8_t* GetMapping() const override; + // |Mapping| + bool IsDontNeedSafe() const override; + private: const uint8_t* const data_; const size_t size_; const ReleaseProc release_proc_; + const bool dontneed_safe_; FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping); }; @@ -162,6 +177,9 @@ class MallocMapping final : public Mapping { // |Mapping| const uint8_t* GetMapping() const override; + // |Mapping| + bool IsDontNeedSafe() const override; + /// Removes ownership of the data buffer. /// After this is called; the mapping will point to nullptr. [[nodiscard]] uint8_t* Release(); @@ -186,6 +204,9 @@ class SymbolMapping final : public Mapping { // |Mapping| const uint8_t* GetMapping() const override; + // |Mapping| + bool IsDontNeedSafe() const override; + private: fml::RefPtr native_library_; const uint8_t* mapping_ = nullptr; diff --git a/fml/mapping_unittests.cc b/fml/mapping_unittests.cc index 0a7309ccf3c4c..7bc1c62d09aba 100644 --- a/fml/mapping_unittests.cc +++ b/fml/mapping_unittests.cc @@ -52,4 +52,11 @@ TEST(MallocMapping, Release) { ASSERT_EQ(0u, mapping.GetSize()); } +TEST(MallocMapping, IsDontNeedSafe) { + size_t length = 10; + MallocMapping mapping(reinterpret_cast(malloc(length)), length); + ASSERT_NE(nullptr, mapping.GetMapping()); + ASSERT_FALSE(mapping.IsDontNeedSafe()); +} + } // namespace fml diff --git a/fml/platform/posix/mapping_posix.cc b/fml/platform/posix/mapping_posix.cc index 535a72ebe4ccf..aff3d645d3a5b 100644 --- a/fml/platform/posix/mapping_posix.cc +++ b/fml/platform/posix/mapping_posix.cc @@ -100,6 +100,10 @@ const uint8_t* FileMapping::GetMapping() const { return mapping_; } +bool FileMapping::IsDontNeedSafe() const { + return mutable_mapping_ == nullptr; +} + bool FileMapping::IsValid() const { return valid_; } diff --git a/fml/platform/win/mapping_win.cc b/fml/platform/win/mapping_win.cc index b32178f54d4e9..6225a9c6d0311 100644 --- a/fml/platform/win/mapping_win.cc +++ b/fml/platform/win/mapping_win.cc @@ -115,6 +115,10 @@ const uint8_t* FileMapping::GetMapping() const { return mapping_; } +bool FileMapping::IsDontNeedSafe() const { + return mutable_mapping_ == nullptr; +} + bool FileMapping::IsValid() const { return valid_; } diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 748e430c7f5a9..b041b40fcd3d1 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -74,6 +74,10 @@ void DartIsolate::Flags::SetNullSafetyEnabled(bool enabled) { flags_.null_safety = enabled; } +void DartIsolate::Flags::SetIsDontNeedSafe(bool value) { + flags_.snapshot_is_dontneed_safe = value; +} + Dart_IsolateFlags DartIsolate::Flags::Get() const { return flags_; } @@ -139,6 +143,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( isolate_flags.SetNullSafetyEnabled( isolate_configration->IsNullSafetyEnabled(*isolate_snapshot)); + isolate_flags.SetIsDontNeedSafe(isolate_snapshot->IsDontNeedSafe()); auto isolate = CreateRootIsolate(settings, // isolate_snapshot, // diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index 39a98dd9ed377..b5a559215eb75 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -70,6 +70,7 @@ class DartIsolate : public UIDartState { ~Flags(); void SetNullSafetyEnabled(bool enabled); + void SetIsDontNeedSafe(bool value); Dart_IsolateFlags Get() const; diff --git a/runtime/dart_snapshot.cc b/runtime/dart_snapshot.cc index 9fada1b046c72..12aab73db5085 100644 --- a/runtime/dart_snapshot.cc +++ b/runtime/dart_snapshot.cc @@ -97,7 +97,11 @@ static std::shared_ptr SearchMapping( static std::shared_ptr ResolveVMData( const Settings& settings) { #if DART_SNAPSHOT_STATIC_LINK - return std::make_unique(kDartVmSnapshotData, 0); + return std::make_unique(kDartVmSnapshotData, + 0, // size + nullptr, // release_func + true // dontneed_safe + ); #else // DART_SNAPSHOT_STATIC_LINK return SearchMapping( settings.vm_snapshot_data, // embedder_mapping_callback @@ -112,7 +116,11 @@ static std::shared_ptr ResolveVMData( static std::shared_ptr ResolveVMInstructions( const Settings& settings) { #if DART_SNAPSHOT_STATIC_LINK - return std::make_unique(kDartVmSnapshotInstructions, 0); + return std::make_unique(kDartVmSnapshotInstructions, + 0, // size + nullptr, // release_func + true // dontneed_safe + ); #else // DART_SNAPSHOT_STATIC_LINK return SearchMapping( settings.vm_snapshot_instr, // embedder_mapping_callback @@ -127,7 +135,11 @@ static std::shared_ptr ResolveVMInstructions( static std::shared_ptr ResolveIsolateData( const Settings& settings) { #if DART_SNAPSHOT_STATIC_LINK - return std::make_unique(kDartIsolateSnapshotData, 0); + return std::make_unique(kDartIsolateSnapshotData, + 0, // size + nullptr, // release_func + true // dontneed_safe + ); #else // DART_SNAPSHOT_STATIC_LINK return SearchMapping( settings.isolate_snapshot_data, // embedder_mapping_callback @@ -143,7 +155,11 @@ static std::shared_ptr ResolveIsolateInstructions( const Settings& settings) { #if DART_SNAPSHOT_STATIC_LINK return std::make_unique( - kDartIsolateSnapshotInstructions, 0); + kDartIsolateSnapshotInstructions, + 0, // size + nullptr, // release_func + true // dontneed_safe + ); #else // DART_SNAPSHOT_STATIC_LINK return SearchMapping( settings.isolate_snapshot_instr, // embedder_mapping_callback @@ -233,6 +249,14 @@ const uint8_t* DartSnapshot::GetInstructionsMapping() const { return instructions_ ? instructions_->GetMapping() : nullptr; } +bool DartSnapshot::IsDontNeedSafe() const { + if (data_ && !data_->IsDontNeedSafe()) + return false; + if (instructions_ && !instructions_->IsDontNeedSafe()) + return false; + return true; +} + bool DartSnapshot::IsNullSafetyEnabled(const fml::Mapping* kernel) const { return ::Dart_DetectNullSafety( nullptr, // script_uri (unsupported by Flutter) diff --git a/runtime/dart_snapshot.h b/runtime/dart_snapshot.h index 0b64936952fa5..47cef0d57ad32 100644 --- a/runtime/dart_snapshot.h +++ b/runtime/dart_snapshot.h @@ -160,6 +160,11 @@ class DartSnapshot : public fml::RefCountedThreadSafe { /// const uint8_t* GetInstructionsMapping() const; + //---------------------------------------------------------------------------- + /// @brief Returns whether both the data and instructions mappings are + /// safe to use with madvise(DONTNEED). + bool IsDontNeedSafe() const; + bool IsNullSafetyEnabled( const fml::Mapping* application_kernel_mapping) const; diff --git a/shell/platform/android/apk_asset_provider.cc b/shell/platform/android/apk_asset_provider.cc index 178227e9b156d..e5c266edab839 100644 --- a/shell/platform/android/apk_asset_provider.cc +++ b/shell/platform/android/apk_asset_provider.cc @@ -48,6 +48,8 @@ class APKAssetMapping : public fml::Mapping { return reinterpret_cast(AAsset_getBuffer(asset_)); } + bool IsDontNeedSafe() const override { return !AAsset_isAllocated(asset_); } + private: AAsset* const asset_; diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index c0c5e43469f34..0cc86073a10c7 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -18,6 +18,8 @@ return static_cast([data_.get() bytes]); } + bool IsDontNeedSafe() const override { return false; } + private: fml::scoped_nsobject data_; FML_DISALLOW_COPY_AND_ASSIGN(NSDataMapping); diff --git a/shell/platform/fuchsia/flutter/component_v1.cc b/shell/platform/fuchsia/flutter/component_v1.cc index 427487f2fbd81..721a4fd2274d9 100644 --- a/shell/platform/fuchsia/flutter/component_v1.cc +++ b/shell/platform/fuchsia/flutter/component_v1.cc @@ -292,27 +292,28 @@ ComponentV1::ComponentV1( } auto hold_snapshot = [snapshot](const uint8_t* _, size_t __) {}; settings_.vm_snapshot_data = [hold_snapshot, vm_data]() { - return std::make_unique(vm_data, 0, - hold_snapshot); + return std::make_unique(vm_data, 0, hold_snapshot, + true /* dontneed_safe */); }; settings_.vm_snapshot_instr = [hold_snapshot, vm_instructions]() { - return std::make_unique(vm_instructions, 0, - hold_snapshot); + return std::make_unique( + vm_instructions, 0, hold_snapshot, true /* dontneed_safe */); }; settings_.isolate_snapshot_data = [hold_snapshot, isolate_data]() { - return std::make_unique(isolate_data, 0, - hold_snapshot); + return std::make_unique( + isolate_data, 0, hold_snapshot, true /* dontneed_safe */); }; settings_.isolate_snapshot_instr = [hold_snapshot, isolate_instructions]() { - return std::make_unique(isolate_instructions, 0, - hold_snapshot); + return std::make_unique( + isolate_instructions, 0, hold_snapshot, true /* dontneed_safe */); }; isolate_snapshot_ = fml::MakeRefCounted( - std::make_shared(isolate_data, 0, - hold_snapshot), + std::make_shared(isolate_data, 0, hold_snapshot, + true /* dontneed_safe */), std::make_shared(isolate_instructions, 0, - hold_snapshot)); + hold_snapshot, + true /* dontneed_safe */)); } else { const int namespace_fd = component_data_directory_.get(); settings_.vm_snapshot_data = [namespace_fd]() { diff --git a/shell/platform/fuchsia/flutter/component_v2.cc b/shell/platform/fuchsia/flutter/component_v2.cc index 5fa34ec439d79..1ba984de66d99 100644 --- a/shell/platform/fuchsia/flutter/component_v2.cc +++ b/shell/platform/fuchsia/flutter/component_v2.cc @@ -310,27 +310,28 @@ ComponentV2::ComponentV2( } auto hold_snapshot = [snapshot](const uint8_t* _, size_t __) {}; settings_.vm_snapshot_data = [hold_snapshot, vm_data]() { - return std::make_unique(vm_data, 0, - hold_snapshot); + return std::make_unique(vm_data, 0, hold_snapshot, + true /* dontneed_safe */); }; settings_.vm_snapshot_instr = [hold_snapshot, vm_instructions]() { - return std::make_unique(vm_instructions, 0, - hold_snapshot); + return std::make_unique( + vm_instructions, 0, hold_snapshot, true /* dontneed_safe */); }; settings_.isolate_snapshot_data = [hold_snapshot, isolate_data]() { - return std::make_unique(isolate_data, 0, - hold_snapshot); + return std::make_unique( + isolate_data, 0, hold_snapshot, true /* dontneed_safe */); }; settings_.isolate_snapshot_instr = [hold_snapshot, isolate_instructions]() { - return std::make_unique(isolate_instructions, 0, - hold_snapshot); + return std::make_unique( + isolate_instructions, 0, hold_snapshot, true /* dontneed_safe */); }; isolate_snapshot_ = fml::MakeRefCounted( - std::make_shared(isolate_data, 0, - hold_snapshot), + std::make_shared(isolate_data, 0, hold_snapshot, + true /* dontneed_safe */), std::make_shared(isolate_instructions, 0, - hold_snapshot)); + hold_snapshot, + true /* dontneed_safe */)); } else { const int namespace_fd = component_data_directory_.get(); settings_.vm_snapshot_data = [namespace_fd]() { diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc index 159499aaae44a..972b303c450e6 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc @@ -70,6 +70,10 @@ size_t FileInNamespaceBuffer::GetSize() const { return size_; } +bool FileInNamespaceBuffer::IsDontNeedSafe() const { + return true; +} + std::unique_ptr LoadFile(int namespace_fd, const char* path, bool executable) { diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h index a299cadfecfb3..2a075edbd0f94 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h @@ -25,6 +25,9 @@ class FileInNamespaceBuffer final : public fml::Mapping { // |fml::Mapping| size_t GetSize() const override; + // |fml::Mapping| + bool IsDontNeedSafe() const override; + private: /// The address that was mapped to the buffer. void* address_;