This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

libstdc++ debug mode ideas 1


These messages are being forwarded to the public list for archiving and
comment with the full permission of all participants.

-benjamin

Begin forwarded message:

Date: Wed, 30 Jul 2003 09:56:36 -0700
From: Doug Gregor <dgregor@apple.com>
To: Benjamin Kosnik <bkoz@redhat.com>
Cc: gdr@integrable-solutions.net
Subject: Re: [libstdc++ PATCH] libstdc++ debug mode (second try)


Benjamin,
	I think I like some of the user-level changes this structure implies, 
but there are some remaining issues.

	Most importantly, this version breaks debug/release compatibility. To 
see what I mean, compile this in debug mode to in_debug.o:

	#include <vector>
	void foo(std::vector<int>& v)
	{
   		v.push_back(17);
	}

Now compile this in release mode to in_release.o:

	#include <vector>
	void bar(std::vector<int>& v)
	{
   		v.push_back(42);
	}

	int main()
	{
   		std::vector<int> v;
   		bar(v);
	}

... and then link the two together. We have an ODR violation, because 
the two std::vector<int>::push_back's are different. The problem is 
that the symbols in both files have the same name:

-bash-2.05b$ nm in_debug.o |grep push_back
000002a8 t __ZN5__std6vectorIiSaIiEE9push_backERKi
00000040 t __ZNSt6vectorIiSaIiEE9push_backERKi

-bash-2.05b$ nm in_release.o |grep push_back
000001e4 t __ZNSt6vectorIiSaIiEE9push_backERKi

Our linker actually doesn't complain, and splits out an object file 
containing both definitions of push_back:

00002b00 t __ZNSt6vectorIiSaIiEE9push_backERKi
00004168 t __ZNSt6vectorIiSaIiEE9push_backERKi

I have *no* idea how this resolution works, but I can guarantee it will 
break on somebody's platform, and it may not break loudly enough for 
them to know what happened. With the link_name version, the debug 
version will be std::_Debug_vector and the release version will be 
std::vector, so there will be no link-time collision. Moreover, if 
in_release.C referred to the function 'foo' in in_debug.C (such that we 
needed to pass a vector between the two), and the two were compiled in 
different modes, the link_name version will fail to link (because 'foo' 
would be mangled differently in each file) instead of silently picking 
up a version that expects a different type of std::vector.

In a talk I gave at Apple about the debug mode, the first bullet 
describing the role of link_name was:

	"It's an ODR violation: get over it."

The fact is that, in the absence of a perfect template aliasing model, 
compiling part of a program in release mode and part of it in debug 
mode is an ODR violation. If you agree with this, then I only see two 
options:
	(1) Find that perfect template aliasing model, or
	(2) Manage the ODR violation so that it can't cause trouble.

	I see that include <debug/vector> makes std::vector the debug version 
even if we are compiling in release mode. This differs from my original 
patch, where <debug/vector> declares a debugging vector as 
__gnu_debug::vector (regardless of whether or not we're in debug mode). 
The benefit of your approach is that users can change one #include to 
get debugging for a certain container type; the disadvantage is that 
you can't change it for a single container instance (since there is no 
separate __gnu_debug::vector to use). The other issue is that

	#include <vector>
	#include <debug/vector>

will fail to compile in release mode, because std::vector will be 
defined twice and __std::vector will not be defined at all. With this 
methodology, the best we could do would be to emit a warning that the 
debug-mode vector will _not_ be used, and then skip the debug version. 
I wonder how many users would leave a stray <debug/vector> somewhere in 
their program and wonder why things are running slowly.
	
	As a minor issue, the definition of _GLIBCXX_NAMESPACE_STD (and _EXT) 
for debug mode will break user code because of the blanket using 
namespace std directive. I think we want:

#define _GLIBCXX_NAMESPACE_STD namespace __std { using namespace std; } 
\
    namespace __std

	Also, _GLIBCXX_NAMESPACE_STD would have to be moved out of c++config 
and into something without include guards (e.g., the late 
debug/support.h). Try a program like this (in release mode):

	#include <vector>
	#include <debug/list>

The release-mode <vector> inclues c++config, which sets 
_GLIBCXX_NAMESPACE_STD correctly for release mode. When <debug/list> is 
included, c++config doesn't get reparsed (because of the include 
guards), so the include of <list> in <debug/list> defines std::list 
(not __std::list), and we get similar errors to the <vector> and 
<debug/vector> example.

	The std -> __std renaming for debug mode is a mixed bag, I think. It 
gets rid of the need for preprocessor-based renaming, e.g.,

	#ifdef _GLIBCXX_DEBUG
	#  define list _Release_list
	#endif

but  we lose a little relative to the link_name version because the 
release-mode containers in debug mode will mangle differently than the 
release-mode containers in release mode. It basically means that there 
will be a bit of code duplication in projects that have some 
release-compiled code and some debug-compiled code. This by itself 
shouldn't break any valid programs, I think. But as I stated above, 
this renaming doesn't get rid of the ODR violation, so it only helps to 
clean up the code a little.

FWIW, I'd be happy to consider implementation ideas (even half-baked 
ones <g>) without full implementations. I've already been through 
several design iterations since I started in June, so even if you just 
send me an outline of the idea I can compare it with the approach I've 
chosen.

	Doug

On Tuesday, July 29, 2003, at 4:25PM, Benjamin Kosnik wrote:
>
> Doug, try today's version:
>
> http://people.redhat.com/~bkoz/
>
> -benjamin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]