RFR 8050485: super() in a try block in a ctor causes VerifyError
harold seigel
harold.seigel at oracle.com
Fri Aug 1 17:20:49 UTC 2014
Hi Keith,
I'm glad you are reviewing this fix. Here's some responses to your
questions:
1. This type of behavior is allowed by the JVMS. See JVM Spec 8,
section 4.10.1.4
<http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.10.1.4>:
A special form of frame assignability is allowed for an exception
handler of an |<init>| method. If the method invokes another
|<init>| method, and the invocation throws an exception, then the
current object is broken; nonetheless, a handler's target is type
safe iff all local variables in the handler's incoming type state
are |top|. This ensures that the handler's frame does not depend on
the current type.
The above text was added as part of the fix for JDK-7020118
<https://bugs.openjdk.java.net/browse/JDK-7020118>.
2. The super() call site does result in 'this' being marked as
initialized. I'll have to think some more about the scenario that you
propose.
Thanks, Harold
On 8/1/2014 12:48 PM, Keith McGuigan wrote:
> Hi Harold,
>
> Sorry for coming into this late and perhaps without full context, and
> I apologize if you've already answered this before, but I have a
> couple fundamental questions:
>
> 1. Is this type of behavior allowed by the JVMS? I seem to remember
> constraints in there that combined to flat-out prevented making a
> super() call be able to be protected by an exception handler...
> something about not being able to represent the uninit this type in
> the stack maps maybe. I could be mis-remembering here, or maybe have
> the rules been relaxed?
>
> 2. I assume that the super() call site still results in 'this' being
> marked as initialized, right? With your change, what's to stop
> someone from writing code that will stash the 'this' object into a
> static field, or stick it into the thrown exception before the
> inevitable throw occurs? If you can do this, the uninit object can
> still leak out of the method.
>
> --
> - Keith
>
>
> On Fri, Aug 1, 2014 at 11:38 AM, harold seigel
> <harold.seigel at oracle.com <mailto:harold.seigel at oracle.com>> wrote:
>
> Hi,
>
> Please review this latest webrev for fixing 8050485. The webrev
> is at: http://cr.openjdk.java.net/~hseigel/bug_8050485_3/
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8050485_3/>
>
> This new webrev has two major changes. The first change is for
> branches. For both conditional forward and unconditional forward
> branches, the implementation now parses both paths. Because the
> parsing starts at the beginning of each catch clause, and parses
> both paths at branches, backward branches can be ignored. In the
> following example, the target of the branch is 10. The
> implementation first parses the bytecodes between 2 and 10 and
> then, if needed, parses the bytecodes from 10 onward. In this
> case, it would detect the return at 2 and throw a VerifyError
> before parsing the bytecodes from 10 on.
>
> 1 goto 10
> 2 return
> 3 ifeq 2
> ...
> 10
>
> In this example the branch target is 3, the code would parse the
> the bytecodes between 1 and 3. It would see the athrow but would
> also parse the bytecodes from 3 onward to make sure that that path
> also ends in an athrow.
>
> 1 ifeql 3
> 2 athrow
> 3 ...
>
> One advantage of this approach is that it handles the following
> case. The TRY block at 5-6 does an unconditional goto around the
> catch clause to the throw at 10. But, since both paths at the
> goto are parsed, the return at 8 is detected and VerifyError is
> thrown. This enables the implementation to avoid checking for
> exception handlers for every bytecode in a catch handler.
>
> 1 public TryNestedTry2() {
> 2 try {
> 3 super();
> 4 } catch(Exception e) {
> 5 try {
> 6 xxxx();
> 7 } catch(Exception f) {
> 8 return;
> 9 }
> 10 throw e;
> 11 }
> 12 }
>
> The second major change adds support for catch clauses containing
> TRY blocks. When the implementation finds an athrow it checks to
> see if the athrow is contained in TRY blocks. If it is, then it
> pushes the starting bytecodes for those TRY blocks' catch handlers
> onto a separate stack. The bytecodes in those catch handlers are
> then parsed after all normal paths have been parsed. For example,
> it would return VerifyError for this case:
>
> static int n = 1;
> public TryNestedTryOK() {
> try {
> super();
> } catch(java.lang.VerifyError g) {
> try {
> throw g;
> } catch(Exception h) {
> n++;
> }
> }
>
> Thanks, Harold
>
>
> On 7/30/2014 4:53 PM, harold seigel wrote:
>
> Hi Dean,
>
> Thanks for finding this problem. I will look into issues with
> backward branches. Perhaps, if I scan every forward path,
> backward branches can be ignored.
>
> Harold
>
> On 7/29/2014 6:28 PM, Dean Long wrote:
>
> Checking the algorithm, it looks like there is a change of
> an infinite loop:
>
> L0:
> goto L2 // 2: set_interval(L2, code_length);
> start_bc_offset:
> L2:
> ifle L0 // 1: set_interval(L0, L2)
> L3:
> code_length:
>
> dl
>
> On 7/29/2014 9:47 AM, harold seigel wrote:
>
> Hi,
>
> Please review this updated webrev for bug 8050485.
> This update does not use recursion. Instead, it uses
> a GrowableArray to push and pop bytecode intervals
> when parsing if*, goto*, tableswitch, lookupswitch and
> athrow bytecodes. This updated webrev was tested using
> the same tests as the previous weberv.
>
> The updated webrev is at:
> http://cr.openjdk.java.net/~hseigel/bug_8050485_2/
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8050485_2/>
>
> Thanks, Harold
>
> On 7/24/2014 1:55 PM, harold seigel wrote:
>
> Hi,
>
> Please review this verifier fix for bug 8050485.
>
> The fix for JDK-8035119
> <https://bugs.openjdk.java.net/browse/JDK-8035119>
> broke existing tools, such as NetBeans Profiler,
> that generate bytecodes which place TRY blocks
> around constructor calls to super() and this().
> The purpose of that fix was to prevent exception
> handlers from handling exceptions thrown from
> super() and this(), and returning malformed
> objects to the callers of the constructors.
>
> The NB Profiler prevents malformed objects from
> being returned to the constructors' callers by
> having the exception handlers re-throw the exceptions.
>
> The purpose of this fix is to allow a TRY block
> around a constructor's call to super() and this(),
> provided that all code paths in the TRY block's
> exception handlers terminate with a throw. This
> prevents malformed objects from being returned and
> does not break tools like NB Profiler.
>
> The fix works by parsing the bytecodes inside of
> the exception handlers, making sure that all code
> paths end in an 'athrow' bytecode. Otherwise, it
> throws a VerifyError exception. This parsing is
> only done when the verifier detects a
> constructor's call to super() or this() from
> inside of a TRY block.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8050485
> Open webrev:
> http://cr.openjdk.java.net/~hseigel/bug_8050485/
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8050485/>
>
> The fix was tested with the JCK lang, vm, and
> api/java_lang tests, the UTE verifier and quick
> tests, the JTREG hotspot tests, and additional
> tests with constructor calls to super() and this()
> from inside of a TRY block, including one provided
> by NB Profiler.
>
> Thanks, Harold
>
>
>
>
>
>
>
>
> --
>
> twitter-icon-large.png
>
>
>
> Keith McGuigan
>
> @kamggg
>
> kmcguigan at twitter.com <mailto:kmcguigan at twitter.com>
>
More information about the hotspot-runtime-dev
mailing list