From 4614c17515c5212167bf2defc5222b280597eb05 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Tue, 9 Jun 2020 00:41:00 -0400 Subject: [PATCH 1/7] i#147: Alternate-bitwidth client support Adds new options and interfaces to specify alternate-bitwidth client libraries for use when the application creates a child process of the other bitwidth. For DR, adds -client_lib32 and -client_lib64 options. Switches main usage to use the appropriate option, with its contents then copied into -client_lib (to avoid the pain of removing that options). For drconfiglib, adds dr_register_client_ex() with dr_client_iterator_next_ex() to support querying other-bitwidth client registrations. Adds a new libutil.drconfig_test for drconfiglib. Fixes a bug found by the test: existing client queries were cutting off the last character of the path and options. For drrun and drconfig, adds "-c32" and "-c64" options, with an additional "--" separating the client options between them. For tool files, adds CLIENT{32,64}_{REL,ABS}. Updates drcov, drcpusim, and drcachesim to use the new syntax and drcachesim's launcher to process it. Tested these manually: =========================================================================== $ ninja install $ rm *.log $ ../exports/bin64/drrun -t drcov -- ~/dr/test/execve64 ~/dr/test/hello32 $ l -t *.log 20K drcov.execve64.109583.0000.proc.log 20K drcov.execve64.109585.0000.proc.log 20K drcov.hello32.109585.0000.proc.log $ rm -rf drm*.dir $ ../exports/bin64/drrun -verbose -t drcachesim -verbose 1 -offline -- ~/dr/test/execve64 ~/dr/test/hello32 $ l -td *.dir 4.0K drmemtrace.hello32.117714.7095.dir/ 4.0K drmemtrace.execve64.117714.6260.dir/ 4.0K drmemtrace.execve64.117713.9314.dir/ =========================================================================== Adds tests of -c32/-c64 to the existing cross-arch linux.execve{32,64} tests (Windows won't work until #803 is addressed). The tests look like this: =========================================================================== $ cmake . && ctest -V -R 'linux.execve(32|64)' 8: Running test command: "/home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun" "-32" "-dr_home" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit" "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve-sub64" 8: large_options passed: -paramA foo -paramB bar 8: parent is running under DynamoRIO 8: parent waiting for child 8: child is running under DynamoRIO 8: large_options passed: -paramA foo -paramB bar 8: it_worked 8: running under DynamoRIO 8: large_options exiting 8: child has exited 8: large_options exiting 8: 1/2 Test #8: linux.execve32 ................... Passed 3.93 sec 9: Test command: /home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve-sub32" 9: Test timeout computed to be: 1500 9: large_options passed: -paramA foo -paramB bar 9: parent is running under DynamoRIO 9: parent waiting for child 9: child is running under DynamoRIO 9: large_options passed: -paramA foo -paramB bar 9: it_worked 9: running under DynamoRIO 9: large_options exiting 9: child has exited 9: large_options exiting 2/2 Test #9: linux.execve64 ................... Passed 0.86 sec =========================================================================== Issue: #147, #803 Fixes: #147 --- api/docs/intro.dox | 29 ++- api/docs/release.dox | 11 +- clients/drcachesim/CMakeLists.txt | 17 +- clients/drcachesim/common/options.cpp | 6 + clients/drcachesim/common/options.h | 1 + clients/drcachesim/launcher.cpp | 24 ++ clients/drcov/CMakeLists.txt | 14 +- clients/drcpusim/CMakeLists.txt | 14 +- core/lib/dr_config.h | 203 +++++++++++++++- core/optionsx.h | 40 +++- core/unix/loader.c | 7 +- core/win32/loader.c | 3 + libutil/dr_config.c | 151 +++++++++--- suite/tests/CMakeLists.txt | 35 ++- .../client-interface/large_options.dll.c | 11 +- .../client-interface/large_options.expect | 6 +- suite/tests/libutil/drconfig_test.c | 220 ++++++++++++++++++ suite/tests/libutil/drconfig_test.template | 1 + suite/tests/linux/execve-client.expect | 10 + tools/drdeploy.c | 143 +++++++++--- 20 files changed, 841 insertions(+), 105 deletions(-) create mode 100644 suite/tests/libutil/drconfig_test.c create mode 100644 suite/tests/libutil/drconfig_test.template create mode 100644 suite/tests/linux/execve-client.expect diff --git a/api/docs/intro.dox b/api/docs/intro.dox index e42989e5bd6..85d20bb58fc 100644 --- a/api/docs/intro.dox +++ b/api/docs/intro.dox @@ -387,6 +387,13 @@ of time for the child application. To instead only follow children that are configured (via \c drconfig.exe), use the \ref op_children "-no_follow_children" runtime option. +To ensure a client is loaded into a child process of a different +bitwidth (i.e., a 32-bit child created by a 64-bit parent), use the \c +-c32 and \c -c64 options to \c drconfig or \c drrun, with \c -- ending +the first client's options: + +bin32/drrun.exe -c32 samples/bin32/bbsize.dll -- -c64 samples/bin64/bbsize.dll -- notepad + To \ref sec_comm "nudge" all instances of \c notepad.exe running under DynamoRIO with argument "5", use: \code @@ -594,6 +601,13 @@ Client options are not allowed to contain semicolons. Additionally, the client option string combined with the path to the client library cannot contain all three quote characters (', ", `) simultaneously. +To ensure a client is loaded into a child process of a different +bitwidth (i.e., a 32-bit child created by a 64-bit parent), use the \c +-c32 and \c -c64 options to \c drconfig or \c drrun, with \c -- ending +the first client's options: + +bin32/drrun -c32 samples/bin32/bbsize.dll -- -c64 samples/bin64/bbsize.dll -- notepad + \section multi_client Multiple Clients DynamoRIO does support multiple clients. It is each client's @@ -626,19 +640,26 @@ option. To use the option, first create a file in the \p tools subdirectory of the root of the DynamoRIO installation called \p toolname.drrun32 or \p toolname.drrun64, depending on the target architecture. Here, \p toolname is the desired external name of the tool. -This file should contain one of the following lines: +This file should contain one of the following lines, or two if +they are a pair of CLIENT32_* and CLIENT64_* specifiers: \code CLIENT_ABS=/absolute/path/to/client +CLIENT32_ABS=/absolute/path/to/32-bit-client +CLIENT64_ABS=/absolute/path/to/64-bit-client \endcode or \code CLIENT_REL=relative/path/to/client/from/DynamoRIO/root +CLIENT32_REL=relative/path/to/32-bit-client/from/DynamoRIO/root +CLIENT64_REL=relative/path/to/64-bit-client/from/DynamoRIO/root \endcode -This enables \p drrun to locate the tool's client library. +This enables \p drrun to locate the tool's client library. The 32 and +64 specifiers allow pointing at alternate-bitwidth paths for use if +the target application creates a child process of a different bitwidth. The file can also modify the default DynamoRIO runtime options (see \ref sec_options) via \p DR_OP= lines. Each line contains only one option string @@ -698,8 +719,8 @@ FRONTEND_REL=relative/path/to/front-end/from/DynamoRIO/root \endcode This will cause \p drrun to transfer control to the specified front-end -executable, passing any tool arguments (including a client path, if \p -CLIENT_ABS or \p CLIENT_REL appears after the \p FRONTEND_* command) +executable, passing any tool arguments (including client paths, if \p +CLIENT{,32,64}_{ABS,REL} appears after the \p FRONTEND_* command) followed by "--" and the target application command line. The path to the DynamoRIO install base can be included in the front-end diff --git a/api/docs/release.dox b/api/docs/release.dox index c7ba7c23400..04b5c8ec1d2 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -141,7 +141,10 @@ changes: The changes between version \DR_VERSION and 8.0.0 include the following minor compatibility changes: - Nothing yet. + - drconfiglib (and thus drrun and drconfig) now sets only the new client path + options added to support other-bitwidth child processes. This means that a + drconfiglib from this version will not properly configure for a DynamoRIO + core library from a prior version. Further non-compatibility-affecting changes include: @@ -155,6 +158,12 @@ Further non-compatibility-affecting changes include: Not all API routines support the setting of error codes. Please look at their documentation to check if they do. - Added -instr_only_trace option to drcachesim. + - Added other-bitwidth child process support, with the other client library + specified by "-c32" "-c64" to drrun or drdeploy, by dr_register_client_ex() + with #dr_config_client_t.is_alt_bitwidth=true to drconfiglib, and by + CLIENT{32,64}_{ABS,REL} in tool files. + Added dr_get_client_info_ex() and dr_client_iterator_next_ex() to support + querying other-bitwidth client registration. **************************************************
diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt index 6f5cc20ab0a..b8c2a2403ff 100644 --- a/clients/drcachesim/CMakeLists.txt +++ b/clients/drcachesim/CMakeLists.txt @@ -414,6 +414,17 @@ install_target(drraw2trace ${INSTALL_CLIENTS_BIN}) set(INSTALL_DRCACHESIM_CONFIG ${INSTALL_CLIENTS_BASE}) function (write_config_file dst bindir libdir) + # We include the alternate-bitwidth path, though it won't be there for + # a single build dir and such a child will have a fatal error. + if (X64) + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + set(CUR_BIT "64") + set(ALT_BIT "32") + else () + set(CUR_BIT "32") + set(ALT_BIT "64") + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + endif () if (DEBUG) set(debugopt "TOOL_OP=-dr_debug") else () @@ -426,8 +437,10 @@ TOOL_OP=-dr\n\ TOOL_OP_DR_PATH\n\ TOOL_OP_DR_BUNDLE=-dr_ops\n\ TOOL_OP=-tracer\n\ -CLIENT_REL=${libdir}/${LIB_PFX}drmemtrace${LIB_EXT}\n\ -${debugopt}") +CLIENT${CUR_BIT}_REL=${libdir}/${LIB_PFX}drmemtrace${LIB_EXT}\n\ +TOOL_OP=-tracer_alt\n\ +CLIENT${ALT_BIT}_REL=${alt_libdir}/${LIB_PFX}drmemtrace${LIB_EXT}\n\ +${debugopt}\n") endfunction () if (X64) diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 833a4eeabae..c30f0ef211e 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -335,6 +335,12 @@ droption_t op_tracer(DROPTION_SCOPE_FRONTEND, "tracer", "", "Path to the tracer", "The full path to the tracer library."); +droption_t op_tracer_alt(DROPTION_SCOPE_FRONTEND, "tracer_alt", "", + "Path to the alternate-bitwidth tracer", + "The full path to the tracer library for the other " + "bitwidth, for use on child processes with a " + "different bitwidth from their parent."); + droption_t op_tracer_ops( DROPTION_SCOPE_FRONTEND, "tracer_ops", DROPTION_FLAG_SWEEP | DROPTION_FLAG_ACCUMULATE | DROPTION_FLAG_INTERNAL, "", diff --git a/clients/drcachesim/common/options.h b/clients/drcachesim/common/options.h index 38d78b43dec..6c08a1a44af 100644 --- a/clients/drcachesim/common/options.h +++ b/clients/drcachesim/common/options.h @@ -110,6 +110,7 @@ extern droption_t op_dr_root; extern droption_t op_dr_debug; extern droption_t op_dr_ops; extern droption_t op_tracer; +extern droption_t op_tracer_alt; extern droption_t op_tracer_ops; extern droption_t op_skip_refs; extern droption_t op_warmup_refs; diff --git a/clients/drcachesim/launcher.cpp b/clients/drcachesim/launcher.cpp index 93d43f1abce..7d7cf3ee9ba 100644 --- a/clients/drcachesim/launcher.cpp +++ b/clients/drcachesim/launcher.cpp @@ -167,6 +167,26 @@ configure_application(char *app_name, char **app_argv, std::string tracer_ops, tracer_ops.c_str()) != DR_SUCCESS) { FATAL_ERROR("failed to register DynamoRIO client configuration"); } + if (!op_tracer_alt.get_value().empty()) { + dr_config_client_t info; + info.struct_size = sizeof(info); + info.id = CLIENT_ID; + info.priority = 1; + char local_path[MAXIMUM_PATH]; + dr_snprintf(local_path, BUFFER_SIZE_ELEMENTS(local_path), "%s", + op_tracer_alt.get_value().c_str()); + info.path = (char *)local_path; + char local_ops[MAXIMUM_PATH]; + dr_snprintf(local_ops, BUFFER_SIZE_ELEMENTS(local_path), "%s", + op_tracer_ops.get_value().c_str()); + info.options = (char *)local_ops; + info.is_alt_bitwidth = true; + NOTIFY(1, "INFO", "configuring alt-bitwidth client \"%s\"", + op_tracer_alt.get_value().c_str()); + if (dr_register_client_ex(process, pid, false /*local*/, DR_PLATFORM_DEFAULT, + &info) != DR_SUCCESS) + FATAL_ERROR("failed to register DynamoRIO client configuration"); + } return true; } @@ -269,6 +289,10 @@ _tmain(int argc, const TCHAR *targv[]) if (!file_is_readable(op_tracer.get_value().c_str())) { FATAL_ERROR("tracer library %s is unreadable", op_tracer.get_value().c_str()); } + // We deliberately do *not* check -tracer_alt, since we're guessing that + // path is available and it won't be for a single build dir. + // An other-bitwidth child will exit with a fatal error if the lib + // is not there and the user runs such an app. if (!file_is_readable(op_dr_root.get_value().c_str())) { FATAL_ERROR("invalid -dr_root %s", op_dr_root.get_value().c_str()); } diff --git a/clients/drcov/CMakeLists.txt b/clients/drcov/CMakeLists.txt index 89916030bc8..4aa3331b987 100644 --- a/clients/drcov/CMakeLists.txt +++ b/clients/drcov/CMakeLists.txt @@ -125,11 +125,23 @@ else (X64) endif (X64) function (write_config_file dst libdir) + # We include the alternate-bitwidth path, though it won't be there for + # a single build dir and such a child will have a fatal error. + if (X64) + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + set(CUR_BIT "64") + set(ALT_BIT "32") + else () + set(CUR_BIT "32") + set(ALT_BIT "64") + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + endif () file(WRITE ${dst} "# drcov tool config file\n") file(APPEND ${dst} "# DynamoRIO options\n") file(APPEND ${dst} "DR_OP=-nop_initial_bblock\n") file(APPEND ${dst} "# client tool path\n") - file(APPEND ${dst} "CLIENT_REL=${libdir}/${LIB_PFX}drcov${LIB_EXT}\n") + file(APPEND ${dst} "CLIENT${CUR_BIT}_REL=${libdir}/${LIB_PFX}drcov${LIB_EXT}\n") + file(APPEND ${dst} "CLIENT${ALT_BIT}_REL=${alt_libdir}/${LIB_PFX}drcov${LIB_EXT}\n") file(APPEND ${dst} "# client tool options\n") file(APPEND ${dst} "TOOL_OP=\n") endfunction () diff --git a/clients/drcpusim/CMakeLists.txt b/clients/drcpusim/CMakeLists.txt index 3c3a1c9b72e..df8e7afe0a2 100644 --- a/clients/drcpusim/CMakeLists.txt +++ b/clients/drcpusim/CMakeLists.txt @@ -60,8 +60,20 @@ install_target(drcpusim ${INSTALL_CLIENTS_LIB}) set(INSTALL_DRCPUSIM_CONFIG ${INSTALL_CLIENTS_BASE}) function (write_config_file dst bindir libdir) + # We include the alternate-bitwidth path, though it won't be there for + # a single build dir and such a child will have a fatal error. + if (X64) + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + set(CUR_BIT "64") + set(ALT_BIT "32") + else () + set(CUR_BIT "32") + set(ALT_BIT "64") + string(REPLACE "lib64" "lib32" alt_libdir ${libdir}) + endif () file(WRITE ${dst} "# drcpusim tool config file\n") - file(APPEND ${dst} "CLIENT_REL=${libdir}/${LIB_PFX}drcpusim${LIB_EXT}\n") + file(APPEND ${dst} "CLIENT${CUR_BIT}_REL=${libdir}/${LIB_PFX}drcpusim${LIB_EXT}\n") + file(APPEND ${dst} "CLIENT${ALT_BIT}_REL=${alt_libdir}/${LIB_PFX}drcpusim${LIB_EXT}\n") endfunction () if (X64) diff --git a/core/lib/dr_config.h b/core/lib/dr_config.h index 25b8cded4b8..89ebe7e8c5f 100644 --- a/core/lib/dr_config.h +++ b/core/lib/dr_config.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2011-2015 Google, Inc. All rights reserved. + * Copyright (c) 2011-2020 Google, Inc. All rights reserved. * Copyright (c) 2008-2010 VMware, Inc. All rights reserved. * **********************************************************/ @@ -143,6 +143,11 @@ typedef enum { */ DR_CONFIG_DIR_NOT_FOUND, + /** + * A parameter was not set. + */ + DR_CONFIG_INVALID_PARAMETER, + } dr_config_status_t; /** Allow targeting both 32-bit and native 64-bit processes separately. */ @@ -153,6 +158,71 @@ typedef enum { DR_PLATFORM_NONE, /**< Invalid platform. */ } dr_platform_t; +/** + * Information about a client library setup. + */ +typedef struct _dr_config_client_t { + /** + * The size of this structure. This field must be set when passing as an input + * to functions like dr_get_client_info_ex(). Used for backward compatibility. + */ + size_t struct_size; + /** + * This is a client_id_t uniquely identifying the client. DynamoRIO provides the + * client ID as a parameter to dr_client_main(). Clients use this ID to retrieve + * client-specific path and option information. Outside entities also identify + * the target of a nudge via this ID. + */ + client_id_t id; + /** + * The client number, or priority. Client registration includes a value + * indicating the priority of a client relative to other clients. In + * multi-client settings, a client's priority influences event callback + * ordering.That is, higher priority clients can register their callbacks first; + * DynamoRIO then calls these routines last. Client priorities range + * consecutively from 0 to N-1, where N is the number of registered clients. + * Specify priority 0 to register a client with highest priority. + */ + size_t priority; + /** + * A NULL-terminated string specifying the full path to a valid client library + * whose bitwidth is the opposite of that specified by dr_platform. The string + * length cannot exceed #MAXIMUM_PATH. The client path may not include any semicolons + * and when combined with \p client_options may not include all three quote + * characters (', ", `) simultaneously. + * + * When querying via dr_get_client_info_ex() or dr_client_iterator_next_ex(), the + * caller must either set this to NULL if that data is not needed or point at a + * caller-allocated array of length #MAXIMUM_PATH. + */ + char *path; + /** + * A NULL-terminated string specifying options that are available to the client + * as arguments of dr_client_main() or via dr_get_option_array(). The string + * length cannot exceed #DR_MAX_OPTIONS_LENGTH. The client options may not + * include any semicolons and when combined with \p client_path may not include + * all three quote characters (', ", `) simultaneously. + * + * When querying via dr_get_client_info_ex() or dr_client_iterator_next_ex(), the + * caller must either set this to NULL if that data is not needed or point at a + * caller-allocated array of length #DR_MAX_OPTIONS_LENGTH. + */ + char *options; + /** + * Specifies whether this is a regular client registeration for the target + * process (this field is false) or whether this is an "alternate bitwidth" + * registration which specifies the configuration to use if the target process + * creates a child process that is a different bitwidth from itself. The regular + * client must first be registered on its own before the alternate client is + * registered. If the target process creates a child application of a different + * bitwdith (e.g., the target process is 64-bit and it creates a 32-bit child), + * this registration allows a client of the appropriate bitwidth to be loaded + * into the child. Unregistering a client with dr_unregister_client() will also + * unregister the alternate bitwidth client. + */ + bool is_alt_bitwidth; +} dr_config_client_t; + /* Note that we provide this as an inlined function for use by * dr_nudge_client and dr_nudge_client_ex without requiring drconfiglib * to be linked into a client. @@ -232,7 +302,7 @@ DR_EXPORT * * \param[in] dr_root_dir A NULL-terminated string specifying the full * path to a valid DynamoRIO root directory. - * The string length cannot exceed MAX_PATH. + * The string length cannot exceed #MAXIMUM_PATH. * * \param[in] dr_mode Specifies the mode under which DynamoRIO should * operate. See dr_operation_mode_t. @@ -418,7 +488,7 @@ DR_EXPORT * directory provided at registration. Callers can * pass NULL if this value is not needed. Otherwise, * the parameter must be a caller-allocated array of - * length MAX_PATH. + * length #MAXIMUM_PATH. * * \param[out] dr_mode If the process is registered, the mode provided * at registration. Callers can pass NULL if this @@ -497,12 +567,12 @@ DR_EXPORT * \param[out] process_name The name of the registered process. Callers can * pass NULL if this value is not needed. Otherwise * the parameter must be a caller-allocated array - * of length MAX_PATH. + * of length #MAXIMUM_PATH. * * \param[out] dr_root_dir The root DynamoRIO directory provided at registration. * Callers can pass NULL if this value is not needed. * Otherwise, the parameter must be a caller-allocated - * array of length MAX_PATH. + * array of length #MAXIMUM_PATH. * * \param[out] dr_mode If the process is registered, the mode provided * at registration. Callers can pass NULL if this @@ -572,7 +642,7 @@ DR_EXPORT * \param[in] dr_platform Configurations are kept separate * for 32-bit processes and 64-bit processes. * This parameter allows selecting which of those - * configurations to unset. + * configurations to set. * * \param[in] client_id A client_id_t uniquely identifying the client. * DynamoRIO provides the client ID as a parameter @@ -594,7 +664,7 @@ DR_EXPORT * * \param[in] client_path A NULL-terminated string specifying the full path * to a valid client library. The string length - * cannot exceed MAX_PATH. The client path may not + * cannot exceed #MAXIMUM_PATH. The client path may not * include any semicolons and when combined with * \p client_options may not include all * three quote characters (', ", `) simultaneously. @@ -614,6 +684,51 @@ dr_register_client(const char *process_name, process_id_t pid, bool global, dr_platform_t dr_platform, client_id_t client_id, size_t client_pri, const char *client_path, const char *client_options); +DR_EXPORT +/** + * Register a client for a particular process. Note that the process must first + * be registered via dr_register_process() before calling this routine. + * The #dr_config_client_t structure allows specifying additional options beyond + * what dr_register_client() supports, such as an alternate bitwidth client. + * For an alternate bitwidth client, the main client must first be + * registered by an earlier call. Unregistering a client with + * dr_unregister_client() will also unregister the alternate bitwidth + * client. + * + * \param[in] process_name A NULL-terminated string specifying the name + * of the target process. The string should + * identify the base name of the process, not the + * full path of the executable (e.g., calc.exe). + * + * \param[in] pid A process id of a target process, typically just + * created and suspended via dr_inject_process_exit(). + * If pid != 0, the one-time configuration for that pid + * will be modified. If pid == 0, the general + * configuration for process_name will be modified. + * + * \param[in] global Whether to use global or user-local config + * files. On Windows, global config files are + * stored in a dir pointed at by the DYNAMORIO_HOME + * registry key. On Linux, they are in + * /etc/dynamorio. Administrative privileges may + * be needed if global is true. Note that + * DynamoRIO gives local config files precedence + * when both exist. The caller must separately + * create the global directory. + * + * \param[in] dr_platform Configurations are kept separate + * for 32-bit processes and 64-bit processes. + * This parameter allows selecting which of those + * configurations to set. + * + * \param[in] client Defines the attributes of the client to be registered. + * + * \return A dr_config_status_t code indicating the result of registration. + */ +dr_config_status_t +dr_register_client_ex(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, IN dr_config_client_t *client); + DR_EXPORT /** * Unregister a client for a particular process. @@ -685,6 +800,7 @@ DR_EXPORT * configurations to unset. * * \return The number of clients registered for the given process and platform. + * An alternative-bitwidth client is counted as a separate client. */ size_t dr_num_registered_clients(const char *process_name, process_id_t pid, bool global, @@ -692,8 +808,9 @@ dr_num_registered_clients(const char *process_name, process_id_t pid, bool globa DR_EXPORT /** - * Retrieve client registration information for a particular process for - * the current user. + * Retrieve client registration information for a particular process for the current + * user. If multiple clients are registered (alternative-bitwidth clients are + * considered separate), information on the highest-priority client is returned. * * \param[in] process_name A NULL-terminated string specifying the name * of the target process. The string should @@ -729,7 +846,7 @@ DR_EXPORT * \param[out] client_path The client's path provided at registration. * Callers can pass NULL if this value is not needed. * Otherwise, the parameter must be a caller-allocated - * array of length MAX_PATH. + * array of length #MAXIMUM_PATH. * * \param[out] client_options The client options provided at registration. * Callers can pass NULL if this value is not needed. @@ -745,6 +862,53 @@ dr_get_client_info(const char *process_name, process_id_t pid, bool global, char *client_path, /* OUT */ char *client_options /* OUT */); +DR_EXPORT +/** + * Retrieve client registration information for a particular process for the current + * user. If multiple clients are registered (alternative-bitwidth clients are + * considered separate), information on the highest-priority client is returned. + * + * \param[in] process_name A NULL-terminated string specifying the name + * of the target process. The string should + * identify the base name of the process, not the + * full path of the executable (e.g., calc.exe). + * + * \param[in] pid A process id of a target process, typically just + * created and suspended via dr_inject_process_exit(). + * If pid != 0, the one-time configuration for that pid + * will be queried. If pid == 0, the general + * configuration for process_name will be queried. + * + * \param[in] global Whether to use global or user-local config + * files. On Windows, global config files are + * stored in a dir pointed at by the DYNAMORIO_HOME + * registry key. On Linux, they are in + * /etc/dynamorio. Administrative privileges may + * be needed if global is true. Note that + * DynamoRIO gives local config files precedence + * when both exist. The caller must separately + * create the global directory. + * + * \param[in] dr_platform Configurations are kept separate + * for 32-bit processes and 64-bit processes. + * This parameter allows selecting which of those + * configurations to unset. + * + * \param[out] client The returned information about the client. On input, + * the struct_size field must be set and the client_id + * field set to the unique client ID provided at client + * registration to be queried. Furthermore, the client_path + * and client_options fields must each either be NULL + * if that data is not needed or point at a caller-allocated + * array of length #MAXIMUM_PATH for client_path or + * #DR_MAX_OPTIONS_LENGTH for client_options. + * + * \return A dr_config_status_t code indicating the result of the call. + */ +dr_config_status_t +dr_get_client_info_ex(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, dr_config_client_t *client /* IN/OUT */); + typedef struct _dr_client_iterator_t dr_client_iterator_t; DR_EXPORT @@ -808,7 +972,7 @@ DR_EXPORT * \param[out] client_path The client's path provided at registration. * Callers can pass NULL if this value is not needed. * Otherwise, the parameter must be a caller-allocated - * array of length MAX_PATH. + * array of length #MAXIMUM_PATH. * * \param[out] client_options The client options provided at registration. * Callers can pass NULL if this value is not needed. @@ -821,6 +985,23 @@ dr_client_iterator_next(dr_client_iterator_t *iter, client_id_t *client_id, /* O char *client_path, /* OUT */ char *client_options /* OUT */); +DR_EXPORT +/** + * Return information about a client. + * + * \param[in] iter A client iterator created with dr_client_iterator_start() + * + * \param[out] client The returned information about the client. On input, + * the struct_size field must be set and the client_path + * and client_options fields must each either be NULL + * if that data is not needed or point at a caller-allocated + * array of length #MAXIMUM_PATH for client_path or + * #DR_MAX_OPTIONS_LENGTH for client_options. + */ +dr_config_status_t +dr_client_iterator_next_ex(dr_client_iterator_t *iter, dr_config_client_t *client + /* IN/OUT */); + DR_EXPORT /** * Stops and frees a client iterator. diff --git a/core/optionsx.h b/core/optionsx.h index 53b1e615b61..44b81b9815b 100644 --- a/core/optionsx.h +++ b/core/optionsx.h @@ -343,14 +343,48 @@ OPTION_NAME_INTERNAL(bool, profile_times, "prof_times", "profiling via measuring # ifdef CLIENT_INTERFACE /* FIXME (xref PR 215082): make these external now that our product is our API? */ -/* XXX: note that this does affect pcaches, but we don't want the - * client option strings to matter, so we check this separately - * from the general -persist_check_options +/* XXX: These -client_lib* options do affect pcaches, but we don't want the + * client option strings to matter, so we check them separately + * from the general -persist_check_options. */ /* This option is ignored for STATIC_LIBRARY. */ +/* XXX: We could save space by removing -client_lib and using only the {32,64} + * versions (or by having some kind of true alias where _lib points to the other). + */ OPTION_DEFAULT_INTERNAL(liststring_t, client_lib, EMPTY_STRING, ";-separated string containing client " "lib paths, IDs, and options") +# ifdef X64 +OPTION_COMMAND_INTERNAL(liststring_t, client_lib64, EMPTY_STRING, "client_lib64", + { + snprintf(options->client_lib, + BUFFER_SIZE_ELEMENTS(options->client_lib), "%s", + options->client_lib64); + NULL_TERMINATE_BUFFER(options->client_lib); + }, + ";-separated string containing client " + "lib paths, IDs, and options", + STATIC, OP_PCACHE_NOP /* See note about pcache above. */) +# define ALT_CLIENT_LIB_NAME client_lib32 +# else +OPTION_COMMAND_INTERNAL(liststring_t, client_lib32, EMPTY_STRING, "client_lib32", + { + snprintf(options->client_lib, + BUFFER_SIZE_ELEMENTS(options->client_lib), "%s", + options->client_lib32); + NULL_TERMINATE_BUFFER(options->client_lib); + }, + ";-separated string containing client " + "lib paths, IDs, and options", + STATIC, OP_PCACHE_NOP /* See note about pcache above. */) +# define ALT_CLIENT_LIB_NAME client_lib64 +# endif +OPTION_COMMAND_INTERNAL(liststring_t, ALT_CLIENT_LIB_NAME, EMPTY_STRING, + IF_X64_ELSE("client_lib32", "client_lib64"), {}, + ";-separated string containing client " + "lib paths, IDs, and options for other-bitwidth children", + STATIC, OP_PCACHE_GLOBAL) + /* If we revive hotpatching should use this there as well: but for now * don't want to mess up any legacy tools that rely on hotp libs in * regular loader list diff --git a/core/unix/loader.c b/core/unix/loader.c index c7426744394..83c35306f55 100644 --- a/core/unix/loader.c +++ b/core/unix/loader.c @@ -516,7 +516,12 @@ privload_map_and_relocate(const char *filename, size_t *size OUT, modload_flags_ ELF_ALTARCH_HEADER_TYPE *altarch = (ELF_ALTARCH_HEADER_TYPE *)elf_header; if (!TEST(MODLOAD_NOT_PRIVLIB, flags) && elf_header->e_version == 1 && altarch->e_ehsize == sizeof(ELF_ALTARCH_HEADER_TYPE) && - altarch->e_machine == IF_X64_ELSE(EM_386, EM_X86_64)) { + altarch->e_machine == + IF_X64_ELSE(IF_AARCHXX_ELSE(EM_ARM, EM_386), + IF_AARCHXX_ELSE(EM_AARCH64, EM_X86_64))) { + /* XXX i#147: Should we try some path substs like s/lib32/lib64/? + * Maybe it's better to error out to avoid loading some unintended lib. + */ SYSLOG(SYSLOG_ERROR, CLIENT_LIBRARY_WRONG_BITWIDTH, 3, get_application_name(), get_application_pid(), filename); } diff --git a/core/win32/loader.c b/core/win32/loader.c index b63298d2d71..537b1ef5475 100644 --- a/core/win32/loader.c +++ b/core/win32/loader.c @@ -1057,6 +1057,9 @@ privload_map_and_relocate(const char *filename, size_t *size OUT, modload_flags_ /* XXX i#828: we may eventually support mixed-mode clients. * Xref dr_load_aux_x64_library() and load_library_64(). */ + /* XXX i#147: Should we try some path substs like s/lib32/lib64/? + * Maybe it's better to error out to avoid loading some unintended lib. + */ SYSLOG(SYSLOG_ERROR, CLIENT_LIBRARY_WRONG_BITWIDTH, 3, get_application_name(), get_application_pid(), filename); return NULL; diff --git a/libutil/dr_config.c b/libutil/dr_config.c index 25299a441a0..34b4a4f382b 100644 --- a/libutil/dr_config.c +++ b/libutil/dr_config.c @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2011-2016 Google, Inc. All rights reserved. + * Copyright (c) 2011-2020 Google, Inc. All rights reserved. * Copyright (c) 2008-2010 VMware, Inc. All rights reserved. * **********************************************************/ @@ -99,6 +99,7 @@ typedef struct _client_opt_t { TCHAR *path; client_id_t id; TCHAR *opts; + bool alt_bitwidth; } client_opt_t; typedef struct _opt_info_t { @@ -174,7 +175,7 @@ get_next_token(TCHAR *ptr, TCHAR *token) /* Allocate a new client_opt_t */ static client_opt_t * -new_client_opt(const TCHAR *path, client_id_t id, const TCHAR *opts) +new_client_opt(const TCHAR *path, client_id_t id, const TCHAR *opts, bool alt_bitwidth) { size_t len; client_opt_t *opt = (client_opt_t *)malloc(sizeof(client_opt_t)); @@ -183,6 +184,7 @@ new_client_opt(const TCHAR *path, client_id_t id, const TCHAR *opts) } opt->id = id; + opt->alt_bitwidth = alt_bitwidth; len = MIN(MAXIMUM_PATH - 1, _tcslen(path)); opt->path = malloc((len + 1) * sizeof(opt->path[0])); @@ -216,7 +218,7 @@ free_client_opt(client_opt_t *opt) /* Add another client to an opt_info_t struct */ static dr_config_status_t add_client_lib(opt_info_t *opt_info, client_id_t id, size_t pri, const TCHAR *path, - const TCHAR *opts) + const TCHAR *opts, bool alt_bitwidth) { size_t i; @@ -233,7 +235,7 @@ add_client_lib(opt_info_t *opt_info, client_id_t id, size_t pri, const TCHAR *pa opt_info->client_opts[i] = opt_info->client_opts[i - 1]; } - opt_info->client_opts[pri] = new_client_opt(path, id, opts); + opt_info->client_opts[pri] = new_client_opt(path, id, opts, alt_bitwidth); opt_info->num_clients++; return DR_SUCCESS; @@ -243,6 +245,7 @@ static dr_config_status_t remove_client_lib(opt_info_t *opt_info, client_id_t id) { size_t i, j; + dr_config_status_t status = DR_ID_INVALID; for (i = 0; i < opt_info->num_clients; i++) { if (opt_info->client_opts[i]->id == id) { free_client_opt(opt_info->client_opts[i]); @@ -253,11 +256,13 @@ remove_client_lib(opt_info_t *opt_info, client_id_t id) } opt_info->num_clients--; - return DR_SUCCESS; + status = DR_SUCCESS; + /* Keep searching to also remove any alt-bitwidth for the same id. */ + i--; } } - return DR_ID_INVALID; + return status; } /* Add an 'extra' option (non-client related option) to an opt_info_t struct */ @@ -799,7 +804,12 @@ read_options(opt_info_t *opt_info, IF_REG_ELSE(ConfigGroup *proc_policy, FILE *f /* * look for client options */ - else if (_tcscmp(token, _TEXT("-client_lib")) == 0) { + else if (_tcscmp(token, _TEXT("-client_lib")) == 0 || + _tcscmp(token, _TEXT("-client_lib32")) == 0 || + _tcscmp(token, _TEXT("-client_lib64")) == 0) { + bool alt_bitwidth = + _tcscmp(token, + IF_X64_ELSE(_TEXT("-client_lib32"), _TEXT("-client_lib64"))) == 0; TCHAR *path_str, *id_str, *opt_str; client_id_t id; @@ -846,8 +856,8 @@ read_options(opt_info_t *opt_info, IF_REG_ELSE(ConfigGroup *proc_policy, FILE *f id = _tcstoul(id_str, NULL, 16); /* add the client info to our opt_info structure */ - if (add_client_lib(opt_info, id, opt_info->num_clients, path_str, opt_str) != - DR_SUCCESS) { + if (add_client_lib(opt_info, id, opt_info->num_clients, path_str, opt_str, + alt_bitwidth) != DR_SUCCESS) { goto error; } } @@ -955,9 +965,13 @@ write_options(opt_info_t *opt_info, TCHAR *wbuf) /* no ; allowed */ if (strchr(client_opts->path, ';') || strchr(client_opts->opts, ';')) return DR_CONFIG_OPTIONS_INVALID; - len = _sntprintf(wbuf + sofar, bufsz - sofar, _TEXT(" -client_lib %c%s;%x;%s%c"), - delim, client_opts->path, client_opts->id, client_opts->opts, - delim); + len = _sntprintf( + wbuf + sofar, bufsz - sofar, + client_opts->alt_bitwidth ? IF_X64_ELSE(_TEXT(" -client_lib32 %c%s;%x;%s%c"), + _TEXT(" -client_lib64 %c%s;%x;%s%c")) + : IF_X64_ELSE(_TEXT(" -client_lib64 %c%s;%x;%s%c"), + _TEXT(" -client_lib32 %c%s;%x;%s%c")), + delim, client_opts->path, client_opts->id, client_opts->opts, delim); if (len >= 0 && len <= bufsz - sofar) sofar += len; } @@ -1214,6 +1228,8 @@ dr_unregister_process(const char *process_name, process_id_t pid, bool global, TCHAR wbuf[MAXIMUM_PATH]; convert_to_tchar(wbuf, fname, BUFFER_SIZE_ELEMENTS(wbuf)); NULL_TERMINATE_BUFFER(wbuf); + if (!file_exists(wbuf)) + return DR_PROC_REG_INVALID; if (_wremove(wbuf) == 0) return DR_SUCCESS; } @@ -1548,7 +1564,7 @@ dr_client_iterator_next(dr_client_iterator_t *iter, client_id_t *client_id, /* O *client_pri = iter->cur; if (client_path != NULL) { - size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path)); + size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); _snprintf(client_path, len, TSTR_FMT, client_opt->path); client_path[len] = '\0'; } @@ -1557,7 +1573,7 @@ dr_client_iterator_next(dr_client_iterator_t *iter, client_id_t *client_id, /* O *client_id = client_opt->id; if (client_options != NULL) { - size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts)); + size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); _snprintf(client_options, len, TSTR_FMT, client_opt->opts); client_options[len] = '\0'; } @@ -1565,6 +1581,29 @@ dr_client_iterator_next(dr_client_iterator_t *iter, client_id_t *client_id, /* O iter->cur++; } +dr_config_status_t +dr_client_iterator_next_ex(dr_client_iterator_t *iter, + dr_config_client_t *client /* IN/OUT */) +{ + client_opt_t *client_opt = iter->opt_info.client_opts[iter->cur]; + if (client == NULL || client->struct_size != sizeof(dr_config_client_t)) + return DR_CONFIG_INVALID_PARAMETER; + client->id = client_opt->id; + client->priority = iter->cur; + if (client->path != NULL) { + size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); + _snprintf(client->path, len, TSTR_FMT, client_opt->path); + client->path[len] = '\0'; + } + if (client->options != NULL) { + size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); + _snprintf(client->options, len, TSTR_FMT, client_opt->opts); + client->options[len] = '\0'; + } + iter->cur++; + return DR_SUCCESS; +} + void dr_client_iterator_stop(dr_client_iterator_t *iter) { @@ -1606,10 +1645,11 @@ dr_num_registered_clients(const char *process_name, process_id_t pid, bool globa } dr_config_status_t -dr_get_client_info(const char *process_name, process_id_t pid, bool global, - dr_platform_t dr_platform, client_id_t client_id, size_t *client_pri, - char *client_path, char *client_options) +dr_get_client_info_ex(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, dr_config_client_t *client) { + if (client == NULL || client->struct_size != sizeof(*client)) + return DR_CONFIG_INVALID_PARAMETER; opt_info_t opt_info; dr_config_status_t status; size_t i; @@ -1630,23 +1670,21 @@ dr_get_client_info(const char *process_name, process_id_t pid, bool global, goto exit; for (i = 0; i < opt_info.num_clients; i++) { - if (opt_info.client_opts[i]->id == client_id) { + if (opt_info.client_opts[i]->id == client->id) { client_opt_t *client_opt = opt_info.client_opts[i]; - if (client_pri != NULL) { - *client_pri = i; + client->priority = i; + if (client->path != NULL) { + size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); + _snprintf(client->path, len, TSTR_FMT, client_opt->path); + client->path[len] = '\0'; } - if (client_path != NULL) { - size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path)); - _snprintf(client_path, len, TSTR_FMT, client_opt->path); - client_path[len] = '\0'; - } - - if (client_options != NULL) { - size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts)); - _snprintf(client_options, len, TSTR_FMT, client_opt->opts); - client_options[len] = '\0'; + if (client->options != NULL) { + size_t len = + MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); + _snprintf(client->options, len, TSTR_FMT, client_opt->opts); + client->options[len] = '\0'; } status = DR_SUCCESS; @@ -1667,10 +1705,29 @@ dr_get_client_info(const char *process_name, process_id_t pid, bool global, } dr_config_status_t -dr_register_client(const char *process_name, process_id_t pid, bool global, - dr_platform_t dr_platform, client_id_t client_id, size_t client_pri, - const char *client_path, const char *client_options) +dr_get_client_info(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, client_id_t client_id, size_t *client_pri, + char *client_path, char *client_options) +{ + dr_config_client_t client; + client.struct_size = sizeof(client); + client.id = client_id; + client.path = client_path; + client.options = client_options; + dr_config_status_t status = + dr_get_client_info_ex(process_name, pid, global, dr_platform, &client); + if (status == DR_SUCCESS && client_pri != NULL) + *client_pri = client.priority; + return status; +} + +dr_config_status_t +dr_register_client_ex(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, dr_config_client_t *client) { + if (client == NULL || client->struct_size != sizeof(*client)) + return DR_CONFIG_INVALID_PARAMETER; + TCHAR new_opts[DR_MAX_OPTIONS_LENGTH]; TCHAR wpath[MAXIMUM_PATH], woptions[DR_MAX_OPTIONS_LENGTH]; #ifdef PARAMS_IN_REGISTRY @@ -1697,23 +1754,26 @@ dr_register_client(const char *process_name, process_id_t pid, bool global, opt_info_alloc = true; for (i = 0; i < opt_info.num_clients; i++) { - if (opt_info.client_opts[i]->id == client_id) { + if (opt_info.client_opts[i]->id == client->id && + ((opt_info.client_opts[i]->alt_bitwidth && client->is_alt_bitwidth) || + (!opt_info.client_opts[i]->alt_bitwidth && !client->is_alt_bitwidth))) { status = DR_ID_CONFLICTING; goto exit; } } - if (client_pri > opt_info.num_clients) { + if (client->priority > opt_info.num_clients) { status = DR_PRIORITY_INVALID; goto exit; } - convert_to_tchar(wpath, client_path, MAXIMUM_PATH); + convert_to_tchar(wpath, client->path, MAXIMUM_PATH); NULL_TERMINATE_BUFFER(wpath); - convert_to_tchar(woptions, client_options, DR_MAX_OPTIONS_LENGTH); + convert_to_tchar(woptions, client->options, DR_MAX_OPTIONS_LENGTH); NULL_TERMINATE_BUFFER(woptions); - status = add_client_lib(&opt_info, client_id, client_pri, wpath, woptions); + status = add_client_lib(&opt_info, client->id, client->priority, wpath, woptions, + client->is_alt_bitwidth); if (status != DR_SUCCESS) { goto exit; } @@ -1752,6 +1812,21 @@ dr_register_client(const char *process_name, process_id_t pid, bool global, return status; } +dr_config_status_t +dr_register_client(const char *process_name, process_id_t pid, bool global, + dr_platform_t dr_platform, client_id_t client_id, size_t client_pri, + const char *client_path, const char *client_options) +{ + dr_config_client_t client; + client.struct_size = sizeof(client); + client.id = client_id; + client.priority = client_pri; + client.path = (char *)client_path; + client.options = (char *)client_options; + client.is_alt_bitwidth = false; + return dr_register_client_ex(process_name, pid, global, dr_platform, &client); +} + dr_config_status_t dr_unregister_client(const char *process_name, process_id_t pid, bool global, dr_platform_t dr_platform, client_id_t client_id) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index b2e7c21e92d..9be4b9f0cb5 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -1816,6 +1816,11 @@ if (CLIENT_INTERFACE AND NOT ANDROID) # XXX i#1874: get working on Android endif () if (CLIENT_INTERFACE) + set(dr_root ${PROJECT_BINARY_DIR}) + # Just like frontend_test we request drdecode. + tobuild_api(libutil.drconfig_test libutil/drconfig_test.c "" "${dr_root}" ON OFF) + target_link_libraries(libutil.drconfig_test drconfiglib) + set(BUILD_TEST_ANNOTATION "${PROJECT_BINARY_DIR}/suite/tests/annotations") configure_file( "${CMAKE_CURRENT_SOURCE_DIR}/client-interface/annotation/test_mode_annotations.h" @@ -3459,7 +3464,11 @@ if (UNIX) # Cross-arch execve test: can only be done via a suite of tests that # build both 32-bit and 64-bit. We have 32-bit just build, and # 64-bit then builds and runs both directions. - if (X86) + # + # i#2971: We test -c32/-c64 here since modern DR focuses on clients, and hence we + # have a CLIENT_INTERFACE condition. TODO i#2971: Remove CLIENT_INTERFACE and + # eliminate all of its conditionals. + if (X86 AND CLIENT_INTERFACE) if (X64) add_exe(linux.execve-sub64 linux/execve-sub.c) if (TEST_SUITE AND TEST_32BIT_PATH) @@ -3479,6 +3488,11 @@ if (UNIX) make_symlink(${32dir}/${INSTALL_LIB_X86} ${PROJECT_BINARY_DIR}/${INSTALL_LIB_X86}) get_target_path_for_execution(drrun_path drrun "${location_suffix}") + # We use a client that takes some options and prints something at + # init and at exit to test cross-arch injection with clients. + get_target_path_for_execution(client64_path client.large_options.dll + "${location_suffix}") + set(client32_path ${32dir}/${test_bindir}/libclient.large_options.dll.so) if (DEBUG) set(config_type -DDEBUG=ON) else () @@ -3490,14 +3504,19 @@ if (UNIX) --build-target dynamorio --build-target linux.execve32 --build-target linux.execve-sub32 + --build-target client.large_options.dll --build-noclean # else it tries to clean between each target! --build-makeprogram ${CMAKE_MAKE_PROGRAM} --build-options ${config_type} -DBUILD_TESTS=ON -DBUILD_DOCS=OFF -DBUILD_SAMPLES=OFF -DBUILD_EXT=OFF -DBUILD_CLIENTS=OFF --test-command ${drrun_path} -32 - -dr_home ${32dir} ${dr_test_ops} -- ${32dir}/${test_bindir}/linux.execve32 + -dr_home ${32dir} ${dr_test_ops} + # We test the "-c32 -c64" syntax here. + -c32 ${client32_path} -paramA foo -paramB bar -- + -c64 ${client64_path} -paramA foo -paramB bar -- + ${32dir}/${test_bindir}/linux.execve32 "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/linux.execve-sub64") - file(READ linux/execve.expect expect) + file(READ linux/execve-client.expect expect) set_tests_properties(linux.execve32 PROPERTIES ENVIRONMENT "CFLAGS=-m32;CXXFLAGS=-m32" # We do not try to indirect this so we have to read .expect manually. @@ -3507,7 +3526,10 @@ if (UNIX) # We can't get the depends property right b/c execve32 has no prefix, # so we do this one directly too. add_exe(linux.execve64 linux/execve.c) - add_test(linux.execve64 ${drrun_path} ${dr_test_ops} -- + add_test(linux.execve64 ${drrun_path} ${dr_test_ops} + # We test the "-c32 -c64" syntax for the other direction. + -c32 ${client32_path} -paramA foo -paramB bar -- + -c64 ${client64_path} -paramA foo -paramB bar -- ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/linux.execve64 "${32dir}/${test_bindir}/linux.execve-sub32") set_tests_properties(linux.execve64 PROPERTIES DEPENDS linux.execve32 @@ -3517,7 +3539,7 @@ if (UNIX) add_exe(linux.execve-sub32 linux/execve-sub.c) add_exe(linux.execve32 linux/execve.c) endif (X64) - endif (X86) + endif () # helper app for execve-null add_exe(linux.execve-sub linux/execve-sub.c) @@ -3986,6 +4008,9 @@ else (UNIX) # everywhere (xref i#381, i#234, i#803). # # XXX i#829: mixed-mode stubs and abs far jmps require -heap_in_lower_4GB + # + # TODO i#803: Once cross-arch injection works on Windows, add a test of + # the "-c32 -c64" syntax like we have for Linux. if (TEST_SUITE OR TEST_32BIT_PATH OR NOT X64) if (X64) # XXX i#1636: the earliest injection tests below seem to have the child diff --git a/suite/tests/client-interface/large_options.dll.c b/suite/tests/client-interface/large_options.dll.c index 3277f041812..f0a92157fbb 100644 --- a/suite/tests/client-interface/large_options.dll.c +++ b/suite/tests/client-interface/large_options.dll.c @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2011-2015 Google, Inc. All rights reserved. + * Copyright (c) 2011-2020 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -41,9 +41,16 @@ #include "dr_api.h" +static void +event_exit(void) +{ + dr_fprintf(STDERR, "large_options exiting\n"); +} + DR_EXPORT void dr_init(client_id_t client_id) { const char *opts = dr_get_options(client_id); - dr_fprintf(STDERR, "client opts: %s\n", opts); + dr_fprintf(STDERR, "large_options passed: %s\n", opts); + dr_register_exit_event(event_exit); } diff --git a/suite/tests/client-interface/large_options.expect b/suite/tests/client-interface/large_options.expect index ddccc54a9bb..6151ee57178 100644 --- a/suite/tests/client-interface/large_options.expect +++ b/suite/tests/client-interface/large_options.expect @@ -1,5 +1,7 @@ -client opts: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +large_options passed: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx parent -client opts: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +large_options passed: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx child +large_options exiting parent exiting +large_options exiting diff --git a/suite/tests/libutil/drconfig_test.c b/suite/tests/libutil/drconfig_test.c new file mode 100644 index 00000000000..afce87bff8c --- /dev/null +++ b/suite/tests/libutil/drconfig_test.c @@ -0,0 +1,220 @@ +/* ********************************************************** + * 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 VMWARE, 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. + */ + +#include +#include +#include "dr_api.h" +#include "dr_config.h" + +static void +check(bool condition, const char *error_msg) +{ + if (!condition) { + fprintf(stderr, "ERROR: %s\n", error_msg); + exit(1); + } +} + +static void +test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_platform, + const char *dr_root) +{ + dr_config_status_t status; + /* Unregister first, in case a stale file from an old aborted test is still there. */ + status = dr_unregister_process(name, pid, global, dr_platform); + check(status == DR_SUCCESS || status == DR_PROC_REG_INVALID, + "dr_unregister_process should succeed or not find"); + + status = dr_register_client(name, pid, global, dr_platform, 0, 0, "/any/path", + "-any -ops"); + check(status == DR_PROC_REG_INVALID, + "dr_register_client w/o first dr_register_process should fail"); + + check( + !dr_process_is_registered(name, pid, global, dr_platform, NULL, NULL, NULL, NULL), + "should not be registered\n"); + + status = dr_register_process(name, pid, global, dr_root, DR_MODE_CODE_MANIPULATION, + false, dr_platform, "-disable_traces"); + check(status == DR_SUCCESS, "dr_register_process should succeed"); + check( + dr_process_is_registered(name, pid, global, dr_platform, NULL, NULL, NULL, NULL), + "should be registered\n"); + + status = dr_register_process(name, pid, global, dr_root, DR_MODE_CODE_MANIPULATION, + false, dr_platform, "-disable_traces"); + if (pid == 0) + check(status != DR_SUCCESS, "dr_register_process duplicate 0-pid should fail"); + else { + check(status == DR_SUCCESS, + "dr_register_process duplicate non-0-pid should succeed"); + } + + client_id_t my_id = 19; + size_t my_priority = 0; + const char *my_path = "/path/to/libclient.so"; + const char *my_alt_path = "/path/to/libclient-alt.so"; + const char *my_ops = "-foo -bar"; + check(dr_num_registered_clients(name, pid, global, dr_platform) == 0, + "should be 0 clients pre-reg"); + + status = dr_register_client(name, pid, global, dr_platform, my_id, my_priority, + my_path, my_ops); + check(status == DR_SUCCESS, "dr_register_client should succeed"); + check(dr_num_registered_clients(name, pid, global, dr_platform) == 1, + "should be 1 client post-reg"); + + size_t client_pri; + char client_path[MAXIMUM_PATH]; + char client_ops[MAXIMUM_PATH]; + status = dr_get_client_info(name, pid, global, dr_platform, my_id, &client_pri, + client_path, client_ops); + check(status == DR_SUCCESS, "dr_get_client_info should succeed"); + check(client_pri == my_priority, "priority query doesn't match"); + check(strcmp(client_path, my_path) == 0, "path doesn't match"); + check(strcmp(client_ops, my_ops) == 0, "options don't match"); + + status = dr_unregister_client(name, pid, global, dr_platform, my_id); + check(status == DR_SUCCESS, "dr_unregister_client should succeed"); + + dr_config_client_t client; + client.struct_size = 0; + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_CONFIG_INVALID_PARAMETER, "bad struct_size should fail"); + client.struct_size = sizeof(client); + client.id = my_id; + client.priority = my_priority; + client.path = (char *)my_path; + client.options = (char *)my_ops; + client.is_alt_bitwidth = false; + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_SUCCESS, "dr_register_client_ex should succeed"); + check(dr_num_registered_clients(name, pid, global, dr_platform) == 1, + "should be 1 client post-reg"); + + client.path = (char *)my_alt_path; + client.options = (char *)my_ops; + client.is_alt_bitwidth = true; + client.priority = my_priority + 1; /* Append. */ + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_SUCCESS, "dr_register_client_ex alt should succeed"); + check(dr_num_registered_clients(name, pid, global, dr_platform) == 2, + "should be 2 clients post-alt-reg"); + + /* Plain query should still find the non-alt info. */ + status = dr_get_client_info(name, pid, global, dr_platform, my_id, &client_pri, + client_path, client_ops); + check(status == DR_SUCCESS, "dr_get_client_info should succeed"); + check(client_pri == my_priority, "priority query doesn't match"); + check(strcmp(client_path, my_path) == 0, "path doesn't match"); + check(strcmp(client_ops, my_ops) == 0, "options don't match"); + client.path = client_path; + client.options = client_ops; + client.id = my_id; + status = dr_get_client_info_ex(name, pid, global, dr_platform, &client); + check(status == DR_SUCCESS, "dr_get_client_info_ex should succeed"); + check(client.priority == my_priority, "priority query doesn't match"); + check(strcmp(client.path, my_path) == 0, "path doesn't match"); + check(strcmp(client.options, my_ops) == 0, "options don't match"); + + dr_client_iterator_t *iter = dr_client_iterator_start(name, pid, global, dr_platform); + check(iter != NULL, "iter should instantiate"); + int client_id; + int count = 0; + while (dr_client_iterator_hasnext(iter)) { + dr_client_iterator_next(iter, &client_id, &client_pri, client_path, client_ops); + check(client_id == my_id, "id doesn't match"); + if (count == 0) { + check(client_pri == my_priority, "priority doesn't match"); + check(strcmp(client_path, my_path) == 0, "path doesn't match"); + check(strcmp(client_ops, my_ops) == 0, "options don't match"); + } else { + check(client_pri == my_priority + 1, "priority doesn't match"); + check(strcmp(client_path, my_alt_path) == 0, "path doesn't match"); + check(strcmp(client_ops, my_ops) == 0, "options don't match"); + } + ++count; + } + dr_client_iterator_stop(iter); + + iter = dr_client_iterator_start(name, pid, global, dr_platform); + check(iter != NULL, "iter should instantiate"); + count = 0; + client.struct_size = sizeof(client); + client.path = client_path; + client.options = client_ops; + while (dr_client_iterator_hasnext(iter)) { + dr_client_iterator_next_ex(iter, &client); + check(client.id == my_id, "id doesn't match"); + if (count == 0) { + check(client.priority == my_priority, "priority doesn't match"); + check(strcmp(client.path, my_path) == 0, "path doesn't match"); + check(strcmp(client.options, my_ops) == 0, "options don't match"); + check(!client.is_alt_bitwidth, "is_alt_bitwidth doesn't match"); + } else { + check(client.priority == my_priority + 1, "priority doesn't match"); + check(strcmp(client.path, my_alt_path) == 0, "path doesn't match"); + check(strcmp(client.options, my_ops) == 0, "options don't match"); + check(client.is_alt_bitwidth, "is_alt_bitwidth doesn't match"); + } + ++count; + } + dr_client_iterator_stop(iter); + + status = dr_unregister_client(name, pid, global, dr_platform, my_id); + check(status == DR_SUCCESS, "dr_unregister_client should succeed"); + check(dr_num_registered_clients(name, pid, global, dr_platform) == 0, + "should be 0 clients post-unreg"); + + status = dr_unregister_process(name, pid, global, dr_platform); + check(status == DR_SUCCESS, "dr_unregister_process should succeed"); +} + +int +main(int argc, const char *argv[]) +{ + if (argc < 2) { + fprintf(stderr, "Requires 1 argument: DR root dir\n"); + return 1; + } + const char *dr_root = argv[1]; + +#define PROC_NAME "fake_process" + + test_register(PROC_NAME, 0, false, DR_PLATFORM_32BIT, dr_root); + test_register(PROC_NAME, 0, false, DR_PLATFORM_64BIT, dr_root); + test_register(PROC_NAME, 42, false, DR_PLATFORM_32BIT, dr_root); + test_register(PROC_NAME, 42, false, DR_PLATFORM_64BIT, dr_root); + + fprintf(stderr, "all done\n"); + return 0; +} diff --git a/suite/tests/libutil/drconfig_test.template b/suite/tests/libutil/drconfig_test.template new file mode 100644 index 00000000000..de639143055 --- /dev/null +++ b/suite/tests/libutil/drconfig_test.template @@ -0,0 +1 @@ +all done diff --git a/suite/tests/linux/execve-client.expect b/suite/tests/linux/execve-client.expect new file mode 100644 index 00000000000..7b96a4854e8 --- /dev/null +++ b/suite/tests/linux/execve-client.expect @@ -0,0 +1,10 @@ +large_options passed: -paramA foo -paramB bar +parent is running under DynamoRIO +parent waiting for child +child is running under DynamoRIO +large_options passed: -paramA foo -paramB bar +it_worked +running under DynamoRIO +large_options exiting +child has exited +large_options exiting diff --git a/tools/drdeploy.c b/tools/drdeploy.c index 0bcf72b4467..64ad0cbf059 100644 --- a/tools/drdeploy.c +++ b/tools/drdeploy.c @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2011-2019 Google, Inc. All rights reserved. + * Copyright (c) 2011-2020 Google, Inc. All rights reserved. * Copyright (c) 2008-2010 VMware, Inc. All rights reserved. * **********************************************************/ @@ -133,6 +133,8 @@ const char *usage_str = " -- \n" " or: " TOOLNAME " [options] [DR options] -t [tool options]" " -- \n" + " or: " TOOLNAME " [options] [DR options] -c32 <32-bit-client> [client options]" + " -- -c64 <64-bit-client> [client options] -- \n" # endif ; #endif @@ -215,6 +217,14 @@ const char *options_list_str = " the options may contain semicolon characters or\n" " all 3 quote characters (\", \', `).\n" "\n" + " -c32 <32-bit-path> * -- -c64 <64-bit-path> *\n" + " Registers two versions of one client to run alongside\n" + " DR. Use this to specify two versions of a client to\n" + " handle applications which create other-bitwidth child\n" + " processes. The options for the 32-bit client are\n" + " separated from the \"-c64\" by \"--\". The behavior\n" + " otherwise matches \"-c\".\n" + "\n" " -client \"\"\n" " Use -c instead, unless you need to set the client ID.\n" " Registers one or more clients to run alongside DR.\n" @@ -601,7 +611,7 @@ check_client_lib(const char *client_lib) bool register_client(const char *process_name, process_id_t pid, bool global, dr_platform_t dr_platform, client_id_t client_id, const char *path, - const char *options) + bool is_alt_bitwidth, const char *options) { size_t priority; dr_config_status_t status; @@ -618,21 +628,26 @@ register_client(const char *process_name, process_id_t pid, bool global, /* just append to the existing client list */ priority = dr_num_registered_clients(process_name, pid, global, dr_platform); - info("registering client with id=%d path=|%s| ops=|%s|", client_id, path, options); - status = dr_register_client(process_name, pid, global, dr_platform, client_id, - priority, path, options); - + info("registering client with id=%d path=|%s| ops=|%s|%s", client_id, path, options, + is_alt_bitwidth ? " alt-bitwidth" : ""); + dr_config_client_t info; + info.struct_size = sizeof(info); + info.id = client_id; + info.priority = priority; + info.path = (char *)path; + info.options = (char *)options; + info.is_alt_bitwidth = is_alt_bitwidth; + status = dr_register_client_ex(process_name, pid, global, dr_platform, &info); if (status != DR_SUCCESS) { if (status == DR_CONFIG_STRING_TOO_LONG) { - error("client %s registration failed: option string too long: \"%s\"", - path == NULL ? "" : path, options); + error("client %s registration failed: option string too long: \"%s\"", path, + options); } else if (status == DR_CONFIG_OPTIONS_INVALID) { error("client %s registration failed: options cannot contain ';' or all " "3 quote types: %s", - path == NULL ? "" : path, options); + path, options); } else { - error("client %s registration failed with error code %d", - path == NULL ? "" : path, status); + error("client %s registration failed with error code %d", path, status); } return false; } @@ -718,20 +733,23 @@ write_pid_to_file(const char *pidfile, process_id_t pid) #if defined(DRCONFIG) || defined(DRRUN) static void -append_client(const char *client, int id, const char *client_ops, +append_client(const char *client, int id, const char *client_ops, bool is_alt_bitwidth, char client_paths[MAX_CLIENT_LIBS][MAXIMUM_PATH], client_id_t client_ids[MAX_CLIENT_LIBS], - const char *client_options[MAX_CLIENT_LIBS], size_t *num_clients) + const char *client_options[MAX_CLIENT_LIBS], + bool alt_bitwidth[MAX_CLIENT_LIBS], size_t *num_clients) { + size_t index = *num_clients; /* We support an empty client for native -t usage */ if (client[0] != '\0') { - get_absolute_path(client, client_paths[*num_clients], - BUFFER_SIZE_ELEMENTS(client_paths[*num_clients])); - NULL_TERMINATE_BUFFER(client_paths[*num_clients]); - info("client %d path: %s", (int)*num_clients, client_paths[*num_clients]); + get_absolute_path(client, client_paths[index], + BUFFER_SIZE_ELEMENTS(client_paths[index])); + NULL_TERMINATE_BUFFER(client_paths[index]); + info("client %d path: %s", (int)index, client_paths[index]); } - client_ids[*num_clients] = id; - client_options[*num_clients] = client_ops; + client_ids[index] = id; + client_options[index] = client_ops; + alt_bitwidth[index] = is_alt_bitwidth; (*num_clients)++; } #endif @@ -764,9 +782,14 @@ add_extra_option(char *buf, size_t bufsz, size_t *sofar, const char *fmt, ...) #if defined(DRCONFIG) || defined(DRRUN) /* Returns the path to the client library. Appends to extra_ops. - * A tool config file must contain one of these line types: + * A tool config file must contain one of these line types, or two if + * they are a pair of CLIENT32_* and CLIENT64_* specifiers: * CLIENT_ABS= * CLIENT_REL= + * CLIENT32_ABS= + * CLIENT32_REL= + * CLIENT64_ABS= + * CLIENT64_REL= * It can contain as many DR_OP= lines as desired. Each must contain * one DynamoRIO option token: * DR_OP= @@ -794,9 +817,10 @@ add_extra_option(char *buf, size_t bufsz, size_t *sofar, const char *fmt, ...) */ static bool read_tool_file(const char *toolname, const char *dr_root, dr_platform_t dr_platform, - char *client, size_t client_size, char *ops, size_t ops_size, - size_t *ops_sofar, char *tool_ops, size_t tool_ops_size, - size_t *tool_ops_sofar, char *native_path OUT, size_t native_path_size) + char *client, size_t client_size, char *alt_client, size_t alt_size, + char *ops, size_t ops_size, size_t *ops_sofar, char *tool_ops, + size_t tool_ops_size, size_t *tool_ops_sofar, char *native_path OUT, + size_t native_path_size) { FILE *f; char config_file[MAXIMUM_PATH]; @@ -835,6 +859,23 @@ read_tool_file(const char *toolname, const char *dr_root, dr_platform_t dr_platf add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", client); } + } else if (strstr(line, IF_X64_ELSE("CLIENT64_REL=", "CLIENT32_REL=")) == line) { + _snprintf(client, client_size, "%s/%s", dr_root, + line + strlen(IF_X64_ELSE("CLIENT64_REL=", "CLIENT32_REL="))); + client[client_size - 1] = '\0'; + found_client = true; + if (native_path[0] != '\0') { + add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", + client); + } + } else if (strstr(line, IF_X64_ELSE("CLIENT32_REL=", "CLIENT64_REL=")) == line) { + _snprintf(alt_client, alt_size, "%s/%s", dr_root, + line + strlen(IF_X64_ELSE("CLIENT32_REL=", "CLIENT64_REL="))); + alt_client[alt_size - 1] = '\0'; + if (native_path[0] != '\0') { + add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", + alt_client); + } } else if (strstr(line, "CLIENT_ABS=") == line) { strncpy(client, line + strlen("CLIENT_ABS="), client_size); found_client = true; @@ -842,6 +883,22 @@ read_tool_file(const char *toolname, const char *dr_root, dr_platform_t dr_platf add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", client); } + } else if (strstr(line, IF_X64_ELSE("CLIENT64_ABS=", "CLIENT32_ABS=")) == line) { + strncpy(client, line + strlen(IF_X64_ELSE("CLIENT64_ABS=", "CLIENT32_ABS=")), + client_size); + found_client = true; + if (native_path[0] != '\0') { + add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", + client); + } + } else if (strstr(line, IF_X64_ELSE("CLIENT32_ABS=", "CLIENT64_ABS=")) == line) { + strncpy(alt_client, + line + strlen(IF_X64_ELSE("CLIENT32_ABS=", "CLIENT64_ABS=")), + alt_size); + if (native_path[0] != '\0') { + add_extra_option(tool_ops, tool_ops_size, tool_ops_sofar, "\"%s\"", + alt_client); + } } else if (strstr(line, "DR_OP=") == line) { if (strcmp(line, "DR_OP=") != 0) { add_extra_option(ops, ops_size, ops_sofar, "\"%s\"", @@ -985,6 +1042,7 @@ _tmain(int argc, TCHAR *targv[]) client_id_t client_ids[MAX_CLIENT_LIBS] = { 0, }; + bool alt_bitwidth[MAX_CLIENT_LIBS]; size_t num_clients = 0; char single_client_ops[DR_MAX_OPTIONS_LENGTH]; #endif @@ -1315,8 +1373,8 @@ _tmain(int argc, TCHAR *targv[]) client = argv[++i]; id = strtoul(argv[++i], NULL, 16); ops = argv[++i]; - append_client(client, id, ops, client_paths, client_ids, client_options, - &num_clients); + append_client(client, id, ops, false, client_paths, client_ids, + client_options, alt_bitwidth, &num_clients); } } else if (strcmp(argv[i], "-ops") == 0) { /* support repeating the option (i#477) */ @@ -1359,8 +1417,10 @@ _tmain(int argc, TCHAR *targv[]) * optionsx.h to do otherwise, or to sanity check the DR options here. */ else if (argv[i][0] == '-') { + bool expect_extra_double_dash = false; while (i < argc) { if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "-t") == 0 || + strcmp(argv[i], "-c32") == 0 || strcmp(argv[i], "-c64") == 0 || strcmp(argv[i], "--") == 0) { break; } @@ -1368,14 +1428,23 @@ _tmain(int argc, TCHAR *targv[]) &extra_ops_sofar, "\"%s\"", argv[i]); i++; } - if (i < argc && (strcmp(argv[i], "-t") == 0 || strcmp(argv[i], "-c") == 0)) { + if (i < argc && + (strcmp(argv[i], "-t") == 0 || strcmp(argv[i], "-c") == 0 || + strcmp(argv[i], "-c32") == 0 || strcmp(argv[i], "-c64") == 0)) { const char *client; char client_buf[MAXIMUM_PATH]; + char alt_buf[MAXIMUM_PATH]; size_t client_sofar = 0; + bool is_alt_bitwidth = false; if (i + 1 >= argc) usage(false, "too few arguments to %s", argv[i]); - if (num_clients != 0) + if (num_clients != 0 && + (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "-t") == 0)) usage(false, "Cannot use -client with %s.", argv[i]); + if (strcmp(argv[i], "-c32") == 0) + expect_extra_double_dash = true; + if (strcmp(argv[i], IF_X64_ELSE("-c32", "-c64")) == 0) + is_alt_bitwidth = true; client = argv[++i]; single_client_ops[0] = '\0'; @@ -1385,7 +1454,8 @@ _tmain(int argc, TCHAR *targv[]) * The user must use -c or -client to do that. */ if (!read_tool_file(client, dr_root, dr_platform, client_buf, - BUFFER_SIZE_ELEMENTS(client_buf), extra_ops, + BUFFER_SIZE_ELEMENTS(client_buf), alt_buf, + BUFFER_SIZE_ELEMENTS(alt_buf), extra_ops, BUFFER_SIZE_ELEMENTS(extra_ops), &extra_ops_sofar, single_client_ops, BUFFER_SIZE_ELEMENTS(single_client_ops), @@ -1394,6 +1464,8 @@ _tmain(int argc, TCHAR *targv[]) usage(false, "unknown %s tool \"%s\" requested", platform_name(dr_platform), client); client = client_buf; + append_client(alt_buf, 0, single_client_ops, true, client_paths, + client_ids, client_options, alt_bitwidth, &num_clients); } /* Treat everything up to -- or end of argv as client args. */ @@ -1409,10 +1481,12 @@ _tmain(int argc, TCHAR *targv[]) &client_sofar, "\"%s\"", argv[i]); i++; } - append_client(client, 0, single_client_ops, client_paths, client_ids, - client_options, &num_clients); + append_client(client, 0, single_client_ops, is_alt_bitwidth, client_paths, + client_ids, client_options, alt_bitwidth, &num_clients); } - if (i < argc && strcmp(argv[i], "--") == 0) { + if (i < argc && strcmp(argv[i], "--") == 0 && + (!expect_extra_double_dash || + (i + 1 < argc && strcmp(argv[i + 1], "-c64") != 0))) { i++; goto done_with_options; } @@ -1529,7 +1603,7 @@ _tmain(int argc, TCHAR *targv[]) die(); for (j = 0; j < num_clients; j++) { if (!register_client(process, 0, global, dr_platform, client_ids[j], - client_paths[j], client_options[j])) + client_paths[j], alt_bitwidth[j], client_options[j])) die(); } } else if (action == action_unregister) { @@ -1599,7 +1673,8 @@ _tmain(int argc, TCHAR *targv[]) /* If this is the first setting of AppInit on NT, warn about reboot */ if (!dr_syswide_is_on(dr_platform, dr_root)) { if (platform == PLATFORM_WIN_NT_4) { - warn("on Windows NT, applications will not be taken over until reboot"); + warn("on Windows NT, applications will not be taken over until " + "reboot"); } else if (platform >= PLATFORM_WIN_7) { /* i#323 will fix this but good to warn the user */ warn("on Windows 7+, syswide_on relaxes system security by removing " @@ -1712,7 +1787,7 @@ _tmain(int argc, TCHAR *targv[]) for (j = 0; j < num_clients; j++) { if (!register_client(process, dr_inject_get_process_id(inject_data), global, dr_platform, client_ids[j], client_paths[j], - client_options[j])) + alt_bitwidth[j], client_options[j])) goto error; } } From c5125f7e4bbbb0781c74be3a7d4e755ca670f045 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Sun, 14 Jun 2020 14:05:55 -0400 Subject: [PATCH 2/7] Fix pre-existing Windows drconfig handle leak found by new test; copy drconfiglib.dll into bin dir on Windows; fix unfilled-in field on query; fix clang build warning --- libutil/dr_config.c | 25 ++++++++++++++++++++++--- suite/tests/CMakeLists.txt | 8 ++++++++ suite/tests/libutil/drconfig_test.c | 3 ++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/libutil/dr_config.c b/libutil/dr_config.c index 34b4a4f382b..126cf7490f0 100644 --- a/libutil/dr_config.c +++ b/libutil/dr_config.c @@ -1401,6 +1401,7 @@ dr_registered_process_iterator_start(dr_platform_t dr_platform, bool global) char dir[MAXIMUM_PATH]; if (!get_config_dir(global, dir, BUFFER_SIZE_ELEMENTS(dir), false)) { iter->has_next = false; + iter->find_handle = INVALID_HANDLE_VALUE; return iter; } convert_to_tchar(iter->wdir, dir, BUFFER_SIZE_ELEMENTS(iter->wdir)); @@ -1460,8 +1461,11 @@ dr_registered_process_iterator_next(dr_registered_process_iterator_t *iter, } if (!FindNextFile(iter->find_handle, &iter->find_data)) iter->has_next = false; - if (f == NULL || !ok) + if (f == NULL || !ok) { + if (f != NULL) + fclose(f); return false; + } read_process_policy(f, process_name, dr_root_dir, dr_mode, debug, dr_options); fclose(f); return true; @@ -1539,10 +1543,24 @@ dr_client_iterator_start(const char *process_name, process_id_t pid, bool global iter->cur = 0; if (IF_REG_ELSE(proc_policy == NULL, f == NULL)) return iter; - if (read_options(&iter->opt_info, IF_REG_ELSE(proc_policy, f)) != DR_SUCCESS) + if (read_options(&iter->opt_info, IF_REG_ELSE(proc_policy, f)) != DR_SUCCESS) { +#ifdef PARAMS_IN_REGISTRY + if (policy != NULL) + free_config_group(policy); +#else + if (f != NULL) + fclose(f); +#endif return iter; + } iter->valid = true; - +#ifdef PARAMS_IN_REGISTRY + if (policy != NULL) + free_config_group(policy); +#else + if (f != NULL) + fclose(f); +#endif return iter; } @@ -1600,6 +1618,7 @@ dr_client_iterator_next_ex(dr_client_iterator_t *iter, _snprintf(client->options, len, TSTR_FMT, client_opt->opts); client->options[len] = '\0'; } + client->is_alt_bitwidth = client_opt->alt_bitwidth; iter->cur++; return DR_SUCCESS; } diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 9be4b9f0cb5..44f30d20cb1 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -869,6 +869,10 @@ function(add_api_exe test source use_decoder use_static_DR) COMMAND ${CMAKE_COMMAND} ARGS -E copy "${MAIN_LIBRARY_OUTPUT_DIRECTORY}/dynamorio.dll" "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/dynamorio.dll" + # libutil.drconfig_test needs drconfiglib. + COMMAND ${CMAKE_COMMAND} ARGS + -E copy "${MAIN_LIBRARY_OUTPUT_DIRECTORY}/../drconfiglib.dll" + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/drconfiblib.dll" # We copy drsyms.dll too, for api.symtest COMMAND ${CMAKE_COMMAND} ARGS -E copy "${PROJECT_BINARY_DIR}/ext/${INSTALL_LIB}/drsyms.dll" @@ -1820,6 +1824,10 @@ if (CLIENT_INTERFACE) # Just like frontend_test we request drdecode. tobuild_api(libutil.drconfig_test libutil/drconfig_test.c "" "${dr_root}" ON OFF) target_link_libraries(libutil.drconfig_test drconfiglib) + if (WIN32) + # We have to copy drconfiglib.dll into the bin dir. + add_dependencies(libutil.drconfig_test drcopy) + endif () set(BUILD_TEST_ANNOTATION "${PROJECT_BINARY_DIR}/suite/tests/annotations") configure_file( diff --git a/suite/tests/libutil/drconfig_test.c b/suite/tests/libutil/drconfig_test.c index afce87bff8c..88a0b4d641d 100644 --- a/suite/tests/libutil/drconfig_test.c +++ b/suite/tests/libutil/drconfig_test.c @@ -32,6 +32,7 @@ #include #include +#include #include "dr_api.h" #include "dr_config.h" @@ -148,7 +149,7 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ dr_client_iterator_t *iter = dr_client_iterator_start(name, pid, global, dr_platform); check(iter != NULL, "iter should instantiate"); - int client_id; + unsigned int client_id; int count = 0; while (dr_client_iterator_hasnext(iter)) { dr_client_iterator_next(iter, &client_id, &client_pri, client_path, client_ops); From 2362e8dcd7d6512119986d893371315b105ea317 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Sun, 14 Jun 2020 17:41:38 -0400 Subject: [PATCH 3/7] Fix drconfiglib copy rule --- suite/tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 44f30d20cb1..6f90201ad46 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -872,7 +872,7 @@ function(add_api_exe test source use_decoder use_static_DR) # libutil.drconfig_test needs drconfiglib. COMMAND ${CMAKE_COMMAND} ARGS -E copy "${MAIN_LIBRARY_OUTPUT_DIRECTORY}/../drconfiglib.dll" - "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/drconfiblib.dll" + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/drconfiglib.dll" # We copy drsyms.dll too, for api.symtest COMMAND ${CMAKE_COMMAND} ARGS -E copy "${PROJECT_BINARY_DIR}/ext/${INSTALL_LIB}/drsyms.dll" From 02e1952b07811154f1b82a211ed05259bac50785 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 15 Jun 2020 13:00:24 -0400 Subject: [PATCH 4/7] Enforce primary then alt; use DR's snprintf; add a copy helper; comment tweaks --- api/docs/release.dox | 6 +-- clients/drcachesim/common/options.cpp | 3 +- core/lib/dr_config.h | 48 ++++++++++-------- libutil/CMakeLists.txt | 2 +- libutil/dr_config.c | 70 +++++++++++++++------------ libutil/our_tchar.h | 3 +- suite/tests/libutil/drconfig_test.c | 13 +++++ 7 files changed, 88 insertions(+), 57 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 04b5c8ec1d2..4449a1e28a1 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -142,9 +142,9 @@ The changes between version \DR_VERSION and 8.0.0 include the following minor compatibility changes: - drconfiglib (and thus drrun and drconfig) now sets only the new client path - options added to support other-bitwidth child processes. This means that a - drconfiglib from this version will not properly configure for a DynamoRIO - core library from a prior version. + options which are added in this release to support other-bitwidth child processes. + This means that a drconfiglib from this version will not properly configure for a + DynamoRIO core library from a prior version. Further non-compatibility-affecting changes include: diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index c30f0ef211e..ee1a5a8fe53 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -339,7 +339,8 @@ droption_t op_tracer_alt(DROPTION_SCOPE_FRONTEND, "tracer_alt", "", "Path to the alternate-bitwidth tracer", "The full path to the tracer library for the other " "bitwidth, for use on child processes with a " - "different bitwidth from their parent."); + "different bitwidth from their parent. If empty, " + "such child processes will die with fatal errors."); droption_t op_tracer_ops( DROPTION_SCOPE_FRONTEND, "tracer_ops", diff --git a/core/lib/dr_config.h b/core/lib/dr_config.h index 89ebe7e8c5f..556ba0990cb 100644 --- a/core/lib/dr_config.h +++ b/core/lib/dr_config.h @@ -148,6 +148,12 @@ typedef enum { */ DR_CONFIG_INVALID_PARAMETER, + /** + * A primary client configuration was not found at the time an alternate bitwidth + * client was attempted to be registered. + */ + DR_CONFIG_CLIENT_NOT_FOUND, + } dr_config_status_t; /** Allow targeting both 32-bit and native 64-bit processes separately. */ @@ -175,21 +181,20 @@ typedef struct _dr_config_client_t { */ client_id_t id; /** - * The client number, or priority. Client registration includes a value - * indicating the priority of a client relative to other clients. In + * The client number, or priority. Client registration includes a value + * indicating the priority of a client relative to other clients. In * multi-client settings, a client's priority influences event callback - * ordering.That is, higher priority clients can register their callbacks first; + * ordering. That is, higher priority clients can register their callbacks first; * DynamoRIO then calls these routines last. Client priorities range * consecutively from 0 to N-1, where N is the number of registered clients. * Specify priority 0 to register a client with highest priority. */ size_t priority; /** - * A NULL-terminated string specifying the full path to a valid client library - * whose bitwidth is the opposite of that specified by dr_platform. The string - * length cannot exceed #MAXIMUM_PATH. The client path may not include any semicolons - * and when combined with \p client_options may not include all three quote - * characters (', ", `) simultaneously. + * A NULL-terminated string specifying the full path to a valid client library. + * The string length cannot exceed #MAXIMUM_PATH. The client path may not + * include any semicolons and when combined with \p client_options may not + * include all three quote characters (', ", `) simultaneously. * * When querying via dr_get_client_info_ex() or dr_client_iterator_next_ex(), the * caller must either set this to NULL if that data is not needed or point at a @@ -209,16 +214,17 @@ typedef struct _dr_config_client_t { */ char *options; /** - * Specifies whether this is a regular client registeration for the target - * process (this field is false) or whether this is an "alternate bitwidth" - * registration which specifies the configuration to use if the target process - * creates a child process that is a different bitwidth from itself. The regular - * client must first be registered on its own before the alternate client is - * registered. If the target process creates a child application of a different - * bitwdith (e.g., the target process is 64-bit and it creates a 32-bit child), - * this registration allows a client of the appropriate bitwidth to be loaded - * into the child. Unregistering a client with dr_unregister_client() will also - * unregister the alternate bitwidth client. + * Specifies whether this is a regular client registration for the target process + * (this field is false) or whether this is an "alternate bitwidth" registration + * which specifies the configuration to use if the target process creates a child + * process that is a different bitwidth from itself. The regular client must + * first be registered on its own before the alternate client is registered, and + * the alternate must have the same client ID. If the target process creates a + * child application of a different bitwdith (e.g., the target process is 64-bit + * and it creates a 32-bit child), this registration allows a client of the + * appropriate bitwidth to be loaded into the child. Unregistering a client with + * dr_unregister_client() will also unregister the alternate bitwidth client with + * the same client ID. */ bool is_alt_bitwidth; } dr_config_client_t; @@ -800,7 +806,7 @@ DR_EXPORT * configurations to unset. * * \return The number of clients registered for the given process and platform. - * An alternative-bitwidth client is counted as a separate client. + * An alternative bitwidth client is counted as a separate client. */ size_t dr_num_registered_clients(const char *process_name, process_id_t pid, bool global, @@ -809,7 +815,7 @@ dr_num_registered_clients(const char *process_name, process_id_t pid, bool globa DR_EXPORT /** * Retrieve client registration information for a particular process for the current - * user. If multiple clients are registered (alternative-bitwidth clients are + * user. If multiple clients are registered (alternative bitwidth clients are * considered separate), information on the highest-priority client is returned. * * \param[in] process_name A NULL-terminated string specifying the name @@ -865,7 +871,7 @@ dr_get_client_info(const char *process_name, process_id_t pid, bool global, DR_EXPORT /** * Retrieve client registration information for a particular process for the current - * user. If multiple clients are registered (alternative-bitwidth clients are + * user. If multiple clients are registered (alternative bitwidth clients are * considered separate), information on the highest-priority client is returned. * * \param[in] process_name A NULL-terminated string specifying the name diff --git a/libutil/CMakeLists.txt b/libutil/CMakeLists.txt index 40e7592f4af..36bad803501 100644 --- a/libutil/CMakeLists.txt +++ b/libutil/CMakeLists.txt @@ -117,7 +117,7 @@ if (WIN32) else () # UNIX # Most of it is unported. We're just reusing some utils.c and the config # file writing code currently. - set(DRCONFIG_SRCS dr_config.c utils.c) + set(DRCONFIG_SRCS dr_config.c utils.c ${PROJECT_SOURCE_DIR}/core/io.c) if (APPLE) # XXX i#1286: implement nudge for MacOS else (APPLE) diff --git a/libutil/dr_config.c b/libutil/dr_config.c index 126cf7490f0..6b83348a194 100644 --- a/libutil/dr_config.c +++ b/libutil/dr_config.c @@ -41,6 +41,14 @@ #include "our_tchar.h" #include "dr_frontend.h" +/* We use DR's snprintf for consistent behavior when len==max. + * TODO: Change all of libutil by changing our_tchar.h. + */ +#undef _snprintf +#define _snprintf d_r_snprintf +int +d_r_snprintf(char *s, size_t max, const char *fmt, ...); + #ifdef WINDOWS # include # include /* for _get_osfhandle */ @@ -55,12 +63,9 @@ # define LIB32_SUBDIR _TEXT("\\lib32") # define PREINJECT32_DLL _TEXT("\\lib32\\drpreinject.dll") # define PREINJECT64_DLL _TEXT("\\lib64\\drpreinject.dll") -# define _snprintf d_r_snprintf # undef _sntprintf # define _sntprintf d_r_snprintf_wide int -d_r_snprintf(char *s, size_t max, const char *fmt, ...); -int d_r_snprintf_wide(wchar_t *s, size_t max, const wchar_t *fmt, ...); #else # include @@ -78,6 +83,10 @@ d_r_snprintf_wide(wchar_t *s, size_t max, const wchar_t *fmt, ...); # define DEBUG64_DLL "/lib64/debug/libdynamorio.so" # define LOG_SUBDIR "/logs" # define LIB32_SUBDIR "/lib32/" +# undef _sntprintf +# define _sntprintf d_r_snprintf +int +d_r_snprintf_wide(wchar_t *s, size_t max, const wchar_t *fmt, ...); extern bool create_nudge_signal_payload(siginfo_t *info OUT, uint action_mask, client_id_t client_id, @@ -173,6 +182,14 @@ get_next_token(TCHAR *ptr, TCHAR *token) return ptr; } +static void +copy_up_to_max(TCHAR *buf, size_t max, TCHAR *src) +{ + size_t len = MIN(max - 1, _tcslen(src)); + _snprintf(buf, len, TSTR_FMT, src); + buf[len] = '\0'; +} + /* Allocate a new client_opt_t */ static client_opt_t * new_client_opt(const TCHAR *path, client_id_t id, const TCHAR *opts, bool alt_bitwidth) @@ -1290,9 +1307,7 @@ read_process_policy(IF_REG_ELSE(ConfigGroup *proc_policy, FILE *f), if (process_name != NULL) *process_name = '\0'; if (process_name != NULL && proc_policy->name != NULL) { - SIZE_T len = MIN(_tcslen(proc_policy->name), MAXIMUM_PATH - 1); - _snprintf(process_name, len, TSTR_FMT, proc_policy->name); - process_name[len] = '\0'; + copy_up_to_max(process_name, MAXIMUM_PATH, proc_policy->name); } #else /* up to caller to fill in! */ @@ -1582,18 +1597,14 @@ dr_client_iterator_next(dr_client_iterator_t *iter, client_id_t *client_id, /* O *client_pri = iter->cur; if (client_path != NULL) { - size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); - _snprintf(client_path, len, TSTR_FMT, client_opt->path); - client_path[len] = '\0'; + copy_up_to_max(client_path, MAXIMUM_PATH, client_opt->path); } if (client_id != NULL) *client_id = client_opt->id; if (client_options != NULL) { - size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); - _snprintf(client_options, len, TSTR_FMT, client_opt->opts); - client_options[len] = '\0'; + copy_up_to_max(client_options, DR_MAX_OPTIONS_LENGTH, client_opt->opts); } iter->cur++; @@ -1609,14 +1620,10 @@ dr_client_iterator_next_ex(dr_client_iterator_t *iter, client->id = client_opt->id; client->priority = iter->cur; if (client->path != NULL) { - size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); - _snprintf(client->path, len, TSTR_FMT, client_opt->path); - client->path[len] = '\0'; + copy_up_to_max(client->path, MAXIMUM_PATH, client_opt->path); } if (client->options != NULL) { - size_t len = MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); - _snprintf(client->options, len, TSTR_FMT, client_opt->opts); - client->options[len] = '\0'; + copy_up_to_max(client->options, DR_MAX_OPTIONS_LENGTH, client_opt->opts); } client->is_alt_bitwidth = client_opt->alt_bitwidth; iter->cur++; @@ -1694,16 +1701,11 @@ dr_get_client_info_ex(const char *process_name, process_id_t pid, bool global, client->priority = i; if (client->path != NULL) { - size_t len = MIN(MAXIMUM_PATH - 1, _tcslen(client_opt->path) + 1); - _snprintf(client->path, len, TSTR_FMT, client_opt->path); - client->path[len] = '\0'; + copy_up_to_max(client->path, MAXIMUM_PATH, client_opt->path); } if (client->options != NULL) { - size_t len = - MIN(DR_MAX_OPTIONS_LENGTH - 1, _tcslen(client_opt->opts) + 1); - _snprintf(client->options, len, TSTR_FMT, client_opt->opts); - client->options[len] = '\0'; + copy_up_to_max(client->options, DR_MAX_OPTIONS_LENGTH, client_opt->opts); } status = DR_SUCCESS; @@ -1772,14 +1774,22 @@ dr_register_client_ex(const char *process_name, process_id_t pid, bool global, } opt_info_alloc = true; + bool order_ok = !client->is_alt_bitwidth; for (i = 0; i < opt_info.num_clients; i++) { - if (opt_info.client_opts[i]->id == client->id && - ((opt_info.client_opts[i]->alt_bitwidth && client->is_alt_bitwidth) || - (!opt_info.client_opts[i]->alt_bitwidth && !client->is_alt_bitwidth))) { - status = DR_ID_CONFLICTING; - goto exit; + if (opt_info.client_opts[i]->id == client->id) { + if (!opt_info.client_opts[i]->alt_bitwidth && client->is_alt_bitwidth) { + /* OK since this is the alt for an existing primary. */ + order_ok = true; + } else { + status = DR_ID_CONFLICTING; + goto exit; + } } } + if (!order_ok) { + status = DR_CONFIG_CLIENT_NOT_FOUND; + goto exit; + } if (client->priority > opt_info.num_clients) { status = DR_PRIORITY_INVALID; diff --git a/libutil/our_tchar.h b/libutil/our_tchar.h index 0905e5bbddb..164ca0f88ae 100644 --- a/libutil/our_tchar.h +++ b/libutil/our_tchar.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2012-2016 Google, Inc. All rights reserved. + * Copyright (c) 2012-2020 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -43,6 +43,7 @@ # define _tcslen strlen # define _tcscmp strcmp # define _tcsncpy strncpy +/* TODO: Switch to DR's snprintf for consistent behavior, like dr_config.c has done. */ # define _sntprintf snprintf # define _tcstoul strtoul # define _TEXT(str) str diff --git a/suite/tests/libutil/drconfig_test.c b/suite/tests/libutil/drconfig_test.c index 88a0b4d641d..1d874aa9660 100644 --- a/suite/tests/libutil/drconfig_test.c +++ b/suite/tests/libutil/drconfig_test.c @@ -112,6 +112,16 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ status = dr_register_client_ex(name, pid, global, dr_platform, &client); check(status == DR_CONFIG_INVALID_PARAMETER, "bad struct_size should fail"); client.struct_size = sizeof(client); + + client.id = my_id; + client.priority = my_priority; + client.path = (char *)my_alt_path; + client.options = (char *)my_ops; + client.is_alt_bitwidth = true; + client.priority = 0; + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_CONFIG_CLIENT_NOT_FOUND, "register alt first should fail"); + client.id = my_id; client.priority = my_priority; client.path = (char *)my_path; @@ -122,6 +132,9 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ check(dr_num_registered_clients(name, pid, global, dr_platform) == 1, "should be 1 client post-reg"); + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_ID_CONFLICTING, "dup id should fail"); + client.path = (char *)my_alt_path; client.options = (char *)my_ops; client.is_alt_bitwidth = true; From a31bb861b0a505e962217f01902fccb5bfb8ea7e Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 15 Jun 2020 14:25:31 -0400 Subject: [PATCH 5/7] Reverse order of client reg for tool files to comply with required primar then alt --- tools/drdeploy.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/drdeploy.c b/tools/drdeploy.c index 64ad0cbf059..6c5bbf42acd 100644 --- a/tools/drdeploy.c +++ b/tools/drdeploy.c @@ -1434,6 +1434,7 @@ _tmain(int argc, TCHAR *targv[]) const char *client; char client_buf[MAXIMUM_PATH]; char alt_buf[MAXIMUM_PATH]; + alt_buf[0] = '\0'; size_t client_sofar = 0; bool is_alt_bitwidth = false; if (i + 1 >= argc) @@ -1464,8 +1465,6 @@ _tmain(int argc, TCHAR *targv[]) usage(false, "unknown %s tool \"%s\" requested", platform_name(dr_platform), client); client = client_buf; - append_client(alt_buf, 0, single_client_ops, true, client_paths, - client_ids, client_options, alt_bitwidth, &num_clients); } /* Treat everything up to -- or end of argv as client args. */ @@ -1483,6 +1482,10 @@ _tmain(int argc, TCHAR *targv[]) } append_client(client, 0, single_client_ops, is_alt_bitwidth, client_paths, client_ids, client_options, alt_bitwidth, &num_clients); + if (alt_buf[0] != '\0') { + append_client(alt_buf, 0, single_client_ops, true, client_paths, + client_ids, client_options, alt_bitwidth, &num_clients); + } } if (i < argc && strcmp(argv[i], "--") == 0 && (!expect_extra_double_dash || From f4720efc275d03b53fb390ffdbbccb50e58163c3 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 15 Jun 2020 17:31:18 -0400 Subject: [PATCH 6/7] Reverse order of -c32 -c64 for x64 builds to get alt 2nd; document order; add another test case; fix Windows TCHAR vs char buf --- api/docs/intro.dox | 4 ++++ libutil/dr_config.c | 2 +- suite/tests/libutil/drconfig_test.c | 15 +++++++++++++-- tools/drdeploy.c | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/api/docs/intro.dox b/api/docs/intro.dox index 85d20bb58fc..a002b516390 100644 --- a/api/docs/intro.dox +++ b/api/docs/intro.dox @@ -394,6 +394,8 @@ the first client's options: bin32/drrun.exe -c32 samples/bin32/bbsize.dll -- -c64 samples/bin64/bbsize.dll -- notepad +The order matters: \c -c32 must come first. + To \ref sec_comm "nudge" all instances of \c notepad.exe running under DynamoRIO with argument "5", use: \code @@ -608,6 +610,8 @@ the first client's options: bin32/drrun -c32 samples/bin32/bbsize.dll -- -c64 samples/bin64/bbsize.dll -- notepad +The order matters: \c -c32 must come first. + \section multi_client Multiple Clients DynamoRIO does support multiple clients. It is each client's diff --git a/libutil/dr_config.c b/libutil/dr_config.c index 6b83348a194..ee83b7a78fa 100644 --- a/libutil/dr_config.c +++ b/libutil/dr_config.c @@ -183,7 +183,7 @@ get_next_token(TCHAR *ptr, TCHAR *token) } static void -copy_up_to_max(TCHAR *buf, size_t max, TCHAR *src) +copy_up_to_max(char *buf, size_t max, TCHAR *src) { size_t len = MIN(max - 1, _tcslen(src)); _snprintf(buf, len, TSTR_FMT, src); diff --git a/suite/tests/libutil/drconfig_test.c b/suite/tests/libutil/drconfig_test.c index 1d874aa9660..7c04b29251b 100644 --- a/suite/tests/libutil/drconfig_test.c +++ b/suite/tests/libutil/drconfig_test.c @@ -135,6 +135,16 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ status = dr_register_client_ex(name, pid, global, dr_platform, &client); check(status == DR_ID_CONFLICTING, "dup id should fail"); + client.id = my_id + 1; + client.priority = my_priority + 1; /* Append. */ + client.path = (char *)my_alt_path; + client.options = (char *)my_ops; + client.is_alt_bitwidth = true; + client.priority = 0; + status = dr_register_client_ex(name, pid, global, dr_platform, &client); + check(status == DR_CONFIG_CLIENT_NOT_FOUND, "register diff-ID alt first should fail"); + + client.id = my_id; client.path = (char *)my_alt_path; client.options = (char *)my_ops; client.is_alt_bitwidth = true; @@ -173,7 +183,7 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ check(strcmp(client_ops, my_ops) == 0, "options don't match"); } else { check(client_pri == my_priority + 1, "priority doesn't match"); - check(strcmp(client_path, my_alt_path) == 0, "path doesn't match"); + check(strcmp(client_path, my_alt_path) == 0, "alt path doesn't match"); check(strcmp(client_ops, my_ops) == 0, "options don't match"); } ++count; @@ -196,7 +206,7 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ check(!client.is_alt_bitwidth, "is_alt_bitwidth doesn't match"); } else { check(client.priority == my_priority + 1, "priority doesn't match"); - check(strcmp(client.path, my_alt_path) == 0, "path doesn't match"); + check(strcmp(client.path, my_alt_path) == 0, "alt path doesn't match"); check(strcmp(client.options, my_ops) == 0, "options don't match"); check(client.is_alt_bitwidth, "is_alt_bitwidth doesn't match"); } @@ -204,6 +214,7 @@ test_register(const char *name, process_id_t pid, bool global, dr_platform_t dr_ } dr_client_iterator_stop(iter); + /* A single unregister should remove both b/c they have the same ID. */ status = dr_unregister_client(name, pid, global, dr_platform, my_id); check(status == DR_SUCCESS, "dr_unregister_client should succeed"); check(dr_num_registered_clients(name, pid, global, dr_platform) == 0, diff --git a/tools/drdeploy.c b/tools/drdeploy.c index 6c5bbf42acd..83333198c67 100644 --- a/tools/drdeploy.c +++ b/tools/drdeploy.c @@ -740,6 +740,20 @@ append_client(const char *client, int id, const char *client_ops, bool is_alt_bi bool alt_bitwidth[MAX_CLIENT_LIBS], size_t *num_clients) { size_t index = *num_clients; + /* Handle "-c32 -c64" order for native 64-bit where we want to swap the 32 + * and 64 to get the alt last. + */ + if (index > 0 && id == client_ids[index - 1] && alt_bitwidth[index - 1] && + !is_alt_bitwidth) { + /* Insert this one before the prior one by first copying the prior to index. */ + client_ids[index] = client_ids[index - 1]; + alt_bitwidth[index] = alt_bitwidth[index - 1]; + _snprintf(client_paths[index], BUFFER_SIZE_ELEMENTS(client_paths[index]), + client_paths[index - 1]); + NULL_TERMINATE_BUFFER(client_paths[index]); + client_options[index] = client_options[index - 1]; + index = index - 1; + } /* We support an empty client for native -t usage */ if (client[0] != '\0') { get_absolute_path(client, client_paths[index], From eac4c1dc8fb95afffbdad946dd231cc914f43446 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 15 Jun 2020 18:37:10 -0400 Subject: [PATCH 7/7] Fix clang warning --- tools/drdeploy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/drdeploy.c b/tools/drdeploy.c index 83333198c67..c97725898ed 100644 --- a/tools/drdeploy.c +++ b/tools/drdeploy.c @@ -748,7 +748,7 @@ append_client(const char *client, int id, const char *client_ops, bool is_alt_bi /* Insert this one before the prior one by first copying the prior to index. */ client_ids[index] = client_ids[index - 1]; alt_bitwidth[index] = alt_bitwidth[index - 1]; - _snprintf(client_paths[index], BUFFER_SIZE_ELEMENTS(client_paths[index]), + _snprintf(client_paths[index], BUFFER_SIZE_ELEMENTS(client_paths[index]), "%s", client_paths[index - 1]); NULL_TERMINATE_BUFFER(client_paths[index]); client_options[index] = client_options[index - 1];