i#975 static DR: add optional checks for mid-run malloc calls (#2855)

Adds a feature where a client library can request that the private loader complain if malloc & co. are called at any time other than process init or exit, to help clients that want to support being linked statically with the app. Because it has to be early, the feature is triggered by a variable declaration DR_DISALLOW_UNSAFE_STATIC. It can be overridden dynamically by a new API routine dr_allow_unsafe_static_behavior(). Fixes drcachesim to use placement new for its offline custom module data allocations. Issue: #975, #2006
DynamoRIO · Feb 28, 2018 · 3bc3315 · 3bc3315
1 parent b6baa11
commit 3bc3315
Show file tree

Hide file tree

Showing 14 changed files with 160 additions and 13 deletions.
diff --git a/api/docs/release.dox b/api/docs/release.dox
@@ -236,6 +236,8 @@ Further non-compatibility-affecting changes include:
    executed on along with an optional simulator scheduling feature to
    schedule threads on simulated cores to match the recorded execution on
    physical cpus.
+ - Added #DR_DISALLOW_UNSAFE_STATIC and dr_disallow_unsafe_static_behavior()
+   for sanity checks to help support statically-linked clients.
 
 **************************************************
 <hr>

diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt
@@ -208,6 +208,7 @@ endmacro()
 
 add_drmemtrace(drmemtrace SHARED)
 add_drmemtrace(drmemtrace_static STATIC)
+append_property_string(TARGET drmemtrace_static COMPILE_FLAGS "-DDRMEMTRACE_STATIC")
 # We export drmemtrace.h to the same place as the analysis tool headers
 # for simplicity, rather than sticking it into ext/include or sthg.
 install_client_nonDR_header(drmemtrace tracer/drmemtrace.h)

diff --git a/clients/drcachesim/tracer/instru_offline.cpp b/clients/drcachesim/tracer/instru_offline.cpp
@@ -39,6 +39,7 @@
 #include "drcovlib.h"
 #include "instru.h"
 #include "../common/trace_entry.h"
+#include <new>
 #include <limits.h> /* for USHRT_MAX */
 #include <stddef.h> /* for offsetof */
 #include <string.h> /* for strlen */
@@ -100,14 +101,17 @@ offline_instru_t::load_custom_module_data(module_data_t *module)
     const char *name = dr_module_preferred_name(module);
     // For vdso we include the entire contents so we can decode it during
     // post-processing.
+    // We use placement new for better isolation, esp w/ static linkage into the app.
     if ((name != nullptr &&
          (strstr(name, "linux-gate.so") == name ||
           strstr(name, "linux-vdso.so") == name)) ||
         (module->names.file_name != NULL && strcmp(name, "[vdso]") == 0)) {
-        return new custom_module_data_t((const char *)module->start,
-                                        module->end - module->start, user_data);
+        void *alloc = dr_global_alloc(sizeof(custom_module_data_t));
+        return new(alloc) custom_module_data_t((const char *)module->start,
+                                               module->end - module->start, user_data);
     } else if (user_data != nullptr) {
-        return new custom_module_data_t(nullptr, 0, user_data);
+        void *alloc = dr_global_alloc(sizeof(custom_module_data_t));
+        return new(alloc) custom_module_data_t(nullptr, 0, user_data);
     }
     return nullptr;
 }
@@ -150,7 +154,8 @@ offline_instru_t::free_custom_module_data(void *data)
         return;
     if (user_free != nullptr)
         (*user_free)(custom->user_data);
-    delete custom;
+    custom->~custom_module_data_t();
+    dr_global_free(custom, sizeof(*custom));
 }
 
 bool

diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp
@@ -68,6 +68,11 @@ DR_EXPORT void drmemtrace_client_main(client_id_t id, int argc, const char *argv
 }
 #endif
 
+/* Request debug-build checks on use of malloc mid-run which will break statically
+ * linking this client into an app.
+ */
+DR_DISALLOW_UNSAFE_STATIC
+
 #define NOTIFY(level, ...) do {            \
     if (op_verbose.get_value() >= (level)) \
         dr_fprintf(STDERR, __VA_ARGS__);   \
@@ -1811,6 +1816,14 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[])
         have_phys = physaddr.init();
         if (!have_phys)
             NOTIFY(0, "Unable to open pagemap: using virtual addresses.\n");
+        /* Unfortunately the use of std::unordered_map in physaddr_t calls malloc
+         * and thus we cannot support it for static linking, so we override the
+         * DR_DISALLOW_UNSAFE_STATIC declaration.
+         */
+        dr_allow_unsafe_static_behavior();
+#ifdef DRMEMTRACE_STATIC
+        NOTIFY(0, "-use_physical is unsafe with statically linked clients\n");
+#endif
     }
 }
 

diff --git a/core/lib/dr_api.h b/core/lib/dr_api.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2014-2015 Google, Inc.  All rights reserved.
+ * Copyright (c) 2014-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2002-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -122,6 +122,18 @@ DR_EXPORT LINK_ONCE int _USES_DR_VERSION_ = ${VERSION_NUMBER_INTEGER};
 /* A flag that can be used to identify whether this file was included */
 #define DYNAMORIO_API
 
+/**
+ * This declaration requests that DR perform sanity checks to ensure that client
+ * libraries will also operate safely when linked statically into an
+ * application.  These checks include ensuring that the system allocator is not
+ * called outside of process initialization or exit, where such calls raise
+ * transparency issues due to the lack of isolation without a private loader.
+ * Currently, these checks are only performed in debug builds on UNIX.  This can
+ * be overridden in client code by calling dr_allow_unsafe_static_behavior().
+ */
+#define DR_DISALLOW_UNSAFE_STATIC \
+    DR_EXPORT LINK_ONCE int _DR_DISALLOW_UNSAFE_STATIC_ = 1;
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/core/lib/instrument.c b/core/lib/instrument.c
@@ -2510,6 +2510,12 @@ dr_set_process_exit_behavior(dr_exit_flags_t flags)
     }
 }
 
+void
+dr_allow_unsafe_static_behavior(void)
+{
+    loader_allow_unsafe_static_behavior();
+}
+
 DR_API
 /* Returns the option string passed along with a client path via DR's
  * -client_lib option.

diff --git a/core/lib/instrument_api.h b/core/lib/instrument_api.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2017 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2002-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -840,6 +840,16 @@ DR_API
 void
 dr_set_process_exit_behavior(dr_exit_flags_t flags);
 
+DR_API
+/**
+ * The #DR_DISALLOW_UNSAFE_STATIC declaration requests that DR perform sanity
+ * checks to ensure that client libraries will also operate safely when linked
+ * statically into an application.  This overrides that request, facilitating
+ * having runtime options that are not supported in a static context.
+ */
+void
+dr_allow_unsafe_static_behavior(void);
+
 #ifdef UNIX
 DR_API
 /**

diff --git a/core/os_shared.h b/core/os_shared.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2017 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2003-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -1191,5 +1191,6 @@ void loader_exit(void);
 void loader_thread_init(dcontext_t *dcontext);
 void loader_thread_exit(dcontext_t *dcontext);
 bool in_private_library(app_pc pc);
+void loader_allow_unsafe_static_behavior(void);
 
 #endif /* OS_SHARED_H */
diff --git a/core/unix/loader.c b/core/unix/loader.c
@@ -1,5 +1,5 @@
 /* *******************************************************************************
- * Copyright (c) 2011-2017 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2011 Massachusetts Institute of Technology  All rights reserved.
  * *******************************************************************************/
 
@@ -1224,6 +1224,19 @@ privload_fill_os_module_info(app_pc base,
  * Function Redirection
  */
 
+#ifdef DEBUG
+/* i#975: used for debug checks for static-link-ready clients. */
+bool disallow_unsafe_static_calls;
+#endif
+
+void
+loader_allow_unsafe_static_behavior(void)
+{
+#ifdef DEBUG
+    disallow_unsafe_static_calls = false;
+#endif
+}
+
 #if defined(LINUX) && !defined(ANDROID)
 /* These are not yet supported by Android's Bionic */
 void *
@@ -1366,11 +1379,32 @@ static const redirect_import_t redirect_imports[] = {
 };
 #define REDIRECT_IMPORTS_NUM (sizeof(redirect_imports)/sizeof(redirect_imports[0]))
 
+#ifdef DEBUG
+static const redirect_import_t redirect_debug_imports[] = {
+    {"calloc",  (app_pc)redirect_calloc_initonly},
+    {"malloc",  (app_pc)redirect_malloc_initonly},
+    {"free",    (app_pc)redirect_free_initonly},
+    {"realloc", (app_pc)redirect_realloc_initonly},
+};
+# define REDIRECT_DEBUG_IMPORTS_NUM \
+    (sizeof(redirect_debug_imports)/sizeof(redirect_debug_imports[0]))
+#endif
+
 bool
 privload_redirect_sym(ptr_uint_t *r_addr, const char *name)
 {
     int i;
     /* iterate over all symbols and redirect syms when necessary, e.g. malloc */
+#ifdef DEBUG
+    if (disallow_unsafe_static_calls) {
+        for (i = 0; i < REDIRECT_DEBUG_IMPORTS_NUM; i++) {
+            if (strcmp(redirect_debug_imports[i].name, name) == 0) {
+                *r_addr = (ptr_uint_t)redirect_debug_imports[i].func;
+                return true;;
+            }
+        }
+    }
+#endif
     for (i = 0; i < REDIRECT_IMPORTS_NUM; i++) {
         if (strcmp(redirect_imports[i].name, name) == 0) {
             *r_addr = (ptr_uint_t)redirect_imports[i].func;

diff --git a/core/unix/module.c b/core/unix/module.c
@@ -1,5 +1,5 @@
 /* *******************************************************************************
- * Copyright (c) 2012-2016 Google, Inc.  All rights reserved.
+ * Copyright (c) 2012-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2011 Massachusetts Institute of Technology  All rights reserved.
  * Copyright (c) 2008-2010 VMware, Inc.  All rights reserved.
  * *******************************************************************************/
@@ -33,6 +33,7 @@
  */
 
 #include "../globals.h"
+#include "module_private.h"
 #include "../module_shared.h"
 #include "os_private.h"
 #include "../utils.h"
@@ -664,6 +665,40 @@ redirect_free(void *mem)
     }
 }
 
+# ifdef DEBUG
+/* i#975: these help clients support static linking with the app. */
+void *
+redirect_malloc_initonly(size_t size)
+{
+    CLIENT_ASSERT(!disallow_unsafe_static_calls || !dynamo_initialized || dynamo_exited,
+                  "malloc invoked mid-run when disallowed by DR_DISALLOW_UNSAFE_STATIC");
+    return redirect_malloc(size);
+}
+
+void *
+redirect_realloc_initonly(void *mem, size_t size)
+{
+    CLIENT_ASSERT(!disallow_unsafe_static_calls || !dynamo_initialized || dynamo_exited,
+                  "realloc invoked mid-run when disallowed by DR_DISALLOW_UNSAFE_STATIC");
+    return redirect_realloc(mem, size);
+}
+
+void *
+redirect_calloc_initonly(size_t nmemb, size_t size)
+{
+    CLIENT_ASSERT(!disallow_unsafe_static_calls || !dynamo_initialized || dynamo_exited,
+                  "calloc invoked mid-run when disallowed by DR_DISALLOW_UNSAFE_STATIC");
+    return redirect_calloc(nmemb, size);
+}
+
+void
+redirect_free_initonly(void *mem)
+{
+    CLIENT_ASSERT(!disallow_unsafe_static_calls || !dynamo_initialized || dynamo_exited,
+                  "free invoked mid-run when disallowed by DR_DISALLOW_UNSAFE_STATIC");
+    redirect_free(mem);
+}
+# endif
 #endif /* !NOT_DYNAMORIO_CORE_PROPER */
 
 bool

diff --git a/core/unix/module.h b/core/unix/module.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2016 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2008-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -169,6 +169,17 @@ redirect_free(void *ptr);
 void *
 redirect_realloc(void *ptr, size_t size);
 
+#ifdef DEBUG
+void *
+redirect_malloc_initonly(size_t size);
+void *
+redirect_realloc_initonly(void *mem, size_t size);
+void *
+redirect_calloc_initonly(size_t nmemb, size_t size);
+void
+redirect_free_initonly(void *mem);
+#endif
+
 #if defined(MACOS) || defined(ANDROID)
 typedef FILE stdfile_t;
 # define STDFILE_FILENO _file

diff --git a/core/unix/module_elf.c b/core/unix/module_elf.c
@@ -1,5 +1,5 @@
 /* *******************************************************************************
- * Copyright (c) 2012-2017 Google, Inc.  All rights reserved.
+ * Copyright (c) 2012-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2011 Massachusetts Institute of Technology  All rights reserved.
  * Copyright (c) 2008-2010 VMware, Inc.  All rights reserved.
  * *******************************************************************************/
@@ -1310,6 +1310,11 @@ module_get_os_privmod_data(app_pc base, size_t size, bool dyn_reloc,
         load_delta = 0;
     }
     module_init_os_privmod_data_from_dyn(pd, dyn, load_delta);
+    DODEBUG({
+        if (get_proc_address_from_os_data(&pd->os_data, pd->load_delta,
+                                          DR_DISALLOW_UNSAFE_STATIC_NAME, NULL) != NULL)
+            disallow_unsafe_static_calls = true;
+    });
 }
 
 /* Returns a pointer to the phdr of the given type.

diff --git a/core/unix/module_private.h b/core/unix/module_private.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2016 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2018 Google, Inc.  All rights reserved.
  * Copyright (c) 2008-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -90,6 +90,12 @@ struct _os_privmod_data_t {
     app_pc         tls_image;      /* tls block address in memory */
 };
 
+#ifdef DEBUG
+/* i#975: used for debug checks for static-link-ready clients. */
+# define DR_DISALLOW_UNSAFE_STATIC_NAME "_DR_DISALLOW_UNSAFE_STATIC_"
+extern bool disallow_unsafe_static_calls;
+#endif
+
 void
 module_get_os_privmod_data(app_pc base, size_t size, bool relocated,
                            OUT os_privmod_data_t *pd);

diff --git a/core/win32/loader.c b/core/win32/loader.c
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2017 Google, Inc.   All rights reserved.
+ * Copyright (c) 2011-2018 Google, Inc.   All rights reserved.
  * Copyright (c) 2009-2010 Derek Bruening   All rights reserved.
  * **********************************************************/
 
@@ -415,6 +415,12 @@ os_loader_thread_exit(dcontext_t *dcontext)
     /* do nothing in Windows */
 }
 
+void
+loader_allow_unsafe_static_behavior(void)
+{
+    /* XXX i#975: NYI */
+}
+
 #ifdef CLIENT_INTERFACE
 /* our copy of the PEB for isolation (i#249) */
 PEB *