D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6461 - multiple definitions with typeid and multiobj
Summary: multiple definitions with typeid and multiobj
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 critical
Assignee: No Owner
URL:
Keywords:
: 9044 (view as issue list)
Depends on:
Blocks:
 
Reported: 2011-08-09 15:43 UTC by Martin Nowak
Modified: 2015-06-09 05:10 UTC (History)
3 users (show)

See Also:


Attachments
Testcase (1.19 KB, application/octet-stream)
2013-06-09 23:23 UTC, Denis Shelomovskii
Details

Note You need to log in before you can comment on or make changes to this issue.
Description Martin Nowak 2011-08-09 15:43:49 UTC
--- tmpl.d:
module tmpl;
struct Tmpl(T) {
  T a;
}

---- a.d:
module a;
import tmpl;
TypeInfo fun() { return typeid(Tmpl!int()); }

--- b.d:
module b;
import tmpl;
TypeInfo fun() { return typeid(Tmpl!long()); }

--- main.d:
import a, b;
void main() {
  auto t1 = a.fun();
  auto t2 = b.fun();
  assert(t1 != t2);
}

reproduce:
dmd -lib -ofliba.a a
dmd -lib -oflibb.a b
dmd main -L-L. -L-la -L-lb

This bug is caused by Type::getTypeInfo causing a call to obj_append while already being in a deferred genobjfile. The appended symbol is not marked as doppelganger module thus writes out ModuleAssert, ModuleArray et al. Now two different libraries can end up with colliding definitions if count in obj_write_deferred is accidentally the same.
Comment 1 Martin Nowak 2011-08-09 18:55:28 UTC
This bug is not caused by the late obj_append but is a direct consequence of the special handling for Dsymbols that don't have a module in obj_write_deferred.
It seems for the D case that TypeInfos are the only DSymbols that don't have a module.
Comment 2 Martin Nowak 2013-05-30 08:02:26 UTC
*** Issue 9044 has been marked as a duplicate of this issue. ***
Comment 3 Denis Shelomovskii 2013-05-30 11:13:21 UTC
Change the title please as if Issue 9044 is dup of this, there is no need to generate multiple object files and the issue can be triggered in dmd itself, not in the linker. Also there is no need for `typeid` (at least in user code) as shown in Rainer's example.
Comment 4 Rainer Schuetze 2013-05-30 11:53:13 UTC
> Also there is no need for `typeid` (at least in user code)
as shown in Rainer's example.

The "new" implicitely accesses typeid, so I think Martin is correct in marking the issues as duplicates.
Comment 5 Martin Nowak 2013-05-30 11:54:37 UTC
(In reply to comment #3)
> Change the title please as if Issue 9044 is dup of this, there is no need to
> generate multiple object files

It's multiobj not multiple objects and in fact you need at least two archives.

> and the issue can be triggered in dmd itself, not in the linker.

True

> Also there is no need for `typeid` (at least in user code)
> as shown in Rainer's example.

It happens when TypeInfo instances are emitted, because they are not put into a
doppelgänger module, i.e. the object contains definitions for assert,
unittest_fail, ModuleInfo.

For these TypeInfo instances s->getModule() returns NULL.
https://github.com/D-Programming-Language/dmd/blob/20655f957f3729298b79e6c695b9d7840ac5ef0f/src/glue.c#L116
Comment 6 Rainer Schuetze 2013-05-30 12:14:03 UTC
Generating a UUID instead of the simple static counter in obj_write_deferred seems to fix the problem.
Comment 7 Denis Shelomovskii 2013-05-30 23:28:04 UTC
(In reply to comment #5)
> (In reply to comment #3)
> > Change the title please as if Issue 9044 is dup of this, there is no need to
> > generate multiple object files
> 
> It's multiobj not multiple objects and in fact you need at least two archives.

Sorry my ignorance, but I sill not sure I understand the terminology. The library produced by `dmd -lib a.b b.d` is multiobj and contains two archives? I thought the library is an archive...
Comment 8 Rainer Schuetze 2013-05-30 23:58:45 UTC
"multiobj" is a term used in the compiler source when a module is split into multiple object files (one for each function) to allow linking only referenced functions. It is enabled when you bulid with "-lib". 

So "dmd -lib a.d" produces a library with 4 objects. If you then run "dmd -lib b.d a.lib", dmd does the same for b.d and then merges the other lib a.lib into b.lib. Doing so it detects the multiple definitions.
Comment 9 Rainer Schuetze 2013-05-31 00:06:58 UTC
(In reply to comment #6)
> Generating a UUID instead of the simple static counter in obj_write_deferred
> seems to fix the problem.

Probably the better fix might be to not generate pseudo-ModuleInfo at all. In addition, without a module the TypeInfo symbol is merged into the previous (possibly unrelated) object file.
Comment 10 Martin Nowak 2013-05-31 05:17:57 UTC
(In reply to comment #9)
> (In reply to comment #6)
> > Generating a UUID instead of the simple static counter in obj_write_deferred
> > seems to fix the problem.
> 
> Probably the better fix might be to not generate pseudo-ModuleInfo at all. In
> addition, without a module the TypeInfo symbol is merged into the previous
> (possibly unrelated) object file.

Yeah UUIDs might work but we should instead remove the ugly hack that causes the multiple definitions, i.e. "mname = lastmname;".

IIRC TypeInfo instances were the only symbols where s->getModule() returns NULL.
But a TypeInfo instance should belong to the module of the described Type.
I hope that we can remove the hack if we fix that behavior and assert s->getModule.
Comment 11 Rainer Schuetze 2013-05-31 07:29:21 UTC
(In reply to comment #10)
> IIRC TypeInfo instances were the only symbols where s->getModule() returns
> NULL.

I had the impression that the init-data has the same problem, but I could also have been confused by other issues (the multiobj library creation doesn't work at all for omf atm).

> But a TypeInfo instance should belong to the module of the described Type.
> I hope that we can remove the hack if we fix that behavior and assert
> s->getModule.

The problem also exists for derived types like const(Struct) or Struct* (maybe also for basic types?). You cannot generate all possible types into the module itself (and you might not have it compiled anyway if the declaration is only in a di file). E.g. for const(Struct) you'll have to at least generate the TypeInfo_Const that refers to TypeInfo_Struct somewhere into the current compiler output.
Comment 12 Martin Nowak 2013-05-31 09:19:19 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > IIRC TypeInfo instances were the only symbols where s->getModule() returns
> > NULL.
> 
> I had the impression that the init-data has the same problem, but I could also
> have been confused by other issues (the multiobj library creation doesn't work
> at all for omf atm).
> 
> > But a TypeInfo instance should belong to the module of the described Type.
> > I hope that we can remove the hack if we fix that behavior and assert
> > s->getModule.
> 
> The problem also exists for derived types like const(Struct) or Struct* (maybe
> also for basic types?). You cannot generate all possible types into the module
> itself (and you might not have it compiled anyway if the declaration is only in
> a di file). E.g. for const(Struct) you'll have to at least generate the
> TypeInfo_Const that refers to TypeInfo_Struct somewhere into the current
> compiler output.

True, but wouldn't it still make sense to emit these into a doppelgänger module of the original module? One problem is, that this required linking against the original module, because it references assert etc.

As this only seems to happen with data we don't actually need the module functions, so we could omit them.
Also emitting them to a doppelgänger of a special anonymous module would make more sense than to reuse the last filename.
Comment 13 Denis Shelomovskii 2013-06-04 11:15:14 UTC
Partial workaround:
One can try to detrigger the issue by minimizing module count i.e. for `mylib.*` library which forces you projects to fail building create `mylib_all` module with copied contents of all library files and replace `import mylib.` with `import mylib_all; //mylib.` for easy replacing back once the issue is fixed.
Comment 15 Denis Shelomovskii 2013-06-09 23:23:08 UTC
Created attachment 1221 [details]
Testcase
Comment 17 Denis Shelomovskii 2013-06-27 00:10:37 UTC
As merged dmd pull #2201 worked for me really good, mark as resolved.