i#975 static DR: Add isolated GPR set class (#4410)

A std::unordered_set, even using dr_allocator_t, raises transparency
risks when statically linked on Windows (from lock functions and other
non-allocator resources).  We thus create our own resource-isolated
class to track GPR register inclusion for drmemtrace elision
optimizations.

Adds unit tests of the set class, which are linked into burst_traceopts.

Further tested manually by ensuring the same number of elided
addresses is seen as with std::unordered_set in the burst_traceopts
test:
  [drmemtrace]: Reconstructed 622 elided addresses.

Even further tested by ensuring that raw2trace using the new set
correctly operates on a raw file that was created using
std::unordered_set.

Issue: #975, #4403

clients/drcachesim/CMakeLists.txt

@@ -357,7 +357,7 @@ macro(add_win32_flags target)
     if (DEBUG)
       get_property(cur TARGET ${target} PROPERTY COMPILE_FLAGS)
       string(REPLACE "/MT " "" cur "${cur}") # Avoid override warning.
-      set_target_properties(${target} PROPERTIES COMPILE_FLAGS "${cur} /EHsc /MTd")
+      set_target_properties(${target} PROPERTIES COMPILE_FLAGS "${cur} /EHsc /MTd /Zi")
       append_property_string(TARGET ${target} LINK_FLAGS "/nodefaultlib:libcmt")
     else ()
       append_property_string(TARGET ${target} COMPILE_FLAGS "/EHsc /MT")
@@ -621,7 +621,9 @@ if (BUILD_TESTS)
     add_split_asm_target("${source_abs}" asm_source gen_asm_tgt
       "_asm" "" "${asm_deps}")
     add_executable(tool.drcacheoff.burst_traceopts ${source_abs}
-      ${asm_source})
+      ${asm_source}
+      # This test includes related unit tests for classes used for trace opts.
+      "${CMAKE_CURRENT_SOURCE_DIR}/tests/reg_id_set_unit_tests.cpp")
     configure_DynamoRIO_static(tool.drcacheoff.burst_traceopts)
     if ("${CMAKE_GENERATOR}" MATCHES "Visual Studio")
       add_dependencies(tool.drcacheoff.burst_traceopts ${gen_asm_tgt})

clients/drcachesim/tests/burst_traceopts.cpp

@@ -50,6 +50,10 @@
 #    include <stdlib.h>
 #    include <string.h>
 
+/* Unit tests for classes used for trace optimizations. */
+void
+reg_id_set_unit_tests();
+
 /* Asm routines. */
 extern "C" {
 void
@@ -212,6 +216,8 @@ gather_trace(const std::string &tracer_ops, const std::string &out_subdir)
 int
 main(int argc, const char *argv[])
 {
+    reg_id_set_unit_tests();
+
     std::string dir_opt = gather_trace("", "opt");
     std::string dir_noopt = gather_trace("-disable_optimizations", "noopt");
 

clients/drcachesim/tests/reg_id_set_unit_tests.cpp

@@ -0,0 +1,89 @@
+/* **********************************************************
+ * Copyright (c) 2020 Google, Inc.  All rights reserved.
+ * **********************************************************/
+
+/*
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Google, Inc. nor the names of its contributors may be
+ *   used to endorse or promote products derived from this software without
+ *   specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+/* Unit tests for class reg_id_set_t, which is used for trace optimizations.
+ * Linked into the burst_traceopts executable which covers trace optimizations
+ * (fewer executables reduces the limited-resource CI time).
+ */
+
+#include <assert.h>
+#include "tracer/instru.h"
+
+void
+reg_id_set_unit_tests()
+{
+    reg_id_set_t set;
+    assert(set.begin() == set.end());
+    // Test non-GPR.
+    auto insert_res = set.insert(IF_X86_ELSE(DR_REG_XMM0, DR_REG_Q0));
+    assert(insert_res.first == set.end());
+    assert(!insert_res.second);
+    assert(set.begin() == set.end());
+    // Test adding a GPR.
+    insert_res = set.insert(DR_REG_START_GPR + 1);
+    assert(insert_res.first != set.end());
+    assert(insert_res.second);
+    assert(set.begin() != set.end());
+    // Test find not-present.
+    auto find_res = set.find(DR_REG_START_GPR);
+    assert(find_res == set.end());
+    // Test find present.
+    find_res = set.find(DR_REG_START_GPR + 1);
+    assert(find_res != set.end());
+    assert(*find_res == DR_REG_START_GPR + 1);
+    // Test erase with two entries.
+    insert_res = set.insert(DR_REG_START_GPR + 4);
+    assert(insert_res.second);
+    auto iter = set.begin();
+    bool found_next = false;
+    while (iter != set.end()) {
+        if (*iter == DR_REG_START_GPR + 1)
+            iter = set.erase(iter);
+        else {
+            if (*iter == DR_REG_START_GPR + 4)
+                found_next = true;
+            ++iter;
+        }
+    }
+    assert(found_next);
+    assert(set.find(DR_REG_START_GPR + 1) == set.end());
+    assert(set.find(DR_REG_START_GPR + 4) != set.end());
+    assert(*set.find(DR_REG_START_GPR + 4) == DR_REG_START_GPR + 4);
+    // Test adding duplicates.
+    insert_res = set.insert(DR_REG_START_GPR + 3);
+    assert(insert_res.second);
+    insert_res = set.insert(DR_REG_START_GPR + 3);
+    assert(!insert_res.second);
+    iter = set.erase(insert_res.first);
+    find_res = set.find(DR_REG_START_GPR + 3);
+    assert(find_res == set.end());
+}

clients/drcachesim/tracer/instru.h

@@ -37,7 +37,8 @@
 #define _INSTRU_H_ 1
 
 #include <stdint.h>
-#include <unordered_set>
+#include <string.h>
+#include "dr_api.h"
 #include "drvector.h"
 #include "trace_entry.h"
 #include "dr_allocator.h"
@@ -47,9 +48,121 @@
 // Versioning for our drmodtrack custom module fields.
 #define CUSTOM_MODULE_VERSION 1
 
-typedef std::unordered_set<reg_id_t, std::hash<reg_id_t>, std::equal_to<reg_id_t>,
-                           dr_allocator_t<reg_id_t>>
-    reg_id_set_t;
+// A std::unordered_set, even using dr_allocator_t, raises transparency risks when
+// statically linked on Windows (from lock functions and other non-allocator
+// resources).  We thus create our own resource-isolated class to track GPR register
+// inclusion, which can easily be implemented with an array.
+// The burst_traceopts test contains unit tests for this class.
+class reg_id_set_t {
+public:
+    reg_id_set_t()
+    {
+        clear();
+    }
+
+    void
+    clear()
+    {
+        memset(present_, 0, sizeof(present_));
+    }
+
+    class reg_id_set_iterator_t
+        : public std::iterator<std::input_iterator_tag, reg_id_t> {
+    public:
+        reg_id_set_iterator_t(reg_id_set_t *set)
+            : set_(set)
+            , index_(-1)
+        {
+        }
+        reg_id_set_iterator_t(reg_id_set_t *set, int index)
+            : set_(set)
+            , index_(index)
+        {
+        }
+
+        reg_id_t operator*()
+        {
+            return static_cast<reg_id_t>(DR_REG_START_GPR + index_);
+        }
+
+        bool
+        operator==(const reg_id_set_iterator_t &rhs) const
+        {
+            return index_ == rhs.index_;
+        }
+        bool
+        operator!=(const reg_id_set_iterator_t &rhs) const
+        {
+            return index_ != rhs.index_;
+        }
+
+        reg_id_set_iterator_t &
+        operator++()
+        {
+            while (++index_ < DR_NUM_GPR_REGS && !set_->present_[index_]) {
+                /* Nothing. */
+            }
+            if (index_ >= DR_NUM_GPR_REGS)
+                index_ = -1;
+            return *this;
+        }
+
+    private:
+        friend class reg_id_set_t;
+        reg_id_set_t *set_;
+        int index_;
+    };
+
+    reg_id_set_iterator_t
+    begin()
+    {
+        reg_id_set_iterator_t iter(this);
+        return ++iter;
+    }
+
+    reg_id_set_iterator_t
+    end()
+    {
+        return reg_id_set_iterator_t(this);
+    }
+
+    reg_id_set_iterator_t
+    erase(const reg_id_set_iterator_t pos)
+    {
+        if (pos.index_ == -1)
+            return end();
+        reg_id_set_iterator_t iter(this, pos.index_);
+        present_[pos.index_] = false;
+        return ++iter;
+    }
+
+    std::pair<reg_id_set_iterator_t, bool>
+    insert(reg_id_t id)
+    {
+        if (id < DR_REG_START_GPR || id > DR_REG_STOP_GPR)
+            return std::make_pair(end(), false);
+        index_ = id - DR_REG_START_GPR;
+        bool exists = present_[index_];
+        if (!exists)
+            present_[index_] = true;
+        return std::make_pair(reg_id_set_iterator_t(this, index_), !exists);
+    }
+
+    reg_id_set_iterator_t
+    find(reg_id_t id)
+    {
+        if (id < DR_REG_START_GPR || id > DR_REG_STOP_GPR)
+            return end();
+        index_ = id - DR_REG_START_GPR;
+        if (!present_[index_])
+            return end();
+        return reg_id_set_iterator_t(this, index_);
+    }
+
+private:
+    bool present_[DR_NUM_GPR_REGS];
+    int index_;
+};
 
 class instru_t {
 public: