Remove exceptions pass: introduce per-function operation #1721
Conversation
This replaces function_name#exception_value with a single java::@inflight_exception variable. No more than one #exception_value variable could be populated at any given time in any case; this reduces the number of globals introduced, and as an added bonus will make it easier to write a per-function version of remove_exceptions.
This removes high-level exception-related instructions from a single function, at the cost of some accuracy as we can no longer analyse other functions to determine whether or not they may throw, and therefore always assume they will.
smowton
requested review from
cesaro,
kroening,
mgudemann,
peterschrammel,
tautschnig and
thk123
as code owners
|
Is this code already being exercised by tests? Else, would it be possible to add some tests? Extra brownie points for making the imprecision show up in the test (for example, by outputting the goto program). |
|
No, but it will be when the lazy-loading-via-symex PR arrives. At that point I'll add tests for exceptions+lazy-loading-v2, since that'll be the primary user. This PR is basically to provide a place to review this change in isolation. |
|
FWIW: looks ok to me, save for the required rebase (which will make Doxygen on Travis happy) and the linter issue. |
| /// Note this is somewhat less accurate than the whole-goto-model version, | ||
| /// because we can't inspect other functions to determine whether they throw | ||
| /// or not, and therefore must assume they do and always introduce post-call | ||
| /// exception dispatch. |
There was a problem hiding this comment.
Consider explaining why we would use this version vs. the more precise version above?
|
CBA, sorry ;) The lazy loading v2 tests should incorporate this; I'll add units too if at that point we still think test coverage is weak. |
|
Subsumed by #1759 |
Depends on #1719; please only review the second commit.
This introduces a per-function entry point for remove-exceptions. Unlike the PR for remove-virtual-functions I don't actually use it yet because it does result in more unreachable code; I'll enable this only when lazy-loading v2 (symex-driven function loading) is enabled.
Therefore don't merge this yet since it isn't tested, but this PR provides an easy place to review the change in isolation.