News:

Simutrans Tools
Know our tools that can help you to create add-ons, install and customize Simutrans.

[SOLVED][RC 11.9001] Linker and simcity.cc error

Started by Vonjo, March 27, 2013, 08:04:59 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Vonjo

I got ld error while compiling RC 11.9001 no-graphics (see attachment). Graphics version doesn't have this problem.
---
I also got this error while compiling with gcc 4.6:

simcity.cc: In member function 'void stadt_t::check_all_private_car_routes()':
simcity.cc:2505:102: error: taking address of temporary [-fpermissive]
simcity.cc:2505:109: error: taking address of temporary [-fpermissive]

-fpermissive flag can be used as temporary solution though.

jamespetts

Vonjo,

thank you for your report. Having looked carefully at the relevant section of the code, taking the address of a temporary here is entirely proper and intended, as the method to which it is passed only uses it internally for the duration of the method, and does not store it for future use (and furthermore, this characteristic is fundamental to the method). I suggest that the -fpermissive flag is always set when compiling for GCC.

Indeed, this appears to be set in config.default:


# Following flags exists
# DOUBLE_GROUNDS: Enables two height tiles
# HALF_HEIGHT: Enables half height tiles (8 pixel instead 16)
# OTTD_LIKE: Enables half height tiles and crossconnected industries; defaul folder pak.ttd/
# DESTINATION_CITYCARS: Citycars can have a destination
# USE_C: no assembler for copying
# BIG_ENDIAN: MUST by set for PPC/Motorola byte order! (old mac, amiga)
# COMMAND_LINE_SERVER: No graphics, use only for network servers. Must also set COLOUR_DEPTH = 0.
FLAGS = -DNDEBUG -fpermissive


Did you perhaps delete by accident the -fpermissive (and perhaps also) -DNDEBUG lines when you added COMMAND_LINE_SERVER to your flags?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Vonjo

#2
Thank you for the explanation. I was just worried that it may cause problems in the future. I'll keep the -fpermissive flag then.
Quote from: jamespetts on March 29, 2013, 01:14:06 PM
Did you perhaps delete by accident the -fpermissive (and perhaps also) -DNDEBUG lines when you added COMMAND_LINE_SERVER to your flags?
I was just lazy. I copied config.default from Simutrans standard. I thought it was just the same. ;D
Why do you need -DNDEBUG while it has been available in Makefile?

---
How about the ld error?

jamespetts

The ld error - I thought that that was the same issue as was solved by the -fpermissive flag? I am a little confused.

The -DNDEBUG line ensures that, for a release build, the DEBUG precompiler directive is not set, which disables assertions and other debugging code that can slow the game and, in some cases, put what is for ordinary play superfluous information in the GUI. I strongly recommend that you use the Simutrans-Experimental specific makefile.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

MCollett

Quote from: jamespetts on March 29, 2013, 01:14:06 PM
Having looked carefully at the relevant section of the code, taking the address of a temporary here is entirely proper and intended

Taking the address of a non-lvalue is undefined behaviour according to the C++ standard.  There is absolutely no guarantee that it will do what you think, or indeed anything even vaguely sensible.  In practice, it will be particularly dangerous as soon as you are compiling with any significant level of optimisation - it can result in one of those nasty bugs that doesn't show up when debugging, but causes random crashes in the released version.

(I almost mentioned this particular problem in the thread I started about compiling with clang, but in the end just cheated and turned the error off.)

Best wishes,
Matthew

jamespetts

Hmm - I didn't think that a temporary variable counts as a non-lvalue? Surely if I write:


int a;
int* b = &a;
some_method(b);


I am taking the address of a temporary? This is a legitimate thing to do, is it not, where we know that some_method() does not store b for later use?

Or is the problem that I put the constructor itself in the method call, rather than a separate variable? Is the following a problem?


class some_class
{
int a;
int b;
some_class::some_class(int x, int y) { a = x; b = y; }
};

void other_class::method()
{
random_method(&some_class new_object(1, 2));
}


Would it solve the problem to replace void other_class::method() with the following?



void other_class::method()
{
some_class new_object(1,2);
random_method(&new_object);
}


Or is it really necessary to use heap allocation like this?


void other_class::method()
{
some_class *new_object;
new_object = new some_class(1,2);
random_method(new_object);
delete new_object;
}
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

MCollett

Quote from: jamespetts on March 29, 2013, 09:31:15 PM
Surely if I write:


int a;
int* b = &a;
some_method(b);


I am taking the address of a temporary?
No, a is not a temporary.  Even just some_method(&a) should be OK.

QuoteOr is the problem that I put the constructor itself in the method call, rather than a separate variable?
Yes, exactly.  If there is not a separate variable, the compiler is free to put the temporary into a register, or to optimise its existence away altogether.  In either case trying to take its address causes problems.

QuoteWould it solve the problem to replace void other_class::method() with the following?



void other_class::method()
{
some_class new_object(1,2);
random_method(&new_object);
}

That should do.  It forces the compiler to allocate real memory on the stack.  (Or more precisely, to behave as if it does: a sufficiently smart global optimiser is still free to eliminate the variable, but it is then the compiler's responsibility to avoid nasal demons, not yours.)

Best wishes,
Matthew

Vonjo

#7
Quote from: jamespetts on March 29, 2013, 02:46:02 PM
The ld error - I thought that that was the same issue as was solved by the -fpermissive flag? I am a little confused.
No, LD error is another unrelated error. It only happens with no-graphic version (BACKEND = posix, COLOUR_DEPTH = 0, OSTYPE = linux). I'm using Simutrans experimental Makefile. It still happens with 11.9002.

jamespetts

Ahh - you need to add the flag "COMMAND_LINE_SERVER" when you set COLOUR_DEPTH=0. Try it with that and see how you get on.

Matthew: thank you: that is very helpful.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Vonjo

Quote from: jamespetts on March 30, 2013, 11:31:04 AM
Ahh - you need to add the flag "COMMAND_LINE_SERVER" when you set COLOUR_DEPTH=0. Try it with that and see how you get on.
Oh, I didn't notice that. Thank you. I'll try it. :)

isidoro

If there is such a dependency, it can be enforced in the Makefile, can't it?

Something like: if COLOUR_DEPTH is defined and is 0, then define COMMAND_LINE_SERVER

jamespetts

Hmm, that's not a bad idea. I don't know makefile syntax well enough to code this - can you suggest appropriate syntax for this?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Vonjo

Quote from: jamespetts on March 30, 2013, 11:31:04 AM
Ahh - you need to add the flag "COMMAND_LINE_SERVER" when you set COLOUR_DEPTH=0. Try it with that and see how you get on.
It works now. Thanks. :)

isidoro

Quote from: jamespetts on March 31, 2013, 01:09:18 AM
Hmm, that's not a bad idea. I don't know makefile syntax well enough to code this - can you suggest appropriate syntax for this?

How is COMMAND_LINE_SERVER defined in the config file?  Is it a stand alone variable (COMMAND_LINE_SERVER=0) or is it a flag added to the FLAGS variable (-DCOMMAND_LINE_SERVER)?

Vonjo

#14
Quote from: jamespetts on March 31, 2013, 01:09:18 AM
Hmm, that's not a bad idea. I don't know makefile syntax well enough to code this - can you suggest appropriate syntax for this?
This works for me:

ifeq ($(COLOUR_DEPTH),0)
  CFLAGS += -DCOMMAND_LINE_SERVER
endif

Put that code anywhere before the SOURCES part.

jamespetts

Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

isidoro

I would check if COMMAND_LINE_SERVER is already defined.  You can even add a warning then:

ifeq ($(COLOUR_DEPTH),0)
  ifeq ($(findstring -DCOMMAND_LINE_SERVER,$(FLAGS)),)
    $(warning If COLOUR_DEPTH is 0, COMMAND_LINE_SERVER must be defined.  Fixing it...)
    FLAGS += -DCOMMAND_LINE_SERVER
  endif
endif


Or, alternatively, produce an error, if you think that it should be solved by the user:

ifeq ($(COLOUR_DEPTH),0)
  ifeq ($(findstring -DCOMMAND_LINE_SERVER,$(FLAGS)),)
    $(error COLOUR_DEPTH is 0 and COMMAND_LINE_SERVER isn't defined.  Aborting...)
  endif
endif


I haven't checked the code...  It can have an error, though...


jamespetts

Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.