hasherezade / tiny_tracer

A Pin Tool for tracing API calls etc
1.25k stars 138 forks source link

Improve building steps of pintool #16

Closed JuliaPoo closed 2 years ago

JuliaPoo commented 2 years ago

The current toolchain is rather cumbersome and manual. Since tiny_tracer only targets Windows, the toolchain could be moved into a Visual Studio Solution outside of the Pin directory. I can make a pull request later as a proposal of how it might look like.

Doing this could also move the compilation of the helper binaries: dll_load32.exe, dll_load64.exe, kdb_check.exe, pe_check.exe, into the Visual Studio Solution.

Somewhat related, is there a reason for using pe_check instead of PINTOOL's -t and -t64 options?

hasherezade commented 2 years ago

I respect your opinion, however I am not agree that the current build is cumbersome. Maybe you saw it on my old Youtube video from 2018, which is very obsolete. In current version it is much easier, and, if you don't change the default paths, and have Visual Studio already installed, it takes exactly 4 steps:

  1. Clone the directory into your Pin tools
  2. Open the Visual Studio Project, and compile 32 and 64 bit version
  3. Run the script move_dlls.bat
  4. Run add_menu.reg

Most of the users with whom I discussed also feel that the whole process is very easy at the point. Of course it could be yet easier if I was allowed to ship compiled binaries - then I would make an automated installer for the whole thing. But since the license does not allow it, the options are limited.

Compiling the helper binaries along with the tool makes just no sense, because they are not a part of the tool itself, also they don't change each release, but very rarely. Additionally, they are independent entities, used in other projects as well.

I also don't see any real benefits of compiling the projects outside of the Pin directory - I think having all the tools in one place makes it actually more handy. But of course we have different opinions in those things, as they are a matter of taste.

Maybe I can change my mind by seeing the alternative solution that you propose, so I am open to discuss it further.

hasherezade commented 2 years ago

Regarding the options -t and -t64 - from what I see they allows to just pass the (previously selected) tool to the execution, i.e.

-t64  [default ]
    Specify tool binary for Intel(R) 64 architecture

So, form what I understand, you need to know the target's architecture prior to using those options.

While pe_check is an app that checks the bitness of the target, and is used to decide what should be a configuration variant selected, including which DLL to pass it. How exactly do you envision replacing pe_check with those options? Can you write an exact commandline that would have exactly the same functionality as the current one, with pe_check? If it works better, of course I will use it instead.

JuliaPoo commented 2 years ago

Sorry for the late reply, I missed the email notification. I think having an automatic installer is a good idea, and I can't find where in the license that disallows doing so. I'm referring to PIN 3.21, which has some changes in licensing from 3.18, so that might be what you're referring to? I'd admit I don't have the habit of looking through licenses.

Another reason for moving it out of the pin directory is to separate this repo's code from PIN stuff, but I can see the benefits of having all your pintools in one place.

Regarding the options -t and -t64, I'm aware of what they stand for, though it is implied that pin would try to detect if the target is 32bit or 64bit (it has to decide which to use for -follow_execv afterall).

I ran a few experiments on PIN 3.21

:: Uses tool64.dll and launches test64.exe (correct)
pin -t64 tool64.dll -t tool32.dll -- test64.exe 

:: Uses tool32.dll and launches test32.exe (correct)
pin -t64 tool64.dll -t tool32.dll -- test32.exe

:: Uses tool32.dll and launches C:\Windows\SysWow64\notepad.exe (wrong)
pin -t64 tool64.dll -t tool32.dll -- C:\Windows\System32\notepad.exe

:: Uses tool32.dll and launches C:\Windows\SysWow64\notepad.exe (correct)
pin -t64 tool64.dll -t tool32.dll -- C:\Windows\SysWow64\notepad.exe

:: Uses tool64.dll and launches C:\Windows\System32\notepad.exe (correct)
pin -t tool64.dll -- C:\Windows\System32\notepad.exe

:: Uses tool32.dll and launches C:\Windows\SysWow64\notepad.exe (wrong)
pin -t tool32.dll -- C:\Windows\System32\notepad.exe

Which I think, means that while pin does know which tool to use, it has some trouble parsing the path of the target binary. I'm unsure if this is intended behavior on PIN's end, but it does mean that pe_check.exe is still necessary.

My main concern with using the pe_check.exe is that -follow_execv would be broken in cases when the target process launches a child with different "bitness", since pin doesn't know the existence of both pintools.

hasherezade commented 2 years ago

@JuliaPoo - thank you, I actually missed that you can pass both tools to the pin. I fixed it now. Regarding the license, I will review the updates, and in case if distribution of the compiled binaries is now permitted, I will make an installer.

JuliaPoo commented 2 years ago

The current fix fails for the case of System32 and SysWow64 where the target binary is 64bit but there is a 32bit binary in the paths with the same name, and this is despite providing PIN the full path to the 64bit target. E.g. pin -t64 tool64.dll -t tool32.dll -- C:\Windows\System32\notepad.exe is expected to launch C:\Windows\System32\notepad.exe but it instead launches C:\Windows\SysWow64\notepad.exe.

I'm convinced this is a bug on PIN's side (tested with 3.21 and 3.18), and this would not have been encountered in the previous solution with pe_check.exe.

So it's either:

  1. Have tiny_tracer fail at the System32 and SysWow64 case (current solution)
  2. Have tiny_tracer fail at follow_execv where the child process is a different bitness. (previous solution).
hasherezade commented 2 years ago

Thanks for checking. In such case, I would rather choose 2) and rollback the changes.

JuliaPoo commented 2 years ago

Nice! Since this issue is settled, I'll be closing it.