Remove non-trivial constructors/destructors from SystemProperties
With the goal of disallowing exit time destructors, SystemProperties's
non-trivial destructor needs to be removed. This means replacing the
union hack with yet another hack as we don't want to allocate anything
despite relying on some polymorphism.
Bug: 73485611
Test: boot bullhead
Change-Id: I64223714c9b26c9724bfb8f3e2b0168e47b56bc8
diff --git a/libc/bionic/system_property_api.cpp b/libc/bionic/system_property_api.cpp
index f2e1032..10d1bb3 100644
--- a/libc/bionic/system_property_api.cpp
+++ b/libc/bionic/system_property_api.cpp
@@ -35,6 +35,8 @@
#include "private/bionic_defs.h"
static SystemProperties system_properties;
+static_assert(__is_trivially_constructible(SystemProperties),
+ "System Properties must be trivially constructable");
// This is public because it was exposed in the NDK. As of 2017-01, ~60 apps reference this symbol.
// It is set to nullptr and never modified.
diff --git a/libc/system_properties/include/system_properties/system_properties.h b/libc/system_properties/include/system_properties/system_properties.h
index 37fe6c0..52ffcaf 100644
--- a/libc/system_properties/include/system_properties/system_properties.h
+++ b/libc/system_properties/include/system_properties/system_properties.h
@@ -29,6 +29,7 @@
#pragma once
#include <stdint.h>
+#include <sys/param.h>
#include <sys/system_properties.h>
#include "contexts.h"
@@ -40,12 +41,13 @@
class SystemProperties {
public:
+ friend struct LocalPropertyTestState;
+ friend class SystemPropertiesTest;
// Note that system properties are initialized before libc calls static initializers, so
// doing any initialization in this constructor is an error. Even a Constructor that zero
// initializes this class will clobber the previous property initialization.
// We rely on the static SystemProperties in libc to be placed in .bss and zero initialized.
- SystemProperties() {
- }
+ SystemProperties() = default;
// Special constructor for testing that also zero initializes the important members.
explicit SystemProperties(bool initialized) : initialized_(initialized) {
}
@@ -71,23 +73,17 @@
const prop_info* FindNth(unsigned n);
int Foreach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie);
- Contexts* contexts() {
- return reinterpret_cast<Contexts*>(&contexts_union_);
- }
-
private:
- // We don't want to use new or malloc in properties (b/31659220), and since these classes
- // are small enough and we place them in a union. See the above note about Constructors
- // for why there is a no-op constructor here.
- union ContextsUnion {
- ContextsUnion() {
- }
- ~ContextsUnion() {
- }
- ContextsSerialized contexts_serialized;
- ContextsSplit contexts_split;
- ContextsPreSplit contexts_pre_split;
- } contexts_union_;
+ // We don't want to use new or malloc in properties (b/31659220), and we don't want to waste a
+ // full page by using mmap(), so we set aside enough space to create any context of the three
+ // contexts.
+ static constexpr size_t kMaxContextsAlign =
+ MAX(alignof(ContextsSerialized), MAX(alignof(ContextsSplit), alignof(ContextsPreSplit)));
+ static constexpr size_t kMaxContextsSize =
+ MAX(sizeof(ContextsSerialized), MAX(sizeof(ContextsSplit), sizeof(ContextsPreSplit)));
+ alignas(kMaxContextsAlign) char contexts_data_[kMaxContextsSize];
+ Contexts* contexts_;
+
bool initialized_;
char property_filename_[PROP_FILENAME_MAX];
};
diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp
index c610df2..7c48b8e 100644
--- a/libc/system_properties/system_properties.cpp
+++ b/libc/system_properties/system_properties.cpp
@@ -63,7 +63,7 @@
ErrnoRestorer errno_restorer;
if (initialized_) {
- contexts()->ResetAccess();
+ contexts_->ResetAccess();
return true;
}
@@ -74,19 +74,19 @@
if (is_dir(property_filename_)) {
if (access("/dev/__properties__/property_info", R_OK) == 0) {
- new (&contexts_union_.contexts_serialized) ContextsSerialized();
- if (!contexts_union_.contexts_serialized.Initialize(false, property_filename_, nullptr)) {
+ contexts_ = new (contexts_data_) ContextsSerialized();
+ if (!contexts_->Initialize(false, property_filename_, nullptr)) {
return false;
}
} else {
- new (&contexts_union_.contexts_split) ContextsSplit();
- if (!contexts_union_.contexts_split.Initialize(false, property_filename_, nullptr)) {
+ contexts_ = new (contexts_data_) ContextsSplit();
+ if (!contexts_->Initialize(false, property_filename_, nullptr)) {
return false;
}
}
} else {
- new (&contexts_union_.contexts_pre_split) ContextsPreSplit();
- if (!contexts_union_.contexts_pre_split.Initialize(false, property_filename_, nullptr)) {
+ contexts_ = new (contexts_data_) ContextsPreSplit();
+ if (!contexts_->Initialize(false, property_filename_, nullptr)) {
return false;
}
}
@@ -100,8 +100,8 @@
}
strcpy(property_filename_, filename);
- new (&contexts_union_.contexts_serialized) ContextsSerialized();
- if (!contexts_union_.contexts_serialized.Initialize(true, property_filename_, fsetxattr_failed)) {
+ contexts_ = new (contexts_data_) ContextsSerialized();
+ if (!contexts_->Initialize(true, property_filename_, fsetxattr_failed)) {
return false;
}
initialized_ = true;
@@ -113,7 +113,7 @@
return -1;
}
- prop_area* pa = contexts()->GetSerialPropArea();
+ prop_area* pa = contexts_->GetSerialPropArea();
if (!pa) {
return -1;
}
@@ -127,7 +127,7 @@
return nullptr;
}
- prop_area* pa = contexts()->GetPropAreaForName(name);
+ prop_area* pa = contexts_->GetPropAreaForName(name);
if (!pa) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Access denied finding property \"%s\"", name);
return nullptr;
@@ -231,7 +231,7 @@
return -1;
}
- prop_area* pa = contexts()->GetSerialPropArea();
+ prop_area* pa = contexts_->GetSerialPropArea();
if (!pa) {
return -1;
}
@@ -269,12 +269,12 @@
return -1;
}
- prop_area* serial_pa = contexts()->GetSerialPropArea();
+ prop_area* serial_pa = contexts_->GetSerialPropArea();
if (serial_pa == nullptr) {
return -1;
}
- prop_area* pa = contexts()->GetPropAreaForName(name);
+ prop_area* pa = contexts_->GetPropAreaForName(name);
if (!pa) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Access denied adding property \"%s\"", name);
return -1;
@@ -319,7 +319,7 @@
return -1;
}
- prop_area* serial_pa = contexts()->GetSerialPropArea();
+ prop_area* serial_pa = contexts_->GetSerialPropArea();
if (serial_pa == nullptr) {
return -1;
}
@@ -364,7 +364,7 @@
return -1;
}
- contexts()->ForEach(propfn, cookie);
+ contexts_->ForEach(propfn, cookie);
return 0;
}