Bug 1479960 - Give SharedStringMap a magic number so that all zeroes isn't a valid instance. r=kmag
authorJed Davis <jld@mozilla.com>
Wed, 14 Aug 2019 22:48:38 +0000
changeset 488144 1fa598bf26998154cfc2c189a2a5f7fb862c7640
parent 488143 0f466f2a18c0fcf9e552ea07158e847f88ca9950
child 488145 c3ca7014893cd993088b4e6ad5f333ef7eed10a9
push id113900
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:53:50 +0000
treeherdermozilla-inbound@0db07ff50ab5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1479960, 1525803
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1479960 - Give SharedStringMap a magic number so that all zeroes isn't a valid instance. r=kmag There was a strange bug when converting SharedStringMap to use shared memory: on Android, some tests would fail because a pref wasn't set or there was something wrong with an expected error message. The root cause was that mapping ashmem with MAP_PRIVATE results in all zeroes (see bug 1525803), but that was read as a valid SharedStringMap with length 0. To prevent any possible future issues like that, this patch adds a nonzero magic number to the header. It fits into padding on 64-bit and the cost of setting and checking it should be essentially free. Depends on D26744 Differential Revision: https://phabricator.services.mozilla.com/D26745
--- a/dom/ipc/SharedStringMap.cpp
+++ b/dom/ipc/SharedStringMap.cpp
@@ -16,35 +16,39 @@ using namespace mozilla::loader;
 namespace mozilla {
 using namespace ipc;
 namespace dom {
 namespace ipc {
+static constexpr uint32_t kSharedStringMapMagic = 0x9e3779b9;
 static inline size_t GetAlignmentOffset(size_t aOffset, size_t aAlign) {
   auto mod = aOffset % aAlign;
   return mod ? aAlign - mod : 0;
 SharedStringMap::SharedStringMap(const FileDescriptor& aMapFile,
                                  size_t aMapSize) {
   auto result = mMap.initWithHandle(aMapFile, aMapSize);
+  MOZ_RELEASE_ASSERT(GetHeader().mMagic == kSharedStringMapMagic);
   // We return literal nsStrings and nsCStrings pointing to the mapped data,
   // which means that we may still have references to the mapped data even
   // after this instance is destroyed. That means that we need to keep the
   // mapping alive until process shutdown, in order to be safe.
 SharedStringMap::SharedStringMap(SharedStringMapBuilder&& aBuilder) {
   auto result = aBuilder.Finalize(mMap);
+  MOZ_RELEASE_ASSERT(GetHeader().mMagic == kSharedStringMapMagic);
 mozilla::ipc::FileDescriptor SharedStringMap::CloneFileDescriptor() const {
   return mMap.cloneHandle();
 bool SharedStringMap::Has(const nsCString& aKey) {
@@ -87,17 +91,17 @@ Result<Ok, nsresult> SharedStringMapBuil
   MOZ_ASSERT(mEntries.Count() == mKeyTable.Count());
   nsTArray<nsCString> keys(mEntries.Count());
   for (auto iter = mEntries.Iter(); !iter.Done(); iter.Next()) {
-  Header header = {uint32_t(keys.Length())};
+  Header header = {kSharedStringMapMagic, uint32_t(keys.Length())};
   size_t offset = sizeof(header);
   offset += GetAlignmentOffset(offset, alignof(Header));
   offset += keys.Length() * sizeof(SharedStringMap::Entry);
   header.mKeyStringsOffset = offset;
   header.mKeyStringsSize = mKeyTable.Size();
--- a/dom/ipc/SharedStringMap.h
+++ b/dom/ipc/SharedStringMap.h
@@ -54,16 +54,17 @@ class SharedStringMap {
    * - Optional alignment padding for char16_t[]
    * - StringTable<nsString>:
    *   A region of flat, null-terminated UTF-16 strings. Entry value strings are
    *   encoded as character (*not* byte) offsets into this region.
   struct Header {
+    uint32_t mMagic;
     // The number of entries in this map.
     uint32_t mEntryCount;
     // The raw byte offset of the beginning of the key string table, from the
     // start of the shared memory region, and its size in bytes.
     size_t mKeyStringsOffset;
     size_t mKeyStringsSize;