inNative v0.1.2 bugfixes

* Fixed certain C export name handling that could cause a crash
* Added a regression test for the name handling bug.
* Optimization is now on by default to avoid confusion (#18)
* Fixed nonstandard use of `static const int`

Author: Erik McClure

include/innative/innative.h

@@ -19,7 +19,7 @@ limitations under the License.
 
 #define INNATIVE_VERSION_MAJOR 0
 #define INNATIVE_VERSION_MINOR 1
-#define INNATIVE_VERSION_REVISION 1
+#define INNATIVE_VERSION_REVISION 2
 #define INNATIVE_VERSION(v, m, r, b) (((v|0ULL)<<48) | ((m|0ULL)<<32) | ((r|0ULL)<<16) | (b|0ULL))
 
 // CPU Architecture (possible pre-defined macros found on http://predef.sourceforge.net/prearch.html)

include/innative/path.h

@@ -15,11 +15,11 @@ namespace innative {
   {
   public:
 #ifdef IN_PLATFORM_WIN32
-    static const char SEPERATOR = '\\';
-    static const char OTHER = '/';
+    enum : char { SEPERATOR = '\\' };
+    enum : char { OTHER = '/' };
 #elif defined(IN_PLATFORM_POSIX)
-    static const char SEPERATOR = '/';
-    static const char OTHER = '\\';
+    enum : char { SEPERATOR = '/' };
+    enum : char { OTHER = '\\' };
 #else
 #error "unknown platform"
 #endif
@@ -118,7 +118,9 @@ namespace innative {
   protected:
     inline void _canonize()
     {
-      std::replace(_path.begin(), _path.end(), OTHER, SEPERATOR);
+      char other = OTHER; // std::replace takes the address of it's operators, which breaks on any constant that doesn't necessarily exist.
+      char seperator = SEPERATOR;
+      std::replace(_path.begin(), _path.end(), other, seperator);
     }
 
     std::string _path;

innative-cmd/main.cpp

@@ -137,7 +137,7 @@ int main(int argc, char* argv[])
   std::vector<std::pair<const char*, int>> embeddings;
   std::vector<const char*> whitelist;
   unsigned int flags = ENV_ENABLE_WAT; // Always enable WAT
-  unsigned int optimize = 0;
+  unsigned int optimize = ENV_OPTIMIZE_O3; // Default to O3
   innative::Path out;
   std::vector<const char*> wast; // WAST files will be executed in the order they are specified, after all other modules are injected into the environment
   const char* libpath = nullptr;
@@ -192,6 +192,8 @@ int main(int argc, char* argv[])
                 std::cout << "Unknown flag: " << argv[i] << std::endl;
                 err = ERR_UNKNOWN_FLAG;
               }
+              else if(flag->second == ENV_OPTIMIZE_O0 || (flag->second & ENV_OPTIMIZE_OMASK))
+                optimize = (optimize & ~ENV_OPTIMIZE_OMASK) | flag->second;
               else
                 optimize |= flag->second;
             }
@@ -454,6 +456,8 @@ int main(int argc, char* argv[])
       printerr(env->log, "Error loading modules", err);
       dump_validation_errors(env);
     }
+
+    (*exports.DestroyEnvironment)(env);
     return err;
   }
 
@@ -464,6 +468,7 @@ int main(int argc, char* argv[])
     if((*exports.AddEmbedding)(env, embeddings[i].second, embeddings[i].first, 0) == ERR_FATAL_FILE_ERROR)
     {
       fprintf(env->log, "Error loading file: %s\n", embeddings[i].first);
+      (*exports.DestroyEnvironment)(env);
       return ERR_FATAL_FILE_ERROR;
     }
 
@@ -478,6 +483,8 @@ int main(int argc, char* argv[])
       printerr(env->log, "Error loading environment", err);
       dump_validation_errors(env);
     }
+
+    (*exports.DestroyEnvironment)(env);
     return err;
   }
 
@@ -500,7 +507,7 @@ int main(int argc, char* argv[])
       innative_serialize_module(env, i, target.c_str());
     }
   }
-
+  
   // Check if this is a .wast file, which must be handled differently because it's an entire environment
   if(wast.size() > 0)
   {
@@ -518,6 +525,7 @@ int main(int argc, char* argv[])
       dump_validation_errors(env);
     }
 
+    (*exports.DestroyEnvironment)(env);
     return err;
   }
 

innative-env/internal.c

@@ -226,3 +226,9 @@ IN_COMPILER_DLLEXPORT extern void _innative_internal_env_memdump(const unsigned
   }
   _innative_internal_write_out("\n", 1);
 }
+
+// This function exists only to test the _WASM_ C export code path
+IN_COMPILER_DLLEXPORT extern void _innative_internal_WASM_print(int32_t a)
+{
+  _innative_internal_env_print(a);
+}

innative-test/test.cpp

@@ -93,7 +93,7 @@ int main(int argc, char *argv[])
 
   if(stages & TEST_INTERNAL)
   {
-    TestHarness harness(stderr);
+    TestHarness harness(exports, !argc ? 0 : argv[0], log, stderr, temppath.c_str());
     harness.Run(stdout);
   }
 

innative-test/test.h

@@ -9,11 +9,14 @@
 #include <stdio.h>
 #include <vector>
 #include <string.h>
+#include "innative/path.h"
+#include "innative/export.h"
 
 class TestHarness
 {
 public:
-  TestHarness(FILE* out);
+  TestHarness(const IRExports& exports, const char* arg0, int loglevel, FILE* out, const char* folder);
+  ~TestHarness();
   size_t Run(FILE* out);
   void test_allocator();
   void test_environment();
@@ -24,6 +27,7 @@ class TestHarness
   void test_util();
   void test_parallel_parsing();
   void test_malloc();
+  int CompileWASM(const char* file);
 
   inline std::pair<uint32_t, uint32_t> Results() { auto r = _testdata; _testdata = { 0,0 }; return r; }
 
@@ -46,6 +50,11 @@ class TestHarness
 
   std::pair<uint32_t, uint32_t> _testdata;
   FILE* _target;
+  const IRExports& _exports;
+  const char* _arg0;
+  int _loglevel;
+  std::vector<std::string> _garbage;
+  innative::Path _folder;
 };
 
 #define TEST(x) DoTest(x, ""#x, __FILE__, __LINE__)

innative-test/test_harness.cpp

@@ -2,9 +2,15 @@
 // For conditions of distribution and use, see copyright notice in innative.h
 
 #include "test.h"
+#include <stdio.h>
 
-TestHarness::TestHarness(FILE* out) : _target(out), _testdata(0, 0) {}
-
+TestHarness::TestHarness(const IRExports& exports, const char* arg0, int loglevel, FILE* out, const char* folder) : _exports(exports), _arg0(arg0), _loglevel(loglevel), _target(out), _folder(folder), _testdata(0, 0){}
+TestHarness::~TestHarness()
+{
+  // Clean up all the files we just produced
+  for(auto f : _garbage)
+    std::remove(f.c_str());
+}
 size_t TestHarness::Run(FILE* out)
 {
   std::pair<const char*, void(TestHarness::*)()> tests[] = {
@@ -37,9 +43,56 @@ size_t TestHarness::Run(FILE* out)
     fprintf(out, "%-*s %-*s %-*s\n", COLUMNS[0], tests[i].first, COLUMNS[1], buf, COLUMNS[2], (results.first == results.second) ? "PASS" : "FAIL");
   }
 
+  {
+    TEST(CompileWASM("../scripts/test-h.wat") == ERR_SUCCESS);
+
+    char buf[COLUMNS[1] + 1] = { 0 };
+    auto results = Results();
+    snprintf(buf, COLUMNS[1] + 1, "%u/%u", results.first, results.second);
+    fprintf(out, "%-*s %-*s %-*s\n", COLUMNS[0], "aux tests", COLUMNS[1], buf, COLUMNS[2], (results.first == results.second) ? "PASS" : "FAIL");
+  }
+
   // Test compiling EXE
   // Test compiling DLL with entry point that gets called in the init function
 
   fprintf(out, "\n");
   return failures;
-}
+}
+
+int TestHarness::CompileWASM(const char* file)
+{
+  Environment* env = (*_exports.CreateEnvironment)(1, 0, 0);
+  env->flags = ENV_ENABLE_WAT | ENV_LIBRARY;
+  env->optimize = ENV_OPTIMIZE_O3;
+  env->features = ENV_FEATURE_ALL;
+  env->log = stdout;
+  env->loglevel = _loglevel;
+
+  int err = (*_exports.AddEmbedding)(env, 0, (void*)INNATIVE_DEFAULT_ENVIRONMENT, 0);
+  if(err < 0)
+    return err;
+
+  (*_exports.AddModule)(env, file, 0, file, &err);
+  if(err < 0)
+    return err;
+
+  (*_exports.FinalizeEnvironment)(env);
+  std::string base = (_folder + innative::Path(file).File().RemoveExtension()).Get();
+  std::string out = base + IN_LIBRARY_EXTENSION;
+
+  err = (*_exports.Compile)(env, out.c_str());
+  if(err < 0)
+    return err;
+
+  _garbage.push_back(out);
+#ifdef IN_PLATFORM_WIN32
+  _garbage.push_back(base + ".lib");
+#endif
+  (*_exports.DestroyEnvironment)(env);
+  void* m = (*_exports.LoadAssembly)(out.c_str());
+  if(!m)
+    return ERR_FATAL_INVALID_MODULE;
+  (*_exports.FreeAssembly)(m);
+
+  return ERR_SUCCESS;
+}

innative/compile.cpp

@@ -1950,7 +1950,7 @@ IN_ERROR CompileModule(const Environment* env, code::Context& context)
         //if(context.dbuilder)
         //  FunctionDebugInfo(context.functions.back().imported, context, false, context.m.importsection.imports[i].func_desc.debug.line);
 
-        auto name = (context.m.importsection.imports[i].func_desc.debug.name.get()) ?
+        auto name = !(context.m.importsection.imports[i].func_desc.debug.name.get()) ?
           "|" + context.functions.back().imported->getName() + "#internal" :
           context.m.importsection.imports[i].func_desc.debug.name.str() + ("#" + std::to_string(i));
 

scripts/test-h.wat

@@ -0,0 +1,17 @@
+(module
+  (type (;0;) (func (param i32)))
+  (type (;1;) (func))
+  (import "_innative_internal" "print" (func (;0;) (type 0)))
+  (func (;1;) (type 1)
+    (local i32 i32 i32)
+    i32.const 10
+    local.set 0
+    i32.const 104
+    local.set 1
+    i32.const 39
+    local.set 2
+    local.get 1
+    call 0
+    local.get 0
+    call 0)
+  (export "h" (func 1)))