-
Notifications
You must be signed in to change notification settings - Fork 565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
i#147: Alternate-bitwidth client support #4324
Conversation
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
@hunterbr72 here is Step 1 of solving the alternate-bitwidth. Step 2 is the Windows injection #803. |
… drconfiglib.dll into bin dir on Windows; fix unfilled-in field on query; fix clang build warning
Appveyor's test is not finding the drconfiglib.dll copy:
It passes locally on my machine. Grrr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look at the rest later on today.
@@ -685,15 +800,17 @@ 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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but has this function ever been used to determine the next available client ID? Now client IDs might not be sequential and may skip valid values due to the registration of alternative bitwidth clients. I don't think this is a problem, just an observation. No biggie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next available priority I think you mean. Yes, it is in fact used for that very thing in drdeploy.c. These alt clients end up being treated as regular clients in most respects so I guess it's not really a gap. The interface is a little weird that way but I think it's better than my first attempt which tacked on an alt path as an attribute to a single client, not allowing different options (which was voted for with the -c32/-c64).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'm also referring to Client IDs, assuming alternative bitwidth clients are assigned with the same ID as their regular clients. But like I said, this is just an observation, not an issue.
Can I have a look now? |
Yes, PTAL |
@@ -685,15 +800,17 @@ 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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'm also referring to Client IDs, assuming alternative bitwidth clients are assigned with the same ID as their regular clients. But like I said, this is just an observation, not an issue.
Sorry, had to put in another commit b/c drdeploy was registering alt clients first from a tool file (just easier in that order w/ the code how it was), so shifted it down to solve. That should fix the CI failures. |
Github won't let me reply in the thread so replying here: but a client ID can be anything and doesn't have to be sequential (the priority is what's sequential -- though looking at it now if we did it again I'd probably make them the same thing). |
Grr this ordering enforcement is causing more problems: drdeploy was just appending, which for 64-bit would mean you'd have to pass "-c64 -c32" order. I guess it can insert. |
No order enforcement, or "-c3264" combined arg, was certainly simpler. |
…der; add another test case; fix Windows TCHAR vs char buf
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 option).
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)'
Issue: #147, #803
Fixes: #147