[PATCH] llvm-cov: Updated file checksum to be timestamp.

[David Blaikie]

On Fri, Nov 15, 2013 at 11:03 PM, Eric Christopher <echristo at gmail.com>wrote:

> On Fri, Nov 15, 2013 at 6:08 PM, Nick Lewycky <nlewycky at google.com> wrote:
> > On 15 November 2013 17:38, Robinson, Paul
> > <Paul_Robinson at playstation.sony.com> wrote:
> >>
> >> > -----Original Message-----
> >> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >> > bounces at cs.uiuc.edu] On Behalf Of Yuchen Wu
> >> > Sent: Friday, November 15, 2013 4:59 PM
> >> > To: Nick Lewycky; Bob Wilson
> >> > Cc: llvm-commits at cs.uiuc.edu
> >> > Subject: RE: [PATCH] llvm-cov: Updated file checksum to be timestamp.
> >> >
> >> > >> 2. Using the output file itself to seed hash function, which makes
> >> > it
> >> > >> deterministic. I've tried implementing this using the size of the
> >> > >> output buffer and it was pretty simple. The problem with it,
> however,
> >> > >> is that there's a lot more chance for a change to the GCNO file to
> go
> >> > >> unnoticed. I also think that even if the source hadn't changed
> >> > between
> >> > >> compiles, the new binary files shouldn't be compatible with the
> old.
> >> > >
> >> > > This is obviously the correct approach. In general, it's important
> to
> >> > > be able to have reproducible builds so that we can reproduce the
> same
> >> > > binaries from source, builds where outputs can be cached (for
> instance
> >> > > by modern non-make build systems that use the md5 of the output
> >> > files),
> >> > > etc. GCC's behaviour is silly and there's no need to replicate it.
> >> > >
> >> > >> "The problem with it, however, is that there's a lot more chance
> for
> >> > a
> >> > >> change to the GCNO file to go unnoticed."
> >> > >
> >> > > What do you mean by this? Are you worried that things could go into
> >> > the
> >> > > GCNO file without being an input to the hash function? The checksum
> is
> >> > > a safety measure to help people avoid accidentally putting
> mismatching
> >> > > GCNO and GCDA files together. Not having something be input to the
> >> > hash
> >> > > is the safe failure. We don't want the checksum to change if other
> >> > > parts of the GCNO file weren't modified.
> >> >
> >> > What I meant by the last statement was that if you are doing something
> >> > like hashing the size of the file to compute a checksum, there is a
> much
> >> > higher chance that you may be using a GCNO file generated from a
> >> > different source that just happens to be the same size. Obviously that
> >> > was just an example, so if you guys came across a better way to seed
> the
> >> > hash for Google's gcc checksum, I'd be happy to hear it :)
> >>
> >> Can we use an MD5 of the source file here? (Not having looked at the
> >> patch, sorry...) The only reason I ask is that there's a DWARF 5 feature
> >> to use MD5 instead of timestamps in the debug-line info, so computing an
> >> MD5 of the source files is something we'll want to do anyway,
> eventually.
> >
> >
> > That's reasonable, but I'd prefer to have the .gcno's checksum not
> depend on
> > things which aren't in the .gcno. Fixing a typo in a comment for instance
> > produces the same .o file and I'd like it to produce the same .gcno file.
> >
> > That applies to DWARF too. I hope we haven't standardized something that
> > requires us to emit a different .o file just because of a typo fix in a
> > comment.
> >
>
> It's not finalized yet :)


But the current spec (defining package hashing in terms of type hashing)
doesn't include line numbers and a bunch of other stuff in the hash - so
no, fixing a typo in a comment won't change the package hash.

(I'm not sure it's right not to include line info, though - that seems sort
of important to regenerate the debug info)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131116/7696426a/attachment.html>