Godot: Using the color bbcode in RichTextLabel crashes on Android

[anissen]

Godot version:
24e1039eb

OS/device including version:
macOS 10.14 + Android 9

Issue description:

Having a RichTextLabel with the following bbcode crashes my game on startup on Android:

[center]Time: [color=blue]30[/color] / Swipes: [color=blue]5[/color][/center]

This bbcode works fine:

[center]Time: [b]30[/b] / Swipes: [b]5[/b][/center]

Here's the crash log:

09-12 19:17:21.076 26897 26919 F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x7ffffffffffff8 in tid 26919 (GLThread 17807), pid 26897 (ne.bouncingball)
09-12 19:17:21.133 26932 26932 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone
09-12 19:17:21.133  3522  3522 I /system/bin/tombstoned: received crash request for pid 26919
09-12 19:17:21.135 26932 26932 I crash_dump64: performing dump of process 26897 (target tid = 26919)
09-12 19:17:21.147 26932 26932 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
09-12 19:17:21.147 26932 26932 F DEBUG   : Build fingerprint: 'OnePlus/OnePlus3/OnePlus3T:9/PKQ1.181203.001/1907311932:user/release-keys'
09-12 19:17:21.147 26932 26932 F DEBUG   : Revision: '0'
09-12 19:17:21.147 26932 26932 F DEBUG   : ABI: 'arm64'
09-12 19:17:21.147 26932 26932 F DEBUG   : pid: 26897, tid: 26919, name: GLThread 17807  >>> org.godotengine.bouncingball <<<
09-12 19:17:21.147 26932 26932 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x7ffffffffffff8
09-12 19:17:21.147 26932 26932 F DEBUG   :     x0  0000000000000001  x1  0000000000000000  x2  0000000000000000  x3  0000007ca08a2f48
09-12 19:17:21.147 26932 26932 F DEBUG   :     x4  0000007ca08a2f38  x5  00000000062cd0a0  x6  00000000062cd0f0  x7  00000000001fffff
09-12 19:17:21.147 26932 26932 F DEBUG   :     x8  3f7ffffffffffff8  x9  0000000000000001  x10 0000000000000001  x11 0000000000000000
09-12 19:17:21.147 26932 26932 F DEBUG   :     x12 0000000000000143  x13 0000000000000002  x14 00000000000000a3  x15 0000007d2e37f8c8
09-12 19:17:21.147 26932 26932 F DEBUG   :     x16 0000007c88400a18  x17 0000007d2e37f908  x18 0000007ca9200080  x19 0000007c885e8290
09-12 19:17:21.147 26932 26932 F DEBUG   :     x20 0000007c883dd410  x21 0000000000000001  x22 0000000000000000  x23 0000000000000033
09-12 19:17:21.147 26932 26932 F DEBUG   :     x24 0000000000000001  x25 0000000000000040  x26 0000007c885e8290  x27 0000007c883dd410
09-12 19:17:21.147 26932 26932 F DEBUG   :     x28 0000000000000000  x29 0000007c885e8290
09-12 19:17:21.147 26932 26932 F DEBUG   :     sp  0000007c8d53a700  lr  0000007c8df42d0c  pc  0000007c8df42eb8
09-12 19:17:21.169 26932 26932 F DEBUG   : 
09-12 19:17:21.169 26932 26932 F DEBUG   : backtrace:
09-12 19:17:21.169 26932 26932 F DEBUG   :     #00 pc 00000000008ffeb8  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (_ZN13RichTextLabel13_process_lineEPNS_9ItemFrameERK7Vector2RiiiNS_11ProcessModeERK3RefI4FontERK5ColorSE_bS4_RK8Vector2iPPNS_4ItemEPiPbi+7452)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #01 pc 0000000000903c10  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (RichTextLabel::_notification(int)+1364)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #02 pc 0000000000911f44  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (RichTextLabel::_notificationv(int, bool)+152)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #03 pc 0000000001341f28  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (Object::notification(int, bool)+28)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #04 pc 0000000000b474d0  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (CanvasItem::_update_callback()+188)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #05 pc 00000000002a4070  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (MethodBind0::call(Object*, Variant const**, int, Variant::CallError&)+120)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #06 pc 00000000013446f4  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (Object::call(StringName const&, Variant const**, int, Variant::CallError&)+352)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #07 pc 000000000133c744  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (MessageQueue::_call_function(Object*, StringName const&, Variant const*, int, bool)+284)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #08 pc 000000000133c9d4  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (MessageQueue::flush()+236)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #09 pc 00000000007d822c  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (SceneTree::iteration(float)+332)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #10 pc 00000000001a2f0c  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (Main::iteration()+476)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #11 pc 000000000017984c  /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/lib/arm64/libgodot_android.so (Java_org_godotengine_godot_GodotLib_step+168)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #12 pc 000000000056a1e0  /system/lib64/libart.so (art_quick_generic_jni_trampoline+144)
09-12 19:17:21.169 26932 26932 F DEBUG   :     #13 pc 000000000056144c  /system/lib64/libart.so (art_quick_invoke_static_stub+604)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #14 pc 00000000000cf6d8  /system/lib64/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+232)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #15 pc 0000000000282b00  /system/lib64/libart.so (art::interpreter::ArtInterpreterToCompiledCodeBridge(art::Thread*, art::ArtMethod*, art::ShadowFrame*, unsigned short, art::JValue*)+344)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #16 pc 000000000027cbb0  /system/lib64/libart.so (bool art::interpreter::DoCall<false, false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, art::JValue*)+960)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #17 pc 00000000005317d8  /system/lib64/libart.so (MterpInvokeStatic+200)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #18 pc 0000000000553a14  /system/lib64/libart.so (ExecuteMterpImpl+14612)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #19 pc 000000000007ae88  /dev/ashmem/dalvik-classes.dex extracted in memory from /data/app/org.godotengine.bouncingball-9ylnYEPseCi1FIpMe1oX1A==/base.apk (deleted) (org.godotengine.godot.GodotRenderer.onDrawFrame)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #20 pc 0000000000256d10  /system/lib64/libart.so (_ZN3art11interpreterL7ExecuteEPNS_6ThreadERKNS_20CodeItemDataAccessorERNS_11ShadowFrameENS_6JValueEb.llvm.3399805955+488)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #21 pc 000000000025c8c0  /system/lib64/libart.so (art::interpreter::ArtInterpreterToInterpreterBridge(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame*, art::JValue*)+216)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #22 pc 000000000027cb94  /system/lib64/libart.so (bool art::interpreter::DoCall<false, false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, art::JValue*)+932)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #23 pc 0000000000531264  /system/lib64/libart.so (MterpInvokeInterface+1376)
09-12 19:17:21.172 26932 26932 F DEBUG   :     #24 pc 0000000000553a94  /system/lib64/libart.so (ExecuteMterpImpl+14740)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #25 pc 0000000000b0b540  /system/framework/boot-framework.vdex (android.opengl.GLSurfaceView$GLThread.guardedRun+1102)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #26 pc 0000000000256d10  /system/lib64/libart.so (_ZN3art11interpreterL7ExecuteEPNS_6ThreadERKNS_20CodeItemDataAccessorERNS_11ShadowFrameENS_6JValueEb.llvm.3399805955+488)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #27 pc 000000000025c8c0  /system/lib64/libart.so (art::interpreter::ArtInterpreterToInterpreterBridge(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame*, art::JValue*)+216)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #28 pc 000000000027cb94  /system/lib64/libart.so (bool art::interpreter::DoCall<false, false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, art::JValue*)+932)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #29 pc 0000000000531618  /system/lib64/libart.so (MterpInvokeDirect+284)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #30 pc 0000000000553994  /system/lib64/libart.so (ExecuteMterpImpl+14484)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #31 pc 0000000000b0ba30  /system/framework/boot-framework.vdex (android.opengl.GLSurfaceView$GLThread.run+48)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #32 pc 0000000000256d10  /system/lib64/libart.so (_ZN3art11interpreterL7ExecuteEPNS_6ThreadERKNS_20CodeItemDataAccessorERNS_11ShadowFrameENS_6JValueEb.llvm.3399805955+488)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #33 pc 0000000000520ad8  /system/lib64/libart.so (artQuickToInterpreterBridge+944)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #34 pc 000000000056a2fc  /system/lib64/libart.so (art_quick_to_interpreter_bridge+92)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #35 pc 0000000000561188  /system/lib64/libart.so (art_quick_invoke_stub+584)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #36 pc 00000000000cf6b8  /system/lib64/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+200)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #37 pc 0000000000466690  /system/lib64/libart.so (art::(anonymous namespace)::InvokeWithArgArray(art::ScopedObjectAccessAlreadyRunnable const&, art::ArtMethod*, art::(anonymous namespace)::ArgArray*, art::JValue*, char const*)+104)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #38 pc 0000000000467794  /system/lib64/libart.so (art::InvokeVirtualOrInterfaceWithJValues(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, _jmethodID*, jvalue*)+424)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #39 pc 0000000000492dc8  /system/lib64/libart.so (art::Thread::CreateCallback(void*)+1116)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #40 pc 0000000000099508  /system/lib64/libc.so (__pthread_start(void*)+36)
09-12 19:17:21.173 26932 26932 F DEBUG   :     #41 pc 0000000000023e18  /system/lib64/libc.so (__start_thread+68)
09-12 19:17:21.520  3522  3522 E /system/bin/tombstoned: Tombstone written to: /data/tombstones/tombstone_07
09-12 19:17:21.521  3646 26935 W ActivityManager:   Force finishing activity org.godotengine.bouncingball/org.godotengine.godot.Godot
09-12 19:17:21.525  3646 26935 D ActivityTrigger: ActivityTrigger activityPauseTrigger 
09-12 19:17:21.534  3646  3751 D RestartProcessManager: increase duration : 1824 for org.godotengine.bouncingball
09-12 19:17:21.537  3646  3752 I ActivityManager: Showing crash dialog for package org.godotengine.bouncingball u0

It's _purely_ speculation but it may be a regression from #23658.

Steps to reproduce:
Put this bbcode into a RichTextLabel and run the project on Android:
[center]Time: [color=blue]30[/color] / Swipes: [color=blue]5[/color][/center]

Minimal reproduction project:
bouncing-ball.zip

[anissen]

Update: I've tried with 56123ece3 (which, as far as I can tell, is the commit prior to 3d76eb8). It _still_ crashes on Android. However, I haven't changed my Android export templates so I don't know if this test actually helps.

[Calinou]

@anissen You should always recompile the export templates to match the editor version, so I'll assume this was caused by https://github.com/godotengine/godot/commit/3d76eb8938815e1f29796d9103f3e5152c7ccbb6.

[anissen]

Yeah, I know 😅
I just haven't set up my computer for compiling them myself -- I usually just grab the latest from https://hugo.pro/projects/godot-builds/ (thank you, by the way! 😉)

[Relintai]

Hi

I managed to run into this bug (although in a different way, not with the color tag.), and I managed to figure out what causes it.

By the way, this also happens on the WebGL backend. And maybe others as well.

We have this code now in rich_text_label.cpp:

(starts at line 469 at the moment)

for (int j = 0; j < fx_stack.size(); j++) {
    ItemCustomFX *item_custom = Object::cast_to<ItemCustomFX>(fx_stack[j]);
    ItemShake *item_shake = Object::cast_to<ItemShake>(fx_stack[j]);
    ItemWave *item_wave = Object::cast_to<ItemWave>(fx_stack[j]);
    ItemTornado *item_tornado = Object::cast_to<ItemTornado>(fx_stack[j]);
    ItemRainbow *item_rainbow = Object::cast_to<ItemRainbow>(fx_stack[j]);

    if (item_custom && custom_fx_ok) {
        Ref<CharFXTransform> charfx = Ref<CharFXTransform>(memnew(CharFXTransform));
        Ref<RichTextEffect> custom_effect = _get_custom_effect_by_code(item_custom->identifier);
        if (!custom_effect.is_null()) {
            ...
        }
    } else if (item_shake) {
        ...
    } else if (item_wave) {
        ...
    } else if (item_tornado) {
        ...
    } else if (item_rainbow) {
        ...
    }
}

What happens, is because the NO_SAFE_CAST flag is set on android, Object::cast_to becomes return dynamic_cast<T *>(p_object);
Apologies, I misread the #ifndef to #ifdef. Actually the other branch runs.

And none of these casts

ItemCustomFX *item_custom = Object::cast_to<ItemCustomFX>(fx_stack[j]);
ItemShake *item_shake = Object::cast_to<ItemShake>(fx_stack[j]);
ItemWave *item_wave = Object::cast_to<ItemWave>(fx_stack[j]);
ItemTornado *item_tornado = Object::cast_to<ItemTornado>(fx_stack[j]);
ItemRainbow *item_rainbow = Object::cast_to<ItemRainbow>(fx_stack[j]);

actually return NULL. so execution goes into the first if statement, and crashes on this line:

Ref<RichTextEffect> custom_effect = _get_custom_effect_by_code(item_custom->identifier); (at the dereference (item_custom->identifier).

Just for completeness's sake this is how I validated it:
Edit: Removed those ifs, I have no idea why I had them in the first place. The results are still the same, unfortunately.

print_error("item_custom " + String::num((uint64_t)item_custom));
print_error("item_shake " + String::num((uint64_t)item_shake));
print_error("item_wave " + String::num((uint64_t)item_wave));
print_error("item_tornado " + String::num((uint64_t)item_tornado));
print_error("item_rainbow " + String::num((uint64_t)item_rainbow));
print_error("--------------------------------------------------");

This is some of new the output:

...
09-19 10:23:37.363 17076 17094 E godot   : item_tornado 2461479184
09-19 10:23:37.363 17076 17094 E godot   : item_rainbow 2461479184
09-19 10:23:37.363 17076 17094 E godot   : --------------------------------------------------
09-19 10:23:37.363 17076 17094 E godot   : item_custom 2471529040
09-19 10:23:37.363 17076 17094 E godot   : item_shake 2471529040
09-19 10:23:37.363 17076 17094 E godot   : item_wave 2471529040
09-19 10:23:37.363 17076 17094 E godot   : item_tornado 2471529040
09-19 10:23:37.363 17076 17094 E godot   : item_rainbow 2471529040
09-19 10:23:37.363 17076 17094 E godot   : --------------------------------------------------
09-19 10:23:37.363 17076 17094 E godot   : item_custom 2461479184
09-19 10:23:37.363 17076 17094 E godot   : item_shake 2461479184
09-19 10:23:37.363 17076 17094 E godot   : item_wave 2461479184
09-19 10:23:37.363 17076 17094 E godot   : item_tornado 2461479184
09-19 10:23:37.363 17076 17094 E godot   : item_rainbow 2461479184
09-19 10:23:37.363 17076 17094 E godot   : --------------------------------------------------
09-19 10:23:37.363 17076 17094 E godot   : item_custom 2471529040
...

It is clearly visible that every cast gives back a pointer (in fact the same one).

Unfortunately I don't yet know enough about c++ to know what a proper solution would be. I've searched for solutions to these type-cast related problems quite a few time already, but I have yet to find a satisfying answer. :(

Edit:

Okay as it turns out dynamic cast should return null if the cast fails. I was thinking about the old c-style cast. I'll try to figure out what's going on.

Edit2:

I still have no idea why the casts fail this way, however I noticed there is a really simple fix using ItemFX's type member. I'll pr it in a second.

[akien-mga]

Reopening as #32202 is being reverted, a different approach is needed.

[Relintai]

23658 will need to be heavily changed, because of the inheritance introduced. Namely Item->ItemFX->(new classes), and the pr itself uses this fact quite a bit. This was also the cause why I changed my pr from using static_casts, to begin with.

I'll think about how to solve that inheritance issue properly. I don't just want to split up the type enum brainlessly. Even though that's probably what I'd do if I did this for myself :P

By the way please wait with reverting #32202, at least until there is a proper fix for this because when I was debugging this I was able to get RichTextLabels to crash, just by setting their text at runtime to a short text with a number and a newline in it. It was along the lines of "aaa0a\naa". (With BBCode disabled)

In fact that was the reason I got crashes in the first place, my game has spells, and I use RichTextLabels to show their description, which is just a few lines of text with a few numbers.

[akien-mga]

By the way please wait with reverting #32202, at least until there is a proper fix for this because when I was debugging this I was able to get RichTextLabels to crash, just by setting their text at runtime to a short text with a number and a newline in it. It was along the lines of "aaa0a\naa". (With BBCode disabled)

Yeah I agree, I'll hold off with that revert PR until we have a better fix to supersede it. I'm not so keen on reintroducing a crash, and reverting the whole of #23658 would be quite counterproductive at this stage.

[akien-mga]

Since I'm not reverting the bugfix for this issue, we can close it again and continue discussion in this new issue: #32347.