Fix undefined behavior in SmallString.withUTF8 #33698

atrick · 2020-08-29T20:29:21Z

Fix undefined behavior in SmallString.withUTF8

withUTF8 currently vends a typed UInt8 pointer to the underlying
SmallString. That pointer type differs from SmallString's
representation. It should simply vend a raw pointer, which would be
both type safe and convenient for UTF8 data. However, since this
method is already @inlinable, I added calls to bindMemory to prevent
the optimizer from reasoning about access to the typed pointer that we
vend.

rdar://67983613 (Undefinied behavior in SmallString.withUTF8 is miscompiled)

Additional commentary:

SmallString creates a situation where there are two types, the
in-memory type, (UInt64, UInt64), vs. the element type,
UInt8. `UnsafePointer<T>` specifies the in-memory type of the pointee,
because that's how C works. If you want to specify an element type,
not the in-memory type, then you need to use something other than
UnsafePointer to view the memory. A trivial `BufferView<UInt8>` would
be fine, although, frankly, I think UnsafeRawPointer is a perfectly
good type on its own for UTF8 bytes.

Unfortunately, a lot of the UTF8 helper code is ABI-exposed, so to
work around this, we need to insert calls to bindMemory at strategic
points to avoid undefined behavior. This is high-risk and can
negatively affect performance. So far, I was able to resolve the
regressions in our microbenchmarks just by tweaking the inliner.

atrick · 2020-08-29T20:29:52Z

Actually there is still one regression that I'll just file a separate bug for:
MapReduceString 39 46 +17.9% 0.85x

atrick · 2020-08-29T20:32:10Z

Incidentally, some of the BufferPointer code could be cleaner by relying on convenience methods that calculate capacity, but I think this code should stick to the most primitive operations to guarantee performance.

atrick · 2020-08-29T20:32:40Z

@swift-ci benchmark

atrick · 2020-08-29T20:32:45Z

@swift-ci test

atrick · 2020-08-29T20:34:53Z

The current stdlib build does not appear to miscompile, but the miscompile is easily exposed by tweaking the optimizer in unrelated ways, and it could happen in user code since withUTF8 is inlinable.

swift-ci · 2020-08-29T21:34:34Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	2726	3557	+30.5%	0.77x (?)
MapReduceString	37	44	+18.9%	0.84x (?)
Set.isSubset.Int.Empty	43	51	+18.6%	0.84x
ArrayLiteral2	67	79	+17.9%	0.85x (?)
Set.isStrictSubset.Int.Empty	45	53	+17.8%	0.85x
UTF8Decode_InitDecoding	143	167	+16.8%	0.86x (?)
UTF8Decode_InitFromCustom_contiguous	143	164	+14.7%	0.87x (?)
CharacterLiteralsLarge	63	71	+12.7%	0.89x (?)
Set.isStrictSubset.Seq.Int.Empty	108	120	+11.1%	0.90x (?)
Set.isSuperset.Seq.Empty.Int	45	50	+11.1%	0.90x (?)
Set.isDisjoint.Box.Empty	93	103	+10.8%	0.90x (?)
ObjectiveCBridgeStubToNSStringRef	70	77	+10.0%	0.91x (?)
Set.isDisjoint.Int.Empty	51	56	+9.8%	0.91x
Set.isDisjoint.Seq.Int.Empty	47	51	+8.5%	0.92x (?)
CharacterLiteralsSmall	217	235	+8.3%	0.92x
CharacterPropertiesStashedMemo	870	940	+8.0%	0.93x (?)
BucketSort	137	148	+8.0%	0.93x (?)
UTF8Decode_InitFromCustom_noncontiguous	296	319	+7.8%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
ArrayAppendRepeatCol	670	400	-40.3%	1.67x (?)
LessSubstringSubstring	30	22	-26.7%	1.36x
EqualStringSubstring	30	22	-26.7%	1.36x
EqualSubstringString	30	22	-26.7%	1.36x
LessSubstringSubstringGenericComparable	30	22	-26.7%	1.36x
EqualSubstringSubstring	29	22	-24.1%	1.32x
EqualSubstringSubstringGenericEquatable	29	22	-24.1%	1.32x
Breadcrumbs.MutatedIdxToUTF16.Mixed	322	278	-13.7%	1.16x
Breadcrumbs.MutatedUTF16ToIdx.Mixed	316	273	-13.6%	1.16x
StringComparison_longSharedPrefix	373	337	-9.7%	1.11x
SuffixAnySequence	1691	1564	-7.5%	1.08x (?)
SequenceAlgosUnfoldSequence	840	780	-7.1%	1.08x (?)
DictionaryOfAnyHashableStrings_lookup	3648	3408	-6.6%	1.07x (?)

Code size: -O

Improvement	OLD	NEW	DELTA	RATIO
CSVParsing.o	54315	52979	-2.5%	1.03x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
ArrayAppendRepeatCol	500	850	+70.0%	0.59x (?)
Set.subtracting.Empty.Box	7	9	+28.6%	0.78x
MapReduceString	33	42	+27.3%	0.79x
DataCreateEmptyArray	800	950	+18.7%	0.84x (?)
UTF8Decode_InitFromCustom_contiguous	144	168	+16.7%	0.86x
UTF8Decode_InitDecoding	146	165	+13.0%	0.88x
Set.isStrictSubset.Int.Empty	47	53	+12.8%	0.89x (?)
CharacterPropertiesStashedMemo	870	960	+10.3%	0.91x (?)
ArrayLiteral2	98	107	+9.2%	0.92x (?)
Set.isStrictSubset.Seq.Int.Empty	112	122	+8.9%	0.92x (?)
UTF8Decode_InitFromCustom_noncontiguous	262	284	+8.4%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
LessSubstringSubstring	30	22	-26.7%	1.36x
LessSubstringSubstringGenericComparable	30	22	-26.7%	1.36x
EqualStringSubstring	29	22	-24.1%	1.32x
EqualSubstringSubstringGenericEquatable	29	22	-24.1%	1.32x (?)
EqualSubstringString	30	23	-23.3%	1.30x
EqualSubstringSubstring	29	23	-20.7%	1.26x
FlattenListLoop	1530	1272	-16.9%	1.20x (?)
Breadcrumbs.MutatedUTF16ToIdx.Mixed	317	272	-14.2%	1.17x
Breadcrumbs.MutatedIdxToUTF16.Mixed	322	278	-13.7%	1.16x
Dictionary4	233	202	-13.3%	1.15x
StringComparison_longSharedPrefix	374	337	-9.9%	1.11x
Dictionary4OfObjects	296	268	-9.5%	1.10x (?)
Set.isSubset.Seq.Empty.Int	84	77	-8.3%	1.09x (?)
RemoveWhereFilterInts	26	24	-7.7%	1.08x (?)
DistinctClassFieldAccesses	201	186	-7.5%	1.08x (?)
DictionaryOfAnyHashableStrings_lookup	3672	3408	-7.2%	1.08x (?)
ObjectiveCBridgeStubFromNSStringRef	99	92	-7.1%	1.08x (?)
ObjectiveCBridgeStubNSDataAppend	1730	1610	-6.9%	1.07x (?)

Code size: -Osize

Improvement	OLD	NEW	DELTA	RATIO
CSVParsing.o	52209	50797	-2.7%	1.03x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
UTF8Decode_InitDecoding	152	171	+12.5%	0.89x (?)
UTF8Decode_InitFromCustom_contiguous	159	174	+9.4%	0.91x (?)
ExclusivityGlobal	106	114	+7.5%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
EqualSubstringSubstringGenericEquatable	34	26	-23.5%	1.31x
LessSubstringSubstringGenericComparable	34	26	-23.5%	1.31x
EqualSubstringString	35	28	-20.0%	1.25x
EqualSubstringSubstring	34	28	-17.6%	1.21x
LessSubstringSubstring	34	28	-17.6%	1.21x
EqualStringSubstring	34	28	-17.6%	1.21x
Breadcrumbs.MutatedUTF16ToIdx.Mixed	334	291	-12.9%	1.15x (?)
Breadcrumbs.MutatedIdxToUTF16.Mixed	340	297	-12.6%	1.14x (?)
SubstringFromLongString	10	9	-10.0%	1.11x

Code size: -swiftlibs

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

swift-ci · 2020-08-29T21:49:46Z

Build failed
Swift Test OS X Platform
Git Sha - 9d5acf50210ce5a51ce430dd9984f91567bbce43

benrimmington · 2020-08-29T22:28:41Z

#33693 fixes macOS test failure.

benrimmington · 2020-08-29T22:28:52Z

@swift-ci Please test macOS platform

atrick · 2020-09-11T23:40:23Z

@milseman do you have any concerns with this?

milseman

LGTM

xwu

Nits :)

stdlib/public/core/SmallString.swift

withUTF8 currently vends a typed UInt8 pointer to the underlying SmallString. That pointer type differs from SmallString's representation. It should simply vend a raw pointer, which would be both type safe and convenient for UTF8 data. However, since this method is already @inlinable, I added calls to bindMemory to prevent the optimizer from reasoning about access to the typed pointer that we vend. rdar://67983613 (Undefinied behavior in SmallString.withUTF8 is miscompiled) Additional commentary: SmallString creates a situation where there are two types, the in-memory type, (UInt64, UInt64), vs. the element type, UInt8. `UnsafePointer<T>` specifies the in-memory type of the pointee, because that's how C works. If you want to specify an element type, not the in-memory type, then you need to use something other than UnsafePointer to view the memory. A trivial `BufferView<UInt8>` would be fine, although, frankly, I think UnsafeRawPointer is a perfectly good type on its own for UTF8 bytes. Unfortunately, a lot of the UTF8 helper code is ABI-exposed, so to work around this, we need to insert calls to bindMemory at strategic points to avoid undefined behavior. This is high-risk and can negatively affect performance. So far, I was able to resolve the regressions in our microbenchmarks just by tweaking the inliner.

bind_memory has no actual code size cost, and this is the only way to allow rebinding memory within critical standard library code like SmallString without regressing performance.

atrick · 2020-09-25T01:37:58Z

Nits :)

Corrections have all been committed. Much appreciated.

atrick · 2020-09-25T01:38:08Z

@swift-ci test

atrick · 2020-09-25T01:38:16Z

@swift-ci benchmark

swift-ci · 2020-09-25T02:47:32Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
CSVParsing.UTF8	30	68	+126.7%	0.44x
ArrayAppendLatin1Substring	25560	48420	+89.4%	0.53x
ArrayAppendUTF16Substring	25056	47412	+89.2%	0.53x
ArrayAppendAsciiSubstring	25056	47412	+89.2%	0.53x
MapReduceString	41	61	+48.8%	0.67x
String.data.Medium	96	111	+15.6%	0.86x (?)
DictionaryKeysContainsCocoa	28	32	+14.3%	0.88x (?)
NSStringConversion.MutableCopy.Rebridge.UTF8	696	791	+13.6%	0.88x (?)
ObjectiveCBridgeFromNSSetAnyObjectToStringForced	75500	82500	+9.3%	0.92x (?)
EqualSubstringSubstring	35	38	+8.6%	0.92x
EqualSubstringSubstringGenericEquatable	35	38	+8.6%	0.92x (?)
EqualSubstringString	35	38	+8.6%	0.92x
Array2D	5984	6480	+8.3%	0.92x (?)
DataToStringLargeUnicode	6200	6700	+8.1%	0.93x (?)
InsertCharacterEndIndexNonASCII	50	54	+8.0%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	5706	5123	-10.2%	1.11x (?)
Set.isSubset.Seq.Int.Empty	209	192	-8.1%	1.09x
Set.isStrictSubset.Seq.Int.Empty	201	185	-8.0%	1.09x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced	6380	5920	-7.2%	1.08x (?)
SubstringEqualString	428	399	-6.8%	1.07x (?)

Code size: -O

Improvement	OLD	NEW	DELTA	RATIO
CSVParsing.o	53691	52787	-1.7%	1.02x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
CSVParsing.UTF8	31	69	+122.6%	0.45x
ArrayAppendLatin1Substring	29052	46944	+61.6%	0.62x
ArrayAppendUTF16Substring	28476	45972	+61.4%	0.62x
ArrayAppendAsciiSubstring	28476	45972	+61.4%	0.62x
FlattenListFlatMap	3790	5669	+49.6%	0.67x (?)
MapReduceString	46	66	+43.5%	0.70x
String.data.Medium	101	110	+8.9%	0.92x (?)
NSStringConversion	379	412	+8.7%	0.92x (?)
EqualSubstringSubstring	35	38	+8.6%	0.92x
EqualStringSubstring	35	38	+8.6%	0.92x (?)
EqualSubstringSubstringGenericEquatable	35	38	+8.6%	0.92x (?)
EqualSubstringString	35	38	+8.6%	0.92x (?)
LessSubstringSubstring	36	39	+8.3%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
StrComplexWalk	4770	4320	-9.4%	1.10x (?)
Set.subtracting.Empty.Box	15	14	-6.7%	1.07x (?)
Set.subtracting.Seq.Empty.Box	196	183	-6.6%	1.07x (?)
StringWalk	2440	2280	-6.6%	1.07x

Code size: -Osize

Improvement	OLD	NEW	DELTA	RATIO
CSVParsing.o	51869	50396	-2.8%	1.03x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
StringToDataMedium	6850	7500	+9.5%	0.91x (?)
ArrayLiteral2	4481	4906	+9.5%	0.91x (?)

Improvement	OLD	NEW	DELTA	RATIO
SevenBoom	1895	1668	-12.0%	1.14x (?)
ConvertFloatingPoint.MockFloat64Exactly2	2191	2006	-8.4%	1.09x (?)

Code size: -swiftlibs

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 8-Core Intel Xeon E5
  Processor Speed: 3 GHz
  Number of Processors: 1
  Total Number of Cores: 8
  L2 Cache (per Core): 256 KB
  L3 Cache: 25 MB
  Memory: 64 GB

atrick · 2020-09-25T05:28:58Z

I haven't changed these commits, but now there are large regressions.

shahmishal · 2020-10-01T06:17:03Z

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

How to change the base branch: (Link)
More detail about the branch update: (Link)

atrick · 2020-10-26T06:04:36Z

I just hit another miscompile because of this after making an a small change to inlining, so I really need to merge it without investigating regressions. The performance regressions that popped up recently weren't originally a problem, so I'm sure they are fixable but will need a little time to investigate.

atrick · 2020-10-26T06:04:44Z

@swift-ci benchmark

atrick · 2020-10-26T06:04:52Z

@swift-ci test

swift-ci · 2020-10-26T06:32:10Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
ArrayAppendRepeatCol	400	670	+67.5%	0.60x (?)
FlattenListLoop	930	1542	+65.8%	0.60x (?)
MapReduceString	32	47	+46.9%	0.68x (?)
InsertCharacterEndIndexNonASCII	39	42	+7.7%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
EqualSubstringSubstring	30	22	-26.7%	1.36x (?)
EqualSubstringSubstringGenericEquatable	30	22	-26.7%	1.36x (?)
EqualSubstringString	30	22	-26.7%	1.36x
LessSubstringSubstring	30	23	-23.3%	1.30x
EqualStringSubstring	30	23	-23.3%	1.30x (?)
LessSubstringSubstringGenericComparable	30	23	-23.3%	1.30x (?)
ArrayAppendAsciiSubstring	24948	19404	-22.2%	1.29x (?)
ArrayAppendLatin1Substring	25452	19836	-22.1%	1.28x (?)
ArrayAppendUTF16Substring	24876	19404	-22.0%	1.28x (?)
UTF8Decode_InitDecoding	172	144	-16.3%	1.19x (?)
UTF8Decode_InitFromCustom_contiguous	171	145	-15.2%	1.18x
Set.isSuperset.Seq.Empty.Int	50	46	-8.0%	1.09x (?)
UTF8Decode_InitFromCustom_noncontiguous	295	272	-7.8%	1.08x (?)
SubstringEqualString	296	275	-7.1%	1.08x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
MapReduceString	35	50	+42.9%	0.70x

Improvement	OLD	NEW	DELTA	RATIO
EqualSubstringSubstring	30	22	-26.7%	1.36x
EqualSubstringSubstringGenericEquatable	30	22	-26.7%	1.36x
EqualSubstringString	30	22	-26.7%	1.36x (?)
LessSubstringSubstring	30	23	-23.3%	1.30x
EqualStringSubstring	30	23	-23.3%	1.30x (?)
LessSubstringSubstringGenericComparable	30	23	-23.3%	1.30x (?)
UTF8Decode_InitDecoding	172	146	-15.1%	1.18x (?)
UTF8Decode_InitFromCustom_contiguous	172	147	-14.5%	1.17x
StringComparison_longSharedPrefix	372	327	-12.1%	1.14x
Set.isSubset.Int.Empty	52	47	-9.6%	1.11x (?)
Set.isStrictSubset.Seq.Int.Empty	125	114	-8.8%	1.10x (?)
Set.isDisjoint.Seq.Box.Empty	86	80	-7.0%	1.07x (?)

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
CharIteration_punctuatedJapanese_unicodeScalars_Backwards	18840	20440	+8.5%	0.92x (?)
CharIteration_tweet_unicodeScalars_Backwards	200520	217480	+8.5%	0.92x (?)
CharIteration_ascii_unicodeScalars_Backwards	102160	110760	+8.4%	0.92x (?)
DataReplaceLargeBuffer	12	13	+8.3%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
UTF8Decode_InitFromCustom_contiguous	180	155	-13.9%	1.16x
UTF8Decode_InitDecoding	177	155	-12.4%	1.14x
ArrayOfGenericPOD2	588	542	-7.8%	1.08x (?)
EqualSubstringSubstring	91	84	-7.7%	1.08x (?)
LessSubstringSubstringGenericComparable	90	84	-6.7%	1.07x (?)
EqualSubstringString	91	85	-6.6%	1.07x (?)

Code size: -swiftlibs

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

benrimmington · 2020-10-26T07:19:31Z

I haven't changed these commits, but now there are large regressions.

@atrick Could the previous regressions (on 25 Sep) have been due to the creation of the main branch (on 23 Sep), somehow affecting the baseline used by benchmarks?

atrick · 2020-10-26T07:25:55Z

I haven't changed these commits, but now there are large regressions.

@atrick Could the previous regressions (on 25 Sep) have been due to the creation of the main branch (on 23 Sep), somehow affecting the baseline used by benchmarks?

Possibly, I never saw a problem with local benchmarking.

atrick requested a review from milseman August 29, 2020 20:29

milseman approved these changes Sep 16, 2020

View reviewed changes

xwu reviewed Sep 16, 2020

View reviewed changes

stdlib/public/core/SmallString.swift Outdated Show resolved Hide resolved

stdlib/public/core/SmallString.swift Outdated Show resolved Hide resolved

stdlib/public/core/SmallString.swift Outdated Show resolved Hide resolved

stdlib/public/core/SmallString.swift Outdated Show resolved Hide resolved

atrick added 2 commits September 24, 2020 18:36

Make bind_memory free to inline.

42bf92a

bind_memory has no actual code size cost, and this is the only way to allow rebinding memory within critical standard library code like SmallString without regressing performance.

atrick force-pushed the fix-smallstring branch from 303f183 to 42bf92a Compare September 25, 2020 01:36

atrick changed the title ~~Fix smallstring~~ Fix undefined behavior in SmallString.withUTF8 Sep 25, 2020

atrick changed the base branch from master to main October 2, 2020 03:04

atrick merged commit 7f09bbf into swiftlang:main Oct 26, 2020

atrick deleted the fix-smallstring branch November 9, 2020 17:15

Fix undefined behavior in SmallString.withUTF8 #33698

Fix undefined behavior in SmallString.withUTF8 #33698

Uh oh!

Conversation

atrick commented Aug 29, 2020

Uh oh!

atrick commented Aug 29, 2020

Uh oh!

atrick commented Aug 29, 2020

Uh oh!

atrick commented Aug 29, 2020

Uh oh!

atrick commented Aug 29, 2020

Uh oh!

atrick commented Aug 29, 2020

Uh oh!

swift-ci commented Aug 29, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

swift-ci commented Aug 29, 2020

Uh oh!

benrimmington commented Aug 29, 2020

Uh oh!

benrimmington commented Aug 29, 2020

Uh oh!

atrick commented Sep 11, 2020

Uh oh!

milseman left a comment

Choose a reason for hiding this comment

Uh oh!

xwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atrick commented Sep 25, 2020

Uh oh!

atrick commented Sep 25, 2020

Uh oh!

atrick commented Sep 25, 2020

Uh oh!

swift-ci commented Sep 25, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

atrick commented Sep 25, 2020

Uh oh!

shahmishal commented Oct 1, 2020

Uh oh!

atrick commented Oct 26, 2020

Uh oh!

atrick commented Oct 26, 2020

Uh oh!

atrick commented Oct 26, 2020

Uh oh!

swift-ci commented Oct 26, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

benrimmington commented Oct 26, 2020

Uh oh!

atrick commented Oct 26, 2020

Uh oh!

Uh oh!