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

Embedded PCK option gives error messages with templates built using LTO. #35590

Closed
ghost opened this issue Jan 26, 2020 · 8 comments · Fixed by #57450
Closed

Embedded PCK option gives error messages with templates built using LTO. #35590

ghost opened this issue Jan 26, 2020 · 8 comments · Fixed by #57450

Comments

@ghost
Copy link

ghost commented Jan 26, 2020

Godot version:
689db68

MSVC platform=windows bits=32 target=release tools=no use_lto=yes

OS/device including version:
Win10 64-bit Editor / 32-bit Release

Issue description:
Tried using the export with embedded PCK option with a template that I compiled with LTO using MSVC.

Got this message.
godot windows tools 64_2020-01-26_09-58-39

Traced to:

godot/editor/editor_export.cpp

Lines 1594 to 1597 in be1bc53

FixUpEmbeddedPckFunc fixup_func = get_fixup_embedded_pck_func();
if (fixup_func) {
err = fixup_func(p_path, embedded_pos, embedded_size);
}

Then debugged it to a loop where it cannot find what it's looking for:

for (int i = 0; i < num_sections; ++i) {
int64_t section_header_pos = section_table_pos + i * 40;
f->seek(section_header_pos);
uint8_t section_name[9];
f->get_buffer(section_name, 8);
section_name[8] = '\0';
if (strcmp((char *)section_name, "pck") == 0) {
// "pck" section found, let's patch!
// Set virtual size to a little to avoid it taking memory (zero would give issues)
f->seek(section_header_pos + 8);
f->store_32(8);
f->seek(section_header_pos + 16);
f->store_32(p_embedded_size);
f->seek(section_header_pos + 20);
f->store_32(p_embedded_start);
found = true;
break;
}
}
f->close();
return found ? OK : ERR_FILE_CORRUPT;

Oddly the executable still runs, but I'm very paranoid about it. It does build fine with external PCK.

@RandomShaper Is it expected not to use LTO when using this feature, or is there something extra that can be done to allow for it? I imagine LTO can arbitrarily shuffle things around. Or is this maybe a false error?

Steps to reproduce:
Use an export template built with MSVC on Windows using LTO flag.

Additionally had these warnings during the LTO phase when building it, though they don't look relevant. (Including for completeness)

Generating code
C:\Godot\Version 3.2\Source\drivers\gles2\rasterizer_canvas_gles2.cpp(1011) : warning C4750: 'void __thiscall RasterizerCanvasGLES2::_draw_polygon(int const *,int,int,struct Vector2 const *,struct Vector2 const *,struct Color const *,bool,float const *,int const *)': function with _alloca() inlined into a loop
C:\Godot\Version 3.2\Source\drivers\gles2\rasterizer_canvas_gles2.cpp(1032) : warning C4750: 'void __thiscall RasterizerCanvasGLES2::_draw_polygon(int const *,int,int,struct Vector2 const *,struct Vector2 const *,struct Color const *,bool,float const *,int const *)': function with _alloca() inlined into a loop
C:\Godot\Version 3.2\Source\drivers\gles2\rasterizer_canvas_gles2.cpp(1037) : warning C4750: 'void __thiscall RasterizerCanvasGLES2::_draw_generic_indices(unsigned int,int const *,int,int,struct Vector2 const *,struct Vector2 const *,struct Color const *,bool)': function with _alloca() inlined into a loop
C:\Godot\Version 3.2\Source\drivers\gles2\rasterizer_canvas_gles2.cpp(1677) : warning C4750: 'void __thiscall RasterizerCanvasGLES2::_canvas_item_render_commands(struct RasterizerCanvas::Item *,struct RasterizerCanvas::Item *,bool &,struct RasterizerStorageGLES2::Material *)': function with _alloca() inlined into a loop
C:\Godot\Version 3.2\Source\drivers\gles2\rasterizer_canvas_gles2.cpp(1756) : warning C4750: 'void __thiscall RasterizerCanvasGLES2::_canvas_item_render_commands(struct RasterizerCanvas::Item *,struct RasterizerCanvas::Item *,bool &,struct RasterizerStorageGLES2::Material *)': function with _alloca() inlined into a loop
  100%   0 seconds remaining
Finished generating code

Minimal reproduction project:

I have included the build template in this project for expedience.
SHA256: E6F8423029B2598F5F262B461EB47BB12A28856587A2A68831372ECD1F996448

Export Embedded PCK Issue.zip

@Calinou
Copy link
Member

Calinou commented Jan 26, 2020

Does this occur if you compile a Windows binary using MSVC, but without LTO? What about a binary compiled with MinGW and LTO?

@RandomShaper
Copy link
Member

I guess LTO is removing the pck section. That could be check with dumpbin.

@ghost
Copy link
Author

ghost commented Jan 26, 2020

@Calinou Yes, fine without LTO. Isolated it at least to that. Haven't had done much with MinGW, so it isn't set up at the moment. Will take a look next chance I get.

@ghost
Copy link
Author

ghost commented Jan 27, 2020

@Calinou No luck with a clean build. I assume I have something mis-configured, unless someone can confirm otherwise. Unfortunately this is a far I can go with it.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 17, 2020

Can anyone still reproduce this bug in Godot 3.2.3 or any later release?

@akien-mga akien-mga modified the milestones: 3.2, 3.3 Mar 17, 2021
@devrique
Copy link

devrique commented Jun 14, 2021

@KoBeWi Still happening on 3.x branch, commit e8581e5 (last commit at this moment).
Removing use_lto=yes parameter from the command to generate a release template creates the embedded executable correctly.

@Calinou
Copy link
Member

Calinou commented Jun 14, 2021

As a workaround, you can compile a Windows export template with the icon at platform/windows/godot.ico changed before compiling the export template.

@akien-mga akien-mga modified the milestones: 3.3, 3.5 Oct 26, 2021
@Listwon
Copy link
Contributor

Listwon commented Jan 30, 2022

I guess LTO is removing the pck section. That could be check with dumpbin.

This is exactly what happens here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants