From b9e801ce913e7b39c3faede19ba20b69f5d4afb8 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Thu, 7 Feb 2019 11:13:13 +0100
Subject: [PATCH] ENH: extend size of NullObject for safer reinterpret cast

- previously had a single pointer/value zeros (8 bytes), this meant
  that the reinterpret cast to a List would yield a reference that
  could be unsafe under certain conditions.

  Eg,
     const labelList& myList = labelList::null();

     Info<< myList.size() << nl; // OK since size is the first parameter

     SubList<label>(myList, 0);  // Unsafe

  The SubList usage is unsafe since it passes in pointer and size into
  the underlying UList. However, the pointer from the labelList::null()
  will be whatever happens to be around in memory immediately after the
  NullObject singleton. This is mostly not a problem if the List size
  is always checked, but does mean that the data pointer is rather
  dubious.

- Increase the size of the nullObject singleton to 32 bytes of zeros
  to ensure that most reinterpret casting will not result in objects
  that reference arbitrary memory.

  The 32-byte data size is rather arbitrary, but covers most basic
  containers.
---
 .../test/nullObject/Test-nullObject.C         | 111 +++++++++++++++---
 .../primitives/nullObject/nullObject.H        |  62 ++++++----
 2 files changed, 132 insertions(+), 41 deletions(-)

diff --git a/applications/test/nullObject/Test-nullObject.C b/applications/test/nullObject/Test-nullObject.C
index 18524006dc..18f79c3069 100644
--- a/applications/test/nullObject/Test-nullObject.C
+++ b/applications/test/nullObject/Test-nullObject.C
@@ -1,11 +1,49 @@
+/*---------------------------------------------------------------------------*\
+  =========                 |
+  \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
+   \\    /   O peration     |
+    \\  /    A nd           | Copyright (C) 2019 OpenCFD Ltd.
+     \\/     M anipulation  |
+-------------------------------------------------------------------------------
+                            | Copyright (C) 2014 OpenFOAM Foundation
+-------------------------------------------------------------------------------
+License
+    This file is part of OpenFOAM.
+
+    OpenFOAM is free software: you can redistribute it and/or modify it
+    under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    OpenFOAM is distributed in the hope that it will be useful, but WITHOUT
+    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+    FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+    for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with OpenFOAM.  If not, see <http://www.gnu.org/licenses/>.
+
+Application
+    Test-nullObject
+
+Description
+    Tests of nullObject
+
+\*---------------------------------------------------------------------------*/
+
 #include "nullObject.H"
+#include "List.H"
+#include "HashSet.H"
+#include "faceList.H"
+#include "pointField.H"
 #include "IOstreams.H"
 
+// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
+
 using namespace Foam;
 
 class SimpleClass
 {
-
 public:
 
     //- Null constructor
@@ -14,6 +52,26 @@ public:
 };
 
 
+template<class T>
+void printInfo(const UList<T>& list)
+{
+    typedef unsigned long ptrval;
+
+    std::cout
+        << nl
+        << "List : addr: " << ptrval(&list)
+        << " (null: " << isNull(list) << ")" << nl
+        << "    size: " << list.size() << " empty: " << list.empty() << nl
+        << "    data: " << ptrval(list.cdata())
+        << " begin=" << ptrval(list.begin())
+        << " end=" << ptrval(list.end()) << nl;
+
+    Info<< list << nl;
+}
+
+
+// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
+
 int main()
 {
     // Test pointer and reference to a class
@@ -23,31 +81,34 @@ int main()
 
     typedef unsigned long ptrval;
 
-    Info<<"nullObject address=" << ptrval(&(nullObjectPtr)) << endl;
-    Info<<"sizeof(nullObject)" << " == "
-        << sizeof(NullObject::nullObject)
-        << " vs. sizeof(void*)" << " == " << sizeof(void*)
-        << endl;
+    std::cout
+        << "nullObject addr=" << ptrval(&(nullObjectPtr)) << nl
+        << "  sizeof(nullObject) = " << sizeof(NullObject::nullObject) << nl
+        << "  sizeof(void*) = " << sizeof(void*) << nl
+        << "  sizeof(labelList) = " << sizeof(labelList) << nl
+        << "  sizeof(wordHashSet) = " << sizeof(wordHashSet) << nl << nl;
 
-    Info<<"nullObject pointer:" << ptrval(nullObjectPtr->pointer()) << endl;
-    Info<<"nullObject value:"   << nullObjectPtr->value() << endl;
+    std::cout
+        << "nullObject" << nl
+        << "  pointer:" << ptrval(nullObjectPtr->pointer()) << nl
+        << "  value:"   << nullObjectPtr->value() << nl << nl;
 
     if (notNull(ptrToClass))
     {
-        Info<< "Pass: ptrToClass is not null" << endl;
+        Info<< "Pass: ptrToClass is not null" << nl;
     }
     else
     {
-        Info<< "FAIL: refToClass is null" << endl;
+        Info<< "FAIL: refToClass is null" << nl;
     }
 
     if (notNull(refToClass))
     {
-        Info<< "Pass: refToClass is not null" << endl;
+        Info<< "Pass: refToClass is not null" << nl;
     }
     else
     {
-        Info<< "FAIL: refToClass is null" << endl;
+        Info<< "FAIL: refToClass is null" << nl;
     }
 
 
@@ -58,24 +119,42 @@ int main()
 
     if (isNull(ptrToNull))
     {
-        Info<< "Pass: ptrToNull is null" << endl;
+        Info<< "Pass: ptrToNull is null" << nl;
     }
     else
     {
-        Info<< "FAIL: ptrToNull is not null" << endl;
+        Info<< "FAIL: ptrToNull is not null" << nl;
     }
 
     if (isNull(refToNull))
     {
-        Info<< "Pass: refToNull is null" << endl;
+        Info<< "Pass: refToNull is null" << nl;
     }
     else
     {
-        Info<< "FAIL: refToNull is not null" << endl;
+        Info<< "FAIL: refToNull is not null" << nl;
     }
 
     // Clean-up
     delete ptrToClass;
 
+
+    // Test List casting
+    {
+        labelList list1;
+        labelList list2({1, 2, 3});
+
+        printInfo(list1);
+        printInfo(list2);
+        printInfo(labelList::null());
+
+        printInfo(faceList::null());
+        printInfo(pointField::null());
+    }
+
+    Info<< nl;
+
     return 0;
 }
+
+// ************************************************************************* //
diff --git a/src/OpenFOAM/primitives/nullObject/nullObject.H b/src/OpenFOAM/primitives/nullObject/nullObject.H
index ab5785326c..a3f2de2ef7 100644
--- a/src/OpenFOAM/primitives/nullObject/nullObject.H
+++ b/src/OpenFOAM/primitives/nullObject/nullObject.H
@@ -2,10 +2,10 @@
   =========                 |
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
-    \\  /    A nd           | Copyright (C) 2017 OpenCFD Ltd.
+    \\  /    A nd           | Copyright (C) 2017-2019 OpenCFD Ltd.
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-                            | Copyright (C) 2014-2016 OpenFOAM Foundation
+                            | Copyright (C) 2014 OpenFOAM Foundation
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -28,9 +28,11 @@ Class
 
 Description
     Singleton null-object class and instance.
-    Its contents occupy just enough space to also be reinterpreted
+
+    Its contents occupy enough space to also be reinterpreted
     as another class with a null pointer or zero long for its first
-    member.
+    member. There is an additional zero second parameter for safe
+    casting to List etc.
 
 SourceFiles
     nullObjectI.H
@@ -58,25 +60,35 @@ class NullObject;
 
 class NullObject
 {
-    //- Ensure it occupies enough space to reinterpret_cast to a class
-    //  having some member data
-    const union
+    //- A %union of zero data types
+    union zeros
     {
         void* ptr;
         unsigned long val;
-    } content;
+    };
 
-    //- Private constructor for singleton only
-    NullObject()
-    :
-        content{nullptr}
-    {}
 
-    //- No copy construct
-    NullObject(const NullObject&) = delete;
+    // Private Data
+
+        //- The zero data content
+        zeros data_[4];
+
+
+    // Constructors
+
+        //- Private constructor for singleton only
+        //  Could also rely on bit-wise zero initialization for union content
+        NullObject()
+        :
+           data_{nullptr, nullptr, nullptr, nullptr}
+        {}
+
+        //- No copy construct
+        NullObject(const NullObject&) = delete;
+
+        //- No copy assignment
+        void operator=(const NullObject&) = delete;
 
-    //- No copy assignment
-    void operator=(const NullObject&) = delete;
 
 public:
 
@@ -91,13 +103,13 @@ public:
         //- A nullptr pointer content
         inline const void* pointer() const
         {
-            return content.ptr;
+            return data_[0].ptr;
         }
 
         //- Zero valued integer content
         inline unsigned long value() const
         {
-            return content.val;
+            return data_[0].val;
         }
 };
 
@@ -121,29 +133,29 @@ inline Ostream& operator<<(Ostream& os, const NullObject&)
 extern const NullObject* nullObjectPtr;
 
 
-//- Return reference to the nullObject of type T
+//- Reference to the nullObject of type T
 template<class T>
 inline const T& NullObjectRef();
 
-//- Return pointer to the nullObject of type T
+//- Pointer to the nullObject of type T
 template<class T>
 inline const T* NullObjectPtr();
 
 
-//- Return true if t is a reference to the nullObject of type T
+//- True if t is a reference to the nullObject of type T
 template<class T>
 inline bool isNull(const T& t);
 
-//- Return true if t is not a reference to the nullObject of type T
+//- True if t is not a reference to the nullObject of type T
 template<class T>
 inline bool notNull(const T& t);
 
 
-//- Return true if t is a pointer to the nullObject of type T
+//- True if t is a pointer to the nullObject of type T
 template<class T>
 inline bool isNull(const T* t);
 
-//- Return true if t is not a pointer to the nullObject of type T
+//- True if t is not a pointer to the nullObject of type T
 template<class T>
 inline bool notNull(const T* t);