bugFreeciv - Bugs: bug #18481, lua-5.1, ldebug.c compilation...

 
 
Show feedback again

bug #18481: lua-5.1, ldebug.c compilation failure

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Wed 10 Aug 2011 08:03:58 PM UTC  
 
Category: bootstrapSeverity: 3 - Normal
Priority: 5 - NormalStatus: Need Info
Assigned to: NoneOpen/Closed: Open
Release: Operating System: None
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat 10 Mar 2012 05:18:09 PM UTC, comment #13:

> The thread starts here.
>
> Skimming the thread, I don't see an obvious conclusion.


Basically as this is just compiler warning (turned to error only because of -Werror) they don't consider it lua bug, and they don't want to change working bug-free code without good reasons. So no upstream fix to be expected.

I tried to fix this in our own copy, but setup suffering from original warning just ended to other similar warnings once this one was fixed, and I soon gave up. I don't want our copy of lua to fork too far from original to keep updating it with upstream bugfixes and new versions easier.

Maybe we just disable certain warnings while compiling lua?

Marko Lindqvist <cazfi>
Project Administrator
Sat 10 Mar 2012 01:51:26 PM UTC, comment #12:

>> Has anybody been in contact with lua folks?
> I sent email now.

The thread starts here.

Skimming the thread, I don't see an obvious conclusion. I take it updating to Lua 5.1.5 didn't happen to fix this?

(This ticket is currently targeted at 2.3.2.)

Jacob Nevins <jtn>
Project Administrator
Tue 07 Feb 2012 08:04:02 AM UTC, comment #11:

> Has anybody been in contact with lua folks?


I sent email now. Looking that clang report more carefully it seems that this can be real bug on some (rather obscure) conditions, and not just cosmetic build warning. restorestack() gives pointer value with theoretically 7*8, or 3*8 (on 32bit systems), bits in undefined state.

Marko Lindqvist <cazfi>
Project Administrator
Fri 18 Nov 2011 09:35:17 PM UTC, comment #10:

This clearly isn't going to make 2.3.1, but is still desirable to fix on S2_3 for portability.

Jacob Nevins <jtn>
Project Administrator
Wed 16 Nov 2011 11:04:36 PM UTC, comment #9:

> Could you report lua compile problem directly to lua project
> also? Is that problem somehow solved in system lua of your OS
> (assuming it ships with one)?


Has anybody been in contact with lua folks? This really should be fixed in upstream and not only in our own copy (and over and over again every time we update it)

I get similar error with clang on x86_64 Debian Wheezy with clang packet from Sid (as there's not yet one in Wheezy). Full clang error output looks like:

CC ldebug.o
../../../../../src.patched/dependencies/lua-5.1/src/ldebug.c:620:21: error: cast
from 'char ' to 'TValue ' (aka 'struct lua_TValue *') increases required
alignment from 1 to 8 [-Werror,-Wcast-align]
StkId errfunc = restorestack(L, L->errfunc);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../../../../src.patched/dependencies/lua-5.1/src/ldebug.c:21:
../../../../../src.patched/dependencies/lua-5.1/src/ldo.h:25:28: note: instantiated from:
#define restorestack(L,n) ((TValue )((char )L->stack + (n)))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[4]: *** [ldebug.o] Error 1

Marko Lindqvist <cazfi>
Project Administrator
Sun 25 Sep 2011 04:37:43 PM UTC, comment #8:

can't close the ticket as the applied patch does NOT fix the bug - it's only (perhaps) a helper to find the reason

Matthias Pfafferodt <syntron>
Project Member
Sun 25 Sep 2011 04:24:37 PM UTC, SVN revision 20285:

dd lua api check if debugging

define LUA_USE_APICHECK if debug=yes|checks

see gna bug #18481

(Browse SVN revision 20285)

Matthias Pfafferodt <syntron>
Project Member
Mon 12 Sep 2011 06:49:33 PM UTC, comment #6:

The attached patch adds a lua api check if debugging is activated. perhaps this allows us to find the reason for the lua error ...

(file #14078)

Matthias Pfafferodt <syntron>
Project Member
Sun 14 Aug 2011 10:34:26 AM UTC, comment #5:

I'm sorry, I posted to the wrong thread and didn't notice Markos answer in bug #18482.

Michal Mazurek <akfaew>
Sun 14 Aug 2011 10:29:17 AM UTC, comment #4:

Here we go, I believe this is the source of the SIGBUS. 2.2 works fine on sparc64, here is what changed:
2.3:
2.2:

The function secfile_lookup_int_vec() is now broken (it was good in 2.2).

I think this is what is called "strict alignment". n-byte data types must be n-byte aligned. game.info.granary_num_inis is 4-byte aligned, and when cast to a 8-byte data type causes the SIGBUS.

Michal Mazurek <akfaew>
Sat 13 Aug 2011 08:24:47 PM UTC, comment #3:

> You split this bug report in two, however the following is
> relevant to both of them:


> cc1: warnings being treated as errors
> ruleset.c: In function 'load_ruleset_game':
> ruleset.c:3231: warning: cast increases required alignment of
> target type


> 3231 &game.info.granary_num_inis,


How is that related to lua compile error? Seems unrelated, except that you get same compiler warning from both. So I really don't understand how:

> The attached diff makes the warnings go away


How can that patch touching only lua code fix ruleset.c warning in code that has nothing to do with lua?!?

Could you report lua compile problem directly to lua project also? Is that problem somehow solved in system lua of your OS (assuming it ships with one)?

Marko Lindqvist <cazfi>
Project Administrator
Thu 11 Aug 2011 07:13:23 AM UTC, comment #2:

It does not make the warning go away. The attached diff does, however I have no idea what consequences it might have. I don't use LUA, I can't really test it.

You split this bug report in two, however the following is relevant to both of them:

cc1: warnings being treated as errors
ruleset.c: In function 'load_ruleset_game':
ruleset.c:3231: warning: cast increases required alignment of target type

And ruleset.c:3231 is:

3230 food_ini = secfile_lookup_int_vec(file, (size_t *)
3231 &game.info.granary_num_inis,
3232 "civstyle.granary_food_ini");

secfile_lookup_int_vec, where the bus error occurs.

The attached diff makes the warnings go away, but not the problem. I believe the "patched" lua code will produce more bus errors, regardless of the type the pointers are cast to. The solution to this is make a temporary variable, and copy stuff to it. That way it will be aligned. If not that, then add some _alignment_ (8) attribute to that LUA struct.

Here is a relevant document: http://cmynhier.blogspot.com/2008/10/memory-alignment-on-sparc-or-300x.html

(file #13833)

Michal Mazurek <akfaew>
Wed 10 Aug 2011 08:30:51 PM UTC, comment #1:

Can you test if attached patch makes error to go away.

This is definitely not suitable patch for committing (being gcc specific) but it can be used to determine if problem is in restorestack() cast from char * to TValue *.

(file #13831)

Marko Lindqvist <cazfi>
Project Administrator
Wed 10 Aug 2011 08:03:58 PM UTC, original submission:

Reported by Michal Mazurek <akfaew> as part of bug #18468:

cc1: warnings being treated as errors
ldebug.c: In function 'luaG_errormsg':
ldebug.c:620: warning: cast increases required alignment of target type
gmake[4]: *** [ldebug.o] Error 1
gmake[4]: Leaving directory `/home/longturn/src/freeciv-2.3.0/dependencies/lua-5.1/src'
gmake[3]: *** [all-recursive] Error 1
gmake[3]: Leaving directory `/home/longturn/src/freeciv-2.3.0/dependencies/lua-5.1'
gmake[2]: *** [all-recursive] Error 1
gmake[2]: Leaving directory `/home/longturn/src/freeciv-2.3.0/dependencies'
gmake[1]: *** [all-recursive] Error 1
gmake[1]: Leaving directory `/home/longturn/src/freeciv-2.3.0'
gmake: *** [all] Error 2

Marko Lindqvist <cazfi>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #13833:  fc.diff added by akfaew (1kB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by syntron (Updated the item)
  • -unavailable- added by akfaew (Updated the item)
  • -unavailable- added by cazfi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 30 Mar 2012 12:35:08 AM UTCjtnPlanned Release2.3.2, 2.4.0=>
    Fri 18 Nov 2011 09:35:17 PM UTCjtnPlanned Release2.3.1, 2.4.0=>2.3.2, 2.4.0
    Sun 25 Sep 2011 04:37:43 PM UTCsyntronStatusFixed=>Need Info
      Assigned tosyntron=>None
      Open/ClosedClosed=>Open
    Sun 25 Sep 2011 04:25:00 PM UTCsyntronStatusNone=>Fixed
      Assigned toNone=>syntron
      Open/ClosedOpen=>Closed
    Mon 12 Sep 2011 06:49:33 PM UTCsyntronAttached File-=>Added 0014-add-lua-api-check-if-debugging.patch, #14078
    Thu 11 Aug 2011 07:13:23 AM UTCakfaewAttached File-=>Added fc.diff, #13833
    Wed 10 Aug 2011 08:30:51 PM UTCcazfiAttached File-=>Added AlignedRestoreStack_18481.diff, #13831
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup