Skip to content

Add cygwin support #704

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Berrysoft
Copy link

There's no OsString::from_wide on cygwin because it is treated as unix. I choose WideCharToMultiByte.

@Berrysoft
Copy link
Author

@alexcrichton Could you review this PR?

@Berrysoft
Copy link
Author

@workingjubilee Could you review this PR, then...?

@Berrysoft
Copy link
Author

@ChrisDenton Could you review this PR?

@Djihads80
Copy link

can you guys review this rp? pweaaatyyy pweeeeaaaseee with a chewwy on top 🍒🥺

@Berrysoft
Copy link
Author

Seems that everyone is going on holidays:(

@mati865
Copy link

mati865 commented Apr 27, 2025

I'm not sure if this is the correct way to solve the problem. This will print Windows style paths, right?
Using APIs provided by Cygwin should be always preferred over calling Win32 APIs.

Is there no way to hook up implementation that other UNIXes use?

@Berrysoft
Copy link
Author

The path style depends on the compiler. If it's cross-compiled, the path is windows style. Otherwise it's unix style.

@Berrysoft
Copy link
Author

Using APIs provided by Cygwin should be always preferred over calling Win32 APIs.

So where is cygwin api to get the dependency dlls' names? Here we use winapi to get the names only, not the paths.

@mati865
Copy link

mati865 commented Apr 27, 2025

The path style depends on the compiler. If it's cross-compiled, the path is windows style. Otherwise it's unix style.

If you call Win32 APIs, they'll return Windows style paths for files.
I think you are mixing host and target paths. Windows host compiler will use Windows style paths, but binaries built for Cygwin (either host or target) will use Cygwin style paths.

So where is cygwin api to get the dependency dlls' names?

I don't know Cygwin APIs, maybe @jeremyd2019 could answer that?

Here we use winapi to get the names only, not the paths.

Ah, that wouldn't be a big deal then, just a small divergence from the Cygwin way.

Can you share a backtrace from binary built for Cygwin that panics?

@jeremyd2019
Copy link

jeremyd2019 commented Apr 27, 2025

I don't understand exactly what you're asking. Cygwin has dladdr which will give you a (Cygwin) path to a DLL given an address within it. I don't know of any Cygwin way to enumerate loaded modules though... It has that information, but I don't see an API to access it.

We could ask Cygwin to add one, especially if there's some POSIX way to do that that they're not implementing yet.

@mati865
Copy link

mati865 commented Apr 27, 2025

I don't know of any Cygwin way to enumerate loaded modules though... It has that information, but I don't see an API to access it.

IIUC that's what we need. We can stick to Win32 APIs then.

@jeremyd2019
Copy link

Here we use winapi to get the names only, not the paths.

I think you're wrong there. szExePath is converted, that's the full path. szModuleName is just the name. Also, that path is then passed to mmap, presumably the Cygwin mmap, so it should probably be getting the Cygwin path. Ideas:

  1. cygwin_conv_path(CCP_WIN_W_TO_POSIX, ... Paths are not 100% guaranteed to be straight converted to valid UTF-8. There are degenerate cases where something else happens.
  2. Pass the modBaseAddr or hModule into dlinfo and let that handle the conversion. That might be a little more wasteful as it would get the module file name again when that's already in the MODULEENTRY32W struct, but maybe easier to get at dlinfo than cygwin_conv_path since that's a more standard function.
  3. look at /proc/self/maps ? The unix side does that, but in combination with what seems to be an ELF-specific method of enumerating modules (which is probably why Cygwin doesn't provide it)

@jeremyd2019
Copy link

@Berrysoft
Copy link
Author

I didn't find dlinfo method in cygwin. I will use cygwin_conv_path since it's straightforward. It's OK to be not UTF-8 because we are using OsString.

@jeremyd2019
Copy link

jeremyd2019 commented Apr 28, 2025

@Berrysoft
Copy link
Author

I'm not sure if dladdr accepts a HMODULE. Anyway it also uses cygwin_conv_path: https://github.com/cygwin/cygwin/blob/d9e9405becbac3c8bc0c9f23f822fb2339990fab/winsup/cygwin/dlfcn.cc#L430-L431

And I even think the code of cygwin above is a little buggy, considering that a UTF-16 path with length MAX_PATH might be converted to a POSIX path that is longer than MAX_PATH (e.g., the UTF-16 path is full of CJK characters)...

@Berrysoft
Copy link
Author

I fixed the path conversion. Now the path is converted by cygwin_conv_path and the buffer length is handled correctly.

I also updated libc version to 0.2.171. It's the minimum version that support cygwin.

@jeremyd2019
Copy link

I'm not sure if dladdr accepts a HMODULE. Anyway it also uses cygwin_conv_path: https://github.com/cygwin/cygwin/blob/d9e9405becbac3c8bc0c9f23f822fb2339990fab/winsup/cygwin/dlfcn.cc#L430-L431

An HMODULE is the base address of the module. dladdr takes an address within the module, the HMODULE of the module is the lowest such address. But I only suggested it because I wasn't sure you had an easy way to call cygwin_conv_path directly from rust (it rather unsafely even for C takes different types depending on the enum value given, so for a "safe" language like rust it's probably a real headache). Since you apparently can call cygwin_conv_path, that's best.

And I even think the code of cygwin above is a little buggy, considering that a UTF-16 path with length MAX_PATH might be converted to a POSIX path that is longer than MAX_PATH (e.g., the UTF-16 path is full of CJK characters)...

Yeah, I see, the array in the DL_info struct is actually PATH_MAX not MAX_PATH, it should probably be passing that to cygwin_conv_path instead, and that should be plenty big to hold a MAX_PATH long UTF-16 string in any supported multibyte encoding. I'll look at sending Cygwin a patch for that.

@Berrysoft
Copy link
Author

Thanks for review from @mati865 & @jeremyd2019 ! However it still needs a maintainer to review...

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

Successfully merging this pull request may close these issues.

4 participants