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

Missing symbols while linking with MinGW and LTO #6592

Closed
DorpsGek opened this issue Jul 25, 2017 · 8 comments
Closed

Missing symbols while linking with MinGW and LTO #6592

DorpsGek opened this issue Jul 25, 2017 · 8 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues

Comments

@DorpsGek
Copy link
Member

adf88 opened the ticket and wrote:

When compiling with LTO enabled (--enable-lto), GCC (-fwhole-program) changes all external symbols into static, except of the 'main' function. This causes some missing symbols when linking Windows builds of OpenTTD (e.g. with MinGW):

* WinMain - Entry point. '-mwindows' and other Windows-specific options didn't help, happened in GCC 6, GCC 7 seems smarter.
* _safe_esp - Variable referenced inside inline assembler code. Probably all GCC versions.

I'm including a fix for the trunk. It adds 'attribute ((used))' to the two symbols.

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6592
@DorpsGek
Copy link
Member Author

frosch wrote:

Does this mean that _safe_esp can now be declared static?
It looks to me the non-staticness tries to achieve the same thing.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6592#comment14475

@DorpsGek
Copy link
Member Author

adf88 wrote:

Seems so, but I'm not sure about other compilers.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6592#comment14476

@DorpsGek
Copy link
Member Author

adf88 wrote:

OK, I found out that 'volatile' works on _safe_esp in GCC 6 and 7. This would be a preferred way I think. I'm uploading updated patch.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6592#comment14477

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
@TrueBrain TrueBrain added patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay and removed bug from FlySpray labels Apr 12, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@TrueBrain
Copy link
Member

The _safe_esp feels more like a work-around than anything else. Rest of the patch looks fine, just the comment is a bit weird. Guess what you are trying to say:

LTO doesn't see WinMain being used (as it isn't from our application), so mark it as used to prevent it beingt optimized out.

@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. Since OpenTTD moved to GitHub, we use pull requests rather than patches, as they are a much more productive workflow.

I'm planning to close this soon (in 7 days), as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun.

If you would like to continue with this patch, the best way would be to move the patch to your own GitHub fork, update it for the current OpenTTD master, and then create a pull request. For more information, please see our CONTRIBUTING.md.

We are also happy to discuss directly on the issue, or in #openttd irc, including help to get this into a pull request. Thanks for your contribution!

@andythenorth andythenorth removed the stale Stale issues label Jan 24, 2019
@andythenorth
Copy link
Contributor

Per previous comment, closing. Thanks!

@LordAro
Copy link
Member

LordAro commented Jan 24, 2019

Just for the lols, compiling with --enable-lto in MinGW (GCC 8.2) results in:

C:/msys64/home/LordAro/OpenTTD/src/order_gui.cpp: In member function 'OnDropdownSelect':
C:/msys64/home/LordAro/OpenTTD/src/core/bitmath_func.hpp:62:4: warning: 'MEM[(unsigned char &)&order + 3]' may be used uninitialized in this function [-Wmaybe-uninitialized]
  x &= (T)(~((((T)1U << n) - 1) << s));
    ^
C:/msys64/home/LordAro/OpenTTD/src/order_gui.cpp: In member function 'OnHotkey':
C:/msys64/home/LordAro/OpenTTD/src/core/bitmath_func.hpp:62:4: warning: 'MEM[(unsigned char &)&order + 3]' may be used uninitialized in this function [-Wmaybe-uninitialized]
  x &= (T)(~((((T)1U << n) - 1) << s));
    ^
C:/msys64/home/LordAro/OpenTTD/src/vehicle_cmd.cpp: In function 'CmdBuildVehicle':
C:/msys64/home/LordAro/OpenTTD/src/order_backup.cpp:91:9: warning: 'v' may be used uninitialized in this function [-Wmaybe-uninitialized]
  if (v->cur_implicit_order_index >= v->GetNumOrders()) v->cur_implicit_order_index = v->cur_real_order_index;
         ^
C:/msys64/home/LordAro/OpenTTD/src/vehicle_cmd.cpp:119:11: note: 'v' was declared here
  Vehicle *v;
           ^
C:/msys64/home/LordAro/OpenTTD/src/strings.cpp: In function 'SetDParamMaxDigits':
C:/msys64/home/LordAro/OpenTTD/src/strings.cpp:125:25: warning: 'front' may be used uninitialized in this function [-Wmaybe-uninitialized]
  uint64 val = count > 1 ? front : next;
                         ^
C:/msys64/home/LordAro/OpenTTD/src/strings.cpp:123:7: note: 'front' was declared here
  uint front, next;
       ^
C:/msys64/home/LordAro/OpenTTD/src/strings.cpp:123:14: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]
  uint front, next;
              ^

Further amusingly, if you install SDL and try to use that, you get a couple of extra errors:

C:/msys64/home/LordAro/OpenTTD/src/video/sdl_v.cpp:425:8: warning: type 'struct VkMapping' violates the C++ One Definition Rule [-Wodr]
 struct VkMapping {
        ^
C:/msys64/home/LordAro/OpenTTD/src/video/win32_v.cpp:122:8: note: a different type is defined in another translation unit
 struct VkMapping {
        ^
C:/msys64/home/LordAro/OpenTTD/src/video/sdl_v.cpp:429:9: note: the first difference of corresponding definitions is field 'vk_from'
  uint16 vk_from;
         ^
C:/msys64/home/LordAro/OpenTTD/src/video/win32_v.cpp:123:7: note: a field of same name but different type is defined in another translation unit
  byte vk_from;
       ^
C:/msys64/home/LordAro/OpenTTD/src/video/sdl_v.cpp:425:8: note: type 'uint16' should match type 'byte'
 struct VkMapping {
        ^

@LordAro LordAro reopened this Jan 24, 2019
@stale
Copy link

stale bot commented Mar 25, 2019

This issue has been automatically marked as stale because it has not had any activity in the last two months.
If you believe the issue is still relevant, please test on the latest nightly and report back.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale Stale issues label Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues
Projects
None yet
Development

No branches or pull requests

5 participants