This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug go/60406] recover.go: test13reflect2 test failure


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406

--- Comment #25 from Ian Lance Taylor <ian at airs dot com> ---
(In reply to Dominik Vogt from comment #22)
> >> Hm, so the patch penalises platforms that cannot deal with the
> >> 16 byte window?
> 
> > Yes, but, recall that on your system almost all tests pass using the
> > code that is in the tree today, before your patch.
> 
> But they really only succeed "by accident".  The call for any platform might
> introduce control flow into the thunk and trigger the problem in any of the
> test cases.

Yes, I understand that.

You suggested that my patch penalized all cases where we have a control flow
problem, because it will do a more costly unwind step.  That is true.  However,
in practice, the case arises very rarely.  We know that the code in the thunk
is always very simple:

    if (__go_set_defer_retaddr (&&L))
      goto L;
    real_function(args);
  L:

The compiler is not going to start randomly throwing additional control flow
statements into that function.  The cases where it does do that are the cases
where args is very large, so that it uses a block copy to copy them onto the
stack.  But that essentially never happens.  The args here are the args in
"defer real_function(args)".  90% of the time there are no arguments at all. 
9.9% of the time it's just a couple of pointer/scalar arguments.  In real code
nobody ever calls defer with large arguments, because it's just a strange way
to write; people write a function closure instead.  The only code in the
standard library that calls defer with a large argument is the recover.go test,
to make sure that it works.

It's necessary to handle all cases correctly.  But there is nothing wrong with
using an efficient mechanism when it works, as long as we can correctly fall
back to a more expensive mechanism when it fails.  I believe that my patch does
that.

If you like, we can incorporate your patch too, as long as it is only an
addition to the existing code.  Before calling runtime_callers, we can call
_Unwind_FindEnclosingFunction.  Then we need only go on to the runtime_callers
code if _Unwind_FindEnclosingFunction returns NULL, or in the cases where
d->__makefunc_can_recover is true.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]