The Static Analysis tool Coverity added that though arg1 is null check, it is dereferenced on the following lines without taking into consideration the null check: >> ParseNode* arg1 = FunctionArgsList(fn, &numFormals); >> ParseNode* arg2 = arg1 ? NextNode(arg1) : nullptr; >> ParseNode* arg3 = arg2 ? NextNode(arg2) : nullptr; >> >> if (numFormals > 3) >> return m.fail(fn, "asm.js modules takes at most 3 argument"); >> >> PropertyName* arg1Name = nullptr; >> if (numFormals >= 1 && !CheckModuleArgument(m, arg1, &arg1Name)) In this case this easily can be considered a false-positive since the numFormulas is checked, but for precaution i would suggest adding an assert in the functional that ultimately null derefenced it - CheckArgument(ModuleValidator& m, ParseNode* arg, PropertyName** name): >> if (!IsDefinition(arg)) >> return m.fail(arg, "duplicate argument name not allowed"); >> >> if (arg->isKind(PNK_ASSIGN)) >> return m.fail(arg, "default arguments not allowed");

I'm not sure the MOZ_ASSERT helps very much since if this bug is made, it will immediately crash (in debug/release). Is the static analysis happier if the "if (numFormals >= 1 &&" is changed to "if (arg1 &&"?

yes this is a better way of calming Coverity.