From bc09128e3ccf3af441218739dc4f3d9a5584e2f5 Mon Sep 17 00:00:00 2001 From: Nilay Vaish Date: Sun, 7 Jan 2018 15:38:15 -0800 Subject: [PATCH 1/5] i#2717 shrink static DR library: do not initialize dynamo_options statically. We currently initialize dynamo_options at the time of declaration. This is not required since the initialization is same as default_options variable. Instead, we now copy the default_options to dynamo_options in the function options_init(). This saves about 27,020 bytes of space. Before: -r-xr-xr-x 1 nilayvaish eng 2594134 Nov 24 23:37 blaze-bin/third_party/dynamorio/libdynamorio.a After: -r-xr-xr-x 1 nilayvaish eng 2567114 Nov 25 12:32 blaze-bin/third_party/dynamorio/libdynamorio.a Issue: #2717 --- core/dynamo.c | 9 +++++---- core/options.c | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/core/dynamo.c b/core/dynamo.c index 4443502e0b..db4a237238 100644 --- a/core/dynamo.c +++ b/core/dynamo.c @@ -853,10 +853,6 @@ standalone_init(void) standalone_library = true; /* We have release-build stats now so this is not just DEBUG */ stats = &nonshared_stats; -#if defined(INTERNAL) && defined(DEADLOCK_AVOIDANCE) - /* avoid issues w/ GLOBAL_DCONTEXT instead of thread dcontext */ - dynamo_options.deadlock_avoidance = false; -#endif #ifdef UNIX os_page_size_init((const char **)our_environ); #endif @@ -867,6 +863,10 @@ standalone_init(void) #endif config_init(); options_init(); +#if defined(INTERNAL) && defined(DEADLOCK_AVOIDANCE) + /* avoid issues w/ GLOBAL_DCONTEXT instead of thread dcontext */ + dynamo_options.deadlock_avoidance = false; +#endif vmm_heap_init(); heap_init(); dynamo_heap_initialized = true; @@ -888,6 +888,7 @@ standalone_init(void) # ifdef DEBUG /* FIXME: share code w/ main init routine? */ nonshared_stats.logmask = LOG_ALL; + /* Why do we need this to options_init()? We did so above. */ options_init(); if (stats->loglevel > 0) { char initial_options[MAX_OPTIONS_STRING]; diff --git a/core/options.c b/core/options.c index 55ea83fb30..20967f0af6 100644 --- a/core/options.c +++ b/core/options.c @@ -209,9 +209,8 @@ char option_string[MAX_OPTIONS_STRING] = {0,}; # define ASSERT_OWN_OPTIONS_LOCK(b, l) ASSERT_OWN_WRITE_LOCK(b, l) # define CORE_STATIC static /* official options */ -options_t dynamo_options = { -# include "optionsx.h" -}; +options_t dynamo_options; + /* Temporary string, static to save stack space in synchronize_dynamic_options(). * FIXME case 8074: should protect this better as w/o DR dll randomization attacker * can repeatedly try to clobber. Move to heap? Or shrink stack space elsewhere @@ -2443,6 +2442,8 @@ options_init() /* .lspdata pages start out writable so no unprotect needed here */ write_lock(&options_lock); ASSERT(sizeof(dynamo_options) == sizeof(options_t)); + /* Set to default options. */ + dynamo_options = default_options; /* get dynamo options */ adjust_defaults_for_page_size(&dynamo_options); retval = get_parameter(PARAM_STR(DYNAMORIO_VAR_OPTIONS), option_string, From ae5ef844a9997fa58f420c6a1b964341d7007268 Mon Sep 17 00:00:00 2001 From: Nilay Vaish Date: Mon, 15 Jan 2018 21:28:39 -0800 Subject: [PATCH 2/5] i#2717 shrink static DR library: do not set dynamo_options statically We currently initialize dynamo_options at the time of declaration. This is not required since the initialization is same as default_options variable. Instead, we now copy the default_options to dynamo_options in the function options_init(). This saves about 27,020 bytes of space. Before: -r-xr-xr-x 1 nilayvaish eng 2594134 Nov 24 23:37 dynamorio/libdynamorio.a After: -r-xr-xr-x 1 nilayvaish eng 2567114 Nov 25 12:32 dynamorio/libdynamorio.a Issue: #2717 --- core/dynamo.c | 11 ++++------- core/options.c | 3 +-- core/optionsx.h | 4 ++++ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/core/dynamo.c b/core/dynamo.c index db4a237238..1b0babc3bf 100644 --- a/core/dynamo.c +++ b/core/dynamo.c @@ -861,12 +861,12 @@ standalone_init(void) if (!syscalls_init()) return NULL; /* typically b/c of unsupported OS version */ #endif +#ifdef DEBUG + nonshared_stats.logmask = LOG_ALL; +#endif + config_init(); options_init(); -#if defined(INTERNAL) && defined(DEADLOCK_AVOIDANCE) - /* avoid issues w/ GLOBAL_DCONTEXT instead of thread dcontext */ - dynamo_options.deadlock_avoidance = false; -#endif vmm_heap_init(); heap_init(); dynamo_heap_initialized = true; @@ -887,9 +887,6 @@ standalone_init(void) # ifdef DEBUG /* FIXME: share code w/ main init routine? */ - nonshared_stats.logmask = LOG_ALL; - /* Why do we need this to options_init()? We did so above. */ - options_init(); if (stats->loglevel > 0) { char initial_options[MAX_OPTIONS_STRING]; main_logfile = open_log_file(main_logfile_name(), NULL, 0); diff --git a/core/options.c b/core/options.c index 20967f0af6..68b3f69c2d 100644 --- a/core/options.c +++ b/core/options.c @@ -2443,9 +2443,8 @@ options_init() write_lock(&options_lock); ASSERT(sizeof(dynamo_options) == sizeof(options_t)); /* Set to default options. */ - dynamo_options = default_options; + set_dynamo_options_defaults(&dynamo_options); /* get dynamo options */ - adjust_defaults_for_page_size(&dynamo_options); retval = get_parameter(PARAM_STR(DYNAMORIO_VAR_OPTIONS), option_string, sizeof(option_string)); if (IS_GET_PARAMETER_SUCCESS(retval)) diff --git a/core/optionsx.h b/core/optionsx.h index 138674afae..a5a207512c 100644 --- a/core/optionsx.h +++ b/core/optionsx.h @@ -294,7 +294,11 @@ #endif #ifdef DEADLOCK_AVOIDANCE +# ifdef STANDALONE_UNIT_TEST + OPTION_DEFAULT_INTERNAL(bool, deadlock_avoidance, false, "enable deadlock avoidance checks") +# else OPTION_DEFAULT_INTERNAL(bool, deadlock_avoidance, true, "enable deadlock avoidance checks") +# endif OPTION_DEFAULT_INTERNAL(uint, mutex_callstack, 0 /* 0 to disable, 4 recommended, MAX_MUTEX_CALLSTACK */, "collect a callstack up to specified depth when a mutex is locked") #endif From 60842f2e9aad2ee5c58ab552914fdabd38083849 Mon Sep 17 00:00:00 2001 From: Nilay Vaish Date: Sat, 20 Jan 2018 08:03:29 -0800 Subject: [PATCH 3/5] Fixes problems due to options not being readable. --- core/options.c | 5 +++++ core/optionsx.h | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/options.c b/core/options.c index 68b3f69c2d..5241aaaa4f 100644 --- a/core/options.c +++ b/core/options.c @@ -2441,9 +2441,14 @@ options_init() /* .lspdata pages start out writable so no unprotect needed here */ write_lock(&options_lock); + SELF_UNPROTECT_OPTIONS(); ASSERT(sizeof(dynamo_options) == sizeof(options_t)); /* Set to default options. */ set_dynamo_options_defaults(&dynamo_options); +#if defined(INTERNAL) && defined(DEADLOCK_AVOIDANCE) + /* avoid issues w/ GLOBAL_DCONTEXT instead of thread dcontext */ + dynamo_options.deadlock_avoidance = false; +#endif /* get dynamo options */ retval = get_parameter(PARAM_STR(DYNAMORIO_VAR_OPTIONS), option_string, sizeof(option_string)); diff --git a/core/optionsx.h b/core/optionsx.h index a5a207512c..138674afae 100644 --- a/core/optionsx.h +++ b/core/optionsx.h @@ -294,11 +294,7 @@ #endif #ifdef DEADLOCK_AVOIDANCE -# ifdef STANDALONE_UNIT_TEST - OPTION_DEFAULT_INTERNAL(bool, deadlock_avoidance, false, "enable deadlock avoidance checks") -# else OPTION_DEFAULT_INTERNAL(bool, deadlock_avoidance, true, "enable deadlock avoidance checks") -# endif OPTION_DEFAULT_INTERNAL(uint, mutex_callstack, 0 /* 0 to disable, 4 recommended, MAX_MUTEX_CALLSTACK */, "collect a callstack up to specified depth when a mutex is locked") #endif From e4984be7fa70f0cc43a4fa3ada5e6724bb209d99 Mon Sep 17 00:00:00 2001 From: Nilay Vaish Date: Sat, 20 Jan 2018 09:47:46 -0800 Subject: [PATCH 4/5] Use already defined functions for making options readonly and writable. --- core/options.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/options.c b/core/options.c index 5241aaaa4f..1430734b3f 100644 --- a/core/options.c +++ b/core/options.c @@ -2439,9 +2439,11 @@ options_init() { int ret = 0, retval; - /* .lspdata pages start out writable so no unprotect needed here */ - write_lock(&options_lock); - SELF_UNPROTECT_OPTIONS(); + /* Apart from when we start of, we might end up here due to post processing. + * When here due to post processing, dynamo_options would not readable. So + * we make them writable first. + */ + options_make_writable(); ASSERT(sizeof(dynamo_options) == sizeof(options_t)); /* Set to default options. */ set_dynamo_options_defaults(&dynamo_options); @@ -2461,8 +2463,7 @@ options_init() options_enable_code_api_dependences(&dynamo_options); #endif check_option_compatibility(); - /* options will be protected when DR init is completed */ - write_unlock(&options_lock); + options_restore_readonly(); return ret; } From 5c9763e18d8949d5b9de4c3375f75a20daf5cbf0 Mon Sep 17 00:00:00 2001 From: Nilay Vaish Date: Wed, 24 Jan 2018 15:06:25 -0800 Subject: [PATCH 5/5] Trying to fix the broken tests on Windows. Almost everything fails with PR when running on Windows. One of the errors reported was 'access violation'. Restoring the code that only releases the writer lock on options variable. Earlier we had made them readable as well. --- core/options.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/options.c b/core/options.c index 1430734b3f..a5088e7075 100644 --- a/core/options.c +++ b/core/options.c @@ -2440,8 +2440,8 @@ options_init() int ret = 0, retval; /* Apart from when we start of, we might end up here due to post processing. - * When here due to post processing, dynamo_options would not readable. So - * we make them writable first. + * When here due to post processing, dynamo_options would not be writable. + * So we make them writable first. */ options_make_writable(); ASSERT(sizeof(dynamo_options) == sizeof(options_t)); @@ -2463,7 +2463,8 @@ options_init() options_enable_code_api_dependences(&dynamo_options); #endif check_option_compatibility(); - options_restore_readonly(); + /* options will be protected when DR init is completed */ + write_unlock(&options_lock); return ret; }