check bitcasts in eatLLVMIdent, so that call-bitcast is checked

1 files changed, 72 insertions, 68 deletions

diff --git a/src/parseTools.js b/src/parseTools.js
index 06e33028..f963a834 100644
--- a/src/parseTools.js
+++ b/src/parseTools.js
@@ -583,7 +583,9 @@ function parseLLVMFunctionCall(segment) {
 function eatLLVMIdent(tokens) {
   var ret;
   if (tokens[0].text in PARSABLE_LLVM_FUNCTIONS) {
-    ret = parseLLVMFunctionCall([{text: 'i0'}].concat(tokens.slice(0,2))).ident; // TODO: Handle more cases, return a full object, process it later
+    var item = parseLLVMFunctionCall([{text: '?'}].concat(tokens.slice(0,2))); // TODO: Handle more cases, return a full object, process it later
+    if (item.intertype == 'bitcast') checkBitcast(item);
+    ret = item.ident;
     tokens.shift();
     tokens.shift();
   } else {
@@ -1630,80 +1632,82 @@ function makeGetSlabs(ptr, type, allowMultiple, unsigned) {
   return [];
 }
 
-function finalizeLLVMFunctionCall(item, noIndexizeFunctions) {
-  if (item.intertype == 'getelementptr') { // TODO finalizeLLVMParameter on the ident and the indexes?
-    return makePointer(makeGetSlabs(item.ident, item.type)[0], getGetElementPtrIndexes(item), null, item.type);
-  }
-  if (item.intertype == 'bitcast') {
-    // Warn about some types of casts, then fall through to the handling code below
-    var oldType = item.params[0].type;
-    var newType = item.type;
-    if (isPossiblyFunctionType(oldType) && isPossiblyFunctionType(newType)) {
-      var oldInfo = {}, newInfo = {};
-      var oldCount = countNormalArgs(oldType, oldInfo);
-      var newCount = countNormalArgs(newType, newInfo);
-      var warned = false;
-      function showWarning() {
-        if (warned) return;
-        warned = true;
-        if (VERBOSE || ASM_JS) {
-          warnOnce('Casting potentially incompatible function pointer ' + oldType + ' to ' + newType + ', for ' + item.params[0].ident.slice(1));
-        } else {
-          warnOnce('Casting a function pointer type to a potentially incompatible one (use VERBOSE=1 to see more)');
-        }
-        if (ASM_JS) warnOnce('Incompatible function pointer casts are very dangerous with ASM_JS=1, you should investigate and correct these');
+function checkBitcast(item) {
+  // Warn about some types of casts, then fall through to the handling code below
+  var oldType = item.params[0].type;
+  var newType = item.type;
+  if (isPossiblyFunctionType(oldType) && isPossiblyFunctionType(newType)) {
+    var oldInfo = {}, newInfo = {};
+    var oldCount = countNormalArgs(oldType, oldInfo);
+    var newCount = countNormalArgs(newType, newInfo);
+    var warned = false;
+    function showWarning() {
+      if (warned) return;
+      warned = true;
+      if (VERBOSE || ASM_JS) {
+        warnOnce('Casting potentially incompatible function pointer ' + oldType + ' to ' + newType + ', for ' + item.params[0].ident.slice(1));
+      } else {
+        warnOnce('Casting a function pointer type to a potentially incompatible one (use VERBOSE=1 to see more)');
       }
-      if (oldCount != newCount && oldCount && newCount) {
+      if (ASM_JS) warnOnce('Incompatible function pointer casts are very dangerous with ASM_JS=1, you should investigate and correct these');
+    }
+    if (oldCount != newCount && oldCount && newCount) {
+      showWarning();
+
+      // General concerns
+      // ================
+      //
+      // This may be dangerous as clang generates different code for C and C++ calling conventions. The only problem
+      // case appears to be passing a structure by value, C will have (field1, field2) as function args, and the
+      // function will internally create a structure with that data, while C++ will have (struct* byVal) and it
+      // will create a copy before calling the function, then call it with a pointer to the copy. Mixing the two
+      // first of all leads to two copies being made, so this is a bad idea even regardless of Emscripten. But,
+      // what is a problem for Emscripten is that mixing these two calling conventions (say, calling a C one from
+      // C++) will then assume that (struct* byVal) is actually the same as (field1, field2). In native code, this
+      // is easily possible, you place the two fields on the stack and call the function (you know to place the
+      // values since there is 'byVal'). In Emscripten, though, this means we would need to always do one or the
+      // other of the two possibilities, for example, always passing by-value structs as (field1, field2). This
+      // would slow down everything, just to handle this corner case. (Which, just to point out how much of a
+      // corner case it is, does not appear to happen with nested structures!)
+      //
+      // The recommended solution for this problem is not to mix C and C++ calling conventions when passing structs
+      // by value. Either always pass structs by value within C code or C++ code, but not mixing the two by
+      // defining a function in one and calling it from the other (so, just changing .c to .cpp, or moving code
+      // from one file to another, would be enough to fix this), or, do not pass structs by value (which in general
+      // is inefficient, and worth avoiding if you can).
+      //
+      // Note that removing all arguments is acceptable, as a vast to void ()*.
+      //
+
+      // asm.js concerns
+      // ===============
+      //
+      // asm must be even more careful, any change in number of args can make function calls simply fail (since
+      // they will look in the wrong table). You should investigate each one to see if it is problematic, and
+      // adjust the source code to avoid potential issues. The warnings will tell you which types and variables
+      // are involved, look in the LLVM IR to see what is going on, and to connect that to the original source.
+    }
+    if (ASM_JS) {
+      if (oldCount != newCount) showWarning();
+      else if (!isIdenticallyImplemented(oldInfo.returnType, newInfo.returnType)) {
         showWarning();
-
-        // General concerns
-        // ================
-        //
-        // This may be dangerous as clang generates different code for C and C++ calling conventions. The only problem
-        // case appears to be passing a structure by value, C will have (field1, field2) as function args, and the
-        // function will internally create a structure with that data, while C++ will have (struct* byVal) and it
-        // will create a copy before calling the function, then call it with a pointer to the copy. Mixing the two
-        // first of all leads to two copies being made, so this is a bad idea even regardless of Emscripten. But,
-        // what is a problem for Emscripten is that mixing these two calling conventions (say, calling a C one from
-        // C++) will then assume that (struct* byVal) is actually the same as (field1, field2). In native code, this
-        // is easily possible, you place the two fields on the stack and call the function (you know to place the
-        // values since there is 'byVal'). In Emscripten, though, this means we would need to always do one or the
-        // other of the two possibilities, for example, always passing by-value structs as (field1, field2). This
-        // would slow down everything, just to handle this corner case. (Which, just to point out how much of a
-        // corner case it is, does not appear to happen with nested structures!)
-        //
-        // The recommended solution for this problem is not to mix C and C++ calling conventions when passing structs
-        // by value. Either always pass structs by value within C code or C++ code, but not mixing the two by
-        // defining a function in one and calling it from the other (so, just changing .c to .cpp, or moving code
-        // from one file to another, would be enough to fix this), or, do not pass structs by value (which in general
-        // is inefficient, and worth avoiding if you can).
-        //
-        // Note that removing all arguments is acceptable, as a vast to void ()*.
-        //
-
-        // asm.js concerns
-        // ===============
-        //
-        // asm must be even more careful, any change in number of args can make function calls simply fail (since
-        // they will look in the wrong table). You should investigate each one to see if it is problematic, and
-        // adjust the source code to avoid potential issues. The warnings will tell you which types and variables
-        // are involved, look in the LLVM IR to see what is going on, and to connect that to the original source.
-      }
-      if (ASM_JS) {
-        if (oldCount != newCount) showWarning();
-        else if (!isIdenticallyImplemented(oldInfo.returnType, newInfo.returnType)) {
-          showWarning();
-        } else {
-          for (var i = 0; i < oldCount; i++) {
-            if (!isIdenticallyImplemented(oldInfo.segments[i][0].text, newInfo.segments[i][0].text)) {
-              showWarning();
-              break;
-            }
+      } else {
+        for (var i = 0; i < oldCount; i++) {
+          if (!isIdenticallyImplemented(oldInfo.segments[i][0].text, newInfo.segments[i][0].text)) {
+            showWarning();
+            break;
           }
         }
       }
     }
   }
+}
+
+function finalizeLLVMFunctionCall(item, noIndexizeFunctions) {
+  if (item.intertype == 'getelementptr') { // TODO finalizeLLVMParameter on the ident and the indexes?
+    return makePointer(makeGetSlabs(item.ident, item.type)[0], getGetElementPtrIndexes(item), null, item.type);
+  }
+  if (item.intertype == 'bitcast') checkBitcast(item);
   var temp = {
     op: item.intertype,
     variant: item.variant,