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 Aug 10 20:03:58 2011  
 
Category: bootstrapSeverity: 3 - Normal
Priority: 5 - NormalStatus: Wont Fix
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: None
Planned Release: Contains string changes: None

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)

Tue Jun 21 15:08:13 2016, comment #14:

Switched to newer lua already.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Mar 10 17:18:09 2012, 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 AdministratorIn charge of this item.
Sat Mar 10 13:51:26 2012, 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 Feb 7 08:04:02 2012, 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 AdministratorIn charge of this item.
Fri Nov 18 21:35:17 2011, 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 Nov 16 23:04:36 2011, 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 AdministratorIn charge of this item.
Sun Sep 25 16:37:43 2011, 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 Sep 25 16:24:37 2011, 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 Sep 12 18:49:33 2011, 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 Aug 14 10:34:26 2011, comment #5:

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

Michal Mazurek <akfaew>
Sun Aug 14 10:29:17 2011, 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 Aug 13 20:24:47 2011, 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 AdministratorIn charge of this item.
Thu Aug 11 07:13:23 2011, 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 Aug 10 20:30:51 2011, 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 AdministratorIn charge of this item.
Wed Aug 10 20:03:58 2011, 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 AdministratorIn charge of this item.

 

(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.

     

    Error: not logged in

     

     

    Follow 14 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue Jun 21 15:08:13 2016cazfiStatusNeed Info=>Wont Fix
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Fri Mar 30 00:35:08 2012jtnPlanned Release2.3.2, 2.4.0=>
    Fri Nov 18 21:35:17 2011jtnPlanned Release2.3.1, 2.4.0=>2.3.2, 2.4.0
    Sun Sep 25 16:37:43 2011syntronStatusFixed=>Need Info
      Assigned tosyntron=>None
      Open/ClosedClosed=>Open
    Sun Sep 25 16:25:00 2011syntronStatusNone=>Fixed
      Assigned toNone=>syntron
      Open/ClosedOpen=>Closed
    Mon Sep 12 18:49:33 2011syntronAttached File-=>Added 0014-add-lua-api-check-if-debugging.patch, #14078
    Thu Aug 11 07:13:23 2011akfaewAttached File-=>Added fc.diff, #13833
    Wed Aug 10 20:30:51 2011cazfiAttached File-=>Added AlignedRestoreStack_18481.diff, #13831
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup