clang llvm 2.9 compiler widelands crash

Bug #744595 reported by David Allwicher on 2011-03-28
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
High
Unassigned

Bug Description

I compiled widelands using clang 2.9. The good part is, the game compiles, the game starts up. I am able to build a building. When the first ware arrives to the construction site... Segmentation fault.

#0 0x0000000006d06601 in ?? ()
#1 0x0000000006cde340 in ?? ()
#2 0x0000000000a0cd90 in ?? ()
#3 0x0000000006d06570 in ?? ()
#4 0x00007fffffff8af0 in ?? ()
#5 0x00007fffffff8b68 in ?? ()
#6 0x00007fffffff8b68 in ?? ()
#7 0x000000000090c1cd in Ware_Index (this=0x7fffffff9dd8, other=...) at /home/aber/export/src/logic/widelands.h:137
#8 0x0000000000b9b90e in transfer_finish (this=0x6d06690, game=..., t=...) at /home/aber/export/src/economy/request.cc:527
#9 0x0000000000ba177a in has_finished (this=0x6cf2df0) at /home/aber/export/src/economy/transfer.cc:215
#10 0x0000000000b783ce in update (this=0x6cf2ae0, game=...) at /home/aber/export/src/economy/ware_instance.cc:336
#11 0x0000000000a643c4 in deliver_to_building (this=0x63a40a0, game=..., state=...) at /home/aber/export/src/logic/carrier.cc:224
#12 0x0000000000a63f83 in transport_update (this=0x63a40a0, game=..., state=...) at /home/aber/export/src/logic/carrier.cc:167
#13 0x0000000000b445d9 in do_act (this=0x63a40a0, game=...) at /home/aber/export/src/logic/bob.cc:224
#14 0x0000000000b44473 in act (this=0x63a40a0, game=..., data=25) at /home/aber/export/src/logic/bob.cc:208
#15 0x0000000000a1271a in execute (this=0x6d9aa70, game=...) at /home/aber/export/src/logic/instances.cc:107
#16 0x0000000000b503c9 in run_queue (this=0x7fffffffa028, interval=108, game_time_var=@0x7fffffff9e38) at /home/aber/export/src/logic/cmd_queue.cc:119
#17 0x0000000000a2fc67 in think (this=0x7fffffff9dd8) at /home/aber/export/src/logic/game.cc:619
#18 0x000000000096de56 in think (this=0x1985580) at /home/aber/export/src/wui/interactive_base.cc:333
#19 0x00000000009ae970 in think (this=0x1985580) at /home/aber/export/src/wui/interactive_player.cc:253
#20 0x00000000009dec45 in do_think (this=0x1985580) at /home/aber/export/src/ui_basic/panel.cc:560
#21 0x00000000009de637 in run (this=0x1985580) at /home/aber/export/src/ui_basic/panel.cc:173
#22 0x0000000000a2dc96 in run (this=0x7fffffff9dd8, loader_ui=0x7fffffff9d68, start_game_type=Widelands::Game::NewNonScenario)
    at /home/aber/export/src/logic/game.cc:573
#23 0x00000000007abcf5 in new_game (this=0x1157b80) at /home/aber/export/src/wlapplication.cc:2022
#24 0x00000000007aa5a4 in mainmenu_singleplayer (this=0x1157b80) at /home/aber/export/src/wlapplication.cc:1594
#25 0x00000000007a36a5 in mainmenu (this=0x1157b80) at /home/aber/export/src/wlapplication.cc:1516
#26 0x00000000007a27db in run (this=0x1157b80) at /home/aber/export/src/wlapplication.cc:441
#27 0x000000000079d6b6 in main (argc=1, argv=0x7fffffffe758) at /home/aber/export/src/main.cc:48

Related branches

SirVer (sirver) wrote :

I expect more problems, especially in the Lua part (though jari's cool fixups might have solved all issues already). We should look into this, I feel CLANG is the future of C compiling.

Changed in widelands:
status: New → Confirmed
importance: Undecided → High
milestone: none → build17-rc1
tags: added: crash
Hans Joachim Desserud (hjd) wrote :

I initially thought this had been posted in the forum, so my comment is probably a bit more off-topic than it could have been.

Anyways, I recently discovered that clang* has a static code analyzer built-in, which we might want to take advantage of. For those unfamiliar with static code analyzers, they will basically go through the source code and look for error patterns. This allows developers to notice and fix issues before they become serious bugs. I haven't tried it on widelands myself, but I think someone should at least take a look to see whether it reports any issues which should be fixed.

Some links I've been reading lately for more background information:
http://grep.be/blog/en/computer/code/static_analysis_with_clang (example of issue found by static code analysis)
http://altdevblogaday.com/2011/12/24/static-code-analysis/ (Carmack's thoughts on static code analysis. One highlight I especially liked was "This seems to imply that if you have a large enough codebase, any class of error that is syntactically legal probably exists there.")

*It also looks like Visual Studio includes one, so we might want to look at that for Windows as well.

Nicolai Hähnle (nha) wrote :

Running static code analysis is a good idea, as are other forms of more testing. However, two things must be kept in mind:

1. Those things slow down builds, so I am against adding them to the default cmake run.

2. If they are not added to the default build, then in the long run they will almost never be done.

As a consequence, the proper way to implement this is as an automated build process on a server somewhere, that sends sufficiently annoying failure messages when somebody introduces a regression. It should probably also automatically send a summary report every two or four weeks, to remind everybody of long-standing problems.

Hans Joachim Desserud (hjd) wrote :

Nicolai: I agree on both points.

It reminded me of the PPA with daily builds we have for Ubuntu [1], which I guess works as a continuous integration system building each revision. Since I don't think we should interfere with (read break :p) the existing one (which anyways builds using gcc), I guess we should attempt to set up an alternative one running clang/llvm with static code analysis. I have no idea how complicated this would be though.

The problem is of course whether anyone will look at these reports or not. Depending on the amount of issues, I think this might require some initial work in order to eliminate existing issues to keep new ones from drowning in the noise. We might want to consider creating a general bug report for errors found by the static analyser, attach new logs every couple of weeks (and comment notable changes).

[1]https://launchpad.net/~widelands-dev/+archive/widelands-daily

Nicolai Hähnle (nha) wrote :

As for whether anyone will look at the reports: That's definitely an important issue.

I've seen the PPA, and I've seen that it occasionally generates some automatic e-mails. The problem with those e-mails is that they're impossible to understand without some knowledge of how the Launchpad PPA system works in the back-end. I don't have that knowledge, and so far I've just ignored those e-mail reports because of that. So to me, the take away of that is that those reports should be sufficiently self-explanatory to anyone who is able to contribute in the first place, otherwise they end up being useless and ignored.

Still, I think with a little bit of thought, it should be possible to set up a system that generates useful reports.

Jens Beyer (qcumber-some) wrote :

Just as a side note regarding static code analysis: we already have integrated a static code checker (CppCheck), and as nha writes, it slows down the build, so it is not active on default but can be activated with a specific cmake parameter (-DWL_EXTRAWARNINGS=on) along with a few altered compiler warning flags. But, also as nha writes, it is being forgotten/ignored long time...

By the way, the cppcheck people often change their command line parameters so the current settings done by widelands are not working with the current version of cppcheck. We could however enable *all* warnings, which creates lots of noise.

Regarding the topic making widelands clang compatible: +1 as it is most probably the future. However, I don't know if clang 2.9 is worth the effort? I'm not deep enough into that topic, but clang 3.0 is completely different (if I can believe what people write about it), so to be future safe, concentrating on clang 3.0 should be enough, I guess.

Hans Joachim Desserud (hjd) wrote :
Download full text (3.2 KiB)

Nicolai: Take this with a pinch of salt, but I think the PPA process is something like this:
- pull in and install dependencies
- pull in latest WL source code
- compile WL
- clean up and remove packages
which naturally adds a couple of unrelated things to the build log.

Jens: ah thanks, I wasn't aware that it activated cppCheck. (And you also explained why it doesn't seem to do anything currently. )
I am not sure if all warnings should be enable by default, I would at least wait a while until the amount of noise goes down. (See bug 825957 for GCC-warnings, and bug 913369 for clang warnings.) I suspect some of the issues trigger warnings would also trigger in static code analysis so fixing warnings might reduce noise here also.

And now, a bit on-topic for once:
I am not sure what happens when wares reach a building, because a builder arrives first when I try. (Didn't that cause a problem for you, David?) Anyways, this is what happens when a builder reaches a constructions site when compiled with clang 3.0:
Program received signal SIGSEGV, Segmentation fault.
0x8b08ec83 in ?? ()
(gdb) bt
#0 0x8b08ec83 in ?? ()
#1 0xb7b6451c in ?? () from /usr/lib/libgcc_s.so.1
#2 0xb7b64a28 in _Unwind_Resume () from /usr/lib/libgcc_s.so.1
#3 0x0865f996 in Widelands::Worker::buildingwork_update (this=0xbfb0ef0, game=..., state=...) at /opt/widelands/src/logic/worker.cc:1727
#4 0x08614233 in Widelands::Bob::do_act (this=0xbfb0ef0, game=...) at /opt/widelands/src/logic/bob.cc:226
#5 0x086140d8 in Widelands::Bob::act (this=0xbfb0ef0, game=..., data=18) at /opt/widelands/src/logic/bob.cc:210
#6 0x086847b2 in Widelands::Cmd_Act::execute (this=0x10673900, game=...) at /opt/widelands/src/logic/instances.cc:107
#7 0x086fc909 in Widelands::Cmd_Queue::run_queue (this=0xbfffd0b4, interval=1230, game_time_var=@0xbfffcf58) at /opt/widelands/src/logic/cmd_queue.cc:133
#8 0x086b8781 in Widelands::Game::think (this=0xbfffcf28) at /opt/widelands/src/logic/game.cc:628
#9 0x0889a07d in Interactive_Base::think (this=0x9377e48) at /opt/widelands/src/wui/interactive_base.cc:333
#10 0x0886aa45 in Interactive_Player::think (this=0x9377e48) at /opt/widelands/src/wui/interactive_player.cc:256
#11 0x087d7652 in UI::Panel::do_think (this=0x9377e48) at /opt/widelands/src/ui_basic/panel.cc:570
#12 0x087d6f9f in UI::Panel::run (this=0x9377e48) at /opt/widelands/src/ui_basic/panel.cc:174
#13 0x086b67f4 in Widelands::Game::run (this=0xbfffcf28, loader_ui=0xbfffbd08, start_game_type=Widelands::Game::NewSPScenario) at /opt/widelands/src/logic/game.cc:582
#14 0x086b5808 in Widelands::Game::run_splayer_scenario_direct (this=0xbfffcf28, mapname=0x8fbe374 "campaigns/t01.wmf") at /opt/widelands/src/logic/game.cc:272
#15 0x084b137c in WLApplication::campaign_game (this=0x8cde3b8) at /opt/widelands/src/wlapplication.cc:2112
#16 0x084af196 in WLApplication::mainmenu_singleplayer (this=0x8cde3b8) at /opt/widelands/src/wlapplication.cc:1596
#17 0x084a9a4c in WLApplication::mainmenu (this=0x8cde3b8) at /opt/widelands/src/wlapplication.cc:1510
#18 0x084a8c02 in WLApplication::run (this=0x8cde3b8) at /opt/widelands/src/wlapplication.cc:441
#19 0x084a3d04 in main ...

Read more...

SirVer (sirver) wrote :

How critical is this? I vote for postponing to build 18.

David Allwicher (aber) wrote :

What's the unit of measurement? You can use clang to build gcc, if you need to...

Changed in widelands:
milestone: build17-rc1 → build-18rc1
SirVer (sirver) wrote :

You are a true philosopher, David.

Hans Joachim Desserud (hjd) wrote :

Fwiw, FreeBSD has plans to use llvm rather than gcc for version 10. While the overlap between FreeBSD users and WL players might be small, it is packaged there...

I also stumbled across someone rebuilding all of Debian's archive using clang [1]. At the moment, this seems to be a "for fun/because I can" project though.

So I think this should be one of the highlights for build 18. We have the stacktraces, so someone should hopefully be able to look into it with a debugger and see where the difference lies...

[1] http://sylvestre.ledru.info/blog/sylvestre/2012/02/29/rebuild_of_the_debian_archive_with_clang

Jens Beyer (qcumber-some) wrote :

Since Revision 6319 cmake issues a warning if the compiler is identified as Clang.

The warning text is:

WARNING!!! You seem to compile widelands using the clang/llvm compiler infrastructure. Please note that we do not yet support compiling with clang. You are very likely to run into problems. We are working on it, though. In the mean time, please use GCC infrastructure to compile widelands. You can use the CMake parameter -DCMAKE_CXX_COMPILER=g++ to force GCC if you have it installed. Thank you for your understanding.

Changed in widelands:
status: Confirmed → Fix Committed
Jens Beyer (qcumber-some) wrote :

Removed the clang warning as of bzr6380 as we don't see any major issues with clang anymore.

SirVer (sirver) wrote :

Released in build-18 rc1.

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers