Issue 320150043: i#1349 build drltrace parallel to drstrace: initial commit (Closed)
|
Created:
7 years, 2 months ago by Maxim Shudrak Modified:
7 years, 2 months ago Reviewers:
bruening CC:
drmemory-devs_googlegroups.com Visibility:
Public. |
DescriptionCommit log for first patchset:
---------------
i#1349 build drltrace parallel to drstrace: initial commit
Drltrace copied in drmemory's dir
Drltrace_frontend is created similar to drstrace
Updateds DR to X
---------------
Patch Set 1 #
Total comments: 15
MessagesTotal messages: 9
Several comments to support this commit. https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt File CMakeLists.txt (right): https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt#newcode881 CMakeLists.txt:881: # DRMF_INTERNAL_BUILD is used to notify DynamoRIO that we're building it I added special flag for DR. So if we compile DrMemory in Windows DR's version of drltrace will be disabled. https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt File drltrace/CMakeLists.txt (right): https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newcode5 drltrace/CMakeLists.txt:5: # Dr. Memory: the memory debugger This file was created based on drstrace's version. https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c File drltrace/drltrace.c (right): https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c#newcode56 drltrace/drltrace.c:56: /* XXX i#1349: features to add: Moved from dynamorio's directory, removed only ASSERT and NOTIFY. https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c File drltrace/drltrace_frontend.c (left): https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... drltrace/drltrace_frontend.c:213: char sym_path[MAXIMUM_PATH]; I will add symbols fetching functionality back in future commits when I start work on arguments printing. We don't need all of these now. https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c File drltrace/drltrace_frontend.c (right): https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... drltrace/drltrace_frontend.c:23: /* front end for drltrace tool */ So the frontend was build based on drstrace_frontend. Probably I can just simply include drstrace's version and put all these changes under IS_DRLTRACE flag to avoid duplications. What do you think ?
Sign in to reply to this message.
> Updateds DR to X Spelling error Please use NOCHECKIN to ensure it is updated before commit.
Sign in to reply to this message.
We'd prefer to keep drltrace as BSD. If the only code it will share with drstrace is newer code where we have all the copyrights, this is doable. Please follow the drfuzz license model: use its BSD headers and add drltrace to the exceptions in license.txt.
Sign in to reply to this message.
https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt File CMakeLists.txt (right): https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt#newcode881 CMakeLists.txt:881: # DRMF_INTERNAL_BUILD is used to notify DynamoRIO that we're building it On 2017/02/04 16:59:46, Maxim Shudrak wrote: > I added special flag for DR. So if we compile DrMemory in Windows DR's version > of drltrace will be disabled. See comments on the DR side: if we're moving it, let's fully move it and remove from the DR repo. https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt#newcode1858 CMakeLists.txt:1858: add_subdirectory(drltrace) Should be for Linux too -- just not yet for Mac b/c of the exports iter (put comment about that) https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt File drltrace/CMakeLists.txt (right): https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newcode5 drltrace/CMakeLists.txt:5: # Dr. Memory: the memory debugger On 2017/02/04 16:59:46, Maxim Shudrak wrote: > This file was created based on drstrace's version. Could we instead create macros or functions to share the common pieces? https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... drltrace/CMakeLists.txt:70: use_DynamoRIO_extension(drltracelib drsyscall_static) drltrace shouldn't need drsyscall https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... drltrace/CMakeLists.txt:72: use_DynamoRIO_extension(drltracelib drwrap_static) style: indent off https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... drltrace/CMakeLists.txt:74: add_dependencies(drltracelib drsyscall) shouldn't be needed https://codereview.appspot.com/320150043/diff/1/drltrace/README.md File drltrace/README.md (right): https://codereview.appspot.com/320150043/diff/1/drltrace/README.md#newcode2 drltrace/README.md:2: based on DrMemory framework that records calls to shared libraries. It reports all the Dr. Memory Framework https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c File drltrace/drltrace.c (right): https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c#newcode56 drltrace/drltrace.c:56: /* XXX i#1349: features to add: On 2017/02/04 16:59:46, Maxim Shudrak wrote: > Moved from dynamorio's directory, removed only ASSERT and NOTIFY. Perhaps the commit message should say which files were moved from where to where. https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c File drltrace/drltrace_frontend.c (right): https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... drltrace/drltrace_frontend.c:23: /* front end for drltrace tool */ On 2017/02/04 16:59:46, Maxim Shudrak wrote: > So the frontend was build based on drstrace_frontend. Probably I can just simply > include drstrace's version and put all these changes under IS_DRLTRACE flag to > avoid duplications. What do you think ? We'd prefer a cleaner approach of putting shared code into a library (adding to drfrontendlib, e.g.). Also, we'd prefer to not use any code we don't have copyright over, for BSD: better to look at DR's drdeploy.c or sthg. https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... drltrace/drltrace_frontend.c:107: fprintf(stderr, "-logdir <dir> Specify log directory where library call data\n"); How about having a C++ frontend and using droption.h?
Sign in to reply to this message.
On 2017/02/09 18:07:40, bruening wrote: > https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt > File CMakeLists.txt (right): > > https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt#newcode881 > CMakeLists.txt:881: # DRMF_INTERNAL_BUILD is used to notify DynamoRIO that we're > building it > On 2017/02/04 16:59:46, Maxim Shudrak wrote: > > I added special flag for DR. So if we compile DrMemory in Windows DR's version > > of drltrace will be disabled. > > See comments on the DR side: if we're moving it, let's fully move it and remove > from the DR repo. > > https://codereview.appspot.com/320150043/diff/1/CMakeLists.txt#newcode1858 > CMakeLists.txt:1858: add_subdirectory(drltrace) > Should be for Linux too -- just not yet for Mac b/c of the exports iter (put > comment about that) > > https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt > File drltrace/CMakeLists.txt (right): > > https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newcode5 > drltrace/CMakeLists.txt:5: # Dr. Memory: the memory debugger > On 2017/02/04 16:59:46, Maxim Shudrak wrote: > > This file was created based on drstrace's version. > > Could we instead create macros or functions to share the common pieces? > > https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... > drltrace/CMakeLists.txt:70: use_DynamoRIO_extension(drltracelib > drsyscall_static) > drltrace shouldn't need drsyscall > > https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... > drltrace/CMakeLists.txt:72: use_DynamoRIO_extension(drltracelib drwrap_static) > style: indent off > > https://codereview.appspot.com/320150043/diff/1/drltrace/CMakeLists.txt#newco... > drltrace/CMakeLists.txt:74: add_dependencies(drltracelib drsyscall) > shouldn't be needed > > https://codereview.appspot.com/320150043/diff/1/drltrace/README.md > File drltrace/README.md (right): > > https://codereview.appspot.com/320150043/diff/1/drltrace/README.md#newcode2 > drltrace/README.md:2: based on DrMemory framework that records calls to shared > libraries. It reports all > the Dr. Memory Framework > > https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c > File drltrace/drltrace.c (right): > > https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace.c#newcode56 > drltrace/drltrace.c:56: /* XXX i#1349: features to add: > On 2017/02/04 16:59:46, Maxim Shudrak wrote: > > Moved from dynamorio's directory, removed only ASSERT and NOTIFY. > > Perhaps the commit message should say which files were moved from where to > where. > > https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c > File drltrace/drltrace_frontend.c (right): > > https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... > drltrace/drltrace_frontend.c:23: /* front end for drltrace tool */ > On 2017/02/04 16:59:46, Maxim Shudrak wrote: > > So the frontend was build based on drstrace_frontend. Probably I can just > simply > > include drstrace's version and put all these changes under IS_DRLTRACE flag to > > avoid duplications. What do you think ? > > We'd prefer a cleaner approach of putting shared code into a library (adding to > drfrontendlib, e.g.). Also, we'd prefer to not use any code we don't have > copyright over, for BSD: better to look at DR's drdeploy.c or sthg. > > https://codereview.appspot.com/320150043/diff/1/drltrace/drltrace_frontend.c#... > drltrace/drltrace_frontend.c:107: fprintf(stderr, "-logdir <dir> Specify log > directory where library call data\n"); > How about having a C++ frontend and using droption.h? Ok, I've implemented frontend using droption.h, it works pretty well for Windows and for Linux. However, I have the following error when I try to build drltracelib in Linux drltrace.c:187:13: error: implicit declaration of function ‘cast_to_func’ [-Werror=implicit-function-declaration] app_pc (*indir)(void) = (app_pc (*)(void)) cast_to_func(sym->addr); ^ drltrace.c:187:37: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] app_pc (*indir)(void) = (app_pc (*)(void)) cast_to_func(sym->addr); cc1: all warnings being treated as errors Something related to static dlls linkage process ? I'm building drltracelib with the following libraries: use_DynamoRIO_extension(drltracelib drmgr_static) use_DynamoRIO_extension(drltracelib drwrap_static) use_DynamoRIO_extension(drltracelib drx_static) use_DynamoRIO_extension(drltracelib drsyms_static)
Sign in to reply to this message.
On 2017/02/18 14:56:00, Maxim Shudrak wrote: > Ok, I've implemented frontend using droption.h, it works pretty well for Windows > and for Linux. For staging the commits, it might be cleanest to first commit the code in an identical or nearly-identical state to what it looked like in the DR repo (even if the only way to do that is to have it not build or run, if it's hard to make it build or run: just don't enable in cmake yet), so we have nice history of what changed in one repo without having to do a manual diff. Seem reasonable? > However, I have the > following error when I try to build drltracelib in Linux > drltrace.c:187:13: error: implicit declaration of function ‘cast_to_func’ > [-Werror=implicit-function-declaration] > app_pc (*indir)(void) = (app_pc (*)(void)) cast_to_func(sym->addr); > ^ > drltrace.c:187:37: error: cast to pointer from integer of different size > [-Werror=int-to-pointer-cast] > app_pc (*indir)(void) = (app_pc (*)(void)) cast_to_func(sym->addr); > cc1: all warnings being treated as errors > > Something related to static dlls linkage process ? Isn't that just a utility func in the DR repo that's not in the DrM repo?
Sign in to reply to this message.
On 2017/02/20 19:35:17, bruening wrote: > On 2017/02/18 14:56:00, Maxim Shudrak wrote: > > Ok, I've implemented frontend using droption.h, it works pretty well for > Windows > > and for Linux. > > For staging the commits, it might be cleanest to first commit the code in an > identical or nearly-identical state to what it looked like in the DR repo (even > if the only way to do that is to have it not build or run, if it's hard to make > it build or run: just don't enable in cmake yet), so we have nice history of > what changed in one repo without having to do a manual diff. Seem reasonable? > > > However, I have the > > following error when I try to build drltracelib in Linux > > drltrace.c:187:13: error: implicit declaration of function ‘cast_to_func’ > > [-Werror=implicit-function-declaration] > > app_pc (*indir)(void) = (app_pc (*)(void)) > cast_to_func(sym->addr); > > ^ > > drltrace.c:187:37: error: cast to pointer from integer of different size > > [-Werror=int-to-pointer-cast] > > app_pc (*indir)(void) = (app_pc (*)(void)) > cast_to_func(sym->addr); > > cc1: all warnings being treated as errors > > > > Something related to static dlls linkage process ? > > Isn't that just a utility func in the DR repo that's not in the DrM repo? Ok, I will prepare clean commit of drltrace. Yes, that was my problem, fixed. Now I have the following error when I try to start drltrace with ping: <Starting application /bin/ping (54512)> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol f_global> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol reported_disk_error> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol tls_idx_util> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol op_print_stderr> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol op_ignore_asserts> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol f_results> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol print_prefix_to_console> <WARNING! symbol lookup error: libdrltracelib.so undefined symbol drmemory_abort> <Paste into GDB to debug DynamoRIO clients: set confirm off add-symbol-file '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so' 0x0000000073807720 add-symbol-file '/home/osboxes/drmemory_clean/build/dynamorio/lib64/debug/libdynamorio.so' 0x00005578dd242a88 > <Initial options = -no_dynamic_options -client_lib '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so;0;-logdir .' -code_api -stack_size 56K -max_elide_jmp 0 -max_elide_call 0 -early_inject -emulate_brk -no_inline_ignored_syscalls -native_exec_default_list '' -no_native_exec_managed_code -no_indcall2direct > drltrace log file is ./drltrace.ping.54512.0000.log <Application /bin/ping (54512). Dr. LTrace internal crash at PC 0x0000000073807fe7. Please report this at http://drmemory.org/issues. Program aborted. Received SIGSEGV at client library pc 0x0000000073807fe7 in thread 54512 Base: 0x00005578dd216000 Registers:eax=0x0000000000000000 ebx=0x00005578dd850270 ecx=0x0000000053ab83a0 edx=0x00005578dd81f8c0 esi=0x0000000000000001 edi=0x0000000053aa1450 esp=0x00007ffefda12190 ebp=0x00007ffefda12220 r8 =0x0000000000000000 r9 =0x0000000000000000 r10=0x0000000053ab6b58 r11=0x0000000053b23e70 r12=0x0000000000000000 r13=0x0000000000000000 r14=0x0000000000000000 r15=0x0000000000000000 eflags=0x0000000000010202 version 6.2.17200, build 17411 -no_dynamic_options -client_lib '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so;0;-logdir .' -code_api -stack_size 56K -max_elide_jmp 0 -max_elide_call 0 -early_inject -emulate_brk -no_inline_ignored_syscalls -native_exec_default_list '' -no_native_exec_managed_code -no_indcall2direct 0x00007ffefda12220 0x000000007380852e 0x00007ffefda12250 0x000000007380b48d 0x00007ffefda124b0 0x00005578dd42d33c 0x00007ffefda12520 0x00005578dd42a216 0x00007ffefda12560 0x00005578dd2a728f 0x00007ffefda12da0 0x00005578dd51e272 0x00007ffefda137f0 0x00005578dd4d3044 /home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so=0x0000000073800000> It seems that I need to start debugging :)
Sign in to reply to this message.
On 2017/02/20 20:24:00, Maxim Shudrak wrote: > On 2017/02/20 19:35:17, bruening wrote: > > On 2017/02/18 14:56:00, Maxim Shudrak wrote: > > > Ok, I've implemented frontend using droption.h, it works pretty well for > > Windows > > > and for Linux. > > > > For staging the commits, it might be cleanest to first commit the code in an > > identical or nearly-identical state to what it looked like in the DR repo > (even > > if the only way to do that is to have it not build or run, if it's hard to > make > > it build or run: just don't enable in cmake yet), so we have nice history of > > what changed in one repo without having to do a manual diff. Seem reasonable? > > > > > However, I have the > > > following error when I try to build drltracelib in Linux > > > drltrace.c:187:13: error: implicit declaration of function ‘cast_to_func’ > > > [-Werror=implicit-function-declaration] > > > app_pc (*indir)(void) = (app_pc (*)(void)) > > cast_to_func(sym->addr); > > > ^ > > > drltrace.c:187:37: error: cast to pointer from integer of different size > > > [-Werror=int-to-pointer-cast] > > > app_pc (*indir)(void) = (app_pc (*)(void)) > > cast_to_func(sym->addr); > > > cc1: all warnings being treated as errors > > > > > > Something related to static dlls linkage process ? > > > > Isn't that just a utility func in the DR repo that's not in the DrM repo? > > Ok, I will prepare clean commit of drltrace. Yes, that was my problem, fixed. > Now I have the following error when I try to start drltrace with ping: > > <Starting application /bin/ping (54512)> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol f_global> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol > reported_disk_error> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol tls_idx_util> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol > op_print_stderr> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol > op_ignore_asserts> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol f_results> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol > print_prefix_to_console> > <WARNING! symbol lookup error: libdrltracelib.so undefined symbol > drmemory_abort> > <Paste into GDB to debug DynamoRIO clients: > set confirm off > add-symbol-file > '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so' > 0x0000000073807720 > add-symbol-file > '/home/osboxes/drmemory_clean/build/dynamorio/lib64/debug/libdynamorio.so' > 0x00005578dd242a88 > > > <Initial options = -no_dynamic_options -client_lib > '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so;0;-logdir .' > -code_api -stack_size 56K -max_elide_jmp 0 -max_elide_call 0 -early_inject > -emulate_brk -no_inline_ignored_syscalls -native_exec_default_list '' > -no_native_exec_managed_code -no_indcall2direct > > drltrace log file is ./drltrace.ping.54512.0000.log > <Application /bin/ping (54512). Dr. LTrace internal crash at PC > 0x0000000073807fe7. Please report this at http://drmemory.org/issues. Program > aborted. > Received SIGSEGV at client library pc 0x0000000073807fe7 in thread 54512 > Base: 0x00005578dd216000 > Registers:eax=0x0000000000000000 ebx=0x00005578dd850270 ecx=0x0000000053ab83a0 > edx=0x00005578dd81f8c0 > esi=0x0000000000000001 edi=0x0000000053aa1450 esp=0x00007ffefda12190 > ebp=0x00007ffefda12220 > r8 =0x0000000000000000 r9 =0x0000000000000000 r10=0x0000000053ab6b58 > r11=0x0000000053b23e70 > r12=0x0000000000000000 r13=0x0000000000000000 r14=0x0000000000000000 > r15=0x0000000000000000 > eflags=0x0000000000010202 > version 6.2.17200, build 17411 > -no_dynamic_options -client_lib > '/home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so;0;-logdir .' > -code_api -stack_size 56K -max_elide_jmp 0 -max_elide_call 0 -early_inject > -emulate_brk -no_inline_ignored_syscalls -native_exec_default_list '' > -no_native_exec_managed_code -no_indcall2direct > 0x00007ffefda12220 0x000000007380852e > 0x00007ffefda12250 0x000000007380b48d > 0x00007ffefda124b0 0x00005578dd42d33c > 0x00007ffefda12520 0x00005578dd42a216 > 0x00007ffefda12560 0x00005578dd2a728f > 0x00007ffefda12da0 0x00005578dd51e272 > 0x00007ffefda137f0 0x00005578dd4d3044 > /home/osboxes/drmemory_clean/build/bin64/debug/libdrltracelib.so=0x0000000073800000> > > It seems that I need to start debugging :) Fixed, remove drsyscall_static and everything started work correctly.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
