swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.57k stars 10.35k forks source link

[SR-10735] Escape file paths for `-emit-dependencies-path` generated files #53127

Open BenchR267 opened 5 years ago

BenchR267 commented 5 years ago
Previous ID SR-10735
Radar rdar://51024983
Original Reporter @BenchR267
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | | |Labels | Bug | |Assignee | @compnerd | |Priority | Medium | md5: a1837a073b8f2a365c5c23bea503c527

Issue Description:

If having a file path that contains e.g. a new line, the dependency file for a primary file that depends on that file contains an unescaped path, in this case with just the new line.
Here's an example:

# generate two files + outputfilemap
echo "class A { static func foo() {} }" >> "$(printf "foo\nbar.swift")"
echo "A.foo()" >> main.swift
echo '{"foo\nbar.swift":{"dependencies":"./foo\nbar.d","diagnostics":"./foo\nbar.dia"},"main.swift":{"dependencies":"./main.d","diagnostics":"./main.dia"}}' > outputfilemap.json

# compile them
swiftc -emit-dependencies -output-file-map outputfilemap.json -serialize-diagnostics main.swift foo$'\n'bar.swift

# open the dependency file
open ./main.d

In the file, we can see that the new line is part of the file name, but it's not escaped. The serialized diagnostics are in a binary format, so it's hard to see what's in there (and I couldn't find a tool to deserialize it), but a project in Xcode with this configuration fails with emitting diagnostics due to the broken .d file while the Swift compile command itself doesn't fail. This is probably a related but different issue.

belkadan commented 5 years ago

I believe part of the problem here is that Makefile syntax is horrendous and unstandardized. :-/ We borrowed our escaping from what Clang does, but maybe it's diverged. cc @compnerd, who looked at this at one point.

BenchR267 commented 5 years ago

Sorry for all the editing, here's what I found out:
For clang, it's actually failing as well. llbuild expects new lines to be escaped with `\`, so that should be done here.
That the command itself is not failing while the dependency parsing fails is actually a llbuild error, that I'll investigate in. (rdar://51069563)

BenchR267 commented 5 years ago

Here's the code for the handling of escaped new lines in llbuild: https://github.com/apple/swift-llbuild/blob/3aeecb428d202afe15633266dc862de27feab723/lib/Core/MakefileDepsParser.cpp#L68.