From 8238a9cd7e8ea54007b958ee8bf9f7ca643e144d Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Sat, 9 Jul 2016 10:52:34 +0200 Subject: [PATCH 1/3] fix --DRT-gcopt string argument parsing - would match more than just an identifier, e.g. 'gc:conservative profile:1' was parsed as gc='conservative profile:1' - was restricted to known GC names, instead we now check that one GC was initialized - parse const(char)[] arguments as simple identifier [_a-zA-Z][_a-zA-Z0-9]* --- src/gc/config.d | 33 +++++++++++--------------------- src/gc/proxy.d | 8 ++++++++ test/exceptions/Makefile | 3 ++- test/exceptions/src/unknown_gc.d | 5 +++++ 4 files changed, 26 insertions(+), 23 deletions(-) create mode 100644 test/exceptions/src/unknown_gc.d diff --git a/src/gc/config.d b/src/gc/config.d index 36a1e0a77f4..978b74b50d3 100644 --- a/src/gc/config.d +++ b/src/gc/config.d @@ -26,7 +26,7 @@ struct Config { bool disable; // start disabled ubyte profile; // enable profiling with summary when terminating program - string gc = "conservative"; // select gc implementation conservative|manual + const(char)[] gc = "conservative"; // select gc implementation conservative|manual size_t initReserve; // initial reserve (MB) size_t minPoolSize = 1; // initial and minimum pool size (MB) @@ -190,27 +190,10 @@ bool parse(T:const(char)[])(const(char)[] optname, ref const(char)[] str, ref T in { assert(str.length); } body { - bool findNext(string next) - { - if(next.length>str.length) - return false; - - return (next.length == str.length)? (next == str):(isspace(str[next.length+1])&&(next == str[0..next.length])); - } - - if(findNext("manual")) - { - res = "manual"; - } - else if(findNext("conservative")) - { - res = "conservative"; - } - else - { - return parseError("conservative or manual", optname, str); - } - + // [_a-zA-Z][_a-zA-Z0-9]*] + if (str[0] != '_' && !isalpha(str[0])) + return parseError("an identifier", optname, str); + res = str[0 .. $ - str[1 .. $].find!(c => c != '_' && !isalnum(c)).length]; str = str[res.length .. $]; return true; } @@ -274,4 +257,10 @@ unittest assert(conf.parseOptions("help")); assert(conf.parseOptions("help profile:1")); assert(conf.parseOptions("help profile:1 help")); + + assert(conf.parseOptions("gc:manual") && conf.gc == "manual"); + assert(conf.parseOptions("gc:conservative help profile:1") && conf.gc == "conservative" && conf.profile == 1); + + // the config parse doesn't know all available GC names, so should accept unknown ones + assert(conf.parseOptions("gc:whatever")); } diff --git a/src/gc/proxy.d b/src/gc/proxy.d index 5e2d4b45493..777fe81421d 100644 --- a/src/gc/proxy.d +++ b/src/gc/proxy.d @@ -42,6 +42,14 @@ extern (C) config.initialize(); ManualGC.initialize(instance); ConservativeGC.initialize(instance); + if (instance is null) + { + import core.stdc.stdio : fprintf, stderr; + import core.stdc.stdlib : exit; + + fprintf(stderr, "No GC was initialized, please recheck the name of the selected GC ('%.*s').\n", cast(int)config.gc.length, config.gc.ptr); + exit(1); + } // NOTE: The GC must initialize the thread library // before its first collection. diff --git a/test/exceptions/Makefile b/test/exceptions/Makefile index 072b8c6de2b..b2ba0971d5a 100644 --- a/test/exceptions/Makefile +++ b/test/exceptions/Makefile @@ -1,6 +1,6 @@ include ../common.mak -TESTS:=stderr_msg unittest_assert invalid_memory_operation +TESTS:=stderr_msg unittest_assert invalid_memory_operation unknown_gc ifeq ($(OS)-$(BUILD),linux-debug) TESTS:=$(TESTS) line_trace endif @@ -24,6 +24,7 @@ $(ROOT)/line_trace.done: $(ROOT)/line_trace $(ROOT)/stderr_msg.done: STDERR_EXP="stderr_msg msg" $(ROOT)/unittest_assert.done: STDERR_EXP="unittest_assert msg" $(ROOT)/invalid_memory_operation.done: STDERR_EXP="InvalidMemoryOperationError" +$(ROOT)/unknown_gc.done: STDERR_EXP="'unknowngc'" $(ROOT)/%.done: $(ROOT)/% @echo Testing $* $(QUIET)$(TIMELIMIT)$(ROOT)/$* $(RUN_ARGS) 2>&1 1>/dev/null | grep -qF $(STDERR_EXP) diff --git a/test/exceptions/src/unknown_gc.d b/test/exceptions/src/unknown_gc.d new file mode 100644 index 00000000000..43e1e6453fe --- /dev/null +++ b/test/exceptions/src/unknown_gc.d @@ -0,0 +1,5 @@ +extern(C) __gshared string[] rt_options = [ "gcopt=gc:unknowngc" ]; + +void main() +{ +} From 0816ae37b633a37629f76a013299ec0375fc0b2e Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Thu, 14 Jul 2016 03:18:45 +0200 Subject: [PATCH 2/3] parse identifier until next terminator --- src/gc/config.d | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/gc/config.d b/src/gc/config.d index 978b74b50d3..bec333dd2af 100644 --- a/src/gc/config.d +++ b/src/gc/config.d @@ -190,11 +190,11 @@ bool parse(T:const(char)[])(const(char)[] optname, ref const(char)[] str, ref T in { assert(str.length); } body { - // [_a-zA-Z][_a-zA-Z0-9]*] - if (str[0] != '_' && !isalpha(str[0])) + auto tail = str.find!(c => c == ':' || c == '=' || c == ' '); + res = str[0 .. $ - tail.length]; + if (!res.length) return parseError("an identifier", optname, str); - res = str[0 .. $ - str[1 .. $].find!(c => c != '_' && !isalnum(c)).length]; - str = str[res.length .. $]; + str = tail; return true; } @@ -259,6 +259,7 @@ unittest assert(conf.parseOptions("help profile:1 help")); assert(conf.parseOptions("gc:manual") && conf.gc == "manual"); + assert(conf.parseOptions("gc:my-gc~modified") && conf.gc == "my-gc~modified"); assert(conf.parseOptions("gc:conservative help profile:1") && conf.gc == "conservative" && conf.profile == 1); // the config parse doesn't know all available GC names, so should accept unknown ones From eacf2ca609b9903e7953604b479c9f9c1db39bda Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Thu, 14 Jul 2016 03:20:02 +0200 Subject: [PATCH 3/3] parse options as string - use inout(char)[] as parse input and as output for identifier parsing - convert parse template overloads that only work w/ a single type to functions --- src/gc/config.d | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/gc/config.d b/src/gc/config.d index bec333dd2af..7cb5b1e6c82 100644 --- a/src/gc/config.d +++ b/src/gc/config.d @@ -26,7 +26,7 @@ struct Config { bool disable; // start disabled ubyte profile; // enable profiling with summary when terminating program - const(char)[] gc = "conservative"; // select gc implementation conservative|manual + string gc = "conservative"; // select gc implementation conservative|manual size_t initReserve; // initial reserve (MB) size_t minPoolSize = 1; // initial and minimum pool size (MB) @@ -74,7 +74,7 @@ struct Config cast(long)maxPoolSize, cast(long)incPoolSize, heapSizeFactor); } - bool parseOptions(const(char)[] opt) + bool parseOptions(string opt) { opt = skip!isspace(opt); while (opt.length) @@ -138,7 +138,7 @@ inout(char)[] find(alias pred)(inout(char)[] str) return null; } -bool parse(T:size_t)(const(char)[] optname, ref const(char)[] str, ref T res) +bool parse(T:size_t)(const(char)[] optname, ref inout(char)[] str, ref T res) in { assert(str.length); } body { @@ -155,7 +155,7 @@ body return true; } -bool parse(T:bool)(const(char)[] optname, ref const(char)[] str, ref T res) +bool parse(const(char)[] optname, ref inout(char)[] str, ref bool res) in { assert(str.length); } body { @@ -169,7 +169,7 @@ body return true; } -bool parse(T:float)(const(char)[] optname, ref const(char)[] str, ref T res) +bool parse(const(char)[] optname, ref inout(char)[] str, ref float res) in { assert(str.length); } body { @@ -186,7 +186,7 @@ body return true; } -bool parse(T:const(char)[])(const(char)[] optname, ref const(char)[] str, ref T res) +bool parse(const(char)[] optname, ref inout(char)[] str, ref inout(char)[] res) in { assert(str.length); } body {