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

Resolve various compiler warnings #11534

Merged
merged 24 commits into from Feb 10, 2023

Conversation

Pokechu22
Copy link
Contributor

Many of these warnings are due to variable shadowing from the deglobalization changes. I'm not super happy about this, but I don't see any alternative (the options for -Wshadow aren't fine-grained enough to disable it for lambdas that don't have capturing enabled or for constructor parameters with the same name as members).

The remaining warnings are these:

/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/Common/Crypto/AES.cpp:253:37: warning: ignoring attributes on template argument ‘__m128i’ [-Wignored-attributes]
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/Common/Crypto/SHA1.cpp:169:43: warning: ignoring attributes on template argument ‘__m128i’ [-Wignored-attributes]
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/Common/Crypto/SHA1.cpp:247:24: warning: ignoring attributes on template argument ‘__m128i’ [-Wignored-attributes]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:674:20: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:946:21: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:949:20: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:1002:19: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:1009:19: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/subprojects/speex/resample.c:1019:16: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/cubeb/cubeb/src/cubeb_alsa.c:8: warning: "_DEFAULT_SOURCE" redefined
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp:599:27: warning: writing 4 bytes into a region of size 1 [-Wstringop-overflow=]
/usr/include/x86_64-linux-gnu/bits/stdio2.h:67:10: warning: ‘%s’ directive argument is null [-Wformat-truncation=]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/sm83/decoder.c:418:9: warning: ‘decoder’ may be used uninitialized in this function [-Wmaybe-uninitialized]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gb/memory.c:287:63: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/arm/decoder.c:565:53: warning: operand of ‘?:’ changes signedness from ‘int32_t’ {aka ‘int’} to ‘uint32_t’ {aka ‘unsigned int’} due to unsignedness of other operand [-Wsign-compare]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gba/renderers/software-bg.c:70:12: warning: ‘localY’ may be used uninitialized in this function [-Wmaybe-uninitialized]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gba/renderers/software-bg.c:69:12: warning: ‘localX’ may be used uninitialized in this function [-Wmaybe-uninitialized]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gba/sio/lockstep.c:454:3: warning: enumeration value ‘SIO_UART’ not handled in switch [-Wswitch]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gba/sio/lockstep.c:454:3: warning: enumeration value ‘SIO_GPIO’ not handled in switch [-Wswitch]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/mGBA/mgba/src/gba/sio/lockstep.c:454:3: warning: enumeration value ‘SIO_JOYBUS’ not handled in switch [-Wswitch]
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/VideoCommon/CPUCull.cpp:28: warning: ignoring ‘#pragma STDC FP_CONTRACT’ [-Wunknown-pragmas]
/mnt/c/users/pokechu22/source/repos/dolphin/Source/Core/VideoCommon/PixelShaderGen.cpp:1188:48: warning: narrowing conversion of ‘(unsigned int)stage.pixel_shader_uid_data::<unnamed struct>::tevind’ from ‘unsigned int’ to ‘unsigned int:21’ [-Wnarrowing]
/mnt/c/users/pokechu22/source/repos/dolphin/Externals/expr/include/expr.h:829:13: warning: declaration of ‘char* s’ shadows a parameter [-Wshadow]

Most of these are in external code. The attributes on __m128i ones are something I don't know how to fix (see https://stackoverflow.com/a/49216021 and https://stackoverflow.com/q/12942548; the latter has comments that imply that this shouldn't apply in C++17). The remainder seem to be compiler bugs which I will investigate further: CPUCull has the warning explicitly ignored, but it still shows up; PixelShaderGen doesn't make sense as tevind is a 21-bit bitfield; and the DualShockUDPClient one is complete nonsense as nothing string-related is happening there. I'll report these to the GCC team after I investigate further.

@Pokechu22
Copy link
Contributor Author

The CPUCull one is because the unknown pragma warning comes from the preprocessor (see bug 89038) and is fixed in GCC 13 (see bug 53431), though that has not yet released.

The DualShockUDPClient one is also fixed in GCC 11 (bug 96963) (though it still exists in GCC 10, which is why they haven't marked it as fixed, I think). I happened to be using GCC 10 to test locally.

I created a report for the PixelShaderGen bitfield issue: bug 108670.

@@ -120,13 +120,13 @@ void ESDevice::InitializeEmulationState()
auto& system = Core::System::GetInstance();
auto& core_timing = system.GetCoreTiming();
s_finish_init_event = core_timing.RegisterEvent(
"IOS-ESFinishInit", [](Core::System& system, u64, s64) { GetIOS()->GetES()->FinishInit(); });
"IOS-ESFinishInit", [](Core::System& system_, u64, s64) { GetIOS()->GetES()->FinishInit(); });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one (and the following) don't use the system_ parameter; maybe we should just omit it like the u64/s64 after it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing at some point GetIOS() will be moved to System, so IMO it's still useful to rename it (if we don't come up with an alternative way of dealing with the shadowing warnings).

@@ -922,7 +922,7 @@ void Init()
auto& core_timing = system.GetCoreTiming();

s_event_enqueue =
core_timing.RegisterEvent("IPCEvent", [](Core::System& system, u64 userdata, s64) {
core_timing.RegisterEvent("IPCEvent", [](Core::System& system_, u64 userdata, s64) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same down here.

This code doesn't need to be portable (since the goal is to have a smaller offset for x64 codegen), so if it's not supported there are other problems. Similar code exists in e.g. DSP.cpp.
Otherwise, files that include the header get warning: ‘Discord::s_using_custom_client’ defined but not used.
…warnings

We need to copy padding in most of these cases, and the objects are trivially copyable; however, BitField prevents trivial copy-assignment.
@phire phire merged commit dad7a32 into dolphin-emu:master Feb 10, 2023
14 checks passed
@Zopolis4
Copy link
Contributor

How are you gathering the warnings? (i.e. how did you make that list of the remaining warnings)

@Pokechu22
Copy link
Contributor Author

In WSL (with GCC):

  1. make clean to force the next build to be from scratch.
  2. make 2>&1 | tee build.log: make to build; 2>&1 to redirect stderr (where warnings are put) to stdout; | tee build.log to write stdout to build.log (but also to the console). Note that this is a single-threaded build; you can also do make -j 8 2>&1 | tee build.log to use 8 cores (or whatever), but then the order of the output won't be deterministic.
  3. grep warning build.log to get the list of warnings.

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