Author Topic: Crash on start up with linux 64 bit filesystem  (Read 4615 times)

0 Members and 1 Guest are viewing this topic.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #35 on: July 28, 2016, 01:13:15 AM »
I believe GGC6 now uses C++14 instead of 98 by default. One of the standard updates in there mangled the placement operator functionality, so try setting it back to using 98.

Nice, once they finally bumped the default C++ standard, they bumped it to one that breaks Simutrans. I guess I will have to keep adding --std arguments in the foreseeable future. (So far, I've only gone up to C++11. Man, there is a lot of new stuff in that. I haven't really noticed C++14. Maybe the changes are so small that they are easy to miss in the documentation I've using. C++17 changes on the other hand, are more visible.)

Offline DrSuperGood

Re: Crash on start up with linux 64 bit filesystem
« Reply #36 on: July 28, 2016, 05:41:57 AM »
Quote
Nice, once they finally bumped the default C++ standard, they bumped it to one that breaks Simutrans. I guess I will have to keep adding --std arguments in the foreseeable future. (So far, I've only gone up to C++11. Man, there is a lot of new stuff in that. I haven't really noticed C++14. Maybe the changes are so small that they are easy to miss in the documentation I've using. C++17 changes on the other hand, are more visible.)
I reported this with MSVC2015 last year. Simutrans uses destructor parameters in at least 1 location, this is done so as to allow a dynamic sized array to directly proceed the class/structure in a rather hacky way. The declaration of the parameter used conflicts with the "sized destructor" functionality in modern C++11 standard. The specific declaration is now reserved for compiler use (cannot be explicitly invoked) so that sized destructors can be called in favour of standard destructors if the type size is fully known at compile time. The advantage to sized destructors is that it can potentially remove the need to resolve what size a type is at runtime allowing for the destructor algorithms to operate more efficiently.

No code based fix could be agreed upon, especially because Simutrans is forced to be compatible with very old versions of the C++ standard. For a possible fix I suggested using a non-basic type to pass the extra allocation size (a type which will not be interpreted as a size) and potentially implementing sized destructor to take advantage of possible speed gains when possible.

The current work around for MSVC2015 is to use a special hidden compiler flag which disables sized destructor functionality.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #37 on: July 28, 2016, 08:27:46 AM »
Are you sure you mean C++11? The only change in operator new or operator delete that I can find (except some thread safety requirements and noexcept) is in C++14.

This is also not about Simutrans using any destructor parameter, but rather placement new parameters (can't find any official name for this). However, it is entirely optional in C++14, according to the documentation I have, to have operator delete with parameters matching operating new or having an explicit size parameter. C++17 even allows the compiler to use the no-args (except the pointer) operator delete even if there exists an operator delete taking a size argument. The only oddity is that C++ apparently will not fall back to any global operator delete if there is a class specific operator new.

Offline DrSuperGood

Re: Crash on start up with linux 64 bit filesystem
« Reply #38 on: July 28, 2016, 05:43:48 PM »
Quote
Are you sure you mean C++11? The only change in operator new or operator delete that I can find (except some thread safety requirements and noexcept) is in C++14.
Finding documentation for the feature with a rough google search is not easy. My primary source of knowledge about it came from proposed ISO revisions for C++11. It is possible the feature was delayed until C++14 or later.

Quote
This is also not about Simutrans using any destructor parameter, but rather placement new parameters (can't find any official name for this). However, it is entirely optional in C++14, according to the documentation I have, to have operator delete with parameters matching operating new or having an explicit size parameter. C++17 even allows the compiler to use the no-args (except the pointer) operator delete even if there exists an operator delete taking a size argument. The only oddity is that C++ apparently will not fall back to any global operator delete if there is a class specific operator new.
The problem is that sized decalocator conflicts with the placement allocator/dealocator used. I forget exactly how, here is the original topic. You even commented on it at the time. It seems MS removed the issue from the list so possibly it has been patched.

Offline itsnotme

Re: Crash on start up with linux 64 bit filesystem
« Reply #39 on: July 28, 2016, 07:48:21 PM »
Could you give the flags and defines you used?

Hi Prissi

Here are the flags that I added to the Makefile
Code: [Select]
CFLAGS   +=   -ansi -Wall -W -Wcast-qual -Wpointer-arith -Wcast-align  -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 $(FLAGS)
CCFLAGS  +=   -ansi -Wstrict-prototypes
LDFLAGS  +=   

here is a snippet of the manpage for my version of gcc
Code: [Select]

       -ansi
           In C mode, this is equivalent to -std=c90. In C++ mode, it is equivalent to -std=c++98.

           This turns off certain features of GCC that are incompatible with ISO C90 (when compiling C code), or of standard C++ (when compiling C++ code),
           such as the "asm" and "typeof" keywords, and predefined macros such as "unix" and "vax" that identify the type of system you are using.  It also
           enables the undesirable and rarely used ISO trigraph feature.  For the C compiler, it disables recognition of C++ style // comments as well as the
           "inline" keyword.

           The alternate keywords "__asm__", "__extension__", "__inline__" and "__typeof__" continue to work despite -ansi.  You would not want to use them in
           an ISO C program, of course, but it is useful to put them in header files that might be included in compilations done with -ansi.  Alternate
           predefined macros such as "__unix__" and "__vax__" are also available, with or without -ansi.

           The -ansi option does not cause non-ISO programs to be rejected gratuitously.  For that, -Wpedantic is required in addition to -ansi.

           The macro "__STRICT_ANSI__" is predefined when the -ansi option is used.  Some header files may notice this macro and refrain from declaring certain
           functions or defining certain macros that the ISO standard doesn't call for; this is to avoid interfering with any programs that might use these
           names for other things.

           Functions that are normally built in but do not have semantics defined by ISO C (such as "alloca" and "ffs") are not built-in functions when -ansi
           is used.


There are a whole host of standards that can be chosen instead of -ansi using -std=
  • c90
  • c99
  • c11
  • gnu90
  • gnu99
  • gnu11
and for c++
  • c++98 or c++03
  • gnu++98 or gnu++03
  • c++11
  • gnu++11
  • c++14
  • gnu++14
  • c++1z
  • gnu++1z

The bolded items are what is selected using -ansi.  I do not know if that is the best option seeing as there are so many language variants.

Edit: forgot to say that my compiler defaults to gnu++14 and gnu11

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #40 on: July 28, 2016, 08:14:49 PM »
There are so many different cases with replacement and fallback rules when it comes to operator delete that I get all dizzy trying to work them all out. However, based on my current favorite C++ reference, it seems clear that when placement new is used for a class that has a placement new operator declared, which is the case for obj_besch_t and by extension all it's subclasses, then it will not call any delete operator at all, if none are declared in the class as well, which is also the case. As such, it should not be possible to confuse the sized delete operator with a placement delete operator that happens to take a size_t as its only parameter, because neither are available in the first place.

However, having only the new operator, but not the delete operator, might be so rare, that this is a situation where compiler bugs might live undetected longer than normal.

Offline DrSuperGood

Re: Crash on start up with linux 64 bit filesystem
« Reply #41 on: July 28, 2016, 10:42:58 PM »
Quote
However, having only the new operator, but not the delete operator, might be so rare, that this is a situation where compiler bugs might live undetected longer than normal.
Some of the documentation points towards the requirement of a placement dealocator for every placement allocator that is defined. This dealocator is called if the placement allocator fails, automatically by the compiler. It is only explicitly called anywhere else, with special syntax. This is where MSVC2015 failed to compile because the placement allocator used for the proceeding array needed a placement dealocator which matched (was too similar to) the declaration of the sized dealocator.

It is possible this requirement has been dropped after much complaint as the item has been removed from their list. I have not tried building Simutrans in nearly a year due to slow development.

At the time a solution as simple as adding a dummy argument to the dealocator could have worked.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4161
  • Total likes: 144
  • Helpful: 147
  • Languages: EN, DE, AT
Re: Crash on start up with linux 64 bit filesystem
« Reply #42 on: July 29, 2016, 05:44:47 AM »
Not to interrupt your discussion, but is there any suggestion on how to make the code standard conforming?
Parsley, sage, rosemary, and maggikraut.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #43 on: July 29, 2016, 08:11:44 AM »
Some of the documentation points towards the requirement of a placement dealocator for every placement allocator that is defined. This dealocator is called if the placement allocator fails, automatically by the compiler. It is only explicitly called anywhere else, with special syntax. This is where MSVC2015 failed to compile because the placement allocator used for the proceeding array needed a placement dealocator which matched (was too similar to) the declaration of the sized dealocator.

In the C++11 draft (the final versions must be bought), I fond the following quote regarding exception handling during initialization or the allocated object(s): "If the object was allocated in a new-expression, the matching deallocation function (3.7.4.2, 5.3.4, 12.5), if any, is called to free the storage occupied by the object." Note the "if any". I can't find anything stating that they must come in pairs, but it is hard to search for if you don't know the terminology they would use to describe it. Some examples with class specific allocators or deallocators also feature only one of them. The C++14 draft contains a similar quote. There it is followed by a note that not having one is for when no memory was allocated (dynamically I assume) by the matching allocation function. If it was, you will leak memory if the constructor fails. (The final version of the standard costs money, while the older ones are withdrawn.)

Simutrans does allocate memory, but these classes don't have constructors that can fail (that I am aware of at least), so there is no case where these matching deallocation functions would be called anyway. Delete operators will call only the single-arg or sized deallocation function (operator delete) irrespective of how new was called (in general, it can't know), but we do not delete besch classes either that I am aware of. If we do, I think it will use the global deallocation functions in the absence of class-specific ones.

Not to interrupt your discussion, but is there any suggestion on how to make the code standard conforming?

As far as I can tell, this particular part of the code is. One might try to declare a single-argument (just the pointer) deallocation function (operator delete) in obj_besch_t to see if that unconfuses the compiler.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8698
  • Total likes: 300
  • Helpful: 228
  • Languages: De,EN,JP
Re: Crash on start up with linux 64 bit filesystem
« Reply #44 on: July 29, 2016, 11:29:11 PM »
Defining -ansi may do the trick with gcc at least, as listed above. At least it does not break GCC 4.x compiles

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #45 on: July 30, 2016, 08:57:22 AM »
Since -ansi is equivalent to -std=c++98 (when compiling C++), might it be better, for those human reading the switches, to use the latter, more explicit one? -ansi makes more sense for C, since there actually was an ANSI C standard, while C++ has been ISO only.

Offline DrSuperGood

Re: Crash on start up with linux 64 bit filesystem
« Reply #46 on: July 30, 2016, 09:57:25 PM »
Instead of forcing old standards, it would be better if the code was fixed up for newer standards, while still meeting the minimum standard requirement. This would make the code more future proof I would imagine.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #47 on: July 31, 2016, 06:54:39 AM »
Instead of forcing old standards, it would be better if the code was fixed up for newer standards, while still meeting the minimum standard requirement.
If only it was easy to figure what the standard demands. I still can't find any definitive words on whether a class-specific allocation function will use a matching global deallocation function if there are no class-specific deallocation functions. If the behavior is undefined, that is usually stated.

This would make the code more future proof I would imagine.
Apparently not in this case, though. I have also seen another case as well, where signatures of standard-defined functions changed between standards. And I think there is a case of a keyword (auto) being reused for something completely different, which I can imagine causing some troubles, but I think it was very rarely used. So it seems that C++ standards are not wholly backwards compatible, at least when overriding or otherwise implementing things covered by the standard (which is not discouraged by the standard). For pure usage, they seem to give you until the next standard to adapt.

If only C++ had a means of passing around instances of classes where the template parameter is unknown, one could have just made the data size a template parameter and avoid the entire placement new/delete fuzz.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4577
  • Total likes: 165
  • Helpful: 106
  • Languages: EN, NO
Re: Crash on start up with linux 64 bit filesystem
« Reply #48 on: August 05, 2016, 07:15:48 PM »
I just got GCC 6 with MSYS2 and the error related to operator delete. The quickest solution does indeed appear to be adding a declaration of operator delete in obj_besch_t.

Code: [Select]
void operator delete(void *ptr) {
return ::operator delete(ptr);
}

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8698
  • Total likes: 300
  • Helpful: 228
  • Languages: De,EN,JP
Re: Crash on start up with linux 64 bit filesystem
« Reply #49 on: August 05, 2016, 10:02:01 PM »
If this solves this, then we should add it indeed. I am away from my usual computer, so either make a patch or someone else please change the code in question.