duktape icon indicating copy to clipboard operation
duktape copied to clipboard

Omitted internal duk_require_constructor_call() with GCC 5+, noreturn enabled, debugger support enabled

Open svaarala opened this issue 5 years ago • 1 comments

Outward symptoms, for make duk on GCC 5+:

duk> ArrayBuffer()
= ||

Expected result is a TypeError:

duk> ArrayBuffer()
TypeError: constructor requires 'new'
    at [anon] (duk_api_call.c:386) internal
    at ArrayBuffer () native strict preventsyield
    at global (input:1) preventsyield

The direct reason for the missing TypeError is that the entire call to duk_require_constructor_call() is not emitted by the compiler for:

DUK_INTERNAL duk_ret_t duk_bi_arraybuffer_constructor(duk_hthread *thr) {
        [...]

        duk_require_constructor_call(thr);   /* <== not emitted */

        len = duk_to_int(thr, 0);
        [...]

Looking at the assembler output, the broken output just omits the entire call (and it's not inlined either):

000000000001e41a <duk_bi_arraybuffer_constructor>:
   1e41a:       55                      push   %rbp
   1e41b:       53                      push   %rbx
   1e41c:       31 f6                   xor    %esi,%esi
   1e41e:       48 89 fb                mov    %rdi,%rbx
   1e421:       48 83 ec 08             sub    $0x8,%rsp
   1e425:       e8 b7 fe ff ff          callq  1e2e1 <duk_to_int>
   1e42a:       85 c0                   test   %eax,%eax
   [...]

whereas the correct output has:

000000000001c145 <duk_bi_arraybuffer_constructor>:
   1c145:       55                      push   %rbp
   1c146:       53                      push   %rbx
   1c147:       48 89 fb                mov    %rdi,%rbx
   1c14a:       48 83 ec 08             sub    $0x8,%rsp
   1c14e:       e8 51 72 ff ff          callq  133a4 <duk_require_constructor_call>
   1c153:       31 f6                   xor    %esi,%esi
   1c155:       48 89 df                mov    %rbx,%rdi
   1c158:       e8 2e ae ff ff          callq  16f8b <duk_to_int>

The duk_require_constructor_call() is simply:

DUK_EXTERNAL void duk_require_constructor_call(duk_hthread *thr) {
        DUK_ASSERT_API_ENTRY(thr);

        if (!duk_is_constructor_call(thr)) {
                DUK_ERROR_TYPE(thr, DUK_STR_CONSTRUCT_ONLY);
                DUK_WO_NORETURN(return;);
        }
}

I don't yet have a good explanation why this happens. The conditions required to trigger this are roughly:

  • GCC 5+ used.
  • Noreturn macros enabled, i.e. gcc is given noreturn function attributes.
  • Debugger support is enabled.

Conversely, the issue can be avoided by (other than not using GCC 5+) either:

  • Disabling noreturn function attributes; I'll merge this as a short term fix soon.
  • Disabling debugger support.

I haven't yet isolated the root cause, but after some quick troubleshooting the issue also disappears by adding a dummy printf() in duk_require_constructor_call():

DUK_EXTERNAL void duk_require_constructor_call(duk_hthread *thr) {
        DUK_ASSERT_API_ENTRY(thr);

        if (!duk_is_constructor_call(thr)) {
                printf("aiee\n");  /* <== this eliminates the issue */
                DUK_ERROR_TYPE(thr, DUK_STR_CONSTRUCT_ONLY);
                DUK_WO_NORETURN(return;);
        }
}

Not sure why this would matter, but from the compiler point of view that printf() is at least a side effect that cannot be omitted. So, conversely, maybe from the point of view of GCC 5+ all the stuff in the error path seems side effect free and thus OK to omit entirely.

Also, commenting out certain debugger method(s) eliminates the issue. IIRC it suffices to comment out the Eval command -- regardless of whether it's ever executed. This is interesting because even if there was "undefined behavior" or similar in there, should it affect any code prior to that?

I've tested this with other GCC versions and Clang, and it doesn't seem to affect older GCC versions or Clang. I'll obviously try to figure out a root cause but for now, I'll disable noreturn for GCC 5+.

svaarala avatar Jul 23 '19 22:07 svaarala

Sounds like a compiler bug to me.

fatcerberus avatar Aug 17 '19 19:08 fatcerberus