Page MenuHomePhabricator

[DebugInfo] Skip MD5 checksums of preprocessed files

Authored by probinson on May 23 2018, 7:38 AM.

Diff Detail


Event Timeline

trixirt created this revision.May 23 2018, 7:38 AM
probinson added a subscriber: rnk.

+ @rnk who did the initial checksum stuff for CodeView.

I am unclear how the notion of checksums should interact with preprocessed files.

Nit: while many tests have dates in the filenames, we no longer use that convention.

trixirt updated this revision to Diff 148228.May 23 2018, 8:37 AM

Rename test to dwarf5-expansion-checksum.c

After thinking about this a bit... The # directive will cause the filename to be identified as the source for the subsequent lines. That means it will show up as the original source file in the line table.
However, the compiler doesn't have the actual source file of the header (or the original source file, for that matter) and so cannot compute a correct checksum for it. That means, it should omit the checksum, for any file identified with a # directive.
I haven't looked at how CodeView handles this situation; for DWARF v5 I made inconsistent use of a checksum into an assertion offense.
Should we reconsider that? Or have the # directive cause Clang to omit checksums for *all* files, even the one it is actually reading?
@aprantl, @rnk, thoughts?

Once we encounter a # directive we are (most likely) looking at some form of preprocessed source and that means that the checksum will inevitably be different than what we would have gotten were we reading the file directly, because of the preprocessing. At this point the value of the hash approaches zero. I think dropping all checksums is reasonable in that situation.

I see that the per-file info does track the presence of # [line] N but it's not immediately obvious how to query "has any file seen one" which is really what I'd want to know. The file information has various levels of indirection...

@trixirt do you mind if I commandeer this review? I think I have a patch.

commandeer away!
Sure that is fine.

probinson commandeered this revision.May 25 2018, 1:43 PM
probinson updated this revision to Diff 148663.
probinson retitled this revision from Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion to [DebugInfo] Skip MD5 checksums of preprocessed files.
probinson edited the summary of this revision. (Show Details)
probinson edited reviewers, added: dblaikie; removed: probinson.
probinson added a project: debug-info.
probinson added a reviewer: trixirt.

Upload patch to suppress checksums when we see a preprocessed file.

aprantl accepted this revision.May 25 2018, 1:50 PM

Minor comment inline.

378 ↗(On Diff #148663)

Can you add a comment explaining why we are doing this here?

This revision is now accepted and ready to land.May 25 2018, 1:50 PM
probinson marked an inline comment as done.May 25 2018, 2:02 PM
probinson added inline comments.
378 ↗(On Diff #148663)

Of course.

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
probinson marked an inline comment as done.