Tokamak icon indicating copy to clipboard operation
Tokamak copied to clipboard

RuntimeError and uninitialized/overwritten @State vars when using `if` in `View` view builder

Open Feuermurmel opened this issue 2 years ago • 6 comments

Describe the bug I'm seeing crashes and other strange behavior on Wasm targets when trying to use a stateful view (using @StateObject) inside of an if statement in a View view builder.

To Reproduce I've tried all examples here with a single main.swift file, running carton dev and loading the page in Chrome. The stack traces are copied out of Chrome's console.

import TokamakDOM

class MyState: ObservableObject { }

struct MyView: View {
    var foo: Int32 = 0
    @StateObject var myState = MyState()
    var body = Text("hello")
}

struct MyApp: App {
    var body = WindowGroup("Test") { if true { MyView() } }
}

MyApp.main()

Running this example produces the following error:

dev.js:96 RuntimeError: memory access out of bounds
    at $sSNsSxRzSZ6StrideRpzrlE5IndexOMr (<anonymous>:wasm-function[34182]:0x7903fc)
    at $sSqMr (<anonymous>:wasm-function[37837]:0x7baf8f)
    at swift::MetadataCacheEntryBase<(anonymous namespace)::GenericCacheEntry, void const*>::doInitialization(swift::ConcurrencyControl&, swift::MetadataCompletionQueueEntry*, swift::MetadataRequest) (<anonymous>:wasm-function[39511]:0x7d8c75)
    at swift_getGenericMetadata (<anonymous>:wasm-function[39507]:0x7d8443)
    at __swift_instantiateGenericMetadata (<anonymous>:wasm-function[423]:0x4075b)
    at $sSqMa (<anonymous>:wasm-function[20001]:0x4f3c16)
    at $ss7KeyPathC16_projectReadOnly4fromq_x_tF (<anonymous>:wasm-function[24208]:0x605a7d)
    at swift_getAtKeyPath (<anonymous>:wasm-function[24266]:0x60a074)
    at $s11TokamakCore11EnvironmentV10setContent4fromyAA0C6ValuesV_tF (<anonymous>:wasm-function[9516]:0x29175e)
    at $s11TokamakCore11EnvironmentVyxGAA0C6ReaderA2aEP10setContent4fromyAA0C6ValuesV_tFTW (<anonymous>:wasm-function[9523]:0x291c83)
handleError	@	dev.js:96
Promise.catch (async)		
eval	@	dev.js:103
./entrypoint/dev.js	@	dev.js:97
__webpack_require__	@	dev.js:20
(anonymous)	@	dev.js:84
(anonymous)	@	dev.js:87

Changing the declaration of foo to @State var foo: Int32 = 0 produces the following error:

dev.js:96 RuntimeError: table index is out of bounds
    at $s11TokamakCore11StateObjectV12wrappedValuexvg (<anonymous>:wasm-function[12593]:0x333a53)
    at $s11TokamakCore11StateObjectV16objectWillChange11OpenCombine12AnyPublisherVyyts5NeverOGvg (<anonymous>:wasm-function[12616]:0x33481c)
    at $s11TokamakCore11StateObjectVyxGAA16ObservedPropertyA2aEP16objectWillChange11OpenCombine12AnyPublisherVyyts5NeverOGvgTW (<anonymous>:wasm-function[12624]:0x334b95)
    at $s11TokamakCore15StackReconcilerC26setupTransientSubscription33_FCAA3CDB23B668BF64A31B25B678943FLL3for2of4bodyyAA12PropertyInfoV_AA23MountedCompositeElementCyxGs7KeyPathCyAMypGtF (<anonymous>:wasm-function[12346]:0x3299d3)
    at $s11TokamakCore15StackReconcilerC6render33_FCAA3CDB23B668BF64A31B25B678943FLL16compositeElement4body6resultqd__AA016MountedCompositeN0CyxG_s24ReferenceWritableKeyPathCyAKypGs0uV0CyAKqd__ypcGtlF (<anonymous>:wasm-function[12358]:0x32abe4)
    at $s11TokamakCore15StackReconcilerC6render13compositeViewAA03AnyG0VAA016MountedCompositeG0CyxG_tF (<anonymous>:wasm-function[12364]:0x32b1be)
    at $s11TokamakCore20MountedCompositeViewC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtF (<anonymous>:wasm-function[10451]:0x2baac7)
    at $s11TokamakCore15MountedHostViewC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtFyAMXEfU0_ (<anonymous>:wasm-function[10724]:0x2cc071)
    at $s11TokamakCore15MountedHostViewC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtFyAMXEfU0_TA (<anonymous>:wasm-function[10718]:0x2cbcb9)
    at $s11TokamakCore14MountedElementCyxGs5Error_pIggzo_ADsAE_pIegnzo_AA8RendererRzlTR (<anonymous>:wasm-function[10339]:0x2b229b)
handleError	@	dev.js:96
Promise.catch (async)		
eval	@	dev.js:103
./entrypoint/dev.js	@	dev.js:97
__webpack_require__	@	dev.js:20
(anonymous)	@	dev.js:84
(anonymous)	@	dev.js:87

Changing the declaration of foo to @State var foo: Int64 = 0 produces the following error:

RuntimeError: function signature mismatch
    at __swift_destroy_boxed_opaque_existential_0 (<anonymous>:wasm-function[1546]:0x89a23)
    at $s11TokamakCore7AnyViewVwxx (<anonymous>:wasm-function[14334]:0x37e288)
    at swift_arrayDestroy (<anonymous>:wasm-function[38817]:0x7c20b9)
    at $ss23_ContiguousArrayStorageCfD (<anonymous>:wasm-function[22276]:0x579e90)
    at _swift_release_dealloc (<anonymous>:wasm-function[38950]:0x7ca9e6)
    at swift_release (<anonymous>:wasm-function[38946]:0x7ca759)
    at $s11TokamakCore15MountedHostViewC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtF (<anonymous>:wasm-function[10711]:0x2ca3e7)
    at $s11TokamakCore20MountedCompositeViewC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtF (<anonymous>:wasm-function[10451]:0x2b9b8e)
    at $s11TokamakCore12MountedSceneC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtF (<anonymous>:wasm-function[10787]:0x2d3680)
    at $s11TokamakCore10MountedAppC5mount6before2on4withy10TargetTypeQzSg_AA0C7ElementCyxGSgAA15StackReconcilerCyxGtF (<anonymous>:wasm-function[10326]:0x2b09ec)
handleError @ dev.js:96
Promise.catch (async)
eval @ dev.js:103
./entrypoint/dev.js @ dev.js:97
__webpack_require__ @ dev.js:20
(anonymous) @ dev.js:84
(anonymous) @ dev.js:87

Changing the declaration of foo back to @State var foo: Int32 = 0 and the declaration of body to var body: some View { Text("hello \(foo)") } lets the application run without producing an error but foo will be initialized to 16 instead of 0 (the Text element will display "hello 16").

Desktop (please complete the following information):

  • macOS 10.15.7
  • Chrome 91.0.4472.114 (currently most recent version)
  • Tokamak 0.7.0
  • carton 0.10.0
  • Swift wasm-5.3.1-RELEASE

Additional context Possibly related tickets: https://github.com/TokamakUI/Tokamak/issues/222 https://github.com/TokamakUI/Tokamak/issues/338

Feuermurmel avatar Jul 20 '21 12:07 Feuermurmel

This does appear to be the same issue as #338. I get the error if the target is named MyApp, but not if it is named TokamakApp instead.

ezraberch avatar Jul 21 '21 17:07 ezraberch

You sure you're using the latest carton? That would detect stack overflow errors with our new sanitizer. I wonder if this is a stack overflow we investigated in https://github.com/TokamakUI/Tokamak/issues/367. Maybe the workaround with setting stack size manually helps, as described in this comment? https://github.com/TokamakUI/Tokamak/issues/367#issuecomment-829229300

MaxDesiatov avatar Jul 21 '21 17:07 MaxDesiatov

This does appear to be the same issue as #338. I get the error if the target is named MyApp, but not if it is named TokamakApp instead.

Ohh, I didn't anticipate that the names I gave to thing would influence whether this crash happens or not…

I can confirm that when naming the target any of these, the crash will not occur: TokamakApp, Tokam, T, m, t. But it will occur with these names: MyApp, M. I haven't tried more that that. This seems to me as if changing the target's name results in the order of some data in the Wasm file changing and that some out-of-bounds access to that data is caught by the runtime in one case but not in another…

You sure you're using the latest carton? That would detect stack overflow errors with our new sanitizer.

Yes. I've compiled carton myself because I've not setup up Homebrew but I compiled it straight from the 0.10.0 tag. See full-output.txt below which hopefully shows all the interesting details.

I wonder if this is a stack overflow we investigated in #367. Maybe the workaround with setting stack size manually helps, as described in this comment? #367 (comment)

I tried adding those linker flags but I haven't seen a change in behaviour (the tests above where I renamed the target were all done with those linker flags). I wasn't 100% sure I placed them in the right spot, look at the top of full-output.txt to confirm.

full-output.txt

Feuermurmel avatar Jul 22 '21 10:07 Feuermurmel

This does appear to be the same issue as #338. I get the error if the target is named MyApp, but not if it is named TokamakApp instead.

Ohh, I didn't anticipate that the names I gave to thing would influence whether this crash happens or not…

I can confirm that when naming the target any of these, the crash will not occur: TokamakApp, Tokam, T, m, t. But it will occur with these names: MyApp, M. I haven't tried more that that. This seems to me as if changing the target's name results in the order of some data in the Wasm file changing and that some out-of-bounds access to that data is caught by the runtime in one case but not in another…

That's exactly what we were seeing with stack overflow issues.

I tried adding those linker flags but I haven't seen a change in behaviour (the tests above where I renamed the target were all done with those linker flags).

Does setting bigger stack size help in any way?

@kateinoigakukun would you be able to help? What would be the best way to diagnose this memory corruption issue if the SO sanitizer doesn't seem to be catching it?

MaxDesiatov avatar Jul 22 '21 13:07 MaxDesiatov

Does setting bigger stack size help in any way?

I couldn't find a way that made the problem go away, but different stack sizes lead to different error messages. Here are the min and max values I found that lead to each error message:

1024 * 1024 * 1024, 2040 * 1024 * 1024
RuntimeError: memory access out of bounds

2044 * 1024 * 1024, 2047 * 1024 * 1024 + 768 * 1024
Fatal error: 'try!' expression unexpectedly raised an error: : file JavaScriptKit/JSFunction.swift, line 22

2047 * 1024 * 1024 + 896 * 1024, 2 * 1024 * 1024 * 1024 - 16
Fatal error: Unexpectedly found nil while unwrapping an Optional value: file TokamakDOM/App.swift, line 43

2 * 1024 * 1024 * 1024
Error: Detected stack-buffer-overflow.

Feuermurmel avatar Jul 22 '21 19:07 Feuermurmel

Looks like a bigger stack size makes it crash earlier. With 1024 * 1024 * 1024, it crashes on line 49 of App.swift in both Safari and Chrome. With 2044 * 1024 * 1024 or 2047 * 1024 * 1024 + 896 * 1024, it crashes on line 43 in Chrome. In Safari, it gives an Out of Memory error.

ezraberch avatar Jul 22 '21 20:07 ezraberch