Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in ImageExporter::_convert_tex when decompiling Psychopomp Gold #202

Closed
Alex-gnzl opened this issue Oct 25, 2024 · 20 comments · Fixed by #215
Closed

Crash in ImageExporter::_convert_tex when decompiling Psychopomp Gold #202

Alex-gnzl opened this issue Oct 25, 2024 · 20 comments · Fixed by #215
Labels
bug Something isn't working

Comments

@Alex-gnzl
Copy link

System information

Arch Linux, 6.11

Issue description

A certain asset (assets?) in Psychopomp Gold causes the engine to abort

GDB trace from own build:

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff79b6463 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2  0x00007ffff795d120 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff79444c3 in __GI_abort () at abort.c:79
#4  0x00007ffff7945354 in __libc_message_impl (fmt=fmt@entry=0x7ffff7ad32f5 "%s\n") at ../sysdeps/posix/libc_fatal.c:132
#5  0x00007ffff79c0765 in malloc_printerr (str=str@entry=0x7ffff7ad6370 "free(): corrupted unsorted chunks") at malloc.c:5772
#6  0x00007ffff79c163c in _int_free_create_chunk (av=av@entry=0x7ffff7b07ac0 <main_arena>, p=p@entry=0x55555e1205c0, size=<optimized out>, size@entry=131120, 
    nextchunk=nextchunk@entry=0x55555e1405f0, nextsize=nextsize@entry=368) at malloc.c:4735
#7  0x00007ffff79c297a in _int_free_merge_chunk (av=av@entry=0x7ffff7b07ac0 <main_arena>, p=0x55555e1205c0, size=131120) at malloc.c:4700
#8  0x00007ffff79c2cfa in _int_free (av=0x7ffff7b07ac0 <main_arena>, p=p@entry=0x55555e1205c0, have_lock=<optimized out>, have_lock@entry=0) at malloc.c:4646
#9  0x00007ffff79c55ce in __GI___libc_free (mem=0x55555e1205d0) at malloc.c:3398
#10 0x0000555558816029 in CowData<unsigned char>::_unref (this=0x55555d88b558) at ./core/templates/cowdata.h:270
#11 CowData<unsigned char>::_ref (this=0x55555d88b558, p_from=...) at ./core/templates/cowdata.h:465
#12 Vector<unsigned char>::operator= (this=0x55555d88b550, p_from=...) at ./core/templates/vector.h:148
#13 Image::initialize_data (this=0x55555d88b350, p_width=384, p_height=128, p_use_mipmaps=true, p_format=Image::FORMAT_RGBA8, p_data=...) at core/io/image.cpp:2310
#14 0x0000555555b0ee65 in image_decompress_bcdec (p_image=0x55555d88b350) at modules/bcdec/image_decompress_bcdec.cpp:181
#15 0x0000555558815dc4 in Image::decompress (this=0x7da6c) at core/io/image.cpp:2711
#16 0x0000555555e624e1 in ImportExporter::_convert_tex (this=this@entry=0x55555e04bfd0, output_dir=..., p_path=..., p_dst=..., lossy=false)
    at modules/gdsdecomp/utility/import_exporter.cpp:1250
#17 0x0000555555e559cf in ImportExporter::export_texture (this=this@entry=0x55555e04bfd0, output_dir=..., iinfo=...) at modules/gdsdecomp/utility/import_exporter.cpp:1122
#18 0x0000555555e50afe in ImportExporter::_export_imports (this=0x55555e04bfd0, p_out_dir=..., files_to_export=..., pr=0x0, error_string=...)
    at modules/gdsdecomp/utility/import_exporter.cpp:260
#19 0x0000555555e71b64 in call_with_validated_variant_args_ret_helper<__UnexistingClass, Error, String const&, Vector<String> const&, 0ul, 1ul> (p_instance=<optimized out>, 
    p_method=<optimized out>, p_args=<optimized out>, r_ret=<optimized out>) at ./core/variant/binder_common.h:375
#20 call_with_validated_object_instance_args_ret<__UnexistingClass, Error, String const&, Vector<String> const&> (base=<optimized out>, p_method=<optimized out>, 
    p_args=<optimized out>, r_ret=<optimized out>) at ./core/variant/binder_common.h:662
#21 MethodBindTR<Error, String const&, Vector<String> const&>::validated_call (this=<optimized out>, p_object=<optimized out>, p_args=<optimized out>, r_ret=0x7fffffffa058)
    at ./core/object/method_bind.h:536
#22 0x0000555555d25724 in GDScriptFunction::call (this=0x55555b982270, p_instance=0x55555b7e2b80, p_args=<optimized out>, p_argcount=<optimized out>, r_err=..., p_state=0x0)
    at modules/gdscript/gdscript_vm.cpp:2246
#23 0x0000555555bee46b in GDScriptInstance::callp (this=0x55555b7e2b80, p_method=..., p_args=0x7fffffffade0, p_argcount=2, r_error=...) at modules/gdscript/gdscript.cpp:2040
#24 0x0000555558caf297 in Object::callp (this=0x55555b7a36f0, p_method=..., p_args=0x7fffffffade0, p_argcount=2, r_error=...) at core/object/object.cpp:791
#25 0x00005555589ac0ea in Variant::callp (this=<optimized out>, p_method=..., p_args=0x7ffff79b63f4 <__pthread_kill_implementation+276>, p_argcount=2, r_ret=..., r_error=...)
    at core/variant/variant_call.cpp:1227
#26 0x0000555555d1fa83 in GDScriptFunction::call (this=0x55555b987550, p_instance=0x55555b7e2b80, p_args=<optimized out>, p_argcount=<optimized out>, r_err=..., p_state=0x0)
    at modules/gdscript/gdscript_vm.cpp:1920
#27 0x0000555555bee46b in GDScriptInstance::callp (this=0x55555b7e2b80, p_method=..., p_args=0x7fffffffbb98, p_argcount=7, r_error=...) at modules/gdscript/gdscript.cpp:2040
#28 0x0000555558caf297 in Object::callp (this=0x55555b7a36f0, p_method=..., p_args=0x7fffffffbb98, p_argcount=7, r_error=...) at core/object/object.cpp:791
#29 0x00005555589ac0ea in Variant::callp (this=<optimized out>, p_method=..., p_args=0x7ffff79b63f4 <__pthread_kill_implementation+276>, p_argcount=7, r_ret=..., r_error=...)
--Type <RET> for more, q to quit, c to continue without paging--c
    at core/variant/variant_call.cpp:1227
#30 0x0000555555d1fa83 in GDScriptFunction::call (this=0x55555b98e370, p_instance=0x55555b7e2b80, p_args=<optimized out>, p_argcount=<optimized out>, r_err=..., p_state=0x0)
    at modules/gdscript/gdscript_vm.cpp:1920
#31 0x0000555555bee46b in GDScriptInstance::callp (this=0x55555b7e2b80, p_method=..., p_args=0x7fffffffc768, p_argcount=1, r_error=...) at modules/gdscript/gdscript.cpp:2040
#32 0x0000555558caf297 in Object::callp (this=0x55555b7a36f0, p_method=..., p_args=0x7fffffffc768, p_argcount=1, r_error=...) at core/object/object.cpp:791
#33 0x00005555589ac0ea in Variant::callp (this=<optimized out>, p_method=..., p_args=0x7ffff79b63f4 <__pthread_kill_implementation+276>, p_argcount=1, r_ret=..., r_error=...)
    at core/variant/variant_call.cpp:1227
#34 0x0000555555d1fac4 in GDScriptFunction::call (this=0x55555b97dbc0, p_instance=0x55555b7e2b80, p_args=<optimized out>, p_argcount=<optimized out>, r_err=..., p_state=0x0)
    at modules/gdscript/gdscript_vm.cpp:1890
#35 0x0000555555bee46b in GDScriptInstance::callp (this=0x55555b7e2b80, p_method=..., p_args=0x0, p_argcount=0, r_error=...) at modules/gdscript/gdscript.cpp:2040
#36 0x0000555556d81527 in Node::_gdvirtual__ready_call (this=0x55555b7a36f0) at scene/main/node.h:384
#37 Node::_notification (this=0x55555b7a36f0, p_notification=<optimized out>) at scene/main/node.cpp:224
#38 0x0000555556f40420 in Node::_notificationv (this=<optimized out>, p_notification=<optimized out>, p_reversed=<optimized out>) at ./scene/main/node.h:50
#39 CanvasItem::_notificationv (this=<optimized out>, p_notification=<optimized out>, p_reversed=<optimized out>) at ./scene/main/canvas_item.h:45
#40 Control::_notificationv (this=0x55555b7a36f0, p_notification=13, p_reversed=false) at scene/gui/control.h:47
#41 0x0000555558cacec4 in Object::notification (this=0x55555b7a36f0, p_notification=13, p_reversed=false) at core/object/object.cpp:875
#42 0x0000555556d8411d in Node::_propagate_ready (this=0x55555b7a36f0) at scene/main/node.cpp:279
#43 0x0000555556d840c9 in Node::_propagate_ready (this=0x55555b69d1a0) at scene/main/node.cpp:270
#44 0x0000555556d8a3f9 in Node::_set_tree (this=0x55555b69d1a0, p_tree=0x55555b69ccd0) at scene/main/node.cpp:3245
#45 0x00005555559c46dc in OS_LinuxBSD::run (this=0x7fffffffd290) at platform/linuxbsd/os_linuxbsd.cpp:950
#46 0x00005555559ba36e in main (argc=<optimized out>, argv=<optimized out>) at platform/linuxbsd/godot_linuxbsd.cpp:85

(First?) asset that causes this:
res://Sprites/Items/ParagonKeyAnim.png

Steps to reproduce

Try to recover Psychopomp Gold.exe using either the Windows or Linux version of gdsdecomp

Recovery log

N/A

Minimal reproduction project

https://store.steampowered.com/app/3243190/Psychopomp_GOLD/

I wish I could provide an example texture that causes this, but I'm not sure what settings result in this happening.

@Alex-gnzl Alex-gnzl added the bug Something isn't working label Oct 25, 2024
@nikitalita
Copy link
Collaborator

To be clear, you're building from the current gdsdecomp master with Godot @ the commit listed in the readme, correct?

@Alex-gnzl
Copy link
Author

Yes, master with the particular commit -- it also seems to happen with the release and (at the time of writing) latest Actions build.

@nikitalita
Copy link
Collaborator

Ah, heck, I thought this was fixed in the godot engine. Lemme guess, that particular texture is 1x1 pixels, right?

@Alex-gnzl
Copy link
Author

I initially thought so too after searching for a bit, but
The readme commit is after that fix:
(modules/bcdec/image_decompress_bcdec.cpp:98)

        // Compressed images' dimensions should be padded to the upper multiple of 4.
        // If they aren't, they need to be realigned (the actual data is correctly padded though).
        if (width % 4 != 0 || height % 4 != 0) {
                int new_width = width + (4 - (width % 4));
                int new_height = height + (4 - (height % 4));

                print_verbose(vformat("Compressed image's dimensions are not multiples of 4 (%dx%d), aligning to (%dx%d)", width, height, new_width, new_height));

                width = new_width;
                height = new_height;
        }

The stack trace suggests it's 384x128:

#13 Image::initialize_data (this=0x55555d883450, p_width=384, p_height=128, p_use_mipmaps=true, p_format=Image::FORMAT_RGBA8, p_data=...) at core/io/image.cpp:2310
2310            data = p_data;

Which checks out because it's a padded up 381x127 (quickly tossed into the Editor to check):
image

@Alex-gnzl
Copy link
Author

Alex-gnzl commented Oct 27, 2024

I had a random thought and cleared mipmaps in image_decompress_bcdec before it gets to them and that made it work!
Something's wrong with the mipmaps -- could it be related to the <4 width/height thing; or did something change in Godot (I believe the game uses 4.2.1? not sure)?

@nikitalita
Copy link
Collaborator

This is definitely a bug in Godot. I would recommend filing an issue that cross-references godotengine/godot#97862 and godotengine/godot#97873.

Protip: mention that it causes a heap buffer overflow in the title. That always gets their attention.

@octogeckoyhtgyrgfvfthhyfhfdyh

How do I do this? Clear mipmaps, I mean.

@nikitalita
Copy link
Collaborator

I got my hands on Psychopomp Gold and attempted to recreate this to no avail. I think I might have fixed it on the latest PR? Could you try the artifacts here and see if you still have the problem? https://github.com/bruvzg/gdsdecomp/actions/runs/11611147300?pr=208

@Alex-gnzl
Copy link
Author

Alex-gnzl commented Nov 1, 2024

I got my hands on Psychopomp Gold and attempted to recreate this to no avail. I think I might have fixed it on the latest PR? Could you try the artifacts here and see if you still have the problem? https://github.com/bruvzg/gdsdecomp/actions/runs/11611147300?pr=208

Still experiencing this with these artifacts (both linux and windows/wine), as well as my own build on latest master ( 3175997 ) (only tried linux)
Backtrace looks the same.

@Alex-gnzl
Copy link
Author

However, I tried this on an actual Windows VM, and indeed it doesn't happen. Really odd it also happens in wine.

@nikitalita
Copy link
Collaborator

nikitalita commented Nov 1, 2024

ah hah; I think it's related to another bug that I just fixed wrt case-sensitive names, give this one a shot: https://github.com/bruvzg/gdsdecomp/actions/runs/11623418896?pr=209

@Alex-gnzl
Copy link
Author

No dice.

Also, since the project changed a bit since the initial stack trace, here's an up-to-date one (not that anything's different):

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff79b6463 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2  0x00007ffff795d120 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff79444c3 in __GI_abort () at abort.c:79
#4  0x00007ffff7945354 in __libc_message_impl (fmt=fmt@entry=0x7ffff7ad32f5 "%s\n") at ../sysdeps/posix/libc_fatal.c:132
#5  0x00007ffff79c0765 in malloc_printerr (str=str@entry=0x7ffff7ad6370 "free(): corrupted unsorted chunks") at malloc.c:5772
#6  0x00007ffff79c163c in _int_free_create_chunk (av=av@entry=0x7fff94000030, p=p@entry=0x7fff94d2ee00, size=<optimized out>, size@entry=131120, nextchunk=nextchunk@entry=0x7fff94d4ee30, nextsize=nextsize@entry=48704)
    at malloc.c:4735
#7  0x00007ffff79c297a in _int_free_merge_chunk (av=av@entry=0x7fff94000030, p=0x7fff94d2ee00, size=131120) at malloc.c:4700
#8  0x00007ffff79c2cfa in _int_free (av=0x7fff94000030, p=p@entry=0x7fff94d2ee00, have_lock=<optimized out>, have_lock@entry=0) at malloc.c:4646
#9  0x00007ffff79c55ce in __GI___libc_free (mem=0x7fff94d2ee10) at malloc.c:3398
#10 0x0000555558878109 in CowData<unsigned char>::_unref (this=0x7fff94d93688) at ./core/templates/cowdata.h:270
#11 CowData<unsigned char>::_ref (this=0x7fff94d93688, p_from=...) at ./core/templates/cowdata.h:465
#12 Vector<unsigned char>::operator= (this=0x7fff94d93680, p_from=...) at ./core/templates/vector.h:148
#13 Image::initialize_data (this=0x7fff94d93480, p_width=384, p_height=128, p_use_mipmaps=true, p_format=Image::FORMAT_RGBA8, p_data=...) at core/io/image.cpp:2310
#14 0x0000555555b30305 in image_decompress_bcdec (p_image=0x7fff94d93480) at modules/bcdec/image_decompress_bcdec.cpp:181
#15 0x0000555558877ea4 in Image::decompress (this=0x386c8c) at core/io/image.cpp:2711
#16 0x0000555555e6bd15 in TextureExporter::_convert_tex (this=<optimized out>, p_path=..., dest_path=..., lossy=false, image_format=...) at modules/gdsdecomp/exporters/texture_exporter.cpp:109
#17 0x0000555555e6d4cb in TextureExporter::export_resource (this=<optimized out>, output_dir=..., iinfo=...) at modules/gdsdecomp/exporters/texture_exporter.cpp:229
#18 0x0000555555e62290 in Exporter::export_resource (output_dir=..., import_infos=...) at modules/gdsdecomp/exporters/resource_exporter.cpp:128
#19 0x0000555555eb73da in ImportExporter::_do_export (this=0x55555d5bd0f0, i=<optimized out>, tokens=0x55555da087c0) at modules/gdsdecomp/utility/import_exporter.cpp:184
#20 0x0000555558d5e642 in WorkerThreadPool::_process_task (this=0x55555a34c090, p_task=p_task@entry=0x55555d923a78) at core/object/worker_thread_pool.cpp:101
#21 0x0000555558d5eedd in WorkerThreadPool::_thread_function (p_user=p_user@entry=0x55555a6501d0) at core/object/worker_thread_pool.cpp:205
#22 0x0000555558786682 in Thread::callback (p_caller_id=<optimized out>, p_settings=..., p_callback=0x555558d5ed90 <WorkerThreadPool::_thread_function(void*)>, p_userdata=0x55555a6501d0) at core/os/thread.cpp:64
#23 0x00007ffff7ce1c34 in std::execute_native_thread_routine (__p=0x55555a650bf0) at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104
#24 0x00007ffff79b439d in start_thread (arg=<optimized out>) at pthread_create.c:447
#25 0x00007ffff7a3949c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
```

@nikitalita
Copy link
Collaborator

It’s odd this only happens on Linux, and fixes itself when mipmaps cleared. The “free” in the stack trace suggests a double free.

@Alex-gnzl
Copy link
Author

It’s odd this only happens on Linux, and fixes itself when mipmaps cleared. The “free” in the stack trace suggests a double free.

I haven't been able to track down the cause, but here's a Valgrind log in the meantime (however, from a pre-multi-threaded version, c61f14a, so that it's more clear):
valgrind.log
Stuff related to this bug begins from line 65 i believe

@nikitalita
Copy link
Collaborator

Yeah, this is related to godotengine/godot#97862 , I'm seeing the same sort of invalid writes that I saw there. They might have missed something related to mipmaps data when doing the resize; the PR DOES assume the actual image data would be of the proper length.

I would recommend making an MRP (which shouldn't be too difficult; basically, copy the original .ctex file into a project, then make a script that loads the texture, gets the image, then decompresses it) and submitting the issue to godot. I would do this myself, but I can't reproduce this on MacOS and I'm not going to be near a computer that can run Linux for at least a month.

@FragileDeviations
Copy link

It’s odd this only happens on Linux, and fixes itself when mipmaps cleared. The “free” in the stack trace suggests a double free.

This is happening to me on Windows. Even with the other builds.

@nikitalita
Copy link
Collaborator

OK, I was finally able to track this down when building with sanitizers. It is related to the previous issue; basically, the last mipmap at the end of the mipmap list has dimensions that are non-divisible by 4, causing it to write past the allocated destination memory. I will submit an issue and hopefully get this fixed soon.

@nikitalita
Copy link
Collaborator

Tracking godotengine/godot#98884 ; when this is closed, I'll just resync with master.

@nikitalita
Copy link
Collaborator

give this artifact a shot: https://github.com/bruvzg/gdsdecomp/actions/runs/11698791421/job/32579668792?pr=215

I decided to take @Alex-gnzl 's advice and just clear_mipmaps() before saving the image, since they're not used by any of the current image saving functions.

@FragileDeviations
Copy link

This is not fixed, I'm still getting the crash when trying to decompile Psychopomp Gold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants