Pass name of object file to LLVM so it can correctly emit S_OBJNAME in pdb files on Windows

[nebulark]

This should be the remaining fix to close #96475
Setting ObjectFilenameForDebug in llvm::TargetOptions, so llvm it can emit S_OBJNAME in pdb files on Windows.

Without a proper pdb parsing I am not able to add a unit test for this. The string is already appearing in the pdb file so I cannot just use grep.

@rustbot label: +A-debuginfo

[rustbot]

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

[nebulark]

@rustbot label: + O-windows

[rustbot]

Error: Parsing relabel command in comment failed: ...' label: +' | error: empty label at >| ' O-windows'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[nebulark]

@rustbot label: +O-windows

[jackh726]

r? rust-lang/wg-llvm

[nagisa]

Without a proper pdb parsing I am not able to add a unit test for this. The string is already appearing in the pdb file so I cannot just use grep.

Rather than testing for the specific output (i.e. "does S_OBJNAME appear in the pdb?") it would be better to test a behaviour that this enables/improves. The original issue reports suggests:

Tools like Live++ (https://liveplusplus.tech/) need this information in order to be able to recompile individual .o files.

Which seems overly complicated for our test, but maybe there's something we can do with debuginfo tests?

[nagisa]

This seems alright, although one thing I want to bring up from my previous experience with the similar code is: please make sure if the paths are expected to be relative or absolute and that our implementation here matches the ecosystem's convention.

[nebulark]

On my windows machine both paths (the one that already was present and the one I am adding) are excatly the same. They are both absolute paths and use backslashes.
They also match the format (absolute + backlashes) of what clang produces when compiling c++ files.

As far as I understand the reason for having it twice is that they may differ if the obj file gets moved before it is passed to the linker.

[nebulark]

@nagisa I am a bit unsure if I am supposed do something.
I am not in a hurry, just want to rule out that we are waiting on each other.

[bors]

☔ The latest upstream changes (presumably #115911) made this pull request unmergeable. Please resolve the merge conflicts.

[nagisa]

Ah, sorry I missed the notification earlier. It would still be good to add a behavioural test, if only to ensure this behaviour does not regress, but I'm also alright with marking the issue this fixes as E-needstest. But if there is truly no good way to write one down neatly, then its fine to not pursue it.

r=me (you'll need to rebase and ping somebody to say @ bors r=nagisa)

[bors]

📌 Commit 0f16237 has been approved by nagisa)`

It is now in the queue for this repository.

[nagisa]

@bors r- (ah, you shouldn't have reacted, bors T_T)

[nebulark]

Rebased, and checked the code again.

Regarding tests, I think it would be best to mark the issue with E-needstest.
The only pdb test that exists so far is the one I added in #113492, but it just uses string search.

A test for this pr would require checking which .o files got produced and check if a path to those exists in the right places. Ideally, doing linking in an extra step so I can validate correct behaviour if the .o files got moved before linking.
I have no clue how to acomplish that right now with the given tools.

I feel this would take me a long time to figure out and/or might be a bigger change to compiletest.

Ideally like to wait until #113026 lands, which should make this easier, potentially allowing including a pdb parsing lib.

@rustbot review

[nagisa]

@bors r+

[bors]

📌 Commit 91544e6 has been approved by nagisa

It is now in the queue for this repository.

[bors]

⌛ Testing commit 91544e6 with merge 6f13ea0...

[bors]

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 6f13ea0 to master...

[rust-timer]

Finished benchmarking commit (6f13ea0): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.0%	[1.0%, 1.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.9%	[-2.9%, -2.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-2.9%, 2.4%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 630.824s -> 630.479s (-0.05%)
Artifact size: 317.29 MiB -> 317.20 MiB (-0.03%)