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

Pathes in depfiles are not correctly escaped #6570

Open
pborsutzki opened this issue Apr 29, 2024 · 4 comments
Open

Pathes in depfiles are not correctly escaped #6570

pborsutzki opened this issue Apr 29, 2024 · 4 comments
Assignees
Labels
bug Bug, regression, crash
Milestone

Comments

@pborsutzki
Copy link

Description
When generating a depfile through any of the -M* switches, the generated output does not contain escaped pathes. Due to the depfile format (output: input0 input1 ...) this can lead to ambiguities in tools that parse these files like the Ninja build system.

Steps to Reproduce

  • Use a Windows machine.
  • Setup a valid source file, e.g.:
[numthreads(1,1,1)]
void main() {}
  • Compile any file using DXC with an -M* switch while specifying an absolute path to the source file, e.g. dxc -T cs_6_0 -M e:\foo.hlsl.
  • Get this output: e:\foo.hlsl: e:\foo.hlsl

Actual Behavior

DXC returns this result: e:\foo.hlsl: e:\foo.hlsl

This result is ambiguous due to the additional colons :. To me, the format of depfiles does not seem to be clearly specified (it seems to be specified "like makefiles"). But other tools (e.g. Ninja build, Slang) avoid the ambiguity by escaping/expecting escaped pathes in depfiles. So a correct output in my opinion would be this:
e\:\\foo.hlsl: e\:\\foo.hlsl

More problems can occur when using other problematic characters for makefiles like whitespace, #, $, \, [ and ].

Environment

  • DXC version 1.7.2308
  • Host Operating System Windows 10 Enterprise 22H2
@pborsutzki pborsutzki added bug Bug, regression, crash needs-triage Awaiting triage labels Apr 29, 2024
@damyanp damyanp removed the needs-triage Awaiting triage label Apr 29, 2024
@damyanp
Copy link
Member

damyanp commented Apr 29, 2024

Assigned to @pborsutzki as they have a PR out for this already. It'd be good to double check what Clang does with this as well.

@damyanp damyanp added this to the Dormant milestone Apr 29, 2024
@llvm-beanz
Copy link
Collaborator

For a trivial example on Windows clang:

#include "test.h"
#include "test space.h"

The output in the dep file is:

test.o: C:\Users\cbieneman\dev\scratch\test.cpp \
  C:\Users\cbieneman\dev\scratch\test.h \
  C:\Users\cbieneman\dev\scratch\test\ space.h

That escapes the space, but not the other slashes. We should match that behavior in DXC, but not escape all the slashes.

@llvm-beanz
Copy link
Collaborator

llvm-beanz commented Apr 29, 2024

Running the above example through DXC produces:

test.dxbc: C:\Users\cbieneman\dev\scratch\test.cpp \
 C:\Users\cbieneman\dev\scratch/test.h \
 C:\Users\cbieneman\dev\scratch/test space.h

Looks like we're generating pathsep incorrectly and not escaping the space. Both are reasonable fixes.

@pborsutzki
Copy link
Author

Thanks for your investigation.

About clang, I am not sure how correct its output is. I tried this on a Linux using clang 15.0.7 with a file named 'foo :[]\#$.cpp containing int main() {}:

$ clang -MD foo\ \:\[\]\\#\$.cpp
$ cat foo\ \:\[\]\\#\$.d
foo\ :[]\\#$$.o: foo\ :[]/\#$$.cpp
$ make -Bdnf foo\ \:\[\]\\#\$.d | -E "Considering|Successfully"
 Considering target file 'foo :[]\#$.d'.
Considering target file 'foo '.
  Considering target file '[]\'.
make: *** No rule to make target '[]\', needed by 'foo '.  Stop.

I am trying to use make as a validation tool here, as depfiles should contain suitable rules for make. So in this, make finds the target files foo and []\ which both weren't the ones intended. To get the correct result, I must escape :, \, #, $ and the whitespace but not [ and ]. When using this as input in the depfile: foo\ \:[]\\\#$$.o: foo\ \:[]\\\#$$.cpp make gives me this:

$ make -Bdnf foo\ \:\[\]\\#\$.d | grep -E "Considering|Successfully"
 Considering target file 'foo :[]\#$.d'.
Considering target file 'foo :[]\#$.o'.
  Considering target file 'foo :[]\#$.cpp'.
Successfully remade target file 'foo :[]\#$.o'.

Which looks about correct to me. This probably is a bit different on Windows, where : and \ are illegal as part of file/directory names anyways.

In the end, after digging through some makefile documentation I found that using "windows drive letters" (e.g. c:) without escaping the colon is legit usage when using a make build on Windows and also supported by the Ninja build system itself. I then found, that the problems I've had were actually caused by other parts of my build system (mainly: using a CMake version pre 3.29.2 with a Ninja generator, absolute paths and CMP0116).

This conclusion makes this issue/PR currently a non-issue for me - but maybe not for you, as you still might be interested in a fix for the uncovered problems. So feel free to close this or tell me if you want me to reshape the PR in any direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash
Projects
Status: Triaged
Development

No branches or pull requests

3 participants