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

Prevent LTCG (MSVC LTO) from removing "pck" section #57450

Merged
merged 1 commit into from
Jan 31, 2022

Conversation

Listwon
Copy link
Contributor

@Listwon Listwon commented Jan 30, 2022

Fixes #35590

MSVC doesn't have equivalent of GCC __attribute__((used)) so this is the easiest workaround I could think of to prevent removing unused section.

@Listwon Listwon requested a review from a team as a code owner January 30, 2022 15:35
@akien-mga akien-mga added this to the 4.0 milestone Jan 30, 2022
@akien-mga akien-mga added cherrypick:3.4 cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jan 30, 2022
@RandomShaper
Copy link
Member

For a bit more safety it may be worthy to surround the assignment with
#pragma optimize( "", off )
and
#pragma optimize( "", on )
.

@Listwon
Copy link
Contributor Author

Listwon commented Jan 31, 2022

According to the documentation this won't work

The optimize pragma must appear outside a function. It takes effect at the first function defined after the pragma is seen. The on and off arguments turn options specified in the optimization-list on or off.

@akien-mga akien-mga merged commit 5b1b545 into godotengine:master Jan 31, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jan 31, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.4.3.

@akien-mga
Copy link
Member

akien-mga commented Feb 1, 2022

This seems to have broken template build with MinGW-GCC at least:

platform/windows/godot_windows.cpp: In function 'int widechar_main(int, wchar_t**)':
platform/windows/godot_windows.cpp:145:22: error: invalid conversion from 'const char*' to 'char*' [-fpermissive]
  145 |  char *dummy_guard = dummy;
      |                      ^~~~~
      |                      |
      |                      const char*

This can either be fixed with:

diff --git a/platform/windows/godot_windows.cpp b/platform/windows/godot_windows.cpp
index c4926209c7..506c903c35 100644
--- a/platform/windows/godot_windows.cpp
+++ b/platform/windows/godot_windows.cpp
@@ -41,7 +41,7 @@
 #pragma section("pck", read)
 __declspec(allocate("pck")) static char dummy[8] = { 0 };
 #elif defined __GNUC__
-static const char dummy[8] __attribute__((section("pck"), used)) = { 0 };
+static char dummy[8] __attribute__((section("pck"), used)) = { 0 };
 #endif
 #endif

or:

diff --git a/platform/windows/godot_windows.cpp b/platform/windows/godot_windows.cpp
index c4926209c7..906403c592 100644
--- a/platform/windows/godot_windows.cpp
+++ b/platform/windows/godot_windows.cpp
@@ -142,7 +142,7 @@ int widechar_main(int argc, wchar_t **argv) {
 
 #ifndef TOOLS_ENABLED
 	// Workaround to prevent LTCG (MSVC LTO) from removing "pck" section
-	char *dummy_guard = dummy;
+	char *dummy_guard = (char *)dummy;
 #endif
 
 	char **argv_utf8 = new char *[argc];

Not sure which is best / preserves the compiler magic that keeps this section in place.

akien-mga added a commit to akien-mga/godot that referenced this pull request Feb 1, 2022
akien-mga added a commit that referenced this pull request Feb 1, 2022
@Listwon
Copy link
Contributor Author

Listwon commented Feb 1, 2022

Hmm, I think that it would be better to just wrap this part in #if defined _MSC_VER as this is MSVC-only workaround. Sorry I didn't think about it earlier (I missed the difference between const char* for GCC and char * for MSVC).

akien-mga added a commit that referenced this pull request Feb 2, 2022
Riordan-DC pushed a commit to Riordan-DC/godot that referenced this pull request Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedded PCK option gives error messages with templates built using LTO.
3 participants