Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interaction of try...catch_br l with unwind blocks. #130

Closed
ioannad opened this issue Oct 1, 2020 · 10 comments
Closed

Interaction of try...catch_br l with unwind blocks. #130

ioannad opened this issue Oct 1, 2020 · 10 comments

Comments

@ioannad
Copy link
Collaborator

ioannad commented Oct 1, 2020

Say we have a catch_br l in the try instructions of a try...unwind...end instruction (an unwind block), and l is "outside" the unwind block, and an exception is thrown by the try of the try...catch_br l. IIUC, the thrown exception is then caught in the try block with label l, and it seems reasonable to assume that, when unwinding that throw, the cleanup forms of the intermediate unwind block would get triggered. This is how I described the interaction between catch_br and unwind in my draft formal spec for this 3rd EH proposal. Is this the intended behaviour?

A concrete example in case my explanation wasn't clear enough.

i32.const 11
global.set 0
try $l
  try
    try
      throw x
    catch_br $l
  unwind
    i32.const 27
    global.set 0
  end
catch_all
end
global.get 0

Does the above return 27 (as I would expect), or 11?

The question in general:

Does catch_br l trigger unwind-cleanup code from unwind blocks between itself and its destination label l?

@KronicDeth
Copy link

@ioannad here's a section from the meeting notes that may be relevant:

LI: How does catch_br interact with unwind, I assume it skips the catch branches, similar to unwind?

HA: we don’t need to consider unwind and catch_br...

RT: I think you can think of catch_br saying reuse the handler, so unwind still uses the same unwind.

HA: we need to consider unwind, because try can have labels, we can consider unwind as a target for catch_br, we don’t need to rename it. It helps us find next frame to search for, if it says unwind, continue the search. Catch_br tells us which to check next, it can be either unwind or catch. In second phase, we have to visit every unwind block. To visit the right sequence of unwind blocks, we need that catch_br.

LI: Yes, that's what I thought. And you addressed my other concern which is that catch_br can branch to another unwind. Combining the catch/unwind names would be verbose.

@ioannad
Copy link
Collaborator Author

ioannad commented Oct 1, 2020

@KronicDeth, thank you so much, it seems I didn't understand that part of the conversation live (I was in that meeting 😊 )

Catch_br tells us which to check next, it can be either unwind or catch.

This makes me think I might have misunderstood the roles of catch_br and unwind.

To visit the right sequence of unwind blocks, we need that catch_br.

This seems to say that my example above should return 11, not 27.

So it seems that during a throw, not all intermediate unwind blocks execute their cleanup forms, but only those targeted by a catch_br? I'm confused.

@KronicDeth
Copy link

This seems to say that my example above should return 11, not 27.

That is my interpretation. catch_br can target either catch or an unwind, so since it doesn't target the unwind it is explicitly wanting to jump over it.

So it seems that during a throw, not all intermediate unwind blocks execute their cleanup forms, but only those targeted by a catch_br? I'm confused.

I think that's only true upto the try's catch or unwind that matches the label for catch_br. So in your code, it jumps directly to the catch_all, but any unwinds above that would be used if the catch_all was changed to rethrow or throw a new exception.

@ioannad
Copy link
Collaborator Author

ioannad commented Oct 1, 2020

Ok, that makes sense, thank you!

It's late here now, I'll edit the draft formal spec comment to match this semantics tomorrow or (probably) Monday.

@ioannad
Copy link
Collaborator Author

ioannad commented Oct 6, 2020

To close this issue, @aheejin do you agree with @KronicDeth 's interpretation?:

catch_br can target either catch or an unwind, so since it doesn't target the unwind it is explicitly wanting to jump over it.

Where "jump over it" means that the unwind block's cleanup forms are not executed, I presume.

@aheejin
Copy link
Member

aheejin commented Oct 6, 2020

Sorry for the late reply. catch_br can target either catch(_all) or unwind. So your example returns 11.

@RossTate
Copy link
Contributor

RossTate commented Oct 6, 2020

Looking ahead for compatibility with possible extensions, what happens if one were to do an unwinding branch (#124) to label $l if that label is declared as follows:

try $t
  block $l
    try
      try
        ... ;; unwinding branch to $l from within here
      catch_br $t
    unwind
      ...
    end
  end
  ...
catch_all
  ...
end

@aheejin
Copy link
Member

aheejin commented Oct 6, 2020

We don't have a concept called 'unwinding branch' now, so I'm not sure if that can be answered. Also unwind is different from finally; it is semantically the same as catch in the current proposal, before we add two-phase.

@RossTate
Copy link
Contributor

RossTate commented Oct 6, 2020

We don't have a concept called 'unwinding branch' now, so I'm not sure if that can be answered.

My concern is that all the answers I have thought of have problems with this skip-over semantics from what I can tell. I had originally thought catch_br was supposed to reuse an outer try's catch or unwind, which I had checked avoids these problems.

@aheejin
Copy link
Member

aheejin commented Oct 6, 2020

@RossTate

I had originally thought catch_br was supposed to reuse an outer try's catch or unwind, which I had checked avoids these problems.

How are these two different? I mean, "Reuse an outer try's catch/unwind" vs. "skip over semantics"

ioannad pushed a commit to ioannad/exception-handling that referenced this issue Feb 23, 2021
Ms2ger pushed a commit to Ms2ger/exception-handling that referenced this issue Jun 24, 2021
* Fix outdated stuff

* Update Overview.md

* Update Overview.md

* Update Overview.md

* Clarify zero table index

* Address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants