On an example of global inline functions in C++

Question

Considering the following setup, I have ran into a quite strange phenomena, which I can not really explain. Using Visual Studio 2005, the following piece of code results in crash. I would like to really know the reason.

playground.cpp

static int local=-1;    
#include "common.h"

int main(int arg)
{

  setit();     
  docastorUpdate();

  return 0;
}

common.h

#include <stdio.h>
#include <iostream>

void docastorUpdate();

static int *gemini;

inline void setit()
{
  gemini = &local;
}

castor.cpp

static int local = 2;

#include "common.h"

void docastorUpdate() {
  setit();

  // crashing here, dereferencing a null pointer
  std::cout << "castor:" << *gemini << std::endl; 
}

The thing is, that the crash disappears when

I move the inline function setit() to an unnamed namespace
I make it static

To put it in a nutshell, I would need help to understand the reasons. Any suggestion is appreciated! (I am aware that, this solution is not one of the best partices, just being curious.)

static in C++ is a bad idea for global variables (rather use extern): http://stackoverflow.com/questions/15235526/the-static-keyword-and-its-various-uses-in-c — Necrolis, Nov 15 '13 at 12:21
Without asking "what are you trying to do", I would say - get rid of your compiler. It should print `2` and it does print `2` with `gcc`. `gemini` in `castor.cpp` points to `local` in `castor`. That's what `docastorUpdate()` should be using. — lapk, Nov 15 '13 at 12:47
@Petr Just because you can get it to compile and run doesn't mean the code is valid. In particular, UB yields unpredictable results and both `static`s and pointers are vulnerable to UB bugs in your code. So getting it to run in GCC doesn't really prove anything. — Lightness Races in Orbit, Nov 15 '13 at 13:24
I'm curious, does compiling in release/optimized versus debug/unoptimized make a difference? FYI in VS2012 it produces what you might expect in both: 2 — mark, Nov 15 '13 at 13:44
@LightnessRacesinOrbit Yup, I was wrong. I'll leave the original wrong comment so your reply will not be addressing emptyness ;). — lapk, Nov 15 '13 at 13:44

R. Martinho Fernandes · Answer 1 · 2013-11-15T13:45:05.293

This breaks because you are violating the one-definition rule. The one-definition rule says that in a program, across all translation units, there is only one definition of any given function. inline is sort of an exception to this rule, and it more or less means "dear compiler, there will be several definitions of this function, but they will all be the same, I promise".

static, when used here for local, means "dear compiler, this is an internal detail that only this translation unit will ever see; please do not confuse it with variables named local from other translation units"

So you promised the compiler that all definitions of setit will be the same, and asked the compiler to give each translation unit its very own local variable.

However, since the setit function uses whatever variable named local is in scope, the end result is two different definitions of setit, each one using a different variable. You just broke your promise. The compiler trusted you and the result was a totally messed up program. It thought it could do certain things with the code based on your promises, but since you broke them behind its back, those things it tried to do with the code didn't work at all.

I had to flip a coin to decide which reply should I accept as answer, sorry. But still, I was really enjoyed reading your explanation! — Klaymen, Nov 15 '13 at 14:32

score 5 · Accepted Answer · answered Nov 15 '13 at 13:37

Your code invokes undefined behaviour, in a very subtle way.

[C++11: 7.1.2/4]: An inline function shall be defined in every translation unit in which it is ODR-used and shall have exactly the same definition in every case. [..]

Although the definition looks the same because it's lexically copy-pasted into each translation unit by your #include, it's not because &local isn't taking the address of the same variable, in each case.

As a result, anything can happen when you run your program, including transferring all your hard-earned savings into my bank account, or taking away all of Jon Skeet's rep.

This is why making the function non-inline solves the problem; also, putting it in an unnamed namespace makes it a different function in each translation unit.

score 0 · Answer 3 · answered Nov 15 '13 at 12:23

0

you can avoid using static in all the places, you can use extern in your case:

common.h:

extern int *gemini;

common.cpp;

int *gemini = nullptr;

also avoid using local like that , instead you can do this:

inline void setit(int * p)
{
    gemini = p;
}

void docastorUpdate()
{
    static int local = 2;
    setit(&local);
    std::cout << "castor:" << *gemini << std::endl;
}

answered Nov 15 '13 at 12:23

Raxvan

6,257
2
25
46

1

Yep, I'm aware - as I mentioned in the question - that this solution is not appropriate. Personally I would not choose that solution too. But I would like to understand what is the reason of the crash. Anyway, thank you for the comment! – Klaymen Nov 15 '13 at 12:37

On an example of global inline functions in C++

3 Answers3