Global variable vs passed pointer

Started by
24 comments, last by Ectara 14 years ago
I wasn't aware that this is allowed, and definitely not in C89:
si8 buffer[size];


What happens if you malloc this buffer?

Other than that, check that inputs are sane, the problem might be elsewhere.
Advertisement
Allowed, and not C89. C99 to be exact. I'm going to go back and change the dynamic stack allocations to dynamic heap once everything is finished.

Malloc would work fine.

The inputs are sane, the problem is elsewhere, the problem is GCC optimizes out the variable( I rewrote the function to avoid the issue ever so slightly, but to give an idea of how bad it is, imagine it optimized out the pointer `p'). The way I got around it in some of the functions, such as ones that use a spinlock to ensure thread safety, is to declare it volatile; of which I'm pretty sure is C99. But, my question was, does anyone have a method of preventing a variable from being optimized out other than declaring it volatile? I know where the problem lies, just looking for an alternate solution.
Quote:Original post by Ectara

But, my question was, does anyone have a method of preventing a variable from being optimized out other than declaring it volatile? I know where the problem lies, just looking for an alternate solution.


A compiler will never optimize out a variable that is used. If it does, it's a bug in compiler.

There is no alternate solution, since the problem doesn't exist in the first place.

With threads, one thread might only write to a variable, but never read from it. Compiler would assume this variable is redundant and remove it. This cannot happen with single-threaded code.

Another reason why compiler might remove a variable is when its value is passed via registers only. So while a debugger would not see it, its value would still be valid and passed.

So unless it's a compiler bug, there might be an issue with taking an address of a variable that compiler has decided should live inside a register only. As long as address-of operation is legal, compiler will generate valid code. There is nothing to correct for here, where and how variables are stored is up to compiler, and it will never generate invalid code. If it does, it's a bug.
Not necessarily. As seen in the function below, An infinite number of threads could be waiting to write, or read if the function decided to. Either way, it needs to wait it's turn.

void E_errorSetErrorStr(const si8 * message){	si32 tlock = E_globalErrorLock;	while(tlock) /* Compiler might make this always true, and infinite loop, or always false, and cause serious problems. */		tlock = E_globalErrorLock;	E_globalErrorLock = 1;	while(E_globalErrorCount>=E_ERROR_MAX_ERRORS){		E_error *terror = NULL;		if(E_globalErrorCount>2)			terror = E_globalError.next->next;		if(E_globalErrorCount>=2){			strncpy(E_globalError.error,E_globalError.next->error,E_ERROR_BUFLEN);			E_globalError.code = E_globalError.next->code;			free(E_globalError.next);		}		if(E_globalErrorCount==1)			memset(&E_globalError,0,sizeof(E_error));		else			E_globalError.next = terror;		--E_globalErrorCount;	}	E_error *terror = &E_globalError;	if(E_globalErrorCount){		while(terror->next)			terror = terror->next;		terror->next = (E_error*)malloc(sizeof(E_error));		if(E_errorGetAllocError(terror->next,E_TEST_CACHE))return;		terror = terror->next;	}	strncpy(terror->error,message,E_ERROR_BUFLEN);#ifdef E_ERROR_DEBUG	fprintf(stderr,"%s\n",message);#endif	terror->code = 0;	terror->next = NULL;	++E_globalErrorCount;	E_globalErrorLock = 0;	return;}


The solution I had to use was to declare `E_globalErrorLock' as volatile, so it works now. It need not be a bug; with only a single thread, this assumption would be correct.
Maybe GCC is similar to Visual-C in that you need to specify if your program is to run in a multi-threaded environment. Your compiler may be assuming your code is singly-threaded and therefore safe to optimise away that E_globalErrorLock check. With VC there is the -MT flag to tell the compiler to assume globals can be changed by other threads and not to make those sorts of assumptions. Perhaps GCC has a similar flag.

Also, in a multi-threaded environment, this code will cause issues:

	si32 tlock = E_globalErrorLock;	while(tlock)		tlock = E_globalErrorLock;	E_globalErrorLock = 1;


The problem is the thread could be switched away from between reading E_globalErrorLock into tlock and checking the contents of tlock. eg. say E_globalErrorLock is zero when you enter this code. The first line is run loading E_globalErrorLock into tlock, which is zero. Your thread gets switched away to another one that also runs this code. The new thread runs through all three lines, setting E_globalErrorLock to one, and starts to run the code afterwards, then it too is switched away. The original thread runs, sees tlock is zero and sets E_globalErrorLock to one (when it is already one) and runs the same code. You now have two threads running the code you only wanted one thread to use.

You ideally should be using the platform-specific atomic test-and-set functions for things like this. They're guaranteed to operate correctly in a multi-threaded, multi-CPU environment.
Quote:Original post by PlayerX
Maybe GCC is similar to Visual-C in that you need to specify if your program is to run in a multi-threaded environment. Your compiler may be assuming your code is singly-threaded and therefore safe to optimise away that E_globalErrorLock check. With VC there is the -MT flag to tell the compiler to assume globals can be changed by other threads and not to make those sorts of assumptions. Perhaps GCC has a similar flag.

Also, in a multi-threaded environment, this code will cause issues:

	si32 tlock = E_globalErrorLock;	while(tlock)		tlock = E_globalErrorLock;	E_globalErrorLock = 1;


The problem is the thread could be switched away from between reading E_globalErrorLock into tlock and checking the contents of tlock. eg. say E_globalErrorLock is zero when you enter this code. The first line is run loading E_globalErrorLock into tlock, which is zero. Your thread gets switched away to another one that also runs this code. The new thread runs through all three lines, setting E_globalErrorLock to one, and starts to run the code afterwards, then it too is switched away. The original thread runs, sees tlock is zero and sets E_globalErrorLock to one (when it is already one) and runs the same code. You now have two threads running the code you only wanted one thread to use.

You ideally should be using the platform-specific atomic test-and-set functions for things like this. They're guaranteed to operate correctly in a multi-threaded, multi-CPU environment.


Which is very true, and I was trying to avoid using platform specific code. But either way, for any of these, is there a compiler-independent way of doing this without using volatile, or should I be spending my energy on something else right now?
Not really. Your compiler is applying an optimisation on the assumption it's in a single-threaded environment. You need to tell the compiler not to do that, to optimise for a multi-threaded environment. That's always going to be compiler-specific.

'volatile' appears to be in the C89 standard.

EDIT: my mistake, I thought you were asking if there was a compiler-independent way of stopping the optimisation. There's no really platform-independent way of doing multi-threading synchronisation unfortunately.


[Edited by - PlayerX on March 29, 2010 11:49:57 PM]
Change "Optimize out" to "optimize". That's what's really happening. You declare a variable on the stack, compiler does subexpression elimination, constant folding, or whatever, and realizes there is an identity based on some other expression such that it need not even allocate memory for this new variable. It doesn't mean the compiler is doing something wrong, but rather probably your program is.

Edit: Nvm, I read more. It was a threading problem. There's not a platform independent way to invoke mutual exclusion, no.

Volatile isn't even what you want, it only guarantees memory ordering it doesn't guarantee atomicity (except in Visual C++, but even *that* isn't enough to solve the problem). Even with memory ordering (i.e. volatile) *and* atomicity, you can run this:

si32 tlock = E_globalErrorLock;while(tlock)   tlock = E_globalErrorLock;E_globalErrorLock = 1;


And 2 threads could run the while (tlock) test at exactly the same time, both determine it's false, and both "acquire" the lock.

Moral of the story here is, don't try to reinvent synchronization primitives.


BTW, to answer your original question, using a global variable is often faster because you don't have to constantly keep pushing the same argument on the stack over and over again every time you call a function. And if you access it frequently, it will usually stay in the cache. But like everyone else said, it's a micro optimization. Nevertheless, I'd *still* probably use a global variable, because passing around tons of extra arguments tends to get annoying pretty fast.
I'm going to disagree that using globals frequently for this sort of thing is a good idea.

From an architectural perspective, a global variable introduces an implicit dependancy everywhere it is visible. This makes debugging very hard, as there is no real way to make any assumption about the contents of the global at any given time. Global Mutable state is A Bad Thing, in fact any mutable state that exists at a higher level than necessary is a bad thing. The higher up the pyramid it exists, the more egregious it becomes. This also makes refactoring difficult if and when a single global variable no longer suffices.

Its also bad because it lets you ignore certain design problems that would be readily aparent in code that had passed a pointer or (better) a reference around -- eg Chain of Responsibility-type issues.

Passing an extra parameter around isn't a big deal, and saying that its a "hassle" is, to me, just an excuse made up by lazy typists who fancy themselves programmers.

That's not to say that globals are evil, just that they should only be used when the solution calls for it -- If a global really fits the natural solution to a problem, then use one; if not, don't. If you claim to adhere to this principle and find that you're still using global's frequently, then your compass probably needs adjusting. When in doubt, don't.

I can honestly say I've used maybe two globals in anything I've written in the last 4 years, most of which are smallish, but non-trivial programs between 5k and 25k lines of code.

throw table_exception("(? ???)? ? ???");

Quote:Original post by Ravyne
I can honestly say I've used maybe two globals in anything I've written in the last 4 years, most of which are smallish, but non-trivial programs between 5k and 25k lines of code.


Have you used any singletons? Because singletons are essentially global variables with another name.

I'm not necessarily saying you should go throw globals all of your code, but the case of passing around some state to tons and tons of functions is really the poster example of when to use a global variable.

This topic is closed to new replies.

Advertisement