Skip to content
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

Skip path conversion for strace arguments #226

Open
atrigent opened this issue Aug 27, 2024 · 6 comments
Open

Skip path conversion for strace arguments #226

atrigent opened this issue Aug 27, 2024 · 6 comments

Comments

@atrigent
Copy link

atrigent commented Aug 27, 2024

There is already a special case for disabling path conversion with strace in the patch series (916afdc), but it only applies to environment variables. The message on that commit seems to be acknowledging that conversion of arguments is also problematic, so I'm not sure why it doesn't disable that as well.

@dscho
Copy link
Collaborator

dscho commented Aug 27, 2024

The message on that commit seems to be acknowledging that conversion of arguments is also problematic, so I'm not sure why it doesn't disable that as well.

The commit message says that:

Strace is a Windows program [...]

This means that it does not understand any Unix-like paths passed to it as arguments. For example, this invocation would not do what you think it should:

$ strace /usr/bin/uname -a

Because strace is a Windows program, it would interpret /usr/bin/uname as a regular Windows path, i.e. equivalent to \usr\bin\uname, which is an absolute path on the current drive. In most instances, therefore, it would resolve to C:\usr\bin\uname, which typically does not exist.

The only reason why it does work is that the MSYS2 runtime converts the arguments automatically, so that strace never sees the Unix-style path.

Now, why is the environment such a different beast?

The reason is that strace itself does not use the environment variables. But its purpose is to launch MSYS programs which do, and which also understand Unix-like paths such as /usr/bin/uname. Therefore, above-mentioned commit strikes me as trying to avoid unnecessary (and potentially error-prone) back-and-forth conversion of the environment. The same isn't an issue for the arguments, they won't be converted back from Windows paths to Unix paths.

@atrigent
Copy link
Author

The only reason why it does work is that the MSYS2 runtime converts the arguments automatically, so that strace never sees the Unix-style path.

Don't you think you should double-check this before saying it? And how exactly are you imagining it works in Cygwin?

https://github.com/mirror/newlib-cygwin/blob/fe2545e9faaf4bf9586f61a7b83d5cb5af501194/winsup/utils/mingw/strace.cc#L344-L345

converted back from Windows paths to Unix paths

Please read https://www.msys2.org/docs/filesystem-paths/#windows-unix-path-conversion

@dscho
Copy link
Collaborator

dscho commented Aug 27, 2024

Don't you think you should double-check this before saying it? And how exactly are you imagining it works in Cygwin?

https://github.com/mirror/newlib-cygwin/blob/fe2545e9faaf4bf9586f61a7b83d5cb5af501194/winsup/utils/mingw/strace.cc#L344-L345

I stand corrected regarding the first parameter (*argv, which is strace.exe itself).

I do have admit that I wrote this all from memory and did not try this out (you could, if you wanted, using MSYS2_ARG_CONV_EXCL).

converted back from Windows paths to Unix paths

Please read https://www.msys2.org/docs/filesystem-paths/#windows-unix-path-conversion

It is admittedly not documented very well, but there are environment variables that are converted from Windows to Unix paths when an MSYS program is called from a Windows program.

@dscho
Copy link
Collaborator

dscho commented Aug 27, 2024

And how exactly are you imagining it works in Cygwin?

Sorry, I forgot to answer that question: Cygwin plays it safe and does the double-conversion, I expect. I.e. when running strace from a Cygwin Bash (or any other Cygwin process), it converts the subset of the environment that Cygwin cares about (and unlike MSYS2 not all of the environment variables, guessing whether their values are Unix paths or Unix path lists) from Unix to Windows paths, then strace spawns the Cygwin process which means that the reverse conversion happens.

@atrigent
Copy link
Author

atrigent commented Aug 27, 2024

I stand corrected regarding the first parameter (*argv, which is strace.exe itself).

My dude, I beg of you, please read the code. Literally just read the function name. This is the code for launching the child process. It is not modifying its own argv[0].

It does a similar conversion for handling the output file path:

https://github.com/mirror/newlib-cygwin/blob/fe2545e9faaf4bf9586f61a7b83d5cb5af501194/winsup/utils/mingw/strace.cc#L1123

@dscho
Copy link
Collaborator

dscho commented Aug 27, 2024

I stand corrected regarding the first parameter (*argv, which is strace.exe itself).

[...] This is the code for launching the child process. It is not modifying its own argv[0].

Oh right! And it also makes sense because we're looking at the Cygwin code directly, without the MSYS2-specific modifications. And Cygwin does not convert the command-line arguments, therefore its strace.exe obviously has to do that itself.

Back to your question, though, whether MSYS2's strace should be exempt from the argument conversion: It does not really matter, does it, whether the MSYS2 runtime does it or strace?

I guess, theoretically, that something like strace ssh.exe some-host /usr/bin/sh would benefit from not converting the /usr/bin/sh argument. But it is most likely as easy to construe examples where the conversion would be desirable (see e.g. this comment about strace mingw32-make.exe for inspiration)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants