GNOME Bugzilla – Bug 94513
weird problem with typecodes from a typelib
Last modified: 2004-12-22 21:47:04 UTC
I have been running into a weird problem with the typecode information found in the "Everything" imodule. The "IDL:orbit/test/StrSeq:1.0" typecode is supposed to be an alias for the string<sequence> type. However, tc->subtypes[0] does not point at a sequence typecode. Attached is a sample program that demonstrates this. On my system, the output is: ** Message: created ORB ** Message: loaded typelib ** Message: tc->repo_id == IDL:orbit/test/StrSeq:1.0 ** Message: (tc->kind == tk_alias) == 1 ** Message: tc->subtypes[0]->repo_id == (null) ** Message: tc->subtypes[0]->kind == 0 This is using the latest version from the gnome-2-0 branch (as of yesterday).
Created attachment 11312 [details] test program
Took a look in the debugger: (gdb) print tc $1 = 0x4005b2e0 (gdb) print tc->subtypes $2 = (CORBA_TypeCode *) 0x4005b2c0 (gdb) print tc->subtypes[0] $3 = 0x4005df20 (gdb) print &TC_CORBA_sequence_CORBA_string_struct $4 = (struct CORBA_TypeCode_struct *) 0x8049dd0 So, it looks like tc->subtypes[0] is pointing to a memory region around the dlopened imodule library, rather than where libORBit is loaded ...
Reasons for observed behaviour: everything-imodule.c defines ORBIT_IDL_C_IMODULE, which is designed to make all typecode definitions in the -common.c file static, and the definitions in the .h header static rather than extern. The intended behaviour is to compile all typecodes into the typelib. Bad things happen because corba-defs.h from libORBit is included after defining ORBIT_IDL_C_IMODULE. This means that a prototype for things like TC_CORBA_sequence_CORBA_string_struct is defined static, but the definition is missing, as corba-defs.c is in libORBit, so gets skipped. End result is that a static symbol for TC_CORBA_sequence_CORBA_string_struct goes into the uninitialised data section of Everything_module.so! Possible solution: everything-imodule.c currently defines both "ORBIT_IDL_C_IMODULE" and "everything_IMODLE" (which is probably meant to "everything_IMODULE"). Change it to only define "everything_IMODULE". In everything.h, after including orbit headers, put: #ifdef everything_IMODULE # define ORBIT_IDL_C_IMODULE #endif This should delay the defining the variable long enough so that prototypes for typecodes defined in libORBit2 get defined extern, while the local typecodes get defined static. With the above change made to everything-imodule.c and everything.h, the external typecodes all match up. => reassigning to IDL compiler component
The Bonobo imodule also has similar problems: it has TC_CORBA_sequence_CORBA_octet and TC_CORBA_sequence_CORBA_string in its uninitialised data section.
Created attachment 11348 [details] [review] proposed fix
The above patch changes the behaviour of the IDL compiler slightly. Now the struct definitions in the header will only be defined static if both ORBIT_IDL_C_IMODULE and filename_IMODULE are defined, which sounds like the desired semantics.
Patch looks good James. I've had a quick look and I can't see any reason for ORBIT_IDL_C_IMODULE at all now. If you can't see any reason either then feel free to remove it in favour of just having filename_IMODULE. Well hunted down. Please commit ... and thanks muchly.
As far as I can see, the only other use of the ORBIT_IDL_C_IMODULE define is in the -common.c file where it sets various structures to static. These should be fine using the filename_IMODULE define. Do you think it is okay using these non-prefixed defines like this? I doubt people are going to use things like "corba_defs_IMODULE" in their programs, but maybe it would be better to have an ORBIT prefix on them? "ORBIT_IDL_C_IMODULE_filename" maybe? (in case we have non-C imodules at some point ...)
Good point. ORBIT_IDL_C_IMODULE_filename sounds fine ...
Created attachment 11351 [details] [review] updated patch
Above patch checked in on HEAD branch. Is this sort of thing appropriate for gnome-2-0 branch?
Looks good. Yes, it should go on gnome-2-0 as well. Cheers.
If we're doing that - we should also back-port the recent arg length / type fixes etc. since without them the typelib stuff is pretty useless. James ... ?
I have just checked in the above patch to the gnome-2-0 branch of ORBit2 (the generated typelib looks correct w.r.t. "nm" output). Getting bug 93727 fixed on the gnome-2-0 branch would be nice. I would prefer not to wait til GNOME 2.2 before people can use pyorbit.