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

Locked PDB file blocks GDExtension compilation when Editor is launched within debugging #82536

Closed
kkolyan opened this issue Sep 29, 2023 · 14 comments · Fixed by #87117
Closed

Comments

@kkolyan
Copy link
Contributor

kkolyan commented Sep 29, 2023

Godot version

v4.2.dev (4c3dc26)

System information

Godot v4.2.dev (4c3dc26) - Windows 10.0.19042 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1660 Ti (NVIDIA; 31.0.15.3713) - Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz (12 Threads)

Issue description

When Godot Editor is launched within debugging session, it locks PDB file, preventing GDextension compilation

Very similar issue with DLL blocking was fixed recently in this PR: #80188. That works fine when Editor launched without debugger, but with debugger it also blocks PDB. Could we copy it the same way?

Steps to reproduce

  1. checkout godot master branch
  2. compile it with scons dev_build=yes debug_symbols=yes platform=windows compiledb=yes -j8
  3. get some gdextension. dodge-the-creeps example from gdext is fine, then git clone https://github.com/godot-rust/gdext.git && cd gdext/examples/dodge-the-creeps/rust
  4. compile this gdextension cargo build --package dodge-the-creeps --lib
  5. start editor with attached debugger (I used CLion) path_to_godot_working_copy\bin\godot.windows.editor.x86_64.exe --editor --path ../godot (that's path from rust dir to gdext/examples/dodge-the-creeps/godot)
  6. wait for scene loading. it should load fine.
  7. open ./src/player.rs (gdext/examples/dodge-the-creeps/rust/src/player.rs) in any texty editor and append following line to the end: impl Player { fn hello() {} }
  8. again compile cargo build --package dodge-the-creeps --lib --features custom-godot
  9. get error: note: LINK : fatal error LNK1201: error writing to program database 'C:\dev\reload_lab\gdext\target\debug\deps\dodge_the_creeps.pdb'; check for insufficient disk space, invalid path, or insufficient privilege

Minimal reproduction project

N/A

@jsjtxietian
Copy link
Contributor

jsjtxietian commented Sep 30, 2023

Ah I remembered that Casey Muratori solved this problem by passing the some pdb related parameter to cl (%random% to make pdb has a random name everytime) when recompiling dll. When vs loads the new DLL, it loads the new PDB file as well.

@kkolyan
Copy link
Contributor Author

kkolyan commented Oct 1, 2023

Here is workaround based on the previous advice.

pdb_safe_build.cmd:

set "pdb_path=c:\dev\godot\fmandate-gd\rust\target\fmandate_%RANDOM%_%RANDOM%.pdb"
echo %pdb_path% > target\pdb_path.txt
cargo build
copy target\debug\fmandate.pdb %pdb_path%

build.rs:

use std::fs;

fn main() {
    //
    if let Ok(pdb_path) = fs::read_to_string("target/pdb_path.txt") {
        println!("cargo:rustc-link-arg=/PDBALTPATH:{}", pdb_path);
    }
}

@dsnopek
Copy link
Contributor

dsnopek commented Oct 2, 2023

I don't use Visual Studio (just the command line tools sometimes), so I don't know much about how .pdb files work. Would it be enough to just copy the .pdb file the same as we do with the .dll file (adding ~ in front of it)? Or, would that cause problems because Visual Studio won't know how to find the .pdb file? How does Visual Studio know how to find the .pdb file associated with a .dll?

If VS is finding the .pdb file simply by looking for a file named just like the .dll, such that copying it would work, I think it'd be fine for Godot to copy it if it's present. But I don't know if there's more to it than that.

@vnen
Copy link
Member

vnen commented Oct 19, 2023

Would it be enough to just copy the .pdb file the same as we do with the .dll file (adding ~ in front of it)?

I don't think that would help, AFAIK the PDB path is part of the binary so this would only work if we somehow patched the DLL, which I don't think it is feasible.

I'm not sure what the solution for this would be, if there's even anything that Godot can do about it.

@dsnopek
Copy link
Contributor

dsnopek commented Oct 19, 2023

Perhaps then this is a documentation thing for folks using Visual Studio?

The code snippets folks posted above seem to configure Visual Studio to use a random name for the .pdb file on each compile. Perhaps putting those in the docs somewhere would be sufficient?

(Do those .pdb files with random names get cleaned up eventually, though? Or, does the directory slowly fill up with more and more .pdb files? If so, those code snippets may need some additional improvements, perhaps attempting to clean up all the .pdb files that aren't locked?)

@kkolyan
Copy link
Contributor Author

kkolyan commented Oct 20, 2023

Do those .pdb files with random names get cleaned up eventually, though? Or, does the directory slowly fill up with more and more .pdb files?

they slowly fill up. though, in is generated in the working directory and the use case is pretty advanced, I think user will not be taken by surprise with this behavior...

khm, actually, it could be easily enhanced - just try to remove all PDBs by pattern and ignore failure of the active PDB removal attempt.

@DmitriySalnikov
Copy link
Contributor

As vnen said, the path to the PDB is written inside the DLL.
image
Therefore, simply renaming them via Godot will not work. You need to change the build scripts.

I solved this at the scons script level for godot-cpp. I think something similar needs to be implemented when building gdext.

I can make these changes to the example project in godot-cpp (and maybe we need to update the script from the documentation).

@kone9
Copy link

kone9 commented Dec 10, 2023

As vnen said, the path to the PDB is written inside the DLL. image Therefore, simply renaming them via Godot will not work. You need to change the build scripts.

I solved this at the scons script level for godot-cpp. I think something similar needs to be implemented when building gdext.

I can make these changes to the example project in godot-cpp (and maybe we need to update the script from the documentation).

##HHere you have a Scontructor rewritten by you that works correctly in Windows 11, but in VScode with Scons, thank you very much.

https://gitlab.com/kone9/godot-engine-4.2-template-gdextension-with-vscode-windows-11/-/blob/main/SConstruct?ref_type=heads

And don't forget to put reloadable=true in the "gdexample.gdextension" file in the [configuration] part. Here is an example of that file.

[configuration]

entry_symbol = "example_init"
compatibility_minimum = "4.2"
reloadable = true

https://gitlab.com/kone9/godot-engine-4.2-template-gdextension-with-vscode-windows-11/-/blob/main/Godot_proyect/bin/gdexample.gdextension?ref_type=heads
Captura de pantalla 2023-12-10 181027

and here is a template with preconfigured Vscode where you can verify correct operation in Windows 11.

https://gitlab.com/kone9/godot-engine-4.2-template-gdextension-with-vscode-windows-11

@vnen
Copy link
Member

vnen commented Dec 15, 2023

I thought just occurred to me that the Windows API has some stuff to handle the PE format (which includes DLLs), so it might be possible to actually patch the copied DLL and change its PDB. This is kind of like how rcedit works, though itself doesn't provide anything about PDB. I'm not sure if it is a regular resource string, but if it is then it's possible to edit.

Naturally, this would be Windows only since it relies on the Windows API, but this problem happens only on Windows anyway.

I'll try to test this later.

@DmitriySalnikov
Copy link
Contributor

DmitriySalnikov commented Dec 15, 2023

The path to the PDB is stored somewhere at the end of the file with the label RSDS. https://godevtool.com/Other/pdb.htm
I have checked several of my C++ and C# DLLs and even Rust DLLs and in them the label RSDS is either present only once or missing.


It looks like you can safely replace the name of the PDB by editing the DLL file. The debugger finds the PDB by its new name and blocks it! So the breakpoints are working and the values of the variables are visible.
image
image

But most likely you cannot change the length of the original string, usually this should not be a problem. And I do not know what is between the RSDS and the path string, and whether the size of this gap can vary (found this).

Also, information about RSDS can be obtained via dumpbin /headers <path-to-dll> | findstr RSDS

@vnen
Copy link
Member

vnen commented Dec 15, 2023

I tested and it's not included in the resources, so my original idea can't really work.

However, as @DmitriySalnikov points out, this information is in the headers of the DLL. We can do a similar thing to figure out where the specific header is and patch it. I've found this project which removes the PDB path and I believe something similar can be done to replace it instead.

This is a complex approach, but I think it's safer than trying to patch the binary directly at some arbitrary address.

@DmitriySalnikov
Copy link
Contributor

I made my project to test the idea of replacing the PDB path inside the DLL (and EXE..) https://github.com/DmitriySalnikov/RenameAndCopyPDB
Action

This project copies the DLL with the prefix ~ and also creates new PDBs up to *_999.pdb and deletes old files except the newest one based on the time of change. Supports UTF (tested with the name libdd3d.windows.editor.x86_64_юникод_çã.dll ).

Limitations:

  • The bitness of the DLL should be like that of the program.
  • The length of the PDB string inside the DLL must be greater than 8 characters to use the prefix and 4 additional characters. This should not be a problem because an absolute path is usually specified.

Testing:

Untitled.mp4

Is it worth trying to port this to Godot?

@dsnopek
Copy link
Contributor

dsnopek commented Dec 22, 2023

@DmitriySalnikov:

Is it worth trying to port this to Godot?

The code in your test project doesn't seem overly large or complex. Personally, I think it'd be worth try to port to Godot.

This feature is only useful to MSVC users, but it seems like the impact would be minimal, both as far as code maintenance (since it looks pretty simple) and performance (since this only needs to be attempted once per reloadable GDExtension on editor startup), so I'd be for it.

@DmitriySalnikov
Copy link
Contributor

@kkolyan can you check if #87117 fixes your problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment