Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Commit

Permalink
fixup for issue 16211 - Add deprecate feature for cycle detection as the
Browse files Browse the repository at this point in the history 
default mode.
  • Loading branch information
@schveiguy
schveiguy committed Dec 22, 2016
1 parent 422c8f7 commit 5ad304a
Show file tree
Hide file tree
Showing 3 changed files with 255 additions and 21 deletions.
262 changes: 250 additions & 12 deletions src/rt/minfo.d
View file
Expand Up @@ -156,26 +156,40 @@ struct ModuleGroup
* Allocate and fill in _ctors[] and _tlsctors[].
* Modules are inserted into the arrays in the order in which the constructors
* need to be run.
*
* Params:
* cycleHandling - string indicating option for cycle handling
* Throws:
* Exception if it fails.
*/
void sortCtors()
void sortCtors(string cycleHandling)
{
import core.bitop : bts, btr, bt, BitRange;
import rt.util.container.hashtab;
import rt.config : rt_configOption;

// used to unwind stack for printing deprecation message.
static class DeprecatedCycleException : Exception
{
this() { super(""); }
}
scope deprecation = new DeprecatedCycleException();

enum OnCycle
{
deprecate,
abort,
print,
ignore
}

auto onCycle = OnCycle.abort;
// Change default to .abort in 2.073
auto onCycle = OnCycle.deprecate;

switch(rt_configOption("oncycle")) with(OnCycle)
switch(cycleHandling) with(OnCycle)
{
case "deprecate":
onCycle = deprecate;
break;
case "abort":
onCycle = abort;
break;
Expand All @@ -190,7 +204,7 @@ struct ModuleGroup
break;
default:
// invalid cycle handling option.
throw new Error("DRT invalid cycle handling option: " ~ rt_configOption("oncycle"));
throw new Error("DRT invalid cycle handling option: " ~ cycleHandling);
}

debug (printModuleDependencies)
Expand Down Expand Up @@ -339,8 +353,7 @@ struct ModuleGroup
else
{
auto midx = edges[sp.curMod][sp.curDep];
// if midx is -1, then this isn't part of this DSO.
if (midx != -1 && !bts(reachable, midx))
if (!bts(reachable, midx))
{
if (bt(relevant, midx))
{
Expand All @@ -350,7 +363,16 @@ struct ModuleGroup
// was already started, this is a cycle.
final switch(onCycle) with(OnCycle)
{
case deprecate:
// check with old algorithm
if(sortCtorsOld(edges))
{
// unwind to print deprecation message.
throw deprecation;
}
goto case abort; // fall through
case abort:

string errmsg = "";
buildCycleMessage(idx, midx, (string x) {errmsg ~= x;});
throw new Error(errmsg, __FILE__, __LINE__);
Expand Down Expand Up @@ -484,8 +506,208 @@ struct ModuleGroup
}

// finally, do the sorting for both shared and tls ctors.
_ctors = doSort(MIctor | MIdtor);
_tlsctors = doSort(MItlsctor | MItlsdtor);
try
{
_ctors = doSort(MIctor | MIdtor);
_tlsctors = doSort(MItlsctor | MItlsdtor);
}
catch(DeprecatedCycleException)
{
// print a warning
import core.stdc.stdio : fprintf, stderr;
fprintf(stderr, "Deprecation 16211 warning:\n"
~ "A cycle has been detected in your program that was undetected prior to DMD\n"
~ "2.072. This program will continue, but will not operate when using DMD 2.073\n"
~ "to compile. Use runtime option --DRT-oncycle=print to see the cycle details.\n");

}
}

/******************************
* This is the old ctor sorting algorithm that does not find all cycles.
*
* It is here to allow the deprecated behavior from the original algorithm
* until people have fixed their code.
*
* If no cycles are found, the _ctors and _tlsctors are replaced with the
* ones generated by this algorithm to preserve the old incorrect ordering
* behavior.
*
* Params:
* edges - The module edges as found in the `importedModules` member of
* each ModuleInfo. Generated in sortCtors.
* Returns:
* true if no cycle is found, false if one was.
*/
bool sortCtorsOld(int[][] edges)
{
immutable len = edges.length;
assert(len == _modules.length);

static struct StackRec
{
@property int mod()
{
return _mods[_idx];
}

int[] _mods;
size_t _idx;
}

auto stack = (cast(StackRec*).calloc(len, StackRec.sizeof))[0 .. len];
// TODO: reuse GCBits by moving it to rt.util.container or core.internal
immutable nwords = (len + 8 * size_t.sizeof - 1) / (8 * size_t.sizeof);
auto ctorstart = cast(size_t*).malloc(nwords * size_t.sizeof);
auto ctordone = cast(size_t*).malloc(nwords * size_t.sizeof);
int[] initialEdges = (cast(int *)malloc(int.sizeof * len))[0 .. len];
if (!stack.ptr || ctorstart is null || ctordone is null || !initialEdges.ptr)
assert(0);
scope (exit)
{
.free(stack.ptr);
.free(ctorstart);
.free(ctordone);
.free(initialEdges.ptr);
}

// initialize the initial edges
foreach (int i, ref v; initialEdges)
v = i;

bool sort(ref immutable(ModuleInfo)*[] ctors, uint mask)
{
import core.bitop;

ctors = (cast(immutable(ModuleInfo)**).malloc(len * size_t.sizeof))[0 .. len];
if (!ctors.ptr)
assert(0);

// clean flags
memset(ctorstart, 0, nwords * size_t.sizeof);
memset(ctordone, 0, nwords * size_t.sizeof);
size_t stackidx = 0;
size_t cidx;

int[] mods = initialEdges;

size_t idx;
while (true)
{
while (idx < mods.length)
{
auto m = mods[idx];

if (bt(ctordone, m))
{
// this module has already been processed, skip
++idx;
continue;
}
else if (bt(ctorstart, m))
{
/* Trace back to the begin of the cycle.
*/
bool ctorInCycle;
size_t start = stackidx;
while (start--)
{
auto sm = stack[start].mod;
if (sm == m)
break;
assert(sm >= 0);
if (bt(ctorstart, sm))
ctorInCycle = true;
}
assert(stack[start].mod == m);
if (ctorInCycle)
{
return false;
}
else
{
/* This is also a cycle, but the import chain does not constrain
* the order of initialization, either because the imported
* modules have no ctors or the ctors are standalone.
*/
++idx;
}
}
else
{
auto curmod = _modules[m];
if (curmod.flags & mask)
{
if (curmod.flags & MIstandalone || !edges[m].length)
{ // trivial ctor => sort in
ctors[cidx++] = curmod;
bts(ctordone, m);
}
else
{ // non-trivial ctor => defer
bts(ctorstart, m);
}
}
else // no ctor => mark as visited
{
bts(ctordone, m);
}

if (edges[m].length)
{
/* Internal runtime error, recursion exceeds number of modules.
*/
(stackidx < len) || assert(0);

// recurse
stack[stackidx++] = StackRec(mods, idx);
idx = 0;
mods = edges[m];
}
}
}

if (stackidx)
{ // pop old value from stack
--stackidx;
mods = stack[stackidx]._mods;
idx = stack[stackidx]._idx;
auto m = mods[idx++];
if (bt(ctorstart, m) && !bts(ctordone, m))
ctors[cidx++] = _modules[m];
}
else // done
break;
}
// store final number and shrink array
ctors = (cast(immutable(ModuleInfo)**).realloc(ctors.ptr, cidx * size_t.sizeof))[0 .. cidx];
return true;
}

/* Do two passes: ctor/dtor, tlsctor/tlsdtor
*/
immutable(ModuleInfo)*[] _ctors2;
immutable(ModuleInfo)*[] _tlsctors2;
auto result = sort(_ctors2, MIctor | MIdtor) && sort(_tlsctors2, MItlsctor | MItlsdtor);
if (result) // no cycle
{
// fall back to original ordering as part of the deprecation.
if(_ctors.ptr)
.free(_ctors.ptr);
_ctors = _ctors2;
if(_tlsctors.ptr)
.free(_tlsctors.ptr);
_tlsctors = _tlsctors2;
}
else
{
// free any allocated memory that will be forgotten
if (_ctors2.ptr)
.free(_ctors2.ptr);
if (_tlsctors2.ptr)
.free(_tlsctors2.ptr);
}
return result;
}

void runCtors()
Expand Down Expand Up @@ -558,9 +780,10 @@ extern (C)
{
void rt_moduleCtor()
{
import rt.config : rt_configOption;
foreach (ref sg; SectionGroup)
{
sg.moduleGroup.sortCtors();
sg.moduleGroup.sortCtors(rt_configOption("oncycle"));
sg.moduleGroup.runCtors();
}
}
Expand Down Expand Up @@ -691,13 +914,13 @@ unittest
return mi;
}

static void checkExp(string testname, bool shouldThrow,
static void checkExp2(string testname, bool shouldThrow, string oncycle,
immutable(ModuleInfo*)[] modules,
immutable(ModuleInfo*)[] dtors=null,
immutable(ModuleInfo*)[] tlsdtors=null)
{
auto mgroup = ModuleGroup(modules);
mgroup.sortCtors();
mgroup.sortCtors(oncycle);

// if we are expecting sort to throw, don't throw because of unexpected
// success!
Expand All @@ -710,6 +933,15 @@ unittest
}
}

static void checkExp(string testname, bool shouldThrow,
immutable(ModuleInfo*)[] modules,
immutable(ModuleInfo*)[] dtors=null,
immutable(ModuleInfo*)[] tlsdtors=null)
{
checkExp2(testname, shouldThrow, "abort", modules, dtors, tlsdtors);
}


{
auto m0 = mockMI(0);
auto m1 = mockMI(0);
Expand Down Expand Up @@ -787,6 +1019,9 @@ unittest
m1.setImports(&m0.mi);
assertThrown!Throwable(checkExp("", true, [&m0.mi, &m1.mi, &m2.mi]),
"detects ctors cycles");
assertThrown!Throwable(checkExp2("", true, "deprecate",
[&m0.mi, &m1.mi, &m2.mi]),
"detects ctors cycles (dep)");
}

{
Expand Down Expand Up @@ -836,6 +1071,9 @@ unittest
m2.setImports(&m0.mi);
assertThrown!Throwable(checkExp("", true, [&m0.mi, &m1.mi, &m2.mi]),
"detects tlsctors cycle");
assertThrown!Throwable(checkExp2("", true, "deprecate",
[&m0.mi, &m1.mi, &m2.mi]),
"detects tlsctors cycle (dep)");
}

{
Expand Down
8 changes: 5 additions & 3 deletions test/cycles/Makefile
View file
@@ -1,6 +1,6 @@
include ../common.mak

TESTS:=cycle_ignore cycle_abort cycle_print
TESTS:=cycle_ignore cycle_abort cycle_print cycle_deprecate

DIFF:=diff
SED:=sed
Expand All @@ -11,9 +11,11 @@ all: $(addprefix $(ROOT)/,$(addsuffix .done,$(TESTS)))
$(ROOT)/cycle_ignore.done: RETCODE=0
$(ROOT)/cycle_ignore.done: LINES=0
$(ROOT)/cycle_abort.done: RETCODE=1
$(ROOT)/cycle_abort.done: LINES=5
$(ROOT)/cycle_abort.done: LINES=7
$(ROOT)/cycle_print.done: RETCODE=0
$(ROOT)/cycle_print.done: LINES=8
$(ROOT)/cycle_print.done: LINES=6
$(ROOT)/cycle_deprecate.done: RETCODE=0
$(ROOT)/cycle_deprecate.done: LINES=4
$(ROOT)/%.done: $(ROOT)/test_cycles
@echo Testing $*
$(QUIET)$(TIMELIMIT)$(ROOT)/test_cycles --DRT-oncycle=$(patsubst cycle_%.done,%, $(notdir $@)) > $@ 2>&1; test $$? -eq $(RETCODE)
Expand Down
6 changes: 0 additions & 6 deletions test/cycles/src/mod2.d
View file
Expand Up @@ -2,12 +2,6 @@ module mod2;
import mod1;
import mod3;

shared int x;
shared static this()
{
x = 2;
}

void main()
{
// do nothing
Expand Down

0 comments on commit 5ad304a

Please sign in to comment.