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

Cross-Compiling for Windows breaks error stack traces #12729

Open
nolanderc opened this issue Sep 3, 2022 · 5 comments
Open

Cross-Compiling for Windows breaks error stack traces #12729

nolanderc opened this issue Sep 3, 2022 · 5 comments
Labels
arch-x86_64 64-bit x86 bug Observed behavior contradicts documented or intended behavior os-windows
Milestone

Comments

@nolanderc
Copy link
Contributor

nolanderc commented Sep 3, 2022

Zig Version

0.10.0-dev.3857+10e11b60e

Steps to Reproduce

mkdir zig-test && cd zig-test
zig init-exe

and replace src/main.zig with the following:

const std = @import("std");

pub fn main() !void {
    std.log.info("running on thread {}", .{std.Thread.getCurrentId()});
    return error.Whatever;
}

Then build the program on a non-windows OS (tested using Ubuntu 20.04 through WSL2):

zig build -Dtarget=x86_64-windows

Expected Behavior

The program should terminate with a message containing the error and a stack trace showing where the error originated from. For example, this is the result when on WSL2 using zig build run:

info: running on thread 24663
error: Whatever
/home/christofer/dev/tmp/zig-test/src/main.zig:5:5: 0x20d416 in main (zig-test)
    return error.Whatever;
    ^

Actual Behavior

Attempting to run this program on a Windows machine (tested on Windows 11 build 22000.856) produces the following output:

info: running on thread 32448
error: Whatever
thread 32448 panic: reached unreachable code
Panicked during a panic. Aborting.
@nolanderc nolanderc added the bug Observed behavior contradicts documented or intended behavior label Sep 3, 2022
@nolanderc
Copy link
Contributor Author

I have a hunch that this might be related to #8205, since all sources live in WSL and thus use UNC paths when creating the stack trace.

@andrewrk andrewrk added this to the 0.11.0 milestone Oct 15, 2022
nolanderc added a commit to nolanderc/zig that referenced this issue Apr 14, 2023
Fixes ziglang#8205 and ziglang#12729.

The old method of prepending `\??\` at the start of the DOS path
produced invalid strings for UNC paths (such as those found in WSL).
Instead, we rely on the conversion routine in ntdll, which should handle
all current and future edge cases properly.
Vexu pushed a commit to nolanderc/zig that referenced this issue Apr 23, 2023
Fixes ziglang#8205 and ziglang#12729.

The old method of prepending `\??\` at the start of the DOS path
produced invalid strings for UNC paths (such as those found in WSL).
Instead, we rely on the conversion routine in ntdll, which should handle
all current and future edge cases properly.
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as rooted and resolved against the current drive (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as rooted and resolved against the current drive (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as 'drive relative' and resolved against the CWD (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
Vexu pushed a commit that referenced this issue May 29, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as 'drive relative' and resolved against the CWD (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes #8205
Might close (untested) #12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to #15607
@squeek502
Copy link
Collaborator

squeek502 commented May 29, 2023

Now that #15768 is merged, could anyone using WSL2 test to see if this was fixed by it?

@star-tek-mb
Copy link
Contributor

Now that #15768 is merged, could anyone using WSL2 test to see if this was fixed by it?

Stack traces are still broken like that on WSL2:
???:?:?: 0x7ff77c7d33a2 in ??? (test.exe)

@kcbanner
Copy link
Contributor

I looked into this, and it's happening because the PDB path is of the form /home/kcbanner/temp/12729/zig-cache/o/84727f745cb2d4778303a1b5b8068004/12729.pdb.

After the call to fs.path.resolve in readCoffDebugInfo, path is \home\kcbanner\temp\12729\zig-cache\o\84727f745cb2d4778303a1b5b8068004\12729.pdb. This then fails to open in the call to openFile in Pdb.init.

@kcbanner
Copy link
Contributor

There is an additional problem, even if you do find the PDB by doing something like:

        const pdb_paths = [_][]const u8{
            path,
            fs.path.basename(path),
        };

        di.debug_data = PdbOrDwarf{ .pdb = undefined };
        di.debug_data.pdb = for (pdb_paths) |pdb_path| {
            break pdb.Pdb.init(allocator, pdb_path) catch |err| switch (err) {
                error.FileNotFound, error.IsDir => continue,
                else => return err,
            };
        } else return error.MissingDebugInfo;

All the source paths are also unix-style, ie:

image

So to support this we'd need to have some path-mapping heuristic for source files as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-x86_64 64-bit x86 bug Observed behavior contradicts documented or intended behavior os-windows
Projects
None yet
Development

No branches or pull requests

5 participants