Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add external import path switch #20899

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog/dmd.external-import-path.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
External import path switch

A new switch is added, the external import path (``-extI``).
It is similar to the import path switch (``-I``) except it indicates that a module found from it is external to the currently compiling binary.

It is used on Windows when the ``dllimport`` override switch is set to anything other than ``none`` to force an external module's symbols as DllImport.

If a build system supports the external import path switch, it is recommend to **not** use the ``all`` option for the ``dllimport`` override switch which applies to symbols that should not be getting DllImport'd.
10 changes: 7 additions & 3 deletions compiler/src/dmd/cli.d
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,12 @@ dmd -cov -unittest myprog.d
(only imports).`,
),
Option("dllimport=<value>",
"Windows only: select symbols to dllimport (none/defaultLibsOnly/all)",
`Which global variables to dllimport implicitly if not defined in a root module
"Windows only: select symbols to dllimport (none/defaultLibsOnly/externalOnly/all)",
`Which symbols to dllimport implicitly if not defined in a module that is being compiled
$(UL
$(LI $(I none): None)
$(LI $(I defaultLibsOnly): Only druntime/Phobos symbols)
$(LI $(I defaultLibsOnly): Only druntime/Phobos symbols and any from a module that is marked as external to binary)
$(LI $(I externalOnly): Only symbols found from a module that is marked as external to binary)
$(LI $(I all): All)
)`,
),
Expand Down Expand Up @@ -460,6 +461,9 @@ dmd -cov -unittest myprog.d
Option("I=<directory>",
"look for imports also in directory"
),
Option("extI=<directory>",
"look for imports that are out of the currently compiling binary, used to set the module as DllImport"
),
Option("i[=<pattern>]",
"include imported modules in the compilation",
q"{$(P Enables "include imports" mode, where the compiler will include imported
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/dmdparams.d
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum SymImport : ubyte
{
none, /// no symbols
defaultLibsOnly, /// only druntime/phobos symbols
externalOnly, /// any module that is known to be external to binary (-extI)
all, /// all non-root symbols
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dmd/dmodule.d
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,8 @@ extern (C++) final class Module : Package
SearchOptFlags searchCacheFlags; // cached flags
bool insearch;

bool isExplicitlyOutOfBinary; // Is this module known to be out of binary, and must be DllImport'd?

/**
* A root module is one that will be compiled all the way to
* object code. This field holds the root module that caused
Expand Down Expand Up @@ -536,6 +538,7 @@ extern (C++) final class Module : Package
auto m = new Module(loc, filename, ident, 0, 0);

// TODO: apply import path information (pathInfo) on to module
m.isExplicitlyOutOfBinary = importPathThatFindUs.isOutOfBinary;

if (!m.read(loc))
return null;
Expand Down
110 changes: 86 additions & 24 deletions compiler/src/dmd/e2ir.d
Original file line number Diff line number Diff line change
Expand Up @@ -498,52 +498,114 @@
/*********************************************
* Figure out whether a data symbol should be dllimported
* Params:
* var = declaration of the symbol
* symbl = declaration of the symbol
* Returns:
* true if symbol should be imported from a DLL
*/
bool isDllImported(Dsymbol var)
bool isDllImported(Dsymbol symbl)
{
// Windows is the only platform which dmd supports, that uses the DllImport/DllExport scheme.
if (!(target.os & Target.OS.Windows))
return false;
if (var.isImportedSymbol())

// If function does not have a body, check to see if its marked as DllImport or is set to be exported.
// If a global variable has both export + extern, it is DllImport
if (symbl.isImportedSymbol())
return true;
if (driverParams.symImport == SymImport.none)
return false;
if (auto vd = var.isDeclaration())

// Functions can go through the generated trampoline function.
// Not efficient, but it works.
if (symbl.isFuncDeclaration())
return false; // can always jump through import table

// Global variables are allowed, but not TLS or read only memory.
if (auto vd = symbl.isDeclaration())
{
if (!vd.isDataseg() || vd.isThreadlocal())
return false;
if (var.isFuncDeclaration())
return false; // can always jump through import table
if (auto tid = var.isTypeInfoDeclaration())
}

final switch(driverParams.symImport)
{
case SymImport.none:
// If DllImport overriding is disabled, do not change dllimport status.
return false;

case SymImport.externalOnly:
// Only modules that are marked as out of binary will be DllImport
break;

case SymImport.defaultLibsOnly:
case SymImport.all:

Check warning on line 539 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L538-L539

Added lines #L538 - L539 were not covered by tests
// If to access anything in druntime/phobos you need DllImport, verify against this.
break;

Check warning on line 541 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L541

Added line #L541 was not covered by tests
}
const systemLibraryNeedDllImport = driverParams.symImport != SymImport.externalOnly;

// For TypeInfo's check to see if its in druntime and DllImport it
if (auto tid = symbl.isTypeInfoDeclaration())
{
// Built in TypeInfo's are defined in druntime
if (builtinTypeInfo(tid.tinfo))
return true;
return systemLibraryNeedDllImport;

Check warning on line 550 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L550

Added line #L550 was not covered by tests

// Convert TypeInfo to its symbol
if (auto ad = isAggregate(tid.type))
var = ad;
symbl = ad;
}
if (driverParams.symImport == SymImport.defaultLibsOnly)

{
auto m = var.getModule();
// Filter the symbol based upon the module it is in.

auto m = symbl.getModule();
if (!m || !m.md)
return false;
const id = m.md.packages.length ? m.md.packages[0] : null;
if (id && id != Id.core && id != Id.std)
return false;
if (!id && m.md.id != Id.std && m.md.id != Id.object)

if (driverParams.symImport == SymImport.all || m.isExplicitlyOutOfBinary)
{
// If a module is specified as being out of binary (-extI), then it is allowed to be DllImport.
}
else if (driverParams.symImport == SymImport.externalOnly)
{
// Module is in binary, therefore not DllImport
return false;
}
else if (systemLibraryNeedDllImport)

Check warning on line 573 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L573

Added line #L573 was not covered by tests
{
// Filter out all modules that are not in druntime/phobos if we are only doing default libs only

const id = m.md.packages.length ? m.md.packages[0] : null;
if (id && id != Id.core && id != Id.std)
return false;
if (!id && m.md.id != Id.std && m.md.id != Id.object)
return false;

Check warning on line 581 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L577-L581

Added lines #L577 - L581 were not covered by tests
}
}
else if (driverParams.symImport != SymImport.all)
return false;
if (auto mod = var.isModule())
return !mod.isRoot(); // non-root ModuleInfo symbol
if (var.inNonRoot())

// If symbol is a ModuleInfo, check to see if module is being compiled.
if (auto mod = symbl.isModule())
{
const isBeingCompiled = mod.isRoot();
return !isBeingCompiled; // non-root ModuleInfo symbol

Check warning on line 589 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L588-L589

Added lines #L588 - L589 were not covered by tests
}

// Check to see if a template has been instatiated in current compilation,
// if it is defined in a external module, its DllImport.
if (symbl.inNonRoot())
return true; // not instantiated, and defined in non-root
if (auto ti = var.isInstantiated()) // && !defineOnDeclare(sym, false))

// If a template has been instatiated, only DllImport if it is codegen'ing
if (auto ti = symbl.isInstantiated()) // && !defineOnDeclare(sym, false))

Check warning on line 598 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L598

Added line #L598 was not covered by tests
return !ti.needsCodegen(); // instantiated but potentially culled (needsCodegen())
if (auto vd = var.isVarDeclaration())

// If a variable declaration and is extern
if (auto vd = symbl.isVarDeclaration())

Check warning on line 602 in compiler/src/dmd/e2ir.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/e2ir.d#L602

Added line #L602 was not covered by tests
{
// Shouldn't this be including an export check too???
if (vd.storage_class & STC.extern_)
return true; // externally defined global variable
}

return false;
}

Expand Down
10 changes: 7 additions & 3 deletions compiler/src/dmd/frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -7111,6 +7111,7 @@ class Module final : public Package
Dsymbol* searchCacheSymbol;
uint32_t searchCacheFlags;
bool insearch;
bool isExplicitlyOutOfBinary;
Module* importedFrom;
Array<Dsymbol* >* decldefs;
Array<Module* > aimports;
Expand Down Expand Up @@ -8349,12 +8350,15 @@ struct Verbose final
struct ImportPathInfo final
{
const char* path;
bool isOutOfBinary;
ImportPathInfo() :
path()
path(),
isOutOfBinary()
{
}
ImportPathInfo(const char* path) :
path(path)
ImportPathInfo(const char* path, bool isOutOfBinary = false) :
path(path),
isOutOfBinary(isOutOfBinary)
{}
};

Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/globals.d
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ extern(C++) struct Verbose

extern (C++) struct ImportPathInfo {
const(char)* path; // char*'s of where to look for import modules
bool isOutOfBinary; // Will any module found from this path be out of binary?
}

/// Put command line switches in here
Expand Down
12 changes: 10 additions & 2 deletions compiler/src/dmd/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,17 @@ struct Verbose
struct ImportPathInfo
{
const char* path;
d_bool isOutOfBinary;

ImportPathInfo() : path(NULL) { }
ImportPathInfo(const char* p) : path(p) { }
ImportPathInfo() :
path(),
isOutOfBinary()
{
}
ImportPathInfo(const char* path, d_bool isOutOfBinary = false) :
path(path),
isOutOfBinary(isOutOfBinary)
{}
};

// Put command line switches in here
Expand Down
12 changes: 11 additions & 1 deletion compiler/src/dmd/mars.d
Original file line number Diff line number Diff line change
Expand Up @@ -829,11 +829,14 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
case "defaultLibsOnly":
driverParams.symImport = SymImport.defaultLibsOnly;
break;
case "externalOnly":
driverParams.symImport = SymImport.externalOnly;
break;
case "all":
driverParams.symImport = SymImport.all;
break;
default:
error("unknown dllimport '%.*s', must be 'none', 'defaultLibsOnly' or 'all'", cast(int) imp.length, imp.ptr);
error("unknown dllimport '%.*s', must be 'none', 'defaultLibsOnly', 'externalOnly' or 'all'", cast(int) imp.length, imp.ptr);
}
}
else if (arg == "-fIBT")
Expand Down Expand Up @@ -1573,6 +1576,13 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
{
params.imppath.push(ImportPathInfo(p + 2 + (p[2] == '=')));
}
else if (startsWith(p + 1, "extI"))
{
// External import path switch -extI
auto importPathInfo = ImportPathInfo(p + 5 + (p[5] == '='));
importPathInfo.isOutOfBinary = true;
params.imppath.push(importPathInfo);
}
else if (p[1] == 'm' && p[2] == 'v' && p[3] == '=') // https://dlang.org/dmd.html#switch-mv
{
if (p[4] && strchr(p + 5, '='))
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/module.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ class Module final : public Package
SearchOptFlags searchCacheFlags; // cached flags
d_bool insearch;

d_bool isExplicitlyOutOfBinary; // Is this module known to be out of binary, and must be DllImport'd?

// module from command line we're imported from,
// i.e. a module that will be taken all the
// way to an object file
Expand Down
4 changes: 2 additions & 2 deletions compiler/test/dshell/dll_cxx.d
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ int main()
Vars.set(`DLL_LIB`, `$OUTPUT_BASE${SEP}mydll.lib`);
// CXX should be cl
dllCmd ~= [`/LD`, `/nologo`, `/Fe` ~ Vars.DLL];
mainExtra = `$DLL_LIB`;
mainExtra = `$DLL_LIB -dllimport=externalOnly`;
}
else version(OSX)
{
Expand All @@ -44,7 +44,7 @@ int main()
// The arguments have to be passed as an array, because run would replace '/' with '\\' otherwise.
run(dllCmd);

run(`$DMD -m$MODEL -g -od=$OUTPUT_BASE -of=$EXE_NAME $SRC/testdll.d ` ~ mainExtra);
run(`$DMD -m$MODEL -g -od=$OUTPUT_BASE -of=$EXE_NAME -extI=$EXTRA_FILES${SEP}dll_cxx${SEP}external $SRC/testdll.d ` ~ mainExtra);

run(`$EXE_NAME`, stdout, stderr, [`LD_LIBRARY_PATH`: Vars.OUTPUT_BASE]);

Expand Down
3 changes: 3 additions & 0 deletions compiler/test/dshell/extra-files/dll_cxx/external/binding.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module binding;

extern (C) __gshared extern int testExternalImportVar;
6 changes: 6 additions & 0 deletions compiler/test/dshell/extra-files/dll_cxx/mydll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,9 @@ EXPORT int getSomeValueCPP19660(void)
{
return someValueCPP19660;
}

extern "C"
{
// tests -extI switch for variables
EXPORT int testExternalImportVar = 0xF1234;
}
3 changes: 3 additions & 0 deletions compiler/test/dshell/extra-files/dll_cxx/testdll.d
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,7 @@ void main()
{
test22323();
test19660();

import binding;
assert(testExternalImportVar == 0xF1234);
}
Loading