Skip to content

Commit 0174528

Browse files
committed
stdlib: fix the problem swept under the rug in 58a97f1
The `-Winvalid-offsetof` warning is valid in this case. `offsetof` is being applied to types with a non-standard layout. The layout of this type is undefined by the specification. There is no guarantee that the type layout is uniform across all ABIs. It is not possible to portably compute the offset statically, especially efficiently. Sink this check into a unit test to avoid performing this test at runtime. In order to do this in the standard library, we would need to do this check through a global constructor.
1 parent 40deafb commit 0174528

File tree

7 files changed

+252
-197
lines changed

7 files changed

+252
-197
lines changed

stdlib/public/Concurrency/DispatchGlobalExecutor.cpp

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -57,30 +57,6 @@
5757

5858
using namespace swift;
5959

60-
// Ensure that Job's layout is compatible with what Dispatch expects.
61-
// Note: MinimalDispatchObjectHeader just has the fields we care about, it is
62-
// not complete and should not be used for anything other than these asserts.
63-
struct MinimalDispatchObjectHeader {
64-
const void *VTable;
65-
int Opaque0;
66-
int Opaque1;
67-
void *Linkage;
68-
};
69-
70-
#pragma clang diagnostic push
71-
#pragma clang diagnostic ignored "-Wgnu-offsetof-extensions"
72-
#pragma clang diagnostic push
73-
#pragma clang diagnostic ignored "-Winvalid-offsetof"
74-
static_assert(
75-
offsetof(Job, metadata) == offsetof(MinimalDispatchObjectHeader, VTable),
76-
"Job Metadata field must match location of Dispatch VTable field.");
77-
static_assert(offsetof(Job, SchedulerPrivate[Job::DispatchLinkageIndex]) ==
78-
offsetof(MinimalDispatchObjectHeader, Linkage),
79-
"Dispatch Linkage field must match Job "
80-
"SchedulerPrivate[DispatchLinkageIndex].");
81-
#pragma clang diagnostic pop
82-
#pragma clang diagnostic pop
83-
8460
/// The function passed to dispatch_async_f to execute a job.
8561
static void __swift_run_job(void *_job) {
8662
SwiftJob *job = (SwiftJob*) _job;

stdlib/public/Concurrency/ExecutorChecks.cpp

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323

2424
#include "ExecutorImpl.h"
2525

26-
#pragma clang diagnostic push
27-
#pragma clang diagnostic ignored "-Wgnu-offsetof-extensions"
28-
2926
// JobFlags
3027
static_assert(sizeof(swift::JobFlags) == sizeof(SwiftJobFlags));
3128

@@ -51,29 +48,14 @@ static_assert((SwiftJobPriority)swift::JobPriority::Background
5148
static_assert((SwiftJobPriority)swift::JobPriority::Unspecified
5249
== SwiftUnspecifiedJobPriority);
5350

54-
#pragma clang diagnostic push
55-
#pragma clang diagnostic ignored "-Winvalid-offsetof"
5651
// Job (has additional fields not exposed via SwiftJob)
5752
static_assert(sizeof(swift::Job) >= sizeof(SwiftJob));
58-
static_assert(offsetof(swift::Job, metadata) == offsetof(SwiftJob, metadata));
59-
#if !SWIFT_CONCURRENCY_EMBEDDED
60-
static_assert(offsetof(swift::Job, refCounts) == offsetof(SwiftJob, refCounts));
61-
#else
62-
static_assert(offsetof(swift::Job, embeddedRefcount) == offsetof(SwiftJob, refCounts));
63-
#endif
64-
static_assert(offsetof(swift::Job, SchedulerPrivate) == offsetof(SwiftJob, schedulerPrivate));
65-
static_assert(offsetof(swift::Job, SchedulerPrivate[0]) == offsetof(SwiftJob, schedulerPrivate[0]));
66-
static_assert(offsetof(swift::Job, SchedulerPrivate[1]) == offsetof(SwiftJob, schedulerPrivate[1]));
67-
static_assert(offsetof(swift::Job, Flags) == offsetof(SwiftJob, flags));
68-
#pragma clang diagnostic pop
6953

7054
// SerialExecutorRef
7155
static_assert(sizeof(swift::SerialExecutorRef) == sizeof(SwiftExecutorRef));
7256

7357
// swift_clock_id
7458
static_assert((SwiftClockId)swift::swift_clock_id_continuous
7559
== SwiftContinuousClock);
76-
static_assert((SwiftClockId)swift::swift_clock_id_suspending
77-
== SwiftSuspendingClock);
78-
79-
#pragma clang diagnostic pop
60+
static_assert((SwiftClockId)swift::swift_clock_id_suspending ==
61+
SwiftSuspendingClock);
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
//===--- GenericCacheEntry.h ------------------------------------*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef SWIFT_RUNTIME_GENERICCACHEENTRY_H
14+
#define SWIFT_RUNTIME_GENERICCACHEENTRY_H
15+
16+
#include "MetadataCache.h"
17+
#include "swift/ABI/Metadata.h"
18+
#include "swift/ABI/MetadataValues.h"
19+
#include "swift/Basic/Unreachable.h"
20+
#include "swift/RemoteInspection/GenericMetadataCacheEntry.h"
21+
#include "swift/Runtime/LibPrespecialized.h"
22+
23+
namespace swift {
24+
25+
bool areAllTransitiveMetadataComplete_cheap(const Metadata *type);
26+
PrivateMetadataState inferStateForMetadata(Metadata *metadata);
27+
MetadataDependency checkTransitiveCompleteness(const Metadata *initialType);
28+
29+
struct GenericCacheEntry final
30+
: VariadicMetadataCacheEntryBase<GenericCacheEntry> {
31+
static const char *getName() { return "GenericCache"; }
32+
33+
// The constructor/allocate operations that take a `const Metadata *`
34+
// are used for the insertion of canonical specializations.
35+
// The metadata is always complete after construction.
36+
37+
GenericCacheEntry(MetadataCacheKey key, MetadataWaitQueue::Worker &worker,
38+
MetadataRequest request, const Metadata *candidate)
39+
: VariadicMetadataCacheEntryBase(key, worker,
40+
PrivateMetadataState::Complete,
41+
const_cast<Metadata *>(candidate)) {}
42+
43+
AllocationResult allocate(const Metadata *candidate) {
44+
swift_unreachable("always short-circuited");
45+
}
46+
47+
static bool allowMangledNameVerification(const Metadata *candidate) {
48+
// Disallow mangled name verification for specialized candidates
49+
// because it will trigger recursive entry into the swift_once
50+
// in cacheCanonicalSpecializedMetadata.
51+
// TODO: verify mangled names in a second pass in that function.
52+
return false;
53+
}
54+
55+
// The constructor/allocate operations that take a descriptor
56+
// and arguments are used along the normal allocation path.
57+
58+
GenericCacheEntry(MetadataCacheKey key, MetadataWaitQueue::Worker &worker,
59+
MetadataRequest request,
60+
const TypeContextDescriptor *description,
61+
const void *const *arguments)
62+
: VariadicMetadataCacheEntryBase(key, worker,
63+
PrivateMetadataState::Allocating,
64+
/*candidate*/ nullptr) {}
65+
66+
AllocationResult allocate(const TypeContextDescriptor *description,
67+
const void *const *arguments) {
68+
if (auto *prespecialized =
69+
getLibPrespecializedMetadata(description, arguments))
70+
return {prespecialized, PrivateMetadataState::Complete};
71+
72+
// Find a pattern. Currently we always use the default pattern.
73+
auto &generics = description->getFullGenericContextHeader();
74+
auto pattern = generics.DefaultInstantiationPattern.get();
75+
76+
// Call the pattern's instantiation function.
77+
auto metadata =
78+
pattern->InstantiationFunction(description, arguments, pattern);
79+
80+
// If there's no completion function, do a quick-and-dirty check to
81+
// see if all of the type arguments are already complete. If they
82+
// are, we can broadcast completion immediately and potentially avoid
83+
// some extra locking.
84+
PrivateMetadataState state;
85+
if (pattern->CompletionFunction.isNull()) {
86+
if (areAllTransitiveMetadataComplete_cheap(metadata)) {
87+
state = PrivateMetadataState::Complete;
88+
} else {
89+
state = PrivateMetadataState::NonTransitiveComplete;
90+
}
91+
} else {
92+
state = inferStateForMetadata(metadata);
93+
}
94+
95+
return {metadata, state};
96+
}
97+
98+
static bool
99+
allowMangledNameVerification(const TypeContextDescriptor *description,
100+
const void *const *arguments) {
101+
return true;
102+
}
103+
104+
MetadataStateWithDependency
105+
tryInitialize(Metadata *metadata, PrivateMetadataState state,
106+
PrivateMetadataCompletionContext *context) {
107+
assert(state != PrivateMetadataState::Complete);
108+
109+
// Finish the completion function.
110+
if (state < PrivateMetadataState::NonTransitiveComplete) {
111+
// Find a pattern. Currently we always use the default pattern.
112+
auto &generics =
113+
metadata->getTypeContextDescriptor()->getFullGenericContextHeader();
114+
auto pattern = generics.DefaultInstantiationPattern.get();
115+
116+
// Complete the metadata's instantiation.
117+
auto dependency =
118+
pattern->CompletionFunction(metadata, &context->Public, pattern);
119+
120+
// If this failed with a dependency, infer the current metadata state
121+
// and return.
122+
if (dependency) {
123+
return {inferStateForMetadata(metadata), dependency};
124+
}
125+
}
126+
127+
// Check for transitive completeness.
128+
if (auto dependency = checkTransitiveCompleteness(metadata)) {
129+
return {PrivateMetadataState::NonTransitiveComplete, dependency};
130+
}
131+
132+
// We're done.
133+
return {PrivateMetadataState::Complete, MetadataDependency()};
134+
}
135+
};
136+
137+
} // namespace swift
138+
139+
#endif

0 commit comments

Comments
 (0)