This is an archive of the discontinued LLVM Phabricator instance.

[clang] Fix DIFile directory root on Windows
ClosedPublic

Authored by keith on Oct 11 2021, 2:37 PM.

Details

Summary

On unix systems this logic would not separate the file and directory of
the DIFile unless they shared more components at the start than just the
root path character. The logic to do this was unix specific so it didn't
work on Windows. Now we check if the entire root_path is the same as
what you were going to set as the Dir and use the full filepath in that
case.

Diff Detail

Event Timeline

keith requested review of this revision.Oct 11 2021, 2:37 PM
keith created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2021, 2:37 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Adding since you might be interested in this, I think the Swift logic is also derived from this so it might have the same difference.

mstorsjo added inline comments.Oct 15 2021, 4:38 AM
clang/lib/CodeGen/CGDebugInfo.cpp
436

This change is correct IMO.

clang/test/CodeGen/debug-prefix-map.c
24

I presume that this patch goes on top of D111457? It might be good to set that one as the parent revision of this one, so that the premerge test runs applies them on top of each other (right now, this one failed to apply).

Isn't this particular change present (directory being "" here) already after the previous patch?

keith added inline comments.Oct 19 2021, 12:09 PM
clang/test/CodeGen/debug-prefix-map.c
24

That is marked as the parent of this patch in phab, but maybe I need to do something else to get them to apply?

24

Isn't this particular change present (directory being "" here) already after the previous patch?

This does still require this change since otherwise directory still gets C:

mstorsjo added inline comments.Oct 19 2021, 12:17 PM
clang/test/CodeGen/debug-prefix-map.c
24

Hmm, it seems like the premerge testing managed to start running this before the parent was set (and I didn't notice it when I looked at the patches the last round). If you reupload the patch later when this already is set, I think it should work though - but no hurry with that right now.

24

Oh, ok.

keith added inline comments.Oct 19 2021, 2:20 PM
clang/test/CodeGen/debug-prefix-map.c
24

Ah that's on my order of operations then, I submitted this, and then started the pre-warm, and haven't updated it since. Good to know, thanks. I will make sure CI passes here before landing anything regardless.

ormris removed a subscriber: ormris.Jan 24 2022, 11:31 AM
keith updated this revision to Diff 411575.Feb 25 2022, 9:22 PM
keith marked 3 inline comments as done.

Add more test path changes

keith updated this revision to Diff 411922.Feb 28 2022, 4:07 PM

Update tests with new syntax and FileCheck variables

keith updated this revision to Diff 414976.Mar 13 2022, 7:43 PM

Remove variables from test expectations

This didn't work out because of the double vs single slash of the expansions

Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2022, 7:43 PM
keith updated this revision to Diff 415004.Mar 13 2022, 10:16 PM

Replace \w with .

keith added a comment.Mar 14 2022, 9:03 AM

Ping, all green here, I just had to mess with the test regex a bit

compnerd accepted this revision.Mar 14 2022, 4:32 PM
compnerd added inline comments.
clang/test/CodeGen/debug-prefix-map.c
7

I think that this illustrates my point on the base differential.

This revision is now accepted and ready to land.Mar 14 2022, 4:32 PM
keith updated this revision to Diff 415291.Mar 14 2022, 6:09 PM
keith marked an inline comment as done.

Update subsitutions with dashes

This revision was automatically updated to reflect the committed changes.