These options are now only applied if compiling with clang.
While it's possible to use libc++ with gcc, it requires a lot of manual
changes, and basicaly no one does it. (libstdc++ with clang is commonly
done on desktop Linux systems, though.)
Don't -D_LIBCPP_ENABLE_ASSERTIONS on clang-17 or later. It's deprecated,
and may result in a compile error. (...though on Android/Termux with
clang-20.1.3, it didn't...)
Newer compilers usually add more warnings, so having -Werror by default
will most likely result in a compile error when new compiler versions
are released.
MSVC: Remove "-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING". This is no
longer relevant, since we don't support anything older than MSVC 2015.
It seems this was broken since it was initially implemented in v2.0
in commit d732fbc010
([cmake] platform/gcc.cmake: Initial support for detecting DT_RELR.)
because TMP_HAVE_DT_RELR was unset too early.
When compiling with LTO enabled on gcc-14.2.0 (e.g. with Ubuntu 25.04),
the following warning (as error) appeared. Work around it by checking for
str.empty() before resizing the string.
[cmake] gcc.cmake: Remove -Wno-error=aggressive-loop-optimizations, since
it's no longe needed.
In function ‘assign’,
inlined from ‘_S_assign’ at /usr/include/c++/14/bits/basic_string.h:455:23,
inlined from ‘_S_assign’ at /usr/include/c++/14/bits/basic_string.h:450:7,
inlined from ‘_M_replace_aux’ at /usr/include/c++/14/bits/basic_string.tcc:471:17,
inlined from ‘append’ at /usr/include/c++/14/bits/basic_string.h:1499:30,
inlined from ‘resize’ at /usr/include/c++/14/bits/basic_string.tcc:405:14,
inlined from ‘resize’ at /usr/include/c++/14/bits/basic_string.h:1119:21,
inlined from ‘TestBody’ at src/librptext/tests/TextFuncsTest.cpp:513:12:
/usr/include/c++/14/bits/char_traits.h:837:25: error: iteration 9223372036854775807 invokes undefined behavior [-Werror=aggressive-loop-optimizations]
837 | assign(__s[__i], __a);
| ^
/usr/include/c++/14/bits/char_traits.h:836:34: note: within this loop
836 | for (size_t __i = 0; __i < __n; ++__i)
| ^
In function ‘assign’,
inlined from ‘_S_assign’ at /usr/include/c++/14/bits/basic_string.h:455:23,
inlined from ‘_S_assign’ at /usr/include/c++/14/bits/basic_string.h:450:7,
inlined from ‘_M_replace_aux’ at /usr/include/c++/14/bits/basic_string.tcc:471:17,
inlined from ‘append’ at /usr/include/c++/14/bits/basic_string.h:1499:30,
inlined from ‘resize’ at /usr/include/c++/14/bits/basic_string.tcc:405:14,
inlined from ‘resize’ at /usr/include/c++/14/bits/basic_string.h:1119:21,
inlined from ‘TestBody’ at src/librptext/tests/TextFuncsTest.cpp:542:12:
/usr/include/c++/14/bits/char_traits.h:837:25: error: iteration 9223372036854775807 invokes undefined behavior [-Werror=aggressive-loop-optimizations]
837 | assign(__s[__i], __a);
| ^
/usr/include/c++/14/bits/char_traits.h:836:34: note: within this loop
836 | for (size_t __i = 0; __i < __n; ++__i)
| ^
lto1: all warnings being treated as errors
lto-wrapper: fatal error: /usr/bin/c++ returned 1 exit status
compilation terminated.
The same error showed up on armhf and amd64 this time.
Since it's happening in LTO and not the main compiler step, the pragmas
likely aren't working properly. Use -Wno-error=stringop-overread instead.
(cherry picked from commit ec441b4cec)
The Ubuntu 16.04 Launchpad build for armhf failed due to a potential
alignment issue:
/<<PKGBUILDDIR>>/src/kde/AchQtDBus.cpp: In member function ‘int AchQtDBus::notifyFunc(LibRpBase::Achievements::ID)’:
/<<PKGBUILDDIR>>/src/kde/AchQtDBus.cpp:149:59: error: cast from ‘uchar* {aka unsigned char*}’ to ‘LibRpTexture::argb32_t*’ increases required alignment of target type [-Werror=cast-align]
argb32_t *bits = reinterpret_cast<argb32_t*>(icon.bits());
^
This shouldn't be an actual problem, since rp_image's image data is
always 16-byte aligned.
NOTE: Not rebuilding the Ubuntu 16.04 packages for ARM because of this.
(Both the armhf and arm64 builds were cancelled.)
(cherry picked from commit 5a08131311)
clang complains about moc headers due to how automoc handles things:
In file included from build/src/kde/kf5/overlayiconplugin_rom-properties-kf5_autogen/mocs_compilation.cpp:2:
build/src/kde/kf5/overlayiconplugin_rom-properties-kf5_autogen/BKZOEHIFDQ/moc_OverlayIconPluginKF5.cpp:1340:17: error: using namespace directive in global context in header [-Werror,-Wheader-hygiene]
1340 | using namespace RomPropertiesKF5;
| ^
1 error generated.
This clang warning is currently "intentional", though I might rename the
not-virtual open() function later:
In file included from src/libromdata/Handheld/Nintendo3DS.cpp:14:
In file included from src/libromdata/Handheld/Nintendo3DS_p.hpp:19:
src/libromdata/Handheld/../disc/NCCHReader.hpp:193:24: error: 'LibRomData::NCCHReader::open' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
193 | LibRpFile::IRpFilePtr open(int section, const char *filename);
| ^
src/libromdata/../librpbase/disc/IPartition.hpp:104:32: note: hidden overloaded virtual function 'LibRpBase::IPartition::open' declared here: different number of parameters (1 vs 2)
104 | virtual LibRpFile::IRpFilePtr open(const char *filename);
| ^
1 error generated.
Fixes what appears to be a false-positive warning-as-error in the
Release build:
src/librpbase/img/RpPng.cpp: In function ‘LibRpTexture::rp_image_ptr LibRpBase::RpPng::loadPng(png_structp, png_infop)’:
src/librpbase/img/RpPng.cpp:181:42: error: variable ‘color’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
181 | argb32_t color;
| ^~~~~
cc1plus: all warnings being treated as errors
This broke the Ubuntu 22.04 AppVeyor build:
extlib/microtar/src/microtar.c: In function ‘mtar_read_header’:
extlib/microtar/src/microtar.c:111:3: error: ‘__builtin_strncpy’ output may be truncated copying 99 bytes from a string of length 99 [-Werror=stringop-truncation]
111 | strncpy(h->name, rh->name, sizeof(h->name));
| ^
extlib/microtar/src/microtar.c:112:3: error: ‘__builtin_strncpy’ output may be truncated copying 99 bytes from a string of length 99 [-Werror=stringop-truncation]
112 | strncpy(h->linkname, rh->linkname, sizeof(h->linkname));
| ^
cc1: all warnings being treated as errors
Disabled these warnings:
- C4005: macro redefinition (libpng's intprefix.out.tf1 is breaking on this...)
- C4091: 'typedef ': ignored on left of 'tagGPFIDL_FLAGS' when no variable is declared
- C4800: 'BOOL': forcing value to bool 'true' or 'false' (performance warning)
TODO:
- Re-enable C4800 as not-an-error?
- Figure out how to fix libpng's C4005 warnings.
The MSVC build (64-bit, at least...) now builds with no warnings.
Tested using MSVC 2022 17.6.5.
NOTE: /W4 adds a *lot* of warnings that are mostly just noise. Need to
check /W4 and selectively enable at least some of them...
Removed /wd4482. I don't think this warning is relevant anymore.
Warning fixes:
[extlib] Add (unsigned int) casts where necessary.
[librpbase] Config::ImgTypePrio_t, KeyManager::KeyData_t:
- Change `length` from unsigned int to size_t.
The struct is 16 bytes on 64-bit either way, but making it size_t
fixes some conversion warnings.
[librpbase] Hash::Process():
- crc32() returns `unsigned long`, not `uint32_t`, for some reason.
- Also, its length parameter is `unsigned int`, not `size_t`.
- KeyManager::hexStringToBytes(): Take a `size_t` length parameter.
[librpfile] scsi:
- Change cdb_len from `uint8_t` to `size_t`. Using a `uint8_t` argument
doesn't actually save any memory, since it uses 4 bytes on the stack
for 32-bit, and one register on 64-bit, regardless.
- Also, limit the maximum cdb size to 260.
[rp-download] Disable warning C4996 when calling GetVersionEx().
[librptexture] DirectDrawSurface:
- Change some `expected_size` variables from `unsigned int` to `size_t`.
[librptexture] PalmOS_Tbmp:
- Change d->bitmapTypeAddr from `off64_t` to `uint32_t`.
- Change some size variables from `unsigned int` to `size_t`.
- loadTbmp(): v3 transparency: Explicitly cast the transparency value
to uint16_t. (It's stored as a big-endian 32-bit value, but only
16 bits are used.)
- getNextTbmpAddress(): Cast addresses to `uint32_t`. PalmOS executables
are 32-bit and cannot possibly exceed 4 GB.
- FIXME: This function isn't used? (Was it ever used?)
[libromdata]
- DMGPrivate::CartType(): Make `end_offset` constexpr.
- NCCHReader: Add casts for EncSections when using sizeof().
- KeyStoreUIPrivate: binToHexStr(), verifyKeyData():
- Take a `size_t` length parameter.
- WiiUPackagePrivate: Added a parseHexBinary32() wrapper function
that returns `uint32_t` instead of `uint64_t`.
[librpbyteswap/tests]
- BitstuffTest: Cast time(nullptr) to `unsigned int`.
srand() takes `unsigned int`, not `time_t`.
[librpbase/tests]
- RpPngFormatTest: crc32()'s length parameter is `unsigned int`,
not `size_t`.
[libromdata/tests]
- SuperMagicDriveTest: zlib uses `unsigned int`, so cast array sizes
to `unsigned int` instead of changing decompress() to take `size_t`.
On Windows 10 1607+, this flag is equivalent to calling the
SetDefaultDllDirectories() function before the program starts.
We're already calling SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32)
in librpsecure/win32/restrict-dll.c, but this ensures that it's set even
before the program starts.
Related changes:
- amiiboc: Use delay-loading for libfmt. Otherwise, it fails with
error code 0xC0000135.
- Added a dummy stdafx.h file, required by DelayLoadHelper.c.
- All tests: Set /DEPENDENTLOADFLAG:0xA00 because gtest cannot be
delay-loaded.
- msvc.cmake: Fix linker flags for RelWithDebInfo.
TODO:
- EXE: Show DEPENDENTLOADFLAG.
- Test svrplus.
FIx an error in the non-clang section that caused the common conformance
flags to be overwritten with the MSVC-only flags. Not sure if this will
break anything, and I can't build with MSVC at the moment...
Move "/Zc:throwingNew" into the same non-clang-cl block as the other
options.
This fixes the 'negative' errors from lcov's geninfo.
Before:
source files: 600
lines.......: 23.2% (10654 of 45904 lines)
functions...: 22.8% (848 of 3721 functions)
branches....: 15.8% (7572 of 47821 branches)
After:
source files: 600
lines.......: 23.2% (10654 of 45904 lines)
functions...: 22.8% (848 of 3721 functions)
branches....: 15.8% (7572 of 47821 branches)
Effectively no change in coverage, but it's likely more stable.
I'll note that the error was triggering in ASTC decoding, which
uses OpenMP for parallel processing.
Assertions are enabled for libstdc++ (gcc) and libc++ (llvm/clang).
Debug mode is enabled for libstdc++ only.
libc++'s hardened mode is also enabled.
libstdc++'s debug mode has already helped find a few bugs:
- GLenumStrings.cpp: sRGB PVRTC v1 was in the wrong place.
- AmiiboData.cpp: Searching for character variants was incorrect.
Not enabled for Release builds, since the assertions cause the program
to crash with SIGABRT.
gcc-7.1 and later print a lot of annoying warnings on armhf whenever
passing an iterator to a function:
In file included from /usr/include/c++/13/memory:69,
from rom-properties/src/librptexture/stdafx.h:23,
from rom-properties/build/src/librptexture/CMakeFiles/rptexture.dir/cmake_pch.hxx:7,
from <command-line>:
/usr/include/c++/13/bits/stl_uninitialized.h: In function ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = move_iterator<_KTX2_Mipmap_Index*>; _ForwardIterator = _KTX2_Mipmap_Index*; _Allocator = rp::default_init_allocator<_KTX2_Mipmap_Index, allocator<_KTX2_Mipmap_Index> >]’:
/usr/include/c++/13/bits/stl_uninitialized.h:344:5: note: parameter passing for argument of type ‘std::move_iterator<_KTX2_Mipmap_Index*>’ changed in GCC 7.1
344 | __uninitialized_copy_a(_InputIterator __first, _InputIterator __last,
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/13/bits/stl_uninitialized.h:344:5: note: parameter passing for argument of type ‘std::move_iterator<_KTX2_Mipmap_Index*>’ changed in GCC 7.1
/usr/include/c++/13/bits/stl_uninitialized.h: In function ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = _KTX2_Mipmap_Index*; _ForwardIterator = _KTX2_Mipmap_Index*; _Allocator = rp::default_init_allocator<_KTX2_Mipmap_Index, allocator<_KTX2_Mipmap_Index> >]’:
/usr/include/c++/13/bits/stl_uninitialized.h:399:9: note: parameter passing for argument of type ‘std::move_iterator<_KTX2_Mipmap_Index*>’ changed in GCC 7.1
398 | return std::__uninitialized_copy_a
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
399 | (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
400 | _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Enable it for both debug and release if using 10.0 or later.
This fixes an issue when running package.cmd that caused
InitPropVariantFromUInt16() to not be detected correctly when using
the Windows 7 SDK. (/permissive- was only enabled for debug builds,
but MSVC builds are technically debug and release, so it got enabled
in the InitPropVariantFromUInt16() check.)
These errors occurred without this fix:
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\objbase.h(239): error C2760: syntax error: unexpected token 'identifier', expected 'type specifier'
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\gdiplusheaders.h(891): error C4596: 'EmfToWmfBits': illegal qualified name in member declaration
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\gdiplusstringformat.h(220): error C4596: 'GetTrimming': illegal qualified name in member declaration
Available in clang, but not gcc (as of 14.2).
This ensures that header files (including .cpp files used with #include)
do not have global `using` statements.
NintendoDS_p.hpp: Removed unnecessary `using namespace` statements.
Removed global `using` statements from TCreateThumbnail.cpp and
TImageTypesConfig.cpp. Functions now have localized `using` statements.
Updated calling files to add `using` statements where necessary.
TImageTypesConfig.cpp: Also add a static_assert() for conf_imageTypeNames[].
FIXME: The generated moc files for OverlayIconPluginKF5 and
OverlayIconPluginKF6 both have `using namespace` directives, which
causes clang to fail due to -Werror=header-hygiene.
We don't need to check errno for any math functions, and this prevents
certain inlining optimizations. In particular, lrintf() can't be inlined
if we don't specify this, even though glibc's documentation says that
its lrintf() implementation doesn't set errno.
Assembly comparison of manual truncation vs. lrintf() with
-fno-math-errno:
amd64:
- Manual: cvttss2si
- lrintf(): cvtss2si (note the single t)
i386 with SSE disabled:
- Manual: fistp with control word manipulation
- lrintf(): fistp without control word manipulation
Presumably the control word on manipulation changes the rounding mode.
lrintf() without errno doesn't change the rounding mode and relies on
the default rounding, which matches amd64.
TODO: MSVC equivalent, if available.
[libromdata] Nintendo3DSFirm: Fix a strict aliasing error on Ubuntu 16.04. (gcc-5.4.0)
In file included from src/libromdata/stdafx.h:62:0,
from src/libromdata/Handheld/Nintendo3DSFirm.cpp:9:
src/libromdata/Handheld/Nintendo3DSFirm.cpp: In member function ‘virtual int LibRomData::Nintendo3DSFirm::loadFieldData()’:
src/libromdata/Handheld/Nintendo3DSFirm.cpp:369:97: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
const uint32_t first4 = be32_to_cpu(*(reinterpret_cast<const uint32_t*>(firmHeader->signature)));
^
src/libromdata/../librpcpu/byteswap_rp.h:63:52: note: in definition of macro ‘__swab32’
# define __swab32(x) ((uint32_t)__builtin_bswap32(x))
^
src/libromdata/Handheld/Nintendo3DSFirm.cpp:369:27: note: in expansion of macro ‘be32_to_cpu’
const uint32_t first4 = be32_to_cpu(*(reinterpret_cast<const uint32_t*>(firmHeader->signature)));
^
cc1plus: some warnings being treated as errors
Even 12.0 might be too low, but 10.0 is definitely not usable anymore.
CMAKE_SYSTEM_VERSION is set to 10.0 so it will use the latest installed
Windows 10 SDK. This is needed for proper handling of /permissive-
on MSVC 2017 and later.
This makes MSVC a bit more standards-compliant, though it does require
working around some common Win32 idioms. In particular, some const_casts
are needed in order to use constant strings in various structs.
src\win32\RP_XAttrView.cpp(263,31): error C2440: '=': cannot convert from 'const wchar_t [5]' to 'LPWSTR'
src\win32\RP_XAttrView.cpp(263,21): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\RP_XAttrView.cpp(265,32): error C2440: '=': cannot convert from 'const wchar_t [6]' to 'LPWSTR'
src\win32\RP_XAttrView.cpp(265,21): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\RP_ShellPropSheetExt.cpp(890,30): error C2440: '=': cannot convert from 'const wchar_t [1]' to 'LPWSTR'
src\win32\RP_ShellPropSheetExt.cpp(890,24): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\config\AboutTab.cpp(1421,30): error C2440: '=': cannot convert from 'const wchar_t [6]' to 'LPWSTR'
src\win32\config\AboutTab.cpp(1421,19): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
CMake 3.1 added CMAKE_<LANG>_STANDARD, which allows CMake to determine
what flags are needed to select a particular language version.
Set it to C17 and C++17. Previously, we were using C11 and C++11,
though gcc11 switched the default C++ version to C++17. The custom
macro handled this by not adding flags for C++, but it still added
the C11 flag, which prevented use of C17.
No code size differences were observed when compiling on gcc-13.2.0
with CMAKE_<LANG>_STANDARD.
On i386, also add -mstackrealign, since i386 was originally defined to
use 4-byte stack alignment. Most programs *should* be updated to use
16-byte stack alignment nowadays, but just in case something isn't,
this will help to prevent alignment exceptions.
Our minimum supported 64-bit Windows is technically NT 5.2, which
includes both Windows Server 2003 and Windows XP x64 Edition,
so we need to disable thread-safe statics on amd64.
This partially reverts commit 648c9d8915.
([cmake] msvc.cmake: Consolidate "/guard:cf"; clean up some other checks.)
C4477: 'function' : format string 'string' requires an argument of type
'type', but variadic argument number has type 'type'
Roughly equivalent to gcc's -Werror=format-security.
Windows 2000 support only partially worked in rp-config. The actual
shell extension component didn't work.
Support for ANSI Windows was mostly just compile-tested, and it was
becoming a burden to support something almost no one will use.
Removed most `#ifdef UNICODE` and `#ifdef _UNICODE` branches in
favor of using the Unicode path exclusively.
Removed oldwincompat, which was an attempt to get things working
properly on Windows 9x and 2000 with the MSVC 2010 runtime.
rom-properties doesn't compile with MSVC 2010 anymore, so this
probably wasn't very useful anyways.
Consolidated tstring macros in tcharx.h.
Removed GUID_fns.c since it was only useful in ANSI builds.
The "/guard:cf" linker check was using the wrong variable, so it didn't
work correctly. Handle it in the compiler flag check instead.
Simplify the checks for "/CETCOMPAT".
Use two underscores in the "/Zc:throwingNew" check for consistency with
the other checks.
Only disable thread-safe statics on i386 for Windows XP compatibility.
We're supporting a minimum of Windows Vista for amd64, and for ARM CPUs,
the minimum is Windows 10.
svrplus is Windows only, so instead of gettext, we can use
string tables in resource files.
NOTE: Due to differences between MSVC's rc.exe and binutils's windres,
strings in the string table must be NULL-terminated.
- rc.exe ignores the explicit NULL-termination, but has an option "/n"
that automatically adds NULL terminators to strings.
- windres.exe doesn't have an equivalent to "/n", but it *does* allow
adding '\0' to strings to NULL-terminate them.
Updated msvc.cmake to add "/n" to CMAKE_RC_FLAGS.
NOTE: The '\0's in VS_VERSION_INFO doesn't actually do anything in the
resulting resource section with either rc.exe or windres.exe, so get
rid of them. I think this was mentioned on The Old New Thing before.
Renamed resource.rc to res-en_US.rc and added en_US identifiers.
Compiled executable size doesn't change much.
MSVC 2019, 32-bit release build: (both LTO and no-LTO are identical)
svrplus.exe: 114176 -> 114176 (no change)
text data bss dec hex filename
109644 2048 0 111692 1b44c svrplus.exe [before]
110650 2048 0 112698 1b83a svrplus.exe [after]
+1006 0 0 +1006 +3ee Difference
The gcc-5 workaround stopped working properly due to some recent change.
I can't be bothered to figure out what broke it, and gcc-5 is rather old.
Just don't allow enabling LTO at all.
Removed the GCC_5xx_LTO_ISSUES workarounds in all CMakeLists files.
Reworked the -lgcc workaround to check the compiler version instead
of GCC_5xx_LTO_ISSUES.
