Make precompiled headers reproducible

[sylvestre.ledru]

Summary

This patch have been applied to Debian packages for a few months without
any regression.

https://bugs.debian.org/877359

Patch by Rebecca N. Palmer

[lebedev.ri]

Needs a test.

lib/Serialization/ASTWriter.cpp

4283

Would it be better to just change the actual type of SemaRef.OpenCLTypeExtMap?
https://github.com/llvm-mirror/clang/blob/18917301298ad6df9e989983ed1e22cb0f9dff29/include/clang/Sema/Sema.h#L8710-L8712

[sylvestre.ledru]

I won't have time to write the test for that (and would not know where to start). I am just the messenger :)

[Anastasia]

lib/Serialization/ASTWriter.cpp

4283

What data structure would you suggest to use instead? I think sorting during serialization or de-serialization of AST isn't too bad because it's not a common use case.

[lebedev.ri]

lib/Serialization/ASTWriter.cpp

4283

Just std::map<>?

[riccibruno]

lib/Serialization/ASTWriter.cpp

4283

Why is this a problem ? I thought that pch files where not supposed to be distributed. If we come to the conclusion that pch files must be reproducible, then I believe that this is by far not the only example.

[riccibruno]

After looking at the bug report (https://bugs.debian.org/877359), I would like to point out that there is currently *absolutely* no stability guarantee for the serialization format (ie, you need the exact same revision). In regard of this I am wondering how sane it is to ship pch files.

[thakis]

In D60379#1458668, @riccibruno wrote:

After looking at the bug report (https://bugs.debian.org/877359), I would like to point out that there is currently *absolutely* no stability guarantee for the serialization format (ie, you need the exact same revision). In regard of this I am wondering how sane it is to ship pch files.

I agree we shouldn't make guarantees about pch file format stability and that distributing pch files isn't a good idea. Having said that, making clang write the same output when given the same input is still a Good Thing as it enables caching these outputs e.g. with tools like distcc.

[sylvestre.ledru]

To be clear, this isn't about format stability and distributing but making sure that two same runs of clang produces the same output.

[riccibruno]

In D60379#1458799, @thakis wrote:

In D60379#1458668, @riccibruno wrote:

After looking at the bug report (https://bugs.debian.org/877359), I would like to point out that there is currently *absolutely* no stability guarantee for the serialization format (ie, you need the exact same revision). In regard of this I am wondering how sane it is to ship pch files.

I agree we shouldn't make guarantees about pch file format stability and that distributing pch files isn't a good idea. Having said that, making clang write the same output when given the same input is still a Good Thing as it enables caching these outputs e.g. with tools like distcc.

Good point.

I went through each of the structures serialized in ASTWriter.cpp, and unless I missed one, OpenCLDeclExtMap and OpenCLTypeExtMap are the only one which are serialized in a non-deterministic way (so my earlier inline comment was mistaken).

[Anastasia]

lib/Serialization/ASTWriter.cpp

4283

Ok, I would normally prefer to use DenseMap for the general case because since the number of elements in OpenCLTypeExtMap is very small we can take advantage of non-heap allocation. But at the same time probably a couple of elements won't make any difference for std::map either and we can keep the code base more maintainable. So I don't mind either way.

[Anastasia]

FYI, I switched to std::map in this review:
https://reviews.llvm.org/D60778

@sylvestre.ledru do you think it's possible to apply this patch and see if it work for your bug?

Btw I am still confused about the reproducibility - does clang produce the same PCH at least for the runs on the same revision?

[Anastasia]

Can this be closed now due to commit r358674?

[sylvestre.ledru]

yeah
thanks

lib/Serialization/ASTWriter.cpp

void ASTWriter::WriteOpenCLExtensions(Sema &SemaRef) {
   Stream.EmitRecord(OPENCL_EXTENSIONS, Record);
 }
 
 void ASTWriter::WriteOpenCLExtensionTypes(Sema &SemaRef) {
   if (!SemaRef.Context.getLangOpts().OpenCL)
     return;
 
   RecordData Record;
+  // Sort to allow reproducible .pch files - https://bugs.debian.org/877359
+  std::map<TypeID, std::set<std::string>> sortedOpenCLTypeExtMap;
   for (const auto &I : SemaRef.OpenCLTypeExtMap) {
-    Record.push_back(
-        static_cast<unsigned>(getTypeID(I.first->getCanonicalTypeInternal())));
+    sortedOpenCLTypeExtMap[getTypeID(I.first->getCanonicalTypeInternal())]=I.second;
+  }
+  for (const auto &I : sortedOpenCLTypeExtMap) {
+    Record.push_back(static_cast<unsigned>(I.first));
     Record.push_back(I.second.size());
     for (auto Ext : I.second)
       AddString(Ext, Record);
   }
   Stream.EmitRecord(OPENCL_EXTENSION_TYPES, Record);
 }
 
 void ASTWriter::WriteOpenCLExtensionDecls(Sema &SemaRef) {
   if (!SemaRef.Context.getLangOpts().OpenCL)
     return;
 
   RecordData Record;
+  std::map<DeclID, std::set<std::string>> sortedOpenCLDeclExtMap;
   for (const auto &I : SemaRef.OpenCLDeclExtMap) {
-    Record.push_back(getDeclID(I.first));
+    sortedOpenCLDeclExtMap[getDeclID(I.first)]=I.second;
+  }
+  for (const auto &I : sortedOpenCLDeclExtMap) {
+    Record.push_back(I.first);
     Record.push_back(static_cast<unsigned>(I.second.size()));
     for (auto Ext : I.second)
       AddString(Ext, Record);
   }
   Stream.EmitRecord(OPENCL_EXTENSION_DECLS, Record);
 }
 
 void ASTWriter::WriteCUDAPragmas(Sema &SemaRef) {