The International Simutrans Forum

Development => Bug Reports => Topic started by: itsnotme on July 22, 2016, 09:14:06 PM

Title: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 22, 2016, 09:14:06 PM
As the title says the application crashes when I start it when the simutrans directory is on an xfs file system

I did an strace which shows the following output (relevant lines only)


chdir("/home/andrew/Downloads/simutrans/simutrans/") = 0
open("text/.", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 12
fstat64(12, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getdents(12, 0x92c219c, 32768)          = -1 EOVERFLOW (Value too large for defined data type)
close(12)                               = 0
open("pak/text/.", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 12
fstat64(12, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getdents(12, /* 1 entries */, 32768)    = 16
getdents(12, 0x92c219c, 32768)          = -1 EOVERFLOW (Value too large for defined data type)
close(12)                               = 0
write(3, "FATAL ERROR: simmain::main() - U"..., 354) = 354
write(2, "FATAL ERROR: simmain::main() - U"..., 354FATAL ERROR: simmain::main() - Unable to load any language files
*** PLEASE INSTALL PROPER BASE FILES ***

either run ./get_lang_files.sh

or

download a complete simutrans archive and put the text/ folder here.
Aborting program execution ...

For help with this error or to file a bug report please see the Simutrans forum at
http://forum.simutrans.com
) = 354
nanosleep({0, 50000000}, 0xfff45fcc)    = 0
futex(0x8538754, FUTEX_WAKE_PRIVATE, 2147483647) = 1
recvmsg(4, 0xfff45cb4, 0)               = -1 EAGAIN (Resource temporarily unavailable)
recvmsg(4, 0xfff45cb4, 0)               = -1 EAGAIN (Resource temporarily unavailable)
recvmsg(4, 0xfff45cb4, 0)               = -1 EAGAIN (Resource temporarily unavailable)
_newselect(5, [4], NULL, NULL, {0, 0})  = 0 (Timeout)
rt_sigprocmask(SIG_UNBLOCK, [ABRT], NULL, 8) = 0
tgkill(15371, 15371, SIGABRT)           = 0
--- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=15371, si_uid=1000} ---
+++ killed by SIGABRT (core dumped) +++
Aborted (core dumped)



The following line is from the mount command and the disk sdb is a 4TB drive
/dev/sdb2 on /home type xfs (rw,relatime,attr2,inode64,noquota)

When I move the folder to a btrfs filesystem on a 2TB drive simutrans functions correctly



Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 23, 2016, 12:00:45 AM
From what I remember, Simutrans does nothing which should depend on the file system, unless the file system implementation is buggy (or behaves very unnaturally, such as not supporting directories). I would rather expect that something went wrong when you copied stuff, but you have provided no information as to how you set up Simutrans to begin with, so I have nothing to go on.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 23, 2016, 04:36:35 AM
it is the getdents not handling a 64bit inode reference

  int getdents(unsigned int fd, struct linux_dirent *dirp,
                    unsigned int count);
       int getdents64(unsigned int fd, struct linux_dirent64 *dirp,
                    unsigned int count);

need to use getdents64!
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Dwachs on July 23, 2016, 06:59:35 AM
We do not use these functions directly, the code uses opendir, readdir, and related posix functions.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 23, 2016, 09:27:14 AM
A quick google indicates that XFS is indeed a bit quirky, as I get lots of hits on problems with opendir/readdir on XFS. The problem seems to be that they can't tell the difference between files and directories, as XFS doesn't store that information as easily accessible as the others. However, Simutrans doesn't actually check if the entries are files or directories on POSIX.

The man page for getdents clearly states that getdents is not a function one should use directly, but an internal system call. One should use readdir, which Simutrans does. Although another post I found does point out that opendir/readdir was made when file systems where much simpler, and it's a pain to implement them correctly on modern file systems. No hints of newer alternatives, though. Maybe you have an OS or CRT bug on your hands.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 23, 2016, 08:35:38 PM
the following link gives a good explanation of what is happening and how to code around it.  No distribution issue just XFS default behaviour of not holding info with the inode.  May be time to return to ext4.  At least you all know about this for the future.

http://linux-tips.org/t/readdir-function-on-xfs-filesystem-not-working-properly/115
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 23, 2016, 09:41:08 PM
I saw something similar, but they stated that it only applied to 32-bit processes, not 64-bit processes (even 32-bit processes can possibly be hacked around during compilation), and I have still no idea what you are running.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi on July 23, 2016, 10:04:29 PM
See the relevant code:

lookfor = path + ".";

if (DIR* const dir = opendir(lookfor.c_str())) {
lookfor = (name == "*") ? ext : name + ext;

while (dirent const* const entry = readdir(dir)) {
if(entry->d_name[0]!='.' || (entry->d_name[1]!='.' && entry->d_name[1]!=0)) {
int entry_len = strlen(entry->d_name);
if (strcasecmp(entry->d_name + entry_len - lookfor.size(), lookfor.c_str()) == 0) {
add_entry(path,entry->d_name,prepend_path);
}
}
}
closedir(dir);
}
(void) only_directories;

Nowhere there is any request of file attributes. Only the name. (And the name should be enough for an fopen call ...)

Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 24, 2016, 05:05:47 AM
I believe this is actually a Simutrans error (not a problem with large XFS partitions)...

This error is caused by a lack of Large File Support (LFS). This can occur in 3 situations...
1. Using an out of date glibc. Only 2.1.3 and newer has LFS.
2. Using an out of date Linux kernel. Only 2.4.0 and newer has full LFS.
3. Using the wrong compiler flags/macros. One must explicitly instruct 32bit applications to have LFS. Due to the nature of 64bit applications LFS should be automatic (I cannot confirm this).

To quote the LFS (http://users.suse.com/~aj/linux_lfs.html) page, any of the following will add LFS...
Quote
• Compile your programs with "gcc -D_FILE_OFFSET_BITS=64". This forces all file access calls to use the 64 bit variants. Several types change also, e.g. off_t becomes off64_t. It's therefore important to always use the correct types and to not use e.g. int instead of off_t. For portability with other platforms you should use getconf LFS_CFLAGS which will return -D_FILE_OFFSET_BITS=64 on Linux platforms but might return something else on e.g. Solaris. For linking, you should use the link flags that are reported via getconf LFS_LDFLAGS. On Linux systems, you do not need special link flags.
• Define _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE. With these defines you can use the LFS functions like open64 directly.
• Use the O_LARGEFILE flag with open to operate on large files.
Looking quickly into the Simutrans Makefile I do not see a "-D_FILE_OFFSET_BITS=64" or "getconf LFS_CFLAGS" (recommended) when generating the compiler flag list. This explains why this error is happening as it is not using LFS.

I would strongly recommend adding the flag. With the use of multi-TB hard disks being common in 2016 it seems reasonable that LFS support should be mandatory. This change might break the game for some poorly maintained Linux installs however it is nothing that some updating would not fix.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 24, 2016, 05:25:25 AM
This became an issue for some games on steam (unity) as well so I have a 1TB ext4 SSD drive in for these.  I have just moved simutrans onto there.  BTW as I didnt say I am using opensuse with tumbleweed and have 6x4TB drives (4 as raid) and 2 running as normal.  It is one of the 2 "nrmal" that has my home drive on it with xfs.  Thanks for your efforts. 

Thanks for your efforts.  We are all forewarned for the future.  I will ping a question on the opensuse list and see if anyone has a suggestion and report back.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 24, 2016, 09:05:37 AM
According to http://www.tcm.phy.cam.ac.uk/sw/inodes64.html, _FILE_OFFSET_BITS is somewhat unsafe to use, but does not go into details why. It might be that it is unsafe to just force it upon a program. One thing is that systems which extract the inode number and put it somewhere, must understand that the field must be bigger now. However, I can think of another issue. If Simutrans is always built with this flags, it will fail on systems (at least when running as 32-bit) which are not built with this flag. As such, it is strictly speaking only an option for self-compilers (and those who compile entire distros for others). (Which I guess is the Unix mentality.)

My 64-bit Linux box gives -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 when I run the 32-bit getconf LFS_CFLAGS. The 64-bit version does not, despite dirent.h being identical. The base type on which off_t is based on is probably 64-bit already, but that makes it unclear what actually defining those things in a 64-bit build will lead to.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 24, 2016, 05:50:10 PM
Quote
According to http://www.tcm.phy.cam.ac.uk/sw/inodes64.html, _FILE_OFFSET_BITS is somewhat unsafe to use, but does not go into details why.
It can only be unsafe to use if one is manipulating inode fields directly. I doubt Simutrans should be doing this as it should only really care about file/folders and their names and not how/where in a partition the files nodes are located.

Quote
If Simutrans is always built with this flags, it will fail on systems (at least when running as 32-bit) which are not built with this flag.
Systems should not be built with the flag. The flag is purely for applications and not the kernel. All kernels since 2.4.0 should support all applications built with -D_FILE_OFFSET_BITS=64 independent of them being 32 or 64 bits. Kernels before 2.4.0 might have problems, but honestly one really should not be using them in 2016 for security and feature reasons.

Quote
My 64-bit Linux box gives -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 when I run the 32-bit getconf LFS_CFLAGS. The 64-bit version does not, despite dirent.h being identical. The base type on which off_t is based on is probably 64-bit already, but that makes it unclear what actually defining those things in a 64-bit build will lead to.
Did you test it out? I would imagine it still works.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 24, 2016, 06:20:56 PM
In the heat, I forgot that the defines also route the calls to different functions (unless they don't, in which case I blame the heat again). None the less, an application that defines those things but is deployed on a system that does not support them, will fail.

And this is not just a question of what the kernel supports, but what glibc supports. I assume 64-bit glibc always supports 64-bit inodes, although we are still just guessing as to whether itsnotme is running a 32-bit or 64-bit Simutrans. With a 32-bit glibc, 64-bit inode support may be optional, although I see no way of deactivating it on Gentoo, and Gentoo is all about being able to tweak such things.

Even if glibc always supports it if new enough and the kernel supports it (can't see anything suggesting that this is configurable, strangely enough), turning this/these flag on in our binary builds means there is possibly a new system requirement. Linux 2.4 is however about the same age (two years older?) as Windows XP, which is our minimum requirement for Windows.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 24, 2016, 06:48:29 PM
I am running a 64 bit opensuse system with the simutrans package from the download link so I expect that is 32 bit.  Running file on the simutrans executable gives
simutrans: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 2.6.18, BuildID[sha1]=f28c913ef313f6526fc74290751ea3d8043eca28, stripped

uname -a
Linux host 4.6.4-1-default #1 SMP PREEMPT Mon Jul 11 17:59:12 UTC 2016 (103c936) x86_64 x86_64 x86_64 GNU/Linux
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi on July 24, 2016, 07:38:08 PM
This is getting offtopic, but why should fopen fail on a system using large inodes? I mean this is the task of the stdlib to take care of all that stuff, not the program. The lib could use a translation table, since a 32 bit program cannot open more than one billion file handles anyway in 4 GB ...

Moreover, some virtual servers still run ancient 2.6.x kernels, like the serverlist which also run games. But since a server simutrans is usually self-compiled this should not be an issue.

In the last time Linux gave more headache and becomes more monolithical than Microsoft it seems, the 32/64 comptibility is still an issue on Linux for us.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 24, 2016, 08:43:25 PM
Quote from: prissi on July 24, 2016, 07:38:08 PM
This is getting offtopic, but why should fopen fail on a system using large inodes? I mean this is the task of the stdlib to take care of all that stuff, not the program.

It's not fopen, it's readdir. readdir returns a dirent structure. In the past, someone got the bright idea to expose the inode number in the dirent structure. On 32-bit platforms, this was exposed as a 32-bit field. But now inodes are no longer 32-bit, because disks have grown since then (probably the 70s, or maybe 80s), and the internal mapping fails when the inode number does not fit in 32-bit. Virtually nobody cares about the inode number, but it is part of the API and must either work correctly or fail. Yes, they messed up, but there is no other solution that doesn't involve breaking backwards compatibility.

Quote from: prissi on July 24, 2016, 07:38:08 PM
In the last time Linux gave more headache and becomes more monolithical than Microsoft it seems, the 32/64 comptibility is still an issue on Linux for us.

Unix-es has always been more monolithical than Microsoft. Their philosophy is to configure and build the entire system from sources, whereas Microsoft has opted for stable ABIs. Even the closed source NVidia driver for Linux needs to compile a few source files in order to work together with however the kernel on that particular system works. And all the executables get lumped together by default in one directory, so that it is impossible to uninstall something again without a package manager, even if it doesn't install system components (which is the only reason for putting something inside c:\windows on Windows).
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 24, 2016, 09:36:20 PM
I think this is one of those cases where actual practical testing is needed. Try running builds on various Linux installs with "-D_FILE_OFFSET_BITS=64" and such flags. I would be very useful to know that it at least fixes the issue itsnotme is having. We can then start to look at backwards compatibility and other issues it may bring.

What is certain is that more and more people will be using large storage solutions in the future. Building targeting 64bit maybe is a solution but it also degrades game performance and only works for 64bit OS installs. A general 32bit solution to the problem will still be best.

Quote
readdir returns a dirent structure
Actually it returns a dirent-like structure. It is a different structure with potentially different memory layout.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 24, 2016, 10:10:02 PM
Quote from: DrSuperGood on July 24, 2016, 09:36:20 PM
I think this is one of those cases where actual practical testing is needed. Try running builds on various Linux installs with "-D_FILE_OFFSET_BITS=64" and such flags. I would be very useful to know that it at least fixes the issue itsnotme is having. We can then start to look at backwards compatibility and other issues it may bring.

Unfortunately, I can't really help there. Not only do I hardly play Simutrans currently (and I'm still playtesting my Unicode changes from last year), I haven't done so on Linux for years. Although I have big disks in my Linux boxes, I don't have the right file system on them. However, all 64-bit file systems should have this problem at least if Simutrans' files happens to end up with an inode numbers above 232. I also have only 64-bit Simutrans on Linux, my oldest Linux box being pretty much the one where 64-bit Simutrans for Linux was playtested years ago.

I can do some small scale testing with small test programs doing readdir calls (I have already made one just to see if it failed with any XFS file system on 64-bit OS, which it didn't), but that might not be all that gets affected by these changes.

Quote from: DrSuperGood on July 24, 2016, 09:36:20 PM
Building targeting 64bit maybe is a solution but it also degrades game performance and only works for 64bit OS installs.

There might actually be more 64-bit OS-es without 32-bit support than 32-bit OS-es without large file support amongst our users and potential users. It seems 32-bit compatibility isn't on by default on many (most?) distros. Because why should it, when the distro comes with 64-bit builds of everything included? Moving binaries between systems is not the way of the Linux world. You are supposed to get the sources and ./configure && make && make install, and that should give 64-bit users a 64-bit executable.

Quote from: DrSuperGood on July 24, 2016, 09:36:20 PM
Actually it returns a dirent-like structure. It is a different structure with potentially different memory layout.

Maybe you confuse the Linux syscall (http://linux.die.net/man/2/readdir) with the POSIX function (http://linux.die.net/man/3/readdir)? The POSIX function says it returns a dirent (well, a pointer to one). It is the POSIX function Simutrans calls. I guess the POSIX readdir used to call the readdir syscall, but now apparently uses the getdent syscall instead.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 24, 2016, 11:13:05 PM
Quote
Maybe you confuse the Linux syscall with the POSIX function? The POSIX function says it returns a dirent (well, a pointer to one). It is the POSIX function Simutrans calls. I guess the POSIX readdir used to call the readdir syscall, but now apparently uses the getdent syscall instead.
If you read the note...
Quote
Only the fields d_name and d_ino are specified in POSIX.1-2001. The remaining fields are available on many, but not all systems. Under glibc, programs can check for the availability of the fields not defined in POSIX.1 by testing whether the macros _DIRENT_HAVE_D_NAMLEN, _DIRENT_HAVE_D_RECLEN, _DIRENT_HAVE_D_OFF, or _DIRENT_HAVE_D_TYPE are defined.

Other than Linux, the d_type field is available mainly only on BSD systems. This field makes it possible to avoid the expense of calling lstat(2) if further actions depend on the type of the file. If the _BSD_SOURCE feature test macro is defined, then glibc defines the following macro constants for the value returned in d_type:
In the implementation there are also comments further supporting this. Although one can in theory return a getdent formatted structure, it is not required to. This is because getdent is a Linux kernel call while readdir is an API, the structures used may be the same but are technically different.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 25, 2016, 12:40:23 AM
Quote from: DrSuperGood on July 24, 2016, 11:13:05 PM
Although one can in theory return a getdent formatted structure, it is not required to. This is because getdent is a Linux kernel call while readdir is an API, the structures used may be the same but are technically different.

Now you write getdent-structure. That's something else.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 25, 2016, 02:56:15 AM
Quote
Now you write getdent-structure. That's something else.
These Linux APIs are too confusing...

If compiler flags (recommend approach?) does not work then what about the hard-linked approach? With the _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE definitions the appropriate function versions should be able to be linked directly. I am unsure if this is just with glibc or not so it might affect portability.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 25, 2016, 08:21:11 AM
Quote from: DrSuperGood on July 25, 2016, 02:56:15 AM
If compiler flags (recommend approach?) does not work then what about the hard-linked approach? With the _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE definitions the appropriate function versions should be able to be linked directly. I am unsure if this is just with glibc or not so it might affect portability.

The 64-bit version of readdir seems to be undocumented (except on BlackBerry). But the possible compatibility problem is at runtime, not compile time. Whether we use compiler flags or simply hardcode calls to readdir64, we end up with an executable that references readdir64. That won't work on a system that doesn't have large file support. It's the compiler flags or nothing.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi on July 25, 2016, 08:13:11 PM
I may also not compile on the nightly compile farm, which is a 32 bit system ...
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 25, 2016, 09:20:20 PM
Quote
I may also not compile on the nightly compile farm, which is a 32 bit system ...
It will compile correctly as long as the server has LFS. In fact I doubt the OS needs to even support LFS in order to compile a build supporting LFS. From what I read pretty much all Linux versions (both 32bit and 64bit) support LFS since 2005. One can argue if one really should still be using a Linux kernel older than 2005, especially seeing how upgrading is usually free and recommended from a feature/security point of view.

Both 32bit and 64bit versions of Linux have LFS as the limit was purely system and C API based and not architectural. Turning on LFS for 64bit builds does nothing (they always have LFS). Any performance impact from using LFS should be completely trivial. With LFS support enabled both partitions and file size of a XFS partition are limited to 8 EiB (8*1024^6 bytes) which is beyond anything reasonable for consumer use currently.

I guess the big question is how many Linux players use very old Linux versions? If >95% use kernel versions newer than 2005 then there should be no problem at all with forcing LFS.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 25, 2016, 09:40:30 PM
Quote from: DrSuperGood on July 25, 2016, 09:20:20 PM
In fact I doubt the OS needs to even support LFS in order to compile a build supporting LFS.

Features on Linux are generally configured at compile time. I consider it likely that glibc will not compile any readdir64 if the kernel does not support it. If the linker needs the symbol in the import library, it will likely fail then. But given the age of LFS support, I strongly doubt the build server is missing that feature. Only way to know is to try it out.

Quote from: DrSuperGood on July 25, 2016, 09:20:20 PM
Any performance impact from using LFS should be completely trivial.

It might actually be a performance increase. At least on 64-bit systems, and possibly LFS 32-bit systems as well, there is a compatibility layer that actually does some checks. But yes, trivial, and in a by no means performance critical part of Simutrans.

Quote from: DrSuperGood on July 25, 2016, 09:20:20 PM
I guess the big question is how many Linux players use very old Linux versions? If >95% use kernel versions newer than 2005 then there should be no problem at all with forcing LFS.

And as a last option, they still have the option of getting the source code, disabling LFS-support and building Simutrans themselves. Although that argument works both ways.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 26, 2016, 06:49:17 PM
Just to confirm that I have compiled trunk on 64bit and it all works fine.  However, haven't got the cross compile working yet  to test the LFS as I get lots of these errors during compile

besch/bild_besch.cc: In member function 'bild_besch_t* bild_besch_t::copy_fliphorizontal() const':
besch/bild_besch.cc:416:74: error: exception cleanup for this placement new selects non-placement operator delete [-fpermissive]
  bild_besch_t* target_besch = new(pic.len * sizeof(PIXVAL)) bild_besch_t();

I will build a 32 bit VM compile it and test for you
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 26, 2016, 07:20:00 PM
Quote from: itsnotme on July 26, 2016, 06:49:17 PM
besch/bild_besch.cc: In member function 'bild_besch_t* bild_besch_t::copy_fliphorizontal() const':
besch/bild_besch.cc:416:74: error: exception cleanup for this placement new selects non-placement operator delete [-fpermissive]
  bild_besch_t* target_besch = new(pic.len * sizeof(PIXVAL)) bild_besch_t();

Doesn't look cross-compilation or 32/64 related to me. Probably something to do with how picky/conforming the compiler you use is. I don't get this with gcc 4.9.3 on the newest code.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 26, 2016, 07:30:29 PM
Yeah I have gcc 6 and it throws this only when cross compiling and then if I add -fpermissive it then fails at the g++ compile so need to look at that (lots of undefined refs).  Just think it is easier to make a 32 bit VM and be lazy than work out all the libs for cpp  32bit

OK further investigation is the linking fails and I am not going through the makefile and ensure all the linking uses 32bit.  So VM it has to be.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 26, 2016, 10:54:45 PM
I didn't have much trouble setting up a cross-compiler on Linux, although it has been over a year since I did it. All I had to do was specify the correct CC, CXX and LD in the config file. This was without/before autoconf, though. I doubt that sets things up properly. Naturally, the cross compile environment must of course exist and contain all necessary libraries.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Dwachs on July 27, 2016, 07:27:32 AM
Quote from: itsnotme on July 26, 2016, 06:49:17 PM
besch/bild_besch.cc: In member function 'bild_besch_t* bild_besch_t::copy_fliphorizontal() const':
besch/bild_besch.cc:416:74: error: exception cleanup for this placement new selects non-placement operator delete [-fpermissive]
  bild_besch_t* target_besch = new(pic.len * sizeof(PIXVAL)) bild_besch_t();
Seems like bild_besch_t is (always?) allocated by placement-new, without corresponding cleanup.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 27, 2016, 08:18:10 AM
Quote from: Dwachs on July 27, 2016, 07:27:32 AM
Seems like bild_besch_t is (always?) allocated by placement-new, without corresponding cleanup.

I think it is complaining about that obj_besch_t declares it's own special new operator, taking an extra parameter, but no corresponding delete operator, to be used to free the memory allocated if the constructor throws an exception. From what I can find, this is legal, but means memory won't be deallocated upon constructor failure (a very hypothetical situation for besch classes if I remember right, but that may not be apparent for the compiler). However, the compiler's complaint seems to indicate that the compiler is generating code that deallocates the memory if an exception is thrown.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 27, 2016, 03:55:38 PM
So good news

I have compiled a 32bit version with large file support (switches detailed above) and it runs correctly (limited testing) on the large filesystem (same files/inodes that had issues initially).  So that means it works with either 64bit compile or 32bit with large filesystem support.   ;D 

Compiled on a 32bit VM running opensuse tumbleweed


#On 32bit VM
andrew@linux-m7sp:~/Downloads/simutrans/code/trunk> gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/i586-suse-linux/6/lto-wrapper
Target: i586-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib --libexecdir=/usr/lib --enable-languages=c,c++,objc,fortran,obj-c++,java,ada,go --enable-checking=release --with-gxx-include-dir=/usr/include/c++/6 --enable-ssp --disable-libssp --disable-libvtv --disable-libcc1 --enable-plugin --with-bugurl=http://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --disable-libgcj --with-slibdir=/lib --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --enable-linker-build-id --enable-linux-futex --program-suffix=-6 --without-system-libunwind --with-arch-32=i586 --with-tune=generic --build=i586-suse-linux --host=i586-suse-linux
Thread model: posix
gcc version 6.1.1 20160707 [gcc-6-branch revision 238088] (SUSE Linux)

#On the host 64bit system
andrew@host:~/Downloads/simutrans/simutrans> file /home/andrew/Downloads/simutrans/simutrans/simutrans-largefs-32bit
/home/andrew/Downloads/simutrans/simutrans/simutrans-largefs-32bit: ELF 32-bit LSB executable, Intel 80386, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 3.0.0, BuildID[sha1]=b44a32da47fc14619e1401725782729e7b0c03ff, not stripped





Note that I still had to add the -fpermissive flag to the compiler flags

Grateful for the efforts you have all put in.  I now know why I can't run it from steam and get no errors.  BTW I was a long term player back in the old days before prissi took over the dev and was around for some time until World of Warcraft took over my life (given it up now so starting to play lots of things). 
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: TurfIt on July 27, 2016, 07:30:44 PM
Quote from: itsnotme on July 27, 2016, 03:55:38 PM
initially).  So that means it works with either 64bit compile or 32bit with large filesystem support.   ;D 

How about compiled 32 bit on your system without the large file flags? i.e. Is it possibly just a problem with the OTTD compile farm provided executable. Also, does OTTD work? Especially their generic 32 bit linux binaries.


Quote from: itsnotme on July 27, 2016, 03:55:38 PM
gcc version 6.1.1 20160707 [gcc-6-branch revision 238088] (SUSE Linux)

Note that I still had to add the -fpermissive flag to the compiler flags

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. -fpermissive should be a last resort as it's usually code errors one is bypassing by it.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 27, 2016, 07:59:04 PM
Good catch.  Adding the -ansi flag to the CFLAGS and CCFLAGS and removing the -fpermissive removed all the errors/warnings. 

To answer your second question I compiled and tried a 32bit version without the large filesystem support and this version of the application cannot detect the pak file directories.

Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi on July 27, 2016, 10:37:54 PM
Could you give the flags and defines you used?
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 28, 2016, 01:13:15 AM
Quote from: TurfIt on July 27, 2016, 07:30:44 PM
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.)
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood 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 (http://forum.simutrans.com/index.php?topic=14705.msg145651#msg145651) 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: itsnotme on July 28, 2016, 07:48:21 PM
Quote from: prissi on July 27, 2016, 10:37:54 PM
Could you give the flags and defines you used?

Hi Prissi

Here are the flags that I added to the Makefile

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


       -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=
and for c++

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
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters 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 (http://en.cppreference.com/), 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood on July 28, 2016, 10:42:58 PM
QuoteHowever, 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Dwachs 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?
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 29, 2016, 08:11:44 AM
Quote from: DrSuperGood on July 28, 2016, 10:42:58 PM
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.

Quote from: Dwachs 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?

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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi 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
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: DrSuperGood 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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters on July 31, 2016, 06:54:39 AM
Quote from: DrSuperGood 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.
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.

Quote from: DrSuperGood on July 30, 2016, 09:57:25 PM
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.
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: Ters 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.


void operator delete(void *ptr) {
return ::operator delete(ptr);
}
Title: Re: Crash on start up with linux 64 bit filesystem
Post by: prissi 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.