r220493 - Add a "signature" to AST files to verify that they haven't changed

[Ben Langmuir]

> On Mar 26, 2015, at 6:54 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> FYI, as of r233348, ever source of non-deterministic output I can find by inspection and some targeted stress testing on the C++ side is fixed. I suspect we're pretty clase.

Woo!

> 
> Ben, if you want to add test cases for objc stuff, that'd be awesome. I just fixed it all by inspection.

Okay, it will probably be a few days before I have time for this.  However, as a quick check I turned off signatures and emitting the mtime for implicit modueles and built our Cocoa module and its dependencies a few times.  It looks like 22/27 of the modules built identically each time, and the other 5 were reliably different so it should be easy to figure out what’s still having trouble.  If you’re interested, a quick sample showed the differences came from the order of  DECL_OBJC_METHOD and  DECL_OBJC_PROPERTY records. I'll look into that when I start adding tests.

> 
> I think that what we should do is add a (potentially optional and only produced by an asserts build of Clang) record to the serialized AST which is a digest (MD5 or some such) of the entire AST up to that record. We could add this to the low layer of the AST writing so it would be easy to plumb through. This would make an actually *good* signature record and we could check it to ensure we don't waste time trying to debug failures if non-deterministic behavior creeps in again. Ben, could you look at making that change? I think that would be a *much* better design for the signature than a random number, and would serve the exact same purpose.

Richard and I previously discussed having the signature be a hash of:
a) the input files to the current module (path, size, modtime)
b) the signature of all the imports

which should be cheap enough to build all the time.  Having a separate more expensive hash of the whole AST in asserts builds would be good too.

Ben

> 
> On Tue, Mar 24, 2015 at 2:21 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
> 
> On Mon, Mar 23, 2015 at 10:37 PM, Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>> On Mar 23, 2015, at 10:21 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
>> 
>> 
>> On Mon, Mar 23, 2015 at 9:54 PM, Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>>> On Mar 23, 2015, at 5:38 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
>>> 
>>> Digging this thread back up, I'm starting to work on deterministic output of modules. This is *really* important. Unfortunately, it's almost impossible to make progress as long as this code is actually firing because it kind of makes things definitively non-deterministic. ;]
>>> 
>>> Ben, do you have any concerns about only emitting the signature record for implicit modules? This would omit it for explicit modules (where I'm working), PCH files, etc. Based on your commit message and explanation, it doesn't really make sense outside of implicit modules AFAICT, and we're hoping it will go away completely.
>> 
>> SGTM although I would prefer we continue to check the /expected/ signature when an AST file imports an implicit module (and in particular when a PCH imports an implicit module).  Thanks for working on this!
>>  
>> I'm not planning to change the checking code path at all. All the tests pass if I just restrict the emission to be when generating implicit modules
> 
> Perfect, that’s what I was hoping for.  Sorry I didn’t phrase it clearly.
> 
> This is in as part of r233115 with the minimal test case for deterministic output. More to come.
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150330/2521773c/attachment.html>