search

milkytracker / MilkyTracker

An FT2 compatible music tracker
http://milkytracker.github.io/
Other
1.67k stars 160 forks source link

Build fails with LTO #340

Closed eli-schwartz closed 3 months ago

eli-schwartz commented 3 months ago

I tried to build with the following *FLAGS to optimize the build: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

Note the -Werror=* flags are used to help detect cases where the compiler tries to optimize by assuming UB cannot exist in the source code -- if it does exist, ordinarily the code would be miscompiled, and this says to make the miscompilation a fatal error.

I got this error:

FAILED: src/tracker/milkytracker 
: && /usr/bin/x86_64-pc-linux-gnu-g++ -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Wl,-O1 -Wl,--as-needed -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wl,--defsym=__gentoo_check_ldflags__=0 src/compression/CMakeFiles/compression.dir/Decompressor.cpp.o src/compression/CMakeFiles/compression.dir/DecompressorLZX.cpp.o src/compression/CMakeFiles/compression.dir/DecompressorPP20.cpp.o src/compression/CMakeFiles/compression.dir/DecompressorUMX.cpp.o src/compression/CMakeFiles/compression.dir/PP20.cpp.o src/compression/CMakeFiles/compression.dir/unlzx.cpp.o src/compression/CMakeFiles/compression.dir/DecompressorGZIP.cpp.o src/compression/CMakeFiles/compression.dir/DecompressorZIP.cpp.o src/compression/CMakeFiles/compression.dir/ZipExtractor.cpp.o src/compression/CMakeFiles/compression.dir/zziplib/MyIO.cpp.o src/tracker/CMakeFiles/tracker.dir/AnimatedFXControl.cpp.o src/tracker/CMakeFiles/tracker.dir/ColorExportImport.cpp.o src/tracker/CMakeFiles/tracker.dir/ColorPaletteContainer.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogHelp.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogChannelSelector.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogEQ.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogGroupSelection.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogHandlers.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogListBox.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogPanning.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogQuickChooseInstrument.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogResample.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogWithValues.cpp.o src/tracker/CMakeFiles/tracker.dir/DialogZap.cpp.o src/tracker/CMakeFiles/tracker.dir/EQConstants.cpp.o src/tracker/CMakeFiles/tracker.dir/EditorBase.cpp.o src/tracker/CMakeFiles/tracker.dir/EnvelopeContainer.cpp.o src/tracker/CMakeFiles/tracker.dir/EnvelopeEditor.cpp.o src/tracker/CMakeFiles/tracker.dir/EnvelopeEditorControl.cpp.o src/tracker/CMakeFiles/tracker.dir/Equalizer.cpp.o src/tracker/CMakeFiles/tracker.dir/FileExtProvider.cpp.o src/tracker/CMakeFiles/tracker.dir/FileIdentificator.cpp.o src/tracker/CMakeFiles/tracker.dir/GlobalColorConfig.cpp.o src/tracker/CMakeFiles/tracker.dir/InputControlListener.cpp.o src/tracker/CMakeFiles/tracker.dir/LogoBig.cpp.o src/tracker/CMakeFiles/tracker.dir/LogoSmall.cpp.o src/tracker/CMakeFiles/tracker.dir/ModuleEditor.cpp.o src/tracker/CMakeFiles/tracker.dir/ModuleServices.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditor.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorClipBoard.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorControl.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorControlEventListener.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorControlKeyboard.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorControlTransposeHandler.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternEditorTools.cpp.o src/tracker/CMakeFiles/tracker.dir/PatternTools.cpp.o src/tracker/CMakeFiles/tracker.dir/PeakLevelControl.cpp.o src/tracker/CMakeFiles/tracker.dir/Piano.cpp.o src/tracker/CMakeFiles/tracker.dir/PianoControl.cpp.o src/tracker/CMakeFiles/tracker.dir/PlayerController.cpp.o src/tracker/CMakeFiles/tracker.dir/PlayerLogic.cpp.o src/tracker/CMakeFiles/tracker.dir/PlayerMaster.cpp.o src/tracker/CMakeFiles/tracker.dir/RecPosProvider.cpp.o src/tracker/CMakeFiles/tracker.dir/RecorderLogic.cpp.o src/tracker/CMakeFiles/tracker.dir/ResamplerHelper.cpp.o src/tracker/CMakeFiles/tracker.dir/SampleEditor.cpp.o src/tracker/CMakeFiles/tracker.dir/SampleEditorControl.cpp.o src/tracker/CMakeFiles/tracker.dir/SampleEditorControlToolHandler.cpp.o src/tracker/CMakeFiles/tracker.dir/SampleEditorResampler.cpp.o src/tracker/CMakeFiles/tracker.dir/SamplePlayer.cpp.o src/tracker/CMakeFiles/tracker.dir/ScopesControl.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionAbout.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionAbstract.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionAdvancedEdit.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionDiskMenu.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionHDRecorder.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionInstruments.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionOptimize.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionQuickOptions.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionSamples.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionSettings.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionSwitcher.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionTranspose.cpp.o src/tracker/CMakeFiles/tracker.dir/SectionUpperLeft.cpp.o src/tracker/CMakeFiles/tracker.dir/SongLengthEstimator.cpp.o src/tracker/CMakeFiles/tracker.dir/SystemMessage.cpp.o src/tracker/CMakeFiles/tracker.dir/TabHeaderControl.cpp.o src/tracker/CMakeFiles/tracker.dir/TabManager.cpp.o src/tracker/CMakeFiles/tracker.dir/TabTitleProvider.cpp.o src/tracker/CMakeFiles/tracker.dir/TitlePageManager.cpp.o src/tracker/CMakeFiles/tracker.dir/ToolInvokeHelper.cpp.o src/tracker/CMakeFiles/tracker.dir/Tracker.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerConfig.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerInit.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerKeyboard.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerSettings.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerSettingsDatabase.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerShortCuts.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerShutDown.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerStartUp.cpp.o src/tracker/CMakeFiles/tracker.dir/TrackerUpdate.cpp.o src/tracker/CMakeFiles/tracker.dir/Undo.cpp.o src/tracker/CMakeFiles/tracker.dir/VRand.cpp.o src/tracker/CMakeFiles/tracker.dir/Zapper.cpp.o src/tracker/CMakeFiles/tracker.dir/sdl/SDL_KeyTranslation.cpp.o src/tracker/CMakeFiles/tracker.dir/sdl/SDL_Main.cpp.o -o src/tracker/milkytracker  src/fx/libfx.a  src/milkyplay/libmilkyplay.a  src/ppui/osinterface/libosinterface.a  src/ppui/libppui.a  src/midi/libmidi.a  /usr/lib64/libz.so  /usr/lib64/libzzip.so  /usr/lib64/libasound.so  src/ppui/osinterface/libosinterface.a  /usr/lib64/libSDL2.so  /usr/lib64/librtmidi.so && :
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/posix/PPMutex.h:36:7: error: type ‘struct PPMutex’ violates the C++ One Definition Rule [-Werror=odr]
   36 | class PPMutex
      |       ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/sdl/PPMutex.h:36:7: note: a different type is defined in another translation unit
   36 | class PPMutex
      |       ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/posix/PPMutex.h:39:26: note: the first difference of corresponding definitions is field ‘mutexpth_p’
   39 |         pthread_mutex_t* mutexpth_p;
      |                          ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/sdl/PPMutex.h:39:20: note: a field with different name is defined in another translation unit
   39 |         SDL_mutex* mutex;
      |                    ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/sdl/SDL_ModalLoop.cpp:43:17: error: ‘globalMutex’ violates the C++ One Definition Rule [-Werror=odr]
   43 | extern PPMutex* globalMutex;
      |                 ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/posix/PPMutex.h:36:7: note: type ‘struct PPMutex’ itself violates the C++ One Definition Rule
   36 | class PPMutex
      |       ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/ppui/osinterface/sdl/PPMutex.h:36:7: note: the incompatible type is defined here
   36 | class PPMutex
      |       ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/tracker/sdl/SDL_Main.cpp:98:33: note: ‘globalMutex’ was previously declared here
   98 | PPMutex*                        globalMutex                             = NULL;
      |                                 ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/tracker/DialogZap.cpp:37:6: error: type ‘ControlIDs’ violates the C++ One Definition Rule [-Werror=odr]
   37 | enum ControlIDs
      |      ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/tracker/SectionAbout.cpp:40:6: note: an enum with different value name is defined in another translation unit
   40 | enum ControlIDs
      |      ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/tracker/DialogZap.cpp:39:9: note: name ‘MESSAGEBOXZAP_BUTTON_ALL’ differs from name ‘FXCONTROL’ defined in another translation unit
   39 |         MESSAGEBOXZAP_BUTTON_ALL = PP_MESSAGEBOX_BUTTON_USER1,
      |         ^
/var/tmp/portage/media-sound/milkytracker-1.04.00-r1/work/MilkyTracker-1.04.00/src/tracker/SectionAbout.cpp:42:9: note: mismatching definition
   42 |         FXCONTROL                                       = 9000
      |         ^
lto1: some warnings being treated as errors
lto-wrapper: fatal error: /usr/bin/x86_64-pc-linux-gnu-g++ returned 1 exit status
compilation terminated.
/usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

Downstream report: https://bugs.gentoo.org/860870 Full build log: build.log.txt

coderofsalvation commented 3 months ago

Thanks for reporting. Not really sure whether this is really a dealbreaker as these flags are not part of the buildscripts in this repo. If this is easy to fix let us know (how).

eli-schwartz commented 3 months ago

Not really sure whether this is really a dealbreaker as these flags are not part of the buildscripts in this repo.

It indicates latent UB, which may or may not segfault or produce broken and incorrect behavior in general.

The only flag that actually changes the compiled program is -flto, which is like -O2 in that it tells the compiler to work harder to optimize the binary more. -flto is a massively global optimization pass, which means it is better at exposing issues which exist either way. Also that it's better at globally optimizing the entire program. :)

If this is easy to fix let us know (how).

PPMutex should be one of:

I guess the question is, why do the posix/ and sdl/ directories have different versions of PPMutex.h? If it's just about having platform-specific versions, then that implies part of the codebase is incorrectly including the wrong version of the header. Maybe the answer is just to fix the header include.

...

The issue is closed as "completed", and not closed as "not planned", implying that it's been fixed. I cannot find the fix -- it looks like there have been no pushes since February -- but probably I'm missing something? I'd be happy to test a fix if you like.