inlining unwind breaks eh intrinsics

[llvmbot]

Bugzilla Link	2149
Resolution	INVALID
Resolved on	Apr 24, 2010 07:06
Version	2.2
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@jayfoad

Extended Description

The comment at the top of HandleInlinedInvoke() in lib/Transforms/Utils/InlineFunction.cpp says:

/// HandleInlinedInvoke - If we inlined an invoke site, we need to convert calls
/// in the body of the inlined function into invokes and turn unwind
/// instructions into branches to the invoke unwind dest.

I don't think it's safe to inline an unwind instruction and turn it into a branch to the invoke unwind destination, because any calls to llvm.eh.exception and llvm.eh.selector at the destination won't work.

(I'm assuming that the code generator implements these eh intrinsics by getting the exception pointer and exception selector from well-known native registers which are set up as part of the stack unwinding process. If you branch directly to the invoke unwind destination then these registers won't be set up.)

My suggested solution is not to inline any function containing an unwind instruction.

[llvmbot]

Currently the unwind instruction is not handled at all by codegen,
so there can't be a problem due to this transform! Unwinding is
actually handled by calls to runtime functions (these calls are
generated by llvm-gccc), for which everything works correctly.
I actually have a plan to make it possible to use the unwind
instruction, which does indeed require copying the contents of
the eh registers into a variable at the start of each landing
pad, and having the intrinsics grab the value of the variable
not of the register directly. I'm marking this report invalid,
since it is. Please feel free to re-open if you have a testcase.

[llvmbot]

Runtime calls are how throwing is currently implemented, but unwind is in some sense the official llvm way to throw an exception (as specified by the LangRef). That it has been completely disabled seems to indicate a serious bug. Either unwind should work (in some situations), or it should be removed from the instruction set. If the LangRef had a large warning that you have to run the LowerInvoke pass for unwind to behave as specified, that might also be an acceptable solution.

So it sounds much more like won't fix than invalid.

[llvmbot]

The bug described in the PR doesn't exist, which sounds invalid to me.
There is a different bug, which is that unwind doesn't get codegen'd.
That can very reasonably get it's own PR. In any case lack of unwind
codegen isn't "wontfix" because we want to fix it, only it's not clear
what the best way to do that is.

[llvmbot]

fair enough.

[llvmbot]

Currently the unwind instruction is not handled at all by codegen,
so there can't be a problem due to this transform!

I'm not saying that the generated code will work if you don't do the transform, I'm just saying that it won't work if you do. And if you do, codegen will never see an unwind instruction.

Regardless of what the code generator currently does with unwind, I thought the idea of llvm.eh.exception was that it would only work if you reached it by doing an unwind (or possibly by doing some target-specific stuff outside the scope of the LLVM language spec, like calling the runtime functions you mention).

But having looked at the LLVM exception handling spec again, it doesn't say anything at all about this, or what the pointer returned by llvm.eh.exception is or where it comes from.

[llvmbot]

I'm not saying that the generated code will work if you don't do the
transform, I'm just saying that it won't work if you do. And if you do,
codegen will never see an unwind instruction.

I'm not sure what you are saying. You seem to be unhappy because the
current implementation of the intrinsics wouldn't work properly after
this transform if codegen was changed to do something interesting with
an unwind instruction. I tried to explain that there is an alternative
implementation of the intrinsics which means that they would work properly
even in the presence of this transform, in that happy future time when
unwind gets codegened. Also see below.

Regardless of what the code generator currently does with unwind, I thought
the idea of llvm.eh.exception was that it would only work if you reached
it by doing an unwind (or possibly by doing some target-specific stuff
outside the scope of the LLVM language spec, like calling the runtime
functions you mention).

That is correct. I think what you're missing is that the definition of
the unwind instruction is most likely going to have to change before we
are able to codegen it at all. One suggestion was to use it as a "rewind"
instruction, used for rethrowing an exception that already occurred.
That way the intrinsics would simply get the values for the exception
being handled. Unfortunately this doesn't interact very well with the
nounwind attribute.

[llvmbot]

You seem to be unhappy because the
current implementation of the intrinsics wouldn't work properly after
this transform if codegen was changed to do something interesting with
an unwind instruction.

It doesn't matter what codegen does with an unwind instruction, because this transform removes the unwind instruction, so codegen won't see it.

(In any case I'm more interested in the definition of the LLVM language than in the current state of codegen. Codegen isn't the only way of executing LLVM, e.g. there's the interpreter, and custom backends like the C backend. The LLVM language ought to be defined independently of any particular execution mechanism.)

Regardless of what the code generator currently does with unwind, I thought
the idea of llvm.eh.exception was that it would only work if you reached
it by doing an unwind [...]

That is correct.

So branching directly to the llvm.eh.exception won't work. But that's what this transform does: it replaces an unwind with a direct branch.

Anyway, I guess there's no point arguing about it if the whole unwind mechanism needs redesigning anyway.

[llvmbot]

Regardless of what the code generator currently does with unwind, I thought
the idea of llvm.eh.exception was that it would only work if you reached
it by doing an unwind [...]

That is correct.

So branching directly to the llvm.eh.exception won't work. But that's what this
transform does: it replaces an unwind with a direct branch.

Sorry, I said it wrong: llvm.eh.exception only works if you called the
runtime library's unwinder routine. That routine has nothing to do
with unwind. The problem is that there is no obvious way to turn an
unwind instruction into a call to the runtime unwinder, except if we
are currently handling an exception in which case unwind could be used
as a rethrow.

Anyway, I guess there's no point arguing about it if the whole unwind mechanism
needs redesigning anyway.

Right, there is an impedance mismatch between the original LLVM concepts
(unwind, invoke) and the dwarf unwinder (selectors, exception objects).
Currently they are cobbled together, but the whole thing needs a careful
rethink. Currently only codegen knows what to do with eh.selector calls,
and it's not at all clear what the intepreter should do to handle the
personality function. On the other hand, the interpreter knows how to
handle an invoke but codegen isn't capable of handling an invoke unless
there is an eh.selector call in the landing pad.