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

Compiler warnings #5350

Closed
sevu opened this issue Dec 9, 2020 · 11 comments · Fixed by #5451
Closed

Compiler warnings #5350

sevu opened this issue Dec 9, 2020 · 11 comments · Fixed by #5451
Labels
Building Build-time issues. SCons Issues involving the SCons build system. Upstream issue Issues that are or involve issues with libraries, tools, or platforms Wesnoth uses.

Comments

@sevu
Copy link
Member

sevu commented Dec 9, 2020

I guess I'm the only one who gets them … (1.15)

In function 'createstrobj',
inlined from 'luaS_createlngstrobj' at src/lua/lstring.cpp:148:29,
inlined from 'luaS_newlstr' at src/lua/lstring.cpp:206:30:
src/lua/lstring.cpp:142:17: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
142 |   getstr(ts)[l] = '\0';  /* ending 0 */
|                 ^
src/lua/lstring.cpp: In function 'luaS_newlstr':
src/lua/lstate.h:213: note: at offset 0 to object 'ts' with size 24 declared here
213 |   struct TString ts;
|
In function 'createstrobj',
inlined from 'luaS_createlngstrobj' at src/lua/lstring.cpp:148:29,
inlined from 'luaS_newlstr' at src/lua/lstring.cpp:206:30,
inlined from 'luaX_newstring' at src/lua/llex.cpp:130:29:
src/lua/lstring.cpp:142:17: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
142 |   getstr(ts)[l] = '\0';  /* ending 0 */
|                 ^
src/lua/llex.cpp: In function 'luaX_newstring':
src/lua/lstate.h:213: note: at offset 0 to object 'ts' with size 24 declared here
213 |   struct TString ts;
|
In function 'createstrobj',
inlined from 'luaS_createlngstrobj' at src/lua/lstring.cpp:148:29,
inlined from 'luaS_newlstr' at src/lua/lstring.cpp:206:30,
inlined from 'pushstr' at src/lua/lobject.cpp:391:3,
inlined from 'luaO_pushvfstring' at src/lua/lobject.cpp:405:12:
src/lua/lstring.cpp:142:17: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
142 |   getstr(ts)[l] = '\0';  /* ending 0 */
|                 ^
src/lua/lobject.cpp: In function 'luaO_pushvfstring':
src/lua/lstate.h:213: note: at offset 0 to object 'ts' with size 24 declared here
213 |   struct TString ts;
|
@sevu sevu added the Building Build-time issues. label Dec 9, 2020
@Pentarctagon
Copy link
Member

How are you building wesnoth? A fix for scons at least was added in d643edc

@irydacea
Copy link
Member

irydacea commented Dec 10, 2020

It would help a lot to know more about the particular compiler version and platform as well.

@Pentarctagon Pentarctagon added the Needs more info Issues deemed to contain insufficient information for reproducing or fixing. label Dec 11, 2020
@sevu sevu removed the Needs more info Issues deemed to contain insufficient information for reproducing or fixing. label Dec 11, 2020
@sevu
Copy link
Member Author

sevu commented Dec 11, 2020

I built from a branch without this commit yet. But I tested again with today's master (63b16ad), and it's the same, with the above remaining unchanged.

As compiler I used gcc 10.2.
build system was scons,
build host was Archlinux,
library versions … I think are not relevant, as it's about out packaged Lua and the compiler.

Fun fact: GCC doesn't print inlined from … lines when using the German localisation.

@Pentarctagon
Copy link
Member

I guess this will have to wait until GCC 10 is more widely available. I can't test this myself yet, and the commit linked above seems to have fixed this for loonycyborg.

@soliton-
Copy link
Member

Are you saying you get stringop-overflow warnings even though it is turned off?

@Pentarctagon Pentarctagon added the Upstream issue Issues that are or involve issues with libraries, tools, or platforms Wesnoth uses. label Dec 14, 2020
@sevu sevu added the SCons Issues involving the SCons build system. label Dec 15, 2020
@sevu
Copy link
Member Author

sevu commented Dec 15, 2020

It happens during linking, scons doesn't pass -Wno-stringop-overflow to the linker.

When executing the linking command myself with the option it's turned off.

@soliton-
Copy link
Member

Can you show the command? Really seems weird that option would do anything when linking.

@sevu
Copy link
Member Author

sevu commented Dec 15, 2020

g++ -o wesnoth -pthread -fstack-protector-strong -fPIE -pie -Wl,-z,relro,-z,now -O3 -march=native -flto=4 -fuse-ld=gold -Wl,-O1,--sort-common,--as-needed -Wl,--whole-archive build/release/libwesnoth-widgets.a -Wl,--no-whole-archive build/release/wesnoth.o build/release/libwesnoth-client.a build/release/liblua.a build/release/libwesnoth-core.a build/release/libwesnoth-game.a build/release/libwesnoth-sdl.a build/release/libwesnoth-client.a -lboost_regex -lboost_filesystem -lboost_locale -lboost_thread -lboost_system -lboost_random -lboost_program_options -lboost_iostreams -lm -lpthread -licudata -licui18n -licuuc -lcrypto -lSDL2 -lSDL2_ttf -lSDL2_mixer -lSDL2_image -lvorbisfile -lpangocairo-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0-lharfbuzz -lcairo -lfontconfig -lfreetype -lX11 -ldbus-1 -lfribidi -lhistory

I specified a few custom options:
extra_flags_config = '-pipe -Wno-stringop-overflow'
extra_flags_release = '-fno-plt -Wl,-O1,--sort-common,--as-needed'
enable_lto = True
arch = 'native'

I see in the command the -Wl,-O1,--sort-common,--as-needed was taken from extra_flags_release, but -fno-plt not, so scons filters them out somewhere.

@Pentarctagon
Copy link
Member

Does it happen if you build without LTO enabled?

@sevu
Copy link
Member Author

sevu commented Dec 16, 2020

No, only with LTO.

@Pentarctagon
Copy link
Member

I'm not sure if it's something really fixable by us then, since it feels like it would be incorrect to add that warning suppression to the entire linking step when we only want it to apply to particular source files.

sevu added a commit that referenced this issue Jan 17, 2021
This adds the change from d543edc (in scons) and 941433e (in cmake) to the linking step as well, but only when using LTO

closes  #5350
sevu added a commit that referenced this issue Jan 17, 2021
This adds the change from d643edc (in scons) and 941433e (in cmake) to the linking step as well, but only when using LTO

closes  #5350
@sevu sevu linked a pull request Jan 17, 2021 that will close this issue
Pentarctagon pushed a commit that referenced this issue Jan 18, 2021
This adds the change from d643edc (in scons) and 941433e (in cmake) to the linking step as well, but only when using LTO

closes  #5350
sevu added a commit that referenced this issue Jan 18, 2021
This adds the change from d643edc (in scons) and 941433e (in cmake) to the linking step as well, but only when using LTO

closes  #5350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Building Build-time issues. SCons Issues involving the SCons build system. Upstream issue Issues that are or involve issues with libraries, tools, or platforms Wesnoth uses.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants