From 0480748ea6728392886931b8470969ae17aaa91f Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Thu, 2 Feb 2023 15:07:16 -0800 Subject: [PATCH] [DeclContext] Sort the Decls before adding into DeclContext Fix a non-deterministic issue in clang module generation, which the anonymous declaration number from a function context is not deterministic. This is due to the unstable iteration order for decls in scope so the order after moving the decls into function decl context is not deterministic. From https://reviews.llvm.org/D135118, we can't use a set that preserves the order without the performance penalty. Fix the issue by sorting the decls based on raw encoding of their source location. rdar://104097976 Reviewed By: akyrtzi, vsapsai Differential Revision: https://reviews.llvm.org/D141625 --- clang/lib/Parse/ParseDecl.cpp | 9 +++ clang/test/Modules/decl-params-determinisim.m | 96 +++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 clang/test/Modules/decl-params-determinisim.m diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index e6812ac..96c25ba 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -6987,6 +6987,15 @@ void Parser::ParseFunctionDeclarator(Declarator &D, continue; DeclsInPrototype.push_back(ND); } + // Sort DeclsInPrototype based on raw encoding of the source location. + // Scope::decls() is iterating over a SmallPtrSet so sort the Decls before + // moving to DeclContext. This provides a stable ordering for traversing + // Decls in DeclContext, which is important for tasks like ASTWriter for + // deterministic output. + llvm::sort(DeclsInPrototype, [](Decl *D1, Decl *D2) { + return D1->getLocation().getRawEncoding() < + D2->getLocation().getRawEncoding(); + }); } // Remember that we parsed a function type, and remember the attributes. diff --git a/clang/test/Modules/decl-params-determinisim.m b/clang/test/Modules/decl-params-determinisim.m new file mode 100644 index 0000000..d7e873b --- /dev/null +++ b/clang/test/Modules/decl-params-determinisim.m @@ -0,0 +1,96 @@ +/// Test determinisim when serializing anonymous decls. Create enough Decls in +/// DeclContext that can overflow the small storage of SmallPtrSet to make sure +/// the serialization does not rely on iteration order of SmallPtrSet. +// RUN: rm -rf %t.dir +// RUN: split-file %s %t.dir +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t.dir/cache -triple x86_64-apple-macosx10.11.0 \ +// RUN: -I%t.dir/headers %t.dir/main.m -fdisable-module-hash -Wno-visibility +// RUN: mv %t.dir/cache/A.pcm %t1.pcm +/// Check the order of the decls first. If LLVM_ENABLE_REVERSE_ITERATION is on, +/// it will fail the test early if the output is depending on the order of items +/// in containers that has non-deterministic orders. +// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm | FileCheck %s +// RUN: rm -rf %t.dir/cache +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t.dir/cache -triple x86_64-apple-macosx10.11.0 \ +// RUN: -I%t.dir/headers %t.dir/main.m -fdisable-module-hash -Wno-visibility +// RUN: mv %t.dir/cache/A.pcm %t2.pcm +// RUN: diff %t1.pcm %t2.pcm + +/// Spot check entries to make sure they are in current ordering. +/// op13 encodes the anonymous decl number which should be in order. +// CHECK: + -- 2.7.4