[Parse] Tweak a utility function that relies on reading past the end #19230

jrose-apple · 2018-09-10T22:38:52Z

Lexer::getEncodedStringSegment (now getEncodedStringSegmentImpl) assumes that it can read one byte past the end of a string segment in order to avoid bounds-checks on things like "is this a \r\n sequence?". However, the function was being used for strings that did not come from source where this assumption was not always valid. Change the reusable form of the function to always copy into a temporary buffer, allowing the fast path to continue to be used for normal parsing.

Caught by ASan!

rdar://problem/44228891

jrose-apple · 2018-09-10T22:39:33Z

cc @johnno1962. Here's my alternate solution to #19219, which might fix your problem there too.

@swift-ci Please asan test

jrose-apple · 2018-09-10T22:39:41Z

@swift-ci Please test

jrose-apple · 2018-09-10T22:39:47Z

@swift-ci Please test compiler performance

jrose-apple · 2018-09-11T00:23:39Z

Uh, this ASan failure in SILCombine isn't related to what I was fixing. @eeckstein?

rintaro

Thanks! This looks good. (after resolving the conflict)

swift-ci · 2018-09-11T01:52:51Z

Build comment file:

Compilation-performance test failed

jrose-apple · 2018-09-11T02:15:51Z

@swift-ci Please smoke test

Lexer::getEncodedStringSegment (now getEncodedStringSegmentImpl) assumes that it can read one byte past the end of a string segment in order to avoid bounds-checks on things like "is this a \r\n sequence?". However, the function was being used for strings that did not come from source where this assumption was not always valid. Change the reusable form of the function to always copy into a temporary buffer, allowing the fast path to continue to be used for normal parsing. Caught by ASan! rdar://problem/44306756

jrose-apple · 2018-09-11T02:16:35Z

@swift-ci Please test compiler performance

swift-ci · 2018-09-11T02:20:33Z

Build comment file:

Compilation-performance test failed

eeckstein · 2018-09-11T04:32:39Z

@jrose-apple Which asan failure in SILCombine failure?

rintaro · 2018-09-11T11:39:21Z

@eeckstein https://ci.swift.org/view/Pull%20Request/job/swift-PR-osx-ASAN-test/49/

log

19:17:29 =================================================================
19:17:29 ==9759==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffee9f539b0 at pc 0x000107e2366a bp 0x7ffee9f52f90 sp 0x7ffee9f52f88
19:17:29 READ of size 8 at 0x7ffee9f539b0 thread T0
19:17:29     #0 0x107e23669 in bool llvm::function_ref<bool (swift::CanType)>::callback_fn<swift::SILCombiner::canReplaceArg(swift::FullApplySite, swift::ConcreteExistentialInfo const&, unsigned int)::$_2>(long, swift::CanType) SILCombinerApplyVisitors.cpp:771
19:17:29     #1 0x109a1e5ec in swift::Type::findIf(llvm::function_ref<bool (swift::Type)>) const::Walker::walkToTypePre(swift::Type) STLExtras.h:132
19:17:29     #2 0x109a46b76 in swift::Type::walk(swift::TypeWalker&) const TypeWalker.cpp:194
19:17:29     #3 0x1099f045c in swift::Type::findIf(llvm::function_ref<bool (swift::Type)>) const Type.h:224
19:17:29     #4 0x107e19f09 in swift::SILCombiner::canReplaceArg(swift::FullApplySite, swift::ConcreteExistentialInfo const&, unsigned int) Type.h:401
19:17:29     #5 0x107e18e52 in swift::SILCombiner::createApplyWithConcreteType(swift::FullApplySite, llvm::SmallDenseMap<unsigned int, swift::ConcreteExistentialInfo const*, 4u, llvm::DenseMapInfo<unsigned int>, llvm::detail::DenseMapPair<unsigned int, swift::ConcreteExistentialInfo const*> > const&, swift::SILBuilderContext&) SILCombinerApplyVisitors.cpp:851
19:17:29     #6 0x107e1c086 in swift::SILCombiner::propagateConcreteTypeOfInitExistential(swift::FullApplySite) SILCombinerApplyVisitors.cpp:1011
19:17:29     #7 0x107e1f147 in swift::SILCombiner::visitApplyInst(swift::ApplyInst*) SILCombinerApplyVisitors.cpp:1246
19:17:29     #8 0x107e06ef1 in swift::SILCombiner::doOneIteration(swift::SILFunction&, unsigned int) SILCombine.cpp:178
19:17:29     #9 0x107e09e56 in swift::SILCombiner::runOnFunction(swift::SILFunction&) SILCombine.cpp:248
19:17:29     #10 0x107e0bc7f in (anonymous namespace)::SILCombine::run() SILCombine.cpp:359
19:17:29     #11 0x107daa4e8 in swift::SILPassManager::runPassOnFunction(unsigned int, swift::SILFunction*) PassManager.cpp:409
19:17:29     #12 0x107dae921 in swift::SILPassManager::runFunctionPasses(unsigned int, unsigned int) PassManager.cpp:498
19:17:29     #13 0x107db29ab in swift::SILPassManager::execute() PassManager.cpp:618
19:17:29     #14 0x106259087 in swift::SILPassManager::executePassPipelinePlan(swift::SILPassPipelinePlan const&) PassManager.h:269
19:17:29     #15 0x107dd24f0 in swift::runSILOptimizationPasses(swift::SILModule&) Passes.cpp:102
19:17:29     #16 0x105db5a7a in performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) FrontendTool.cpp:1083
19:17:29     #17 0x105da7c9b in swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) FrontendTool.cpp:1838
19:17:29     #18 0x105caaf59 in run_driver(llvm::StringRef, llvm::ArrayRef<char const*>) driver.cpp:122
19:17:29     #19 0x105ca8334 in main driver.cpp:246
19:17:29     #20 0x7fff6e15f014 in start (libdyld.dylib:x86_64+0x1014)
19:17:29 
19:17:29 Address 0x7ffee9f539b0 is located in stack of thread T0 at offset 368 in frame
19:17:29     #0 0x107e1b72f in swift::SILCombiner::propagateConcreteTypeOfInitExistential(swift::FullApplySite) SILCombinerApplyVisitors.cpp:981
19:17:29 
19:17:29   This frame has 7 object(s):
19:17:29     [32, 40) 'Apply'
19:17:29     [64, 128) 'BuilderCtx' (line 986)
19:17:29     [160, 216) 'OpenedArchetypesTracker' (line 987)
19:17:29     [256, 336) 'CEIs' (line 989)
19:17:29     [368, 440) 'CEI' (line 994) <== Memory access at offset 368 is inside this variable
19:17:29     [480, 496) 'ref.tmp25' (line 998)
19:17:29     [512, 552) 'tmp' (line 998)
19:17:29 HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
19:17:29       (longjmp and C++ exceptions *are* supported)
19:17:29 SUMMARY: AddressSanitizer: stack-use-after-scope SILCombinerApplyVisitors.cpp:771 in bool llvm::function_ref<bool (swift::CanType)>::callback_fn<swift::SILCombiner::canReplaceArg(swift::FullApplySite, swift::ConcreteExistentialInfo const&, unsigned int)::$_2>(long, swift::CanType)
19:17:29 Shadow bytes around the buggy address:
19:17:29   0x1fffdd3ea6e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
19:17:29   0x1fffdd3ea6f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
19:17:29   0x1fffdd3ea700: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2
19:17:29   0x1fffdd3ea710: 00 00 00 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00
19:17:29   0x1fffdd3ea720: 00 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00
19:17:29 =>0x1fffdd3ea730: 00 00 f2 f2 f2 f2[f8]f8 f8 f8 f8 f8 f8 f8 f8 f2
19:17:29   0x1fffdd3ea740: f2 f2 f2 f2 f8 f8 f2 f2 f8 f8 f8 f8 f8 f3 f3 f3
19:17:29   0x1fffdd3ea750: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
19:17:29   0x1fffdd3ea760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
19:17:29   0x1fffdd3ea770: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
19:17:29   0x1fffdd3ea780: f8 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 f2
19:17:29 Shadow byte legend (one shadow byte represents 8 application bytes):
19:17:29   Addressable:           00
19:17:29   Partially addressable: 01 02 03 04 05 06 07 
19:17:29   Heap left redzone:       fa
19:17:29   Freed heap region:       fd
19:17:29   Stack left redzone:      f1
19:17:29   Stack mid redzone:       f2
19:17:29   Stack right redzone:     f3
19:17:29   Stack after return:      f5
19:17:29   Stack use after scope:   f8
19:17:29   Global redzone:          f9
19:17:29   Global init order:       f6
19:17:29   Poisoned by user:        f7
19:17:29   Container overflow:      fc
19:17:29   Array cookie:            ac
19:17:29   Intra object redzone:    bb
19:17:29   ASan internal:           fe
19:17:29   Left alloca redzone:     ca
19:17:29   Right alloca redzone:    cb
19:17:29 ==9759==ABORTING

eeckstein · 2018-09-11T15:50:33Z

Thanks. The asan failure is caused by #17370.
Reverting in #19248

jrose-apple · 2018-09-11T20:23:48Z

@swift-ci Please test compiler performance

jrose-apple · 2018-09-11T20:24:00Z

@swift-ci Please asan test

jrose-apple · 2018-09-11T21:31:12Z

@swift-ci Please asan test

jrose-apple · 2018-09-11T21:32:36Z

@swift-ci Please test compiler performance

swift-ci · 2018-09-12T03:08:04Z

Build comment file:

Summary for master full

Unexpected test results, excluded stats for RxSwift, ProcedureKit, ChattoAdditions, Wordy, ReactiveSwift

No regressions above thresholds

Debug-batch
- debug-batch brief
- debug-batch detailed
Release
- release brief
- release detailed

Debug-batch

debug-batch brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (3)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	11,347,063,955,734	11,346,873,597,699	-190,358,035	-0.0%
LLVM.NumLLVMBytesOutput	476,919,772	476,919,798	26	0.0%
time.swift-driver.wall	1147.5s	1150.3s	2.8s	0.24%

debug-batch detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (2)

name	old	new	delta	delta_pct
Driver.NumDriverPipePolls	265,221	260,997	-4,224	-1.59% ✅
Driver.NumDriverPipeReads	278,861	273,218	-5,643	-2.02% ✅

Unchanged (delta < 1.0% or delta < 100.0ms) (92)

name	old	new	delta	delta_pct
AST.NumASTBytesAllocated	11,264,914,213	11,264,937,854	23,641	0.0%
AST.NumDecls	31,738	31,738	0	0.0%
AST.NumDependencies	66,864	66,864	0	0.0%
AST.NumImportedExternalDefinitions	627,579	627,579	0	0.0%
AST.NumInfixOperators	12,659	12,659	0	0.0%
AST.NumLinkLibraries	0	0	0	0.0%
AST.NumLoadedModules	77,198	77,198	0	0.0%
AST.NumLocalTypeDecls	106	106	0	0.0%
AST.NumObjCMethods	11,744	11,744	0	0.0%
AST.NumPostfixOperators	14	14	0	0.0%
AST.NumPrecedenceGroups	6,661	6,661	0	0.0%
AST.NumPrefixOperators	55	55	0	0.0%
AST.NumReferencedDynamicNames	101	101	0	0.0%
AST.NumReferencedMemberNames	1,739,157	1,739,157	0	0.0%
AST.NumReferencedTopLevelNames	110,861	110,861	0	0.0%
AST.NumSourceBuffers	102,837	102,837	0	0.0%
AST.NumSourceLines	1,140,644	1,140,644	0	0.0%
AST.NumSourceLinesPerSecond	570,682	568,026	-2,656	-0.47%
AST.NumTotalClangImportedEntities	2,090,324	2,090,324	0	0.0%
AST.NumUsedConformances	97,005	97,005	0	0.0%
Driver.ChildrenMaxRSS	35,854,782,464	35,772,538,880	-82,243,584	-0.23%
Driver.DriverDepCascadingDynamic	0	0	0	0.0%
Driver.DriverDepCascadingExternal	0	0	0	0.0%
Driver.DriverDepCascadingMember	0	0	0	0.0%
Driver.DriverDepCascadingNominal	0	0	0	0.0%
Driver.DriverDepCascadingTopLevel	0	0	0	0.0%
Driver.DriverDepDynamic	0	0	0	0.0%
Driver.DriverDepExternal	0	0	0	0.0%
Driver.DriverDepMember	0	0	0	0.0%
Driver.DriverDepNominal	0	0	0	0.0%
Driver.DriverDepTopLevel	0	0	0	0.0%
Driver.NumDriverJobsRun	6,944	6,944	0	0.0%
Driver.NumDriverJobsSkipped	0	0	0	0.0%
Driver.NumProcessFailures	0	0	0	0.0%
Frontend.MaxMallocUsage	172,858,380,488	172,666,856,440	-191,524,048	-0.11%
Frontend.NumInstructionsExecuted	11,347,063,955,734	11,346,873,597,699	-190,358,035	-0.0%
Frontend.NumProcessFailures	0	0	0	0.0%
IRModule.NumIRAliases	54,598	54,598	0	0.0%
IRModule.NumIRBasicBlocks	1,661,832	1,661,832	0	0.0%
IRModule.NumIRComdatSymbols	0	0	0	0.0%
IRModule.NumIRFunctions	885,386	885,386	0	0.0%
IRModule.NumIRGlobals	1,115,890	1,115,890	0	0.0%
IRModule.NumIRIFuncs	0	0	0	0.0%
IRModule.NumIRInsts	19,403,148	19,403,148	0	0.0%
IRModule.NumIRNamedMetaData	34,293	34,293	0	0.0%
IRModule.NumIRValueSymbols	1,760,582	1,760,582	0	0.0%
LLVM.NumLLVMBytesOutput	476,919,772	476,919,798	26	0.0%
Parse.NumFunctionsParsed	427,935	427,935	0	0.0%
Parse.NumIterableDeclContextParsed	253,407	253,407	0	0.0%
SILModule.NumSILGenDefaultWitnessTables	0	0	0	0.0%
SILModule.NumSILGenFunctions	911,515	911,515	0	0.0%
SILModule.NumSILGenGlobalVariables	14,857	14,857	0	0.0%
SILModule.NumSILGenVtables	3,414	3,414	0	0.0%
SILModule.NumSILGenWitnessTables	17,709	17,709	0	0.0%
SILModule.NumSILOptDefaultWitnessTables	0	0	0	0.0%
SILModule.NumSILOptFunctions	644,463	644,463	0	0.0%
SILModule.NumSILOptGlobalVariables	15,217	15,217	0	0.0%
SILModule.NumSILOptVtables	6,549	6,549	0	0.0%
SILModule.NumSILOptWitnessTables	34,723	34,723	0	0.0%
Sema.AccessLevelRequest	756,180	756,180	0	0.0%
Sema.DefaultAndMaxAccessLevelRequest	18,461	18,461	0	0.0%
Sema.EnumRawTypeRequest	5,952	5,952	0	0.0%
Sema.ExtendedNominalRequest	1,162,921	1,162,921	0	0.0%
Sema.InheritedDeclsReferencedRequest	46,196,796	46,196,796	0	0.0%
Sema.InheritedTypeRequest	336,828	336,828	0	0.0%
Sema.IsDynamicRequest	671,268	671,268	0	0.0%
Sema.IsObjCRequest	557,526	557,526	0	0.0%
Sema.NamedLazyMemberLoadFailureCount	12,083	12,083	0	0.0%
Sema.NamedLazyMemberLoadSuccessCount	1,800,624	1,800,624	0	0.0%
Sema.NominalTypeLookupDirectCount	13,482,094	13,482,099	5	0.0%
Sema.NumConformancesDeserialized	1,644,609	1,644,609	0	0.0%
Sema.NumConstraintScopes	7,126,611	7,126,611	0	0.0%
Sema.NumConstraintsConsideredForEdgeContraction	13,058,866	13,058,866	0	0.0%
Sema.NumDeclsDeserialized	13,136,294	13,136,294	0	0.0%
Sema.NumDeclsValidated	762,947	762,947	0	0.0%
Sema.NumFunctionsTypechecked	545,845	545,845	0	0.0%
Sema.NumGenericSignatureBuilders	614,885	614,885	0	0.0%
Sema.NumLazyGenericEnvironments	2,510,395	2,510,395	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	243,260	243,260	0	0.0%
Sema.NumLazyIterableDeclContexts	2,250,161	2,250,161	0	0.0%
Sema.NumTypesDeserialized	5,729,922	5,729,922	0	0.0%
Sema.NumTypesValidated	513,184	513,184	0	0.0%
Sema.NumUnloadedLazyIterableDeclContexts	1,516,411	1,516,411	0	0.0%
Sema.OverriddenDeclsRequest	600,948	600,948	0	0.0%
Sema.RequirementRequest	16,836	16,836	0	0.0%
Sema.SelfBoundsFromWhereClauseRequest	46,740	46,740	0	0.0%
Sema.SetterAccessLevelRequest	36,288	36,288	0	0.0%
Sema.SuperclassDeclRequest	36,848,016	36,848,016	0	0.0%
Sema.SuperclassTypeRequest	12,461	12,461	0	0.0%
Sema.TypeDeclsFromWhereClauseRequest	9,460	9,460	0	0.0%
Sema.USRGenerationRequest	164,419	164,419	0	0.0%
Sema.UnderlyingTypeDeclsReferencedRequest	1,362,399	1,362,399	0	0.0%

Release

release brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (3)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	12,522,787,412,055	12,528,027,075,187	5,239,663,132	0.04%
LLVM.NumLLVMBytesOutput	445,790,316	445,790,292	-24	-0.0%
time.swift-driver.wall	2149.7s	2158.7s	8.9s	0.42%

release detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	139,373	139,373	0	0.0%
AST.NumLoadedModules	6,473	6,473	0	0.0%
AST.NumTotalClangImportedEntities	456,261	456,261	0	0.0%
AST.NumUsedConformances	100,140	100,140	0	0.0%
IRModule.NumIRBasicBlocks	1,627,638	1,627,638	0	0.0%
IRModule.NumIRFunctions	721,701	721,701	0	0.0%
IRModule.NumIRGlobals	844,510	844,510	0	0.0%
IRModule.NumIRInsts	13,978,171	13,978,171	0	0.0%
IRModule.NumIRValueSymbols	1,442,203	1,442,203	0	0.0%
LLVM.NumLLVMBytesOutput	445,790,316	445,790,292	-24	-0.0%
SILModule.NumSILGenFunctions	305,160	305,160	0	0.0%
SILModule.NumSILOptFunctions	427,219	427,219	0	0.0%
Sema.NumConformancesDeserialized	929,559	929,559	0	0.0%
Sema.NumConstraintScopes	7,093,710	7,093,710	0	0.0%
Sema.NumDeclsDeserialized	2,850,796	2,850,796	0	0.0%
Sema.NumDeclsValidated	396,234	396,234	0	0.0%
Sema.NumFunctionsTypechecked	250,089	250,089	0	0.0%
Sema.NumGenericSignatureBuilders	111,467	111,467	0	0.0%
Sema.NumLazyGenericEnvironments	499,703	499,703	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	56,552	56,552	0	0.0%
Sema.NumLazyIterableDeclContexts	335,136	335,136	0	0.0%
Sema.NumTypesDeserialized	1,621,645	1,621,645	0	0.0%
Sema.NumTypesValidated	178,074	178,074	0	0.0%

jrose-apple · 2018-09-12T03:11:52Z

That's a lot of failures, enough that I'm not sure I can trust the results again. Still, this fixes an out-of-bounds issue, so I'm going to merge it.

jrose-apple requested review from rintaro and benlangmuir September 10, 2018 22:38

rintaro approved these changes Sep 11, 2018

View reviewed changes

jrose-apple force-pushed the happy-ending branch from 653a198 to ce4c576 Compare September 11, 2018 02:15

jrose-apple merged commit e180bf9 into swiftlang:master Sep 12, 2018

jrose-apple deleted the happy-ending branch September 12, 2018 03:12

This was referenced Sep 12, 2018

Implementation for SE-200 (raw string escaping) #19191

Closed

[Parse] 2nd attempt at multiline attribute messages (SE-200) #19219

Merged

[Parse] Tweak a utility function that relies on reading past the end #19230

[Parse] Tweak a utility function that relies on reading past the end #19230

Uh oh!

Conversation

jrose-apple commented Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrose-apple commented Sep 10, 2018

Uh oh!

jrose-apple commented Sep 10, 2018

Uh oh!

jrose-apple commented Sep 10, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

rintaro left a comment

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Sep 11, 2018

Build comment file:

Compilation-performance test failed

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

swift-ci commented Sep 11, 2018

Build comment file:

Compilation-performance test failed

Uh oh!

eeckstein commented Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro commented Sep 11, 2018

Uh oh!

eeckstein commented Sep 11, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

jrose-apple commented Sep 11, 2018

Uh oh!

swift-ci commented Sep 12, 2018

Build comment file:

Summary for master full

Debug-batch

debug-batch brief

debug-batch detailed

Release

release brief

release detailed

Uh oh!

jrose-apple commented Sep 12, 2018

Uh oh!

Uh oh!

jrose-apple commented Sep 10, 2018 •

edited

Loading

eeckstein commented Sep 11, 2018 •

edited

Loading