Skip to content

Commit 9bc5a55

Browse files
jrose-applejrose-apple
authored
Merge pull request #26685 from jrose-apple/po-tay-toes
Use a better hash seed when doing string hashing in the compiler
2 parents 2c9def8 + c06e105 commit 9bc5a55

21 files changed

+166
-165
lines changed

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,13 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 516; // encode GenericSignature and GenericEnvironment together
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 517; // better string hash seed
56+
57+
/// A standard hash seed used for all string hashes in a serialized module.
58+
///
59+
/// This is the same as the default used by llvm::djbHash, just provided
60+
/// explicitly here to note that it's part of the format.
61+
const uint32_t SWIFTMODULE_HASH_SEED = 5381;
5662

5763
using DeclIDField = BCFixed<31>;
5864

‎lib/ClangImporter/SwiftLookupTable.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,8 +1090,7 @@ namespace {
10901090
}
10911091

10921092
hash_value_type ComputeHash(key_type_ref key) {
1093-
// FIXME: DJB seed=0, audit whether the default seed could be used.
1094-
return static_cast<unsigned>(key.first) + llvm::djbHash(key.second, 0);
1093+
return static_cast<unsigned>(key.first) + llvm::djbHash(key.second);
10951094
}
10961095

10971096
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,
@@ -1340,8 +1339,7 @@ namespace {
13401339
}
13411340

13421341
hash_value_type ComputeHash(internal_key_type key) {
1343-
// FIXME: DJB seed=0, audit whether the default seed could be used.
1344-
return static_cast<unsigned>(key.first) + llvm::djbHash(key.second, 0);
1342+
return static_cast<unsigned>(key.first) + llvm::djbHash(key.second);
13451343
}
13461344

13471345
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ struct RawValueKey {
9797
// FIXME: doesn't accommodate >64-bit or signed raw integer or float values.
9898
union {
9999
StringRef stringValue;
100-
uint32_t charValue;
101100
IntValueTy intValue;
102101
FloatValueTy floatValue;
103102
};
@@ -181,8 +180,7 @@ class DenseMapInfo<RawValueKey> {
181180
return DenseMapInfo<uint64_t>::getHashValue(k.intValue.v0) &
182181
DenseMapInfo<uint64_t>::getHashValue(k.intValue.v1);
183182
case RawValueKey::Kind::String:
184-
// FIXME: DJB seed=0, audit whether the default seed could be used.
185-
return llvm::djbHash(k.stringValue, 0);
183+
return DenseMapInfo<StringRef>::getHashValue(k.stringValue);
186184
case RawValueKey::Kind::Empty:
187185
case RawValueKey::Kind::Tombstone:
188186
return 0;

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ class SILDeserializer::FuncTableInfo {
103103
external_key_type GetExternalKey(internal_key_type ID) { return ID; }
104104

105105
hash_value_type ComputeHash(internal_key_type key) {
106-
// FIXME: DJB seed=0, audit whether the default seed could be used.
107-
return llvm::djbHash(key, 0);
106+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
108107
}
109108

110109
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {

‎lib/Serialization/DocFormat.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@ const uint16_t SWIFTDOC_VERSION_MAJOR = 1;
6969
/// adhering to the 80-column limit for this line.
7070
const uint16_t SWIFTDOC_VERSION_MINOR = 1; // Last change: skipping 0 for testing purposes
7171

72+
/// The hash seed used for the string hashes in a Swift 5.1 swiftdoc file.
73+
///
74+
/// 0 is not a good seed for llvm::djbHash, but swiftdoc files use a stable
75+
/// format, so we can't change the hash seed without a version bump. Any new
76+
/// hashed strings should use a new stable hash seed constant. (No such constant
77+
/// has been defined at the time this doc comment was last updated because there
78+
/// are no other strings to hash.)
79+
const uint32_t SWIFTDOC_HASH_SEED_5_1 = 0;
80+
7281
/// The record types within the comment block.
7382
///
7483
/// Be very careful when changing this block; it must remain

‎lib/Serialization/ModuleFile.cpp

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,7 @@ class ModuleFile::DeclTableInfo {
391391

392392
hash_value_type ComputeHash(internal_key_type key) {
393393
if (key.first == DeclBaseName::Kind::Normal) {
394-
// FIXME: DJB seed=0, audit whether the default seed could be used.
395-
return llvm::djbHash(key.second, 0);
394+
return llvm::djbHash(key.second, SWIFTMODULE_HASH_SEED);
396395
} else {
397396
return (hash_value_type)key.first;
398397
}
@@ -454,8 +453,7 @@ class ModuleFile::ExtensionTableInfo {
454453
}
455454

456455
hash_value_type ComputeHash(internal_key_type key) {
457-
// FIXME: DJB seed=0, audit whether the default seed could be used.
458-
return llvm::djbHash(key, 0);
456+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
459457
}
460458

461459
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {
@@ -515,8 +513,7 @@ class ModuleFile::LocalDeclTableInfo {
515513
}
516514

517515
hash_value_type ComputeHash(internal_key_type key) {
518-
// FIXME: DJB seed=0, audit whether the default seed could be used.
519-
return llvm::djbHash(key, 0);
516+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
520517
}
521518

522519
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {
@@ -551,8 +548,7 @@ class ModuleFile::NestedTypeDeclsTableInfo {
551548
}
552549

553550
hash_value_type ComputeHash(internal_key_type key) {
554-
// FIXME: DJB seed=0, audit whether the default seed could be used.
555-
return llvm::djbHash(key, 0);
551+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
556552
}
557553

558554
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {
@@ -607,8 +603,7 @@ class ModuleFile::DeclMemberNamesTableInfo {
607603

608604
hash_value_type ComputeHash(internal_key_type key) {
609605
if (key.first == DeclBaseName::Kind::Normal) {
610-
// FIXME: DJB seed=0, audit whether the default seed could be used.
611-
return llvm::djbHash(key.second, 0);
606+
return llvm::djbHash(key.second, SWIFTMODULE_HASH_SEED);
612607
} else {
613608
return (hash_value_type)key.first;
614609
}
@@ -782,8 +777,7 @@ class ModuleFile::ObjCMethodTableInfo {
782777
}
783778

784779
hash_value_type ComputeHash(internal_key_type key) {
785-
// FIXME: DJB seed=0, audit whether the default seed could be used.
786-
return llvm::djbHash(key, 0);
780+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
787781
}
788782

789783
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {
@@ -972,8 +966,7 @@ class ModuleFile::DeclCommentTableInfo {
972966

973967
hash_value_type ComputeHash(internal_key_type key) {
974968
assert(!key.empty());
975-
// FIXME: DJB seed=0, audit whether the default seed could be used.
976-
return llvm::djbHash(key, 0);
969+
return llvm::djbHash(key, SWIFTDOC_HASH_SEED_5_1);
977970
}
978971

979972
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {

‎lib/Serialization/Serialization.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ namespace {
102102
switch (key.getKind()) {
103103
case DeclBaseName::Kind::Normal:
104104
assert(!key.empty());
105-
// FIXME: DJB seed=0, audit whether the default seed could be used.
106-
return llvm::djbHash(key.getIdentifier().str(), 0);
105+
return llvm::djbHash(key.getIdentifier().str(),
106+
SWIFTMODULE_HASH_SEED);
107107
case DeclBaseName::Kind::Subscript:
108108
return static_cast<uint8_t>(DeclNameKind::Subscript);
109109
case DeclBaseName::Kind::Constructor:
@@ -179,8 +179,7 @@ namespace {
179179

180180
hash_value_type ComputeHash(key_type_ref key) {
181181
assert(!key.empty());
182-
// FIXME: DJB seed=0, audit whether the default seed could be used.
183-
return llvm::djbHash(key.str(), 0);
182+
return llvm::djbHash(key.str(), SWIFTMODULE_HASH_SEED);
184183
}
185184

186185
int32_t getNameDataForBase(const NominalTypeDecl *nominal,
@@ -244,8 +243,7 @@ namespace {
244243

245244
hash_value_type ComputeHash(key_type_ref key) {
246245
assert(!key.empty());
247-
// FIXME: DJB seed=0, audit whether the default seed could be used.
248-
return llvm::djbHash(key, 0);
246+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
249247
}
250248

251249
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,
@@ -286,8 +284,7 @@ namespace {
286284

287285
hash_value_type ComputeHash(key_type_ref key) {
288286
assert(!key.empty());
289-
// FIXME: DJB seed=0, audit whether the default seed could be used.
290-
return llvm::djbHash(key.str(), 0);
287+
return llvm::djbHash(key.str(), SWIFTMODULE_HASH_SEED);
291288
}
292289

293290
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,
@@ -332,8 +329,7 @@ namespace {
332329
switch (key.getKind()) {
333330
case DeclBaseName::Kind::Normal:
334331
assert(!key.empty());
335-
// FIXME: DJB seed=0, audit whether the default seed could be used.
336-
return llvm::djbHash(key.getIdentifier().str(), 0);
332+
return llvm::djbHash(key.getIdentifier().str(), SWIFTMODULE_HASH_SEED);
337333
case DeclBaseName::Kind::Subscript:
338334
return static_cast<uint8_t>(DeclNameKind::Subscript);
339335
case DeclBaseName::Kind::Constructor:
@@ -4402,8 +4398,7 @@ namespace {
44024398

44034399
hash_value_type ComputeHash(key_type_ref key) {
44044400
llvm::SmallString<32> scratch;
4405-
// FIXME: DJB seed=0, audit whether the default seed could be used.
4406-
return llvm::djbHash(key.getString(scratch), 0);
4401+
return llvm::djbHash(key.getString(scratch), SWIFTMODULE_HASH_SEED);
44074402
}
44084403

44094404
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,

‎lib/Serialization/SerializeDoc.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,7 @@ class DeclCommentTableInfo {
197197

198198
hash_value_type ComputeHash(key_type_ref key) {
199199
assert(!key.empty());
200-
// FIXME: DJB seed=0, audit whether the default seed could be used.
201-
return llvm::djbHash(key, 0);
200+
return llvm::djbHash(key, SWIFTDOC_HASH_SEED_5_1);
202201
}
203202

204203
std::pair<unsigned, unsigned>

‎lib/Serialization/SerializeSIL.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ namespace {
109109

110110
hash_value_type ComputeHash(key_type_ref key) {
111111
assert(!key.empty());
112-
// FIXME: DJB seed=0, audit whether the default seed could be used.
113-
return llvm::djbHash(key, 0);
112+
return llvm::djbHash(key, SWIFTMODULE_HASH_SEED);
114113
}
115114

116115
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,

‎test/Frontend/sil-merge-partial-modules.swift

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
// RUN: %target-swift-frontend -emit-module %t/partial.a.swiftmodule %t/partial.b.swiftmodule -module-name test -enable-library-evolution -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -o %t/test.swiftmodule
77

8-
// RUN: %target-sil-opt %t/test.swiftmodule -disable-sil-linking > %t/dump.sil
8+
// RUN: %target-sil-opt %t/test.swiftmodule -disable-sil-linking -emit-sorted-sil > %t/dump.sil
99
// RUN: %FileCheck %s < %t/dump.sil
1010
// RUN: %FileCheck %s --check-prefix=NEGATIVE < %t/dump.sil
1111

@@ -40,28 +40,30 @@ public class CircleManager : ShapeManager {
4040
public override func manage() {}
4141
}
4242

43-
// FIXME: Why is the definition order totally random?
43+
// CHECK-LABEL: sil [canonical] @$s4test14publicFunctionyyF : $@convention(thin) () -> ()
44+
45+
// CHECK-LABEL: sil [serialized] [canonical] @$s4test17inlinableFunctionyyF : $@convention(thin) () -> () {
46+
// CHECK: function_ref @$s4test17inlinableFunctionyyFyycfU_
47+
// CHECK: }
48+
49+
// CHECK-LABEL: sil shared [serialized] [canonical] @$s4test17inlinableFunctionyyFyycfU_ : $@convention(thin) () -> () {
50+
// CHECK: function_ref @$s4test17versionedFunctionyyF
51+
// CHECK: }
4452

4553
// CHECK-LABEL: sil [canonical] @$s4test17versionedFunctionyyF : $@convention(thin) () -> ()
4654

4755
// CHECK-LABEL: sil [canonical] @$s4test9RectangleV4areaSfvg : $@convention(method) (Rectangle) -> Float
4856

49-
// CHECK-LABEL: sil shared [transparent] [serialized] [thunk] [canonical] @$s4test9RectangleVAA5ShapeA2aDP4drawyyFTW : $@convention(witness_method: Shape) (@in_guaranteed Rectangle) -> () {
57+
// CHECK-LABEL: sil [serialized] [canonical] @$s4test9RectangleV4drawyyF : $@convention(method) (Rectangle) -> () {
5058
// CHECK: function_ref @$s4test14publicFunctionyyF
5159
// CHECK: }
5260

53-
// CHECK-LABEL: sil [canonical] @$s4test14publicFunctionyyF : $@convention(thin) () -> ()
54-
5561
// CHECK-LABEL: sil shared [transparent] [serialized] [thunk] [canonical] @$s4test9RectangleVAA5ShapeA2aDP4areaSfvgTW : $@convention(witness_method: Shape) (@in_guaranteed Rectangle) -> Float {
5662
// CHECK: function_ref @$s4test9RectangleV4areaSfvg
5763
// CHECK: }
5864

59-
// CHECK-LABEL: sil shared [serialized] [canonical] @$s4test17inlinableFunctionyyFyycfU_ : $@convention(thin) () -> () {
60-
// CHECK: function_ref @$s4test17versionedFunctionyyF
61-
// CHECK: }
62-
63-
// CHECK-LABEL: sil [serialized] [canonical] @$s4test17inlinableFunctionyyF : $@convention(thin) () -> () {
64-
// CHECK: function_ref @$s4test17inlinableFunctionyyFyycfU_
65+
// CHECK-LABEL: sil shared [transparent] [serialized] [thunk] [canonical] @$s4test9RectangleVAA5ShapeA2aDP4drawyyFTW : $@convention(witness_method: Shape) (@in_guaranteed Rectangle) -> () {
66+
// CHECK: function_ref @$s4test9RectangleV4drawyyF
6567
// CHECK: }
6668

6769
// CHECK-LABEL: sil_witness_table [serialized] Rectangle: Shape module test {

‎test/SIL/Serialization/Recovery/function.sil

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend -parse-sil %s -emit-sib -o %t/Library.sib -module-name Library -I %S/Inputs/good-modules -parse-stdlib
3-
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/good-modules | %FileCheck %s
4-
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules | %FileCheck -check-prefix=CHECK-RECOVERY %s
5-
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules | %FileCheck -check-prefix=CHECK-RECOVERY-NEGATIVE %s
3+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/good-modules -emit-sorted-sil | %FileCheck %s
4+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules -emit-sorted-sil | %FileCheck -check-prefix=CHECK-RECOVERY %s
5+
// RUN: %target-sil-opt %t/Library.sib -I %S/Inputs/bad-modules -emit-sorted-sil | %FileCheck -check-prefix=CHECK-RECOVERY-NEGATIVE %s
66

77
// CHECK-LABEL: sil_stage raw
88
// CHECK-RECOVERY-LABEL: sil_stage raw

‎test/SIL/Serialization/assignattr.sil

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,12 @@
22
// RUN: %empty-directory(%t)
33
// RUN: %target-sil-opt %s -emit-sib -o %t/tmp.sib -module-name assignattr
44
// RUN: %target-sil-opt %t/tmp.sib -o %t/tmp.2.sib -module-name assignattr
5-
// RUN: %target-sil-opt %t/tmp.2.sib -module-name assignattr | %FileCheck %s
5+
// RUN: %target-sil-opt %t/tmp.2.sib -module-name assignattr -emit-sorted-sil | %FileCheck %s
66

77
sil_stage raw
88

99
import Builtin
1010

11-
// CHECK-LABEL: sil [serialized] [ossa] @trivial_assign : $@convention(thin) (@in Builtin.Int32, Builtin.Int32) -> () {
12-
// CHECK: bb0([[ARG1:%[0-9]+]] : $*Builtin.Int32, [[ARG2:%[0-9]+]] : $Builtin.Int32):
13-
// CHECK: assign [[ARG2]] to [init] [[ARG1]] : $*Builtin.Int32
14-
sil [serialized] [ossa] @trivial_assign : $@convention(thin) (@in Builtin.Int32, Builtin.Int32) -> () {
15-
bb0(%0 : $*Builtin.Int32, %1 : $Builtin.Int32):
16-
assign %1 to [init] %0 : $*Builtin.Int32
17-
%2 = tuple()
18-
return %2 : $()
19-
}
20-
2111
// CHECK-LABEL: sil [serialized] [ossa] @non_trivial_assign : $@convention(thin) (@in Builtin.NativeObject, @owned Builtin.NativeObject) -> () {
2212
// CHECK: bb0([[ARG1:%[0-9]+]] : $*Builtin.NativeObject, [[ARG2:%[0-9]+]] : @owned $Builtin.NativeObject):
2313
// CHECK: [[ARG2_COPY:%.*]] = copy_value [[ARG2]]
@@ -31,3 +21,13 @@ bb0(%0 : $*Builtin.NativeObject, %1 : @owned $Builtin.NativeObject):
3121
%9999 = tuple()
3222
return %9999 : $()
3323
}
24+
25+
// CHECK-LABEL: sil [serialized] [ossa] @trivial_assign : $@convention(thin) (@in Builtin.Int32, Builtin.Int32) -> () {
26+
// CHECK: bb0([[ARG1:%[0-9]+]] : $*Builtin.Int32, [[ARG2:%[0-9]+]] : $Builtin.Int32):
27+
// CHECK: assign [[ARG2]] to [init] [[ARG1]] : $*Builtin.Int32
28+
sil [serialized] [ossa] @trivial_assign : $@convention(thin) (@in Builtin.Int32, Builtin.Int32) -> () {
29+
bb0(%0 : $*Builtin.Int32, %1 : $Builtin.Int32):
30+
assign %1 to [init] %0 : $*Builtin.Int32
31+
%2 = tuple()
32+
return %2 : $()
33+
}

0 commit comments

Comments
 (0)