Default to STACK_OVERFLOW_CHECK=2 in debug builds

[sbc100]

Previously we defaulted to 1 in this case, but we want to improve the error reporting around stack overflow in preparation for lowering the default stack size. See #14177

Benchmark results:

                     Node.js_no_stack_check	Node.js_with_stack_check
              base64 1.000	0.972
        conditionals 1.000	1.096
                copy 1.000	1.003
         corrections 1.000	0.982
       corrections64 1.000	1.004
            fannkuch 1.000	1.004
         fasta_float 1.000	1.002
              havlak 1.000	1.025
                 ifs 1.000	0.962
     matrix_multiply 1.000	1.021
              memops 1.000	0.988
              primes 1.000	0.914
      primes_nocheck 1.000	1.113
            skinning 1.000	0.914
           zzz_box2d 1.000	1.076
          zzz_bullet 1.000	0.988
        zzz_coremark 1.000	0.981
         zzz_linpack 1.000	1.001
 zzz_lua_binarytrees 1.000	1.049
     zzz_lua_scimark 1.000	1.060
            zzz_lzma 1.000	0.984
         zzz_poppler 1.000	1.047
          zzz_sqlite 1.000	1.026
            zzz_zlib 1.000	0.996

[kripken]

Are the benchmarks comparing -sASSERTIONS before and after this PR (with the benchmarks modified to enable assertions), or do they compare -O3 -sSTACK_OVERFLOW_CHECK=1 vs -O3 -sSTACK_OVERFLOW_CHECK=2, or something else?

[kripken]

I think this probably makes sense to do, since performance in debug builds isn't critical, and it looks like this is not a significant regression there anyhow.

[kripken]

Are these just to add more test coverage? Or is it necessary for some reason? Please add a comment.

[kripken]

Hmm, this means the default build will still do wasm changes after link even after we default to wasm-bigint. Maybe it makes sense though.

Please add to this test. We want to emit a good error message in this case, that is, ERROR_ON_WASM_CHANGES_AFTER_LINK should do like it does with WASM_BIGINT - if it's not set right, suggest the proper value.

[sbc100]

Are the benchmarks comparing -sASSERTIONS before and after this PR (with the benchmarks modified to enable assertions), or do they compare -O3 -sSTACK_OVERFLOW_CHECK=1 vs -O3 -sSTACK_OVERFLOW_CHECK=2, or something else?

These benchmarks are actually from main I ran with:

a: ['-sASSERTIONS', '-sSTACK_OVERFLOW_CHECK=1']
b: ['-sASSERTIONS', '-sSTACK_OVERFLOW_CHECK=2']

[kripken]

Are those flags on top of -O3 or something else? Or are those all the flags passed to emcc?

[sbc100]

Are those flags on top of -O3 or something else? Or are those all the flags passed to emcc?

I think they are on top of -O3.

EmscriptenBenchmarker('Node.js no stack check', config.NODE_JS, ['-sASSERTIONS', '-sSTACK_OVERFLOW_CHECK=1'])

I think the EmscriptenBenchmarker injects OPTIMIZATIONS in addition to these flags, right?

[kripken]

Yes, that sounds right.

[sbc100]

Closing a favor of #18154. After speaking with @dschuff we decided we really didn't want to add any binaryen passed to debug builds by default since that would inhibit that ability to skip binaryen rewriting completely (which is very important for fast debug link times)