Skip to content

Commit c85d669

Browse files
[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 (cherry picked from commit 0480748)
1 parent 9c3ff12 commit c85d669

File tree

2 files changed

+105
-0
lines changed

2 files changed

+105
-0
lines changed

clang/lib/Parse/ParseDecl.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6987,6 +6987,15 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
69876987
continue;
69886988
DeclsInPrototype.push_back(ND);
69896989
}
6990+
// Sort DeclsInPrototype based on raw encoding of the source location.
6991+
// Scope::decls() is iterating over a SmallPtrSet so sort the Decls before
6992+
// moving to DeclContext. This provides a stable ordering for traversing
6993+
// Decls in DeclContext, which is important for tasks like ASTWriter for
6994+
// deterministic output.
6995+
llvm::sort(DeclsInPrototype, [](Decl *D1, Decl *D2) {
6996+
return D1->getLocation().getRawEncoding() <
6997+
D2->getLocation().getRawEncoding();
6998+
});
69906999
}
69917000

69927001
// Remember that we parsed a function type, and remember the attributes.
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/// Test determinisim when serializing anonymous decls. Create enough Decls in
2+
/// DeclContext that can overflow the small storage of SmallPtrSet to make sure
3+
/// the serialization does not rely on iteration order of SmallPtrSet.
4+
// RUN: rm -rf %t.dir
5+
// RUN: split-file %s %t.dir
6+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
7+
// RUN: -fmodules-cache-path=%t.dir/cache -triple x86_64-apple-macosx10.11.0 \
8+
// RUN: -I%t.dir/headers %t.dir/main.m -fdisable-module-hash -Wno-visibility
9+
// RUN: mv %t.dir/cache/A.pcm %t1.pcm
10+
/// Check the order of the decls first. If LLVM_ENABLE_REVERSE_ITERATION is on,
11+
/// it will fail the test early if the output is depending on the order of items
12+
/// in containers that has non-deterministic orders.
13+
// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm | FileCheck %s
14+
// RUN: rm -rf %t.dir/cache
15+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
16+
// RUN: -fmodules-cache-path=%t.dir/cache -triple x86_64-apple-macosx10.11.0 \
17+
// RUN: -I%t.dir/headers %t.dir/main.m -fdisable-module-hash -Wno-visibility
18+
// RUN: mv %t.dir/cache/A.pcm %t2.pcm
19+
// RUN: diff %t1.pcm %t2.pcm
20+
21+
/// Spot check entries to make sure they are in current ordering.
22+
/// op13 encodes the anonymous decl number which should be in order.
23+
// CHECK: <TYPE_FUNCTION_PROTO
24+
// CHECK-NEXT: <DECL_PARM_VAR
25+
// CHECK-SAME: op13=2
26+
// CHECK-NEXT: <DECL_PARM_VAR
27+
// CHECK-SAME: op13=3
28+
// CHECK-NEXT: <DECL_PARM_VAR
29+
// CHECK-SAME: op13=4
30+
// CHECK-NEXT: <DECL_PARM_VAR
31+
// CHECK-SAME: op13=5
32+
33+
/// Decl records start at 43
34+
// CHECK: <DECL_RECORD
35+
// CHECK-SAME: op13=43
36+
// CHECK-NEXT: <DECL_RECORD
37+
// CHECK-SAME: op13=44
38+
// CHECK-NEXT: <DECL_RECORD
39+
// CHECK-SAME: op13=45
40+
// CHECK-NEXT: <DECL_RECORD
41+
// CHECK-SAME: op13=46
42+
43+
//--- headers/a.h
44+
void f(struct A0 *a0,
45+
struct A1 *a1,
46+
struct A2 *a2,
47+
struct A3 *a3,
48+
struct A4 *a4,
49+
struct A5 *a5,
50+
struct A6 *a6,
51+
struct A7 *a7,
52+
struct A8 *a8,
53+
struct A9 *a9,
54+
struct A10 *a10,
55+
struct A11 *a11,
56+
struct A12 *a12,
57+
struct A13 *a13,
58+
struct A14 *a14,
59+
struct A15 *a15,
60+
struct A16 *a16,
61+
struct A17 *a17,
62+
struct A18 *a18,
63+
struct A19 *a19,
64+
struct A20 *a20,
65+
struct A21 *a21,
66+
struct A22 *a22,
67+
struct A23 *a23,
68+
struct A24 *a24,
69+
struct A25 *a25,
70+
struct A26 *a26,
71+
struct A27 *a27,
72+
struct A28 *a28,
73+
struct A29 *a29,
74+
struct A30 *a30,
75+
struct A31 *a31,
76+
struct A32 *a32,
77+
struct A33 *a33,
78+
struct A34 *a34,
79+
struct A35 *a35,
80+
struct A36 *a36,
81+
struct A37 *a37,
82+
struct A38 *a38,
83+
struct A39 *a39,
84+
struct A40 *a40);
85+
86+
87+
//--- headers/module.modulemap
88+
89+
module A {
90+
header "a.h"
91+
}
92+
93+
//--- main.m
94+
95+
#import <a.h>
96+

0 commit comments

Comments
 (0)