Skip to content

Commit 9bfae70

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.
1 parent f1b859c commit 9bfae70

File tree

5 files changed

+68
-42
lines changed

5 files changed

+68
-42
lines changed

include/swift/Runtime/STLCompatibility.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,21 @@ bit_cast(const Source &src) noexcept {
4242
}
4343
#endif
4444

45+
namespace {
46+
// NOTE: this is not possible to mark as `constexpr` as `reinterpret_cast` is
47+
// not permitted in `constexpr` statements. Additionally, even if we were able
48+
// to get away from the `reinterpret_cast`, we cannot de-reference the `nullptr`
49+
// in a `constexpr` statement. Finally, because we are checking the offsets of
50+
// members in non-standard layouts, we must delay this check until runtime as
51+
// the static computation is not well defined by the language standard.
52+
inline namespace __swift {
53+
template <typename Type, typename PMFType>
54+
size_t offset_of(PMFType Type::*member) {
55+
return reinterpret_cast<uintptr_t>(std::addressof(static_cast<Type *>(nullptr)->*member));
56+
}
57+
}
58+
59+
#define offset_of(T,d) __swift::offset_of(&T::d)
60+
}
61+
4562
#endif

stdlib/public/Concurrency/DispatchGlobalExecutor.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,6 @@ struct MinimalDispatchObjectHeader {
6767
void *Linkage;
6868
};
6969

70-
#pragma clang diagnostic push
71-
#pragma clang diagnostic ignored "-Wgnu-offsetof-extensions"
72-
static_assert(
73-
offsetof(Job, metadata) == offsetof(MinimalDispatchObjectHeader, VTable),
74-
"Job Metadata field must match location of Dispatch VTable field.");
75-
static_assert(offsetof(Job, SchedulerPrivate[Job::DispatchLinkageIndex]) ==
76-
offsetof(MinimalDispatchObjectHeader, Linkage),
77-
"Dispatch Linkage field must match Job "
78-
"SchedulerPrivate[DispatchLinkageIndex].");
79-
#pragma clang diagnostic pop
80-
8170
/// The function passed to dispatch_async_f to execute a job.
8271
static void __swift_run_job(void *_job) {
8372
SwiftJob *job = (SwiftJob*) _job;

stdlib/public/Concurrency/ExecutorChecks.cpp

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,6 @@ static_assert((SwiftJobPriority)swift::JobPriority::Unspecified
5353

5454
// Job (has additional fields not exposed via SwiftJob)
5555
static_assert(sizeof(swift::Job) >= sizeof(SwiftJob));
56-
static_assert(offsetof(swift::Job, metadata) == offsetof(SwiftJob, metadata));
57-
#if !SWIFT_CONCURRENCY_EMBEDDED
58-
static_assert(offsetof(swift::Job, refCounts) == offsetof(SwiftJob, refCounts));
59-
#else
60-
static_assert(offsetof(swift::Job, embeddedRefcount) == offsetof(SwiftJob, refCounts));
61-
#endif
62-
static_assert(offsetof(swift::Job, SchedulerPrivate) == offsetof(SwiftJob, schedulerPrivate));
63-
static_assert(offsetof(swift::Job, SchedulerPrivate[0]) == offsetof(SwiftJob, schedulerPrivate[0]));
64-
static_assert(offsetof(swift::Job, SchedulerPrivate[1]) == offsetof(SwiftJob, schedulerPrivate[1]));
65-
static_assert(offsetof(swift::Job, Flags) == offsetof(SwiftJob, flags));
6656

6757
// SerialExecutorRef
6858
static_assert(sizeof(swift::SerialExecutorRef) == sizeof(SwiftExecutorRef));
@@ -74,3 +64,31 @@ static_assert((SwiftClockId)swift::swift_clock_id_suspending
7464
== SwiftSuspendingClock);
7565

7666
#pragma clang diagnostic pop
67+
68+
#if !defined(NDEBUG)
69+
namespace swift {
70+
namespace concurrency {
71+
struct StaticAssertions {
72+
StaticAssertions() {
73+
assert(offset_of(swift::Job, metadata) == offset_of(SwiftJob, metadata));
74+
#if !SWIFT_CONCURRENCY_EMBEDDED
75+
assert(offset_of(swift::Job, refCounts) == offset_of(SwiftJob, refCounts));
76+
#else
77+
assert(offset_of(swift::Job, embeddedRefcount) == offset_of(SwiftJob, refCounts));
78+
#endif
79+
assert(offset_of(swift::Job, SchedulerPrivate) == offset_of(SwiftJob, schedulerPrivate));
80+
assert(offset_of(swift::Job, SchedulerPrivate[0]) == offset_of(SwiftJob, schedulerPrivate[0]));
81+
assert(offset_of(swift::Job, SchedulerPrivate[1]) == offset_of(SwiftJob, schedulerPrivate[1]));
82+
assert(offset_of(swift::Job, Flags) == offset_of(SwiftJob, flags));
83+
84+
assert(offset_of(Job, metadata) == offset_of(MinimalDispatchObjectHeader, VTable) &&
85+
"Job Metadata field must match location of Dispatch VTable field.");
86+
assert(offset_of(Job, SchedulerPrivate[Job::DispatchLinkageIndex]) == offset_of(MinimalDispatchObjectHeader, Linkage) &&
87+
"Dispatch Linkage field must match Job SchedulerPrivate[DispatchLinkageIndex].");
88+
}
89+
}
90+
}
91+
}
92+
93+
static swift::concurrency::StaticAssertions assertions;
94+
#endif

stdlib/public/runtime/Metadata.cpp

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@
3939
#include "swift/Runtime/Metadata.h"
4040
#include "swift/Runtime/Once.h"
4141
#include "swift/Runtime/Portability.h"
42+
#include "swift/Runtime/STLCompatibility.h"
4243
#include "swift/Strings.h"
4344
#include "swift/Threading/Mutex.h"
4445
#include "llvm/ADT/StringExtras.h"
4546
#include <algorithm>
4647
#include <cctype>
4748
#include <cinttypes>
4849
#include <condition_variable>
50+
#include <memory>
4951
#include <new>
5052
#include <unordered_set>
5153
#include <vector>
@@ -338,9 +340,9 @@ namespace {
338340
MetadataWaitQueue::Worker &worker,
339341
MetadataRequest request,
340342
const Metadata *candidate)
341-
: VariadicMetadataCacheEntryBase(key, worker,
342-
PrivateMetadataState::Complete,
343-
const_cast<Metadata*>(candidate)) {}
343+
: VariadicMetadataCacheEntryBase(key, worker,
344+
PrivateMetadataState::Complete,
345+
const_cast<Metadata*>(candidate)) {}
344346

345347
AllocationResult allocate(const Metadata *candidate) {
346348
swift_unreachable("always short-circuited");
@@ -362,9 +364,9 @@ namespace {
362364
MetadataRequest request,
363365
const TypeContextDescriptor *description,
364366
const void * const *arguments)
365-
: VariadicMetadataCacheEntryBase(key, worker,
366-
PrivateMetadataState::Allocating,
367-
/*candidate*/ nullptr) {}
367+
: VariadicMetadataCacheEntryBase(key, worker,
368+
PrivateMetadataState::Allocating,
369+
/*candidate*/ nullptr) {}
368370

369371
AllocationResult allocate(const TypeContextDescriptor *description,
370372
const void * const *arguments) {
@@ -438,19 +440,6 @@ namespace {
438440
};
439441
} // end anonymous namespace
440442

441-
namespace swift {
442-
#pragma clang diagnostic push
443-
#pragma clang diagnostic ignored "-Winvalid-offsetof"
444-
struct StaticAssertGenericMetadataCacheEntryValueOffset {
445-
static_assert(
446-
offsetof(GenericCacheEntry, Value) ==
447-
offsetof(swift::GenericMetadataCacheEntry<InProcess::StoredPointer>,
448-
Value),
449-
"The generic metadata cache entry layout mismatch");
450-
};
451-
#pragma clang diagnostic pop
452-
}
453-
454443
namespace {
455444
class GenericMetadataCache :
456445
public MetadataCache<GenericCacheEntry, GenericMetadataCacheTag> {
@@ -8264,3 +8253,18 @@ const HeapObject *swift_getKeyPathImpl(const void *pattern,
82648253
#if defined(__MACH__)
82658254
asm(".linker_option \"-lc++\"\n");
82668255
#endif // defined(__MACH__)
8256+
8257+
#if !defined(NDEBUG)
8258+
namespace swift {
8259+
namespace runtime {
8260+
struct StaticAssertions {
8261+
StaticAssertions() {
8262+
assert(offset_of(GenericCacheEntry, Value) == offset_of(swift::GenericMetadataCacheEntry<InProcess::StoredPointer>, Value) &&
8263+
"GenericMetadataCacheEntry layout mismatch");
8264+
}
8265+
};
8266+
}
8267+
}
8268+
8269+
static swift::runtime::StaticAssertions assertions;
8270+
#endif

stdlib/public/runtime/MetadataCache.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,8 +1611,6 @@ class VariadicMetadataCacheEntryBase :
16111611
bool matchesKey(const MetadataCacheKey &key) const {
16121612
return key == getKey();
16131613
}
1614-
1615-
friend struct StaticAssertGenericMetadataCacheEntryValueOffset;
16161614
};
16171615

16181616
template <class EntryType, uint16_t Tag>

0 commit comments

Comments
 (0)