RFR: [8218152] javac fails and exits with no error if a bad annotation processor is on the classpath
Jan Lahoda
jan.lahoda at oracle.com
Mon Feb 11 12:45:28 UTC 2019
Hi Steve,
I apologize for a late reply.
One thing to note is that according to the ServiceLoader spec, the
LinkageErrors shouldn't be thrown out of the Iterator. That is my
reading at least. Please see:
https://bugs.openjdk.java.net/browse/JDK-8196182
Given there are some issues associated with that, I don't see a problem
with workarounding that in javac. But I guess I'd personally rather kept
it simple. Would there be an issue with simply changing the "catch
(Throwable)" to also report the same error as "catch
(ServiceConfigurationError)"? I.e. something like:
---
> diff -r 4ef401769c36 src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java Fri Feb 08 14:06:35 2019 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java Mon Feb 11 13:33:50 2019 +0100
> @@ -437,6 +437,7 @@
> log.error(Errors.ProcBadConfigFile(sce.getLocalizedMessage()));
> throw new Abort(sce);
> } catch (Throwable t) {
> + log.error(Errors.ProcBadConfigFile(t.getLocalizedMessage()));
> throw new Abort(t);
> }
> }
> @@ -453,6 +454,7 @@
> log.error(Errors.ProcBadConfigFile(sce.getLocalizedMessage()));
> throw new Abort(sce);
> } catch (Throwable t) {
> + log.error(Errors.ProcBadConfigFile(t.getLocalizedMessage()));
> throw new Abort(t);
> }
> }
---
Looking at the tests, one thing to note is that binary files are
generally not allowed in the repository. I wonder if a test that would
run javac programmaticaly wouldn't be easier to do, see e.g.:
test/langtools/tools/javac/processing/rounds/GetElementsAnnotatedWithOnMissing.java
This should allow arbitrary changes to the classfiles, so that binaries
don't need to be in the repository.
Thanks,
Jan
On 11.2.2019 12:30, Steve Groeger wrote:
> Hi all,
>
> Is there anyone out there that would be able to review the webrevs
> mentioned in the earlier post.
> Jonathan Gibbons has agreed to sponsor these changes but we need 2
> reviewers. Anyone?
>
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: groeges at uk.ibm.com
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
>
> From: Steve Groeger <GROEGES at uk.ibm.com>
> To: compiler-dev at openjdk.java.net
> Date: 01/02/2019 12:47
> Subject: RFR: [8218152] javac fails and exits with no error if a bad
> annotation processor is on the classpath
> Sent by: "compiler-dev" <compiler-dev-bounces at openjdk.java.net>
> ------------------------------------------------------------------------
>
>
>
> Hi all,
>
> I have made some changes to the code for issue [1] for both jdk8u and
> jdk (jdk13)
> The latest webrevs have a few extra changes suggested by Jonathan
> Gibbons mentioned in an earlier mailing [2]
> The webrevs are for jdk8u [3] and for jdk [4].
> Please could someone review these changes and let me know if there are
> any issues.
>
> [1] _https://bugs.openjdk.java.net/browse/JDK-8218152_
> [2]
> _https://mail.openjdk.java.net/pipermail/compiler-dev/2019-January/012920.html_
> [3] _http://cr.openjdk.java.net/~sgroeger/8218152/jdk8u/webrev.01/_
> [4] _http://cr.openjdk.java.net/~sgroeger/8218152/jdk/webrev.01/_
>
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: groeges at uk.ibm.com
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
>
> From: Jonathan Gibbons <jonathan.gibbons at oracle.com>
> To: Steve Groeger <GROEGES at uk.ibm.com>
> Cc: compiler-dev at openjdk.java.net
> Date: 31/01/2019 16:56
> Subject: Re: [NEW_BUG] javac fails and exits with no error if a bad
> annotation processor is on the classpath
> ------------------------------------------------------------------------
>
>
>
> Steve,
>
> When you post the new webrevs, can I suggest that you change the Subject
> line of this thread to something like
>
> RFR: 8218152 [javac] fails and exits with no error if a bad annotation
> processor provided
>
> That will help get attention from reviewers, and help any subsequent
> bug-archaeology.
>
> I can sponsor the changes for you once they have been reviewed.
>
> -- Jon
>
> On 1/31/19 8:49 AM, Steve Groeger wrote:
> Hi Jonathan,
>
> Thanks for responding.
>
> I have created a couple of webrevs for jdk8u [1] and jdk [2] which also
> include some JTReg tests.
> I would be grateful if you, or someone else could review these just to
> ensure I am doing the right thing.
> I will update the code with the additional 'throw FatalError' that you
> mentioned and will then create some new webrevs
>
> If you are able to sponsor these changes for me I would be very grateful.
>
> PS. As you saw I have created a JBS issue for this here [3]
>
> [1] _http://cr.openjdk.java.net/~sgroeger/8218152/jdk8u/webrev.00/_
> [2] _http://cr.openjdk.java.net/~sgroeger/8218152/jdk/webrev.00/_
> [3_https://bugs.openjdk.java.net/browse/JDK-8218152_
>
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: _groeges at uk.ibm.com_ <mailto:groeges at uk.ibm.com>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
>
> From: Jonathan Gibbons _<jonathan.gibbons at oracle.com>_
> <mailto:jonathan.gibbons at oracle.com>
> To: Steve Groeger _<GROEGES at uk.ibm.com>_ <mailto:GROEGES at uk.ibm.com>,
> _compiler-dev at openjdk.java.net_ <mailto:compiler-dev at openjdk.java.net>
> Date: 31/01/2019 16:33
> Subject: Re: [NEW_BUG] javac fails and exits with no error if a bad
> annotation processor is on the classpath
>
> ------------------------------------------------------------------------
>
>
>
> Steve,
>
> Your fix is generally good: Abort should only be used after a suitable
> message has been reported.
>
> There's a `catch (Throwable t)` in the same set of catch blocks that you
> did not update. Rather than add yet another message for that case, it
> might be better to throw FatalError, to provoke the standard "javac
> crash" output for the unknown exception.
>
> You should provide a test case ... i.e. as a jtreg test.
>
> -- Jon
>
> On 1/29/19 3:18 AM, Steve Groeger wrote:
> Hi all,
>
> I have identified a potential issue where javac fails and exits with no
> error if a bad annotation processor is on the classpath.
>
> Background: an Annotation Processor class can be packaged in a jar file
> and is identified by a special text file inside the jar named:
> META-INF/services/javax.annotation.processing.Processor. This text file
> has only one line, specifying the class name of the annotation process
> class to run. When javac runs, it checks all jars on the classpath and
> if it finds the special text file in any jar file, then it attempts to
> run the class listed there.
>
> The issue here is that when the annotation processor class can't be
> executed, javac catches the exception and exits without reporting on the
> reason for the exit. Examples of reasons why the annotation processor
> can't be executed include:
>
> * the annotation processor class is compiled for a higher version of
> Java than the javac is running, giving UnsupportedClassVersionError.
> * the annotation processor class file is corrupt so can't be loaded.
>
>
> In either of the above cases javac will swallow the error and exit
> without telling the user why it failed to compile the java source code
> as expected.
>
> Testcase
> put HelloWorld.java in current directory, edit <path_to_jar> and run:
> javac -cp .:<path_to_jar>/bad.annotation.processor.jar
> HelloWorld.java
>
> Instead of producing HelloWorld.class as expected, javac exits silently.
>
> Here, bad.annotation.processor.jar is a file that I created to contain:
> META-INF/services/javax.annotation.processing.Processor <<-- text
> file to list bad class
> bad/notaclass.class <<-- dummy file, can't be loaded as a java class
>
> Does anyone know whether this is a bug and that this should throw an
> error in these cases?
> If so, I will raise a bug for this.
>
> I have looked at the code and seen where javac Aborts the processing,
> and if I make the following changes the javac detects the issue and
> reports an error before Aborting.
>
> diff -r 5178e4b58b17
> src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
> ---
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
> Mon Jan 28 09:56:00 2019 +0100
> +++
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
> Tue Jan 29 11:14:57 2019 +0000
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -436,6 +436,12 @@
> } catch(ServiceConfigurationError sce) {
>
> log.error(Errors.ProcBadConfigFile(sce.getLocalizedMessage()));
> throw new Abort(sce);
> + } catch (UnsupportedClassVersionError ucve) {
> +
> log.error(Errors.ProcCantLoadClass(ucve.getLocalizedMessage()));
> + throw new Abort(ucve);
> + } catch (ClassFormatError cfe) {
> +
> log.error(Errors.ProcCantLoadClass(cfe.getLocalizedMessage()));
> + throw new Abort(cfe);
> } catch (Throwable t) {
> throw new Abort(t);
> }
>
> and to add a new message there is this change
>
> diff -r 5178e4b58b17
> src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> ---
> a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> Mon Jan 28 09:56:00 2019 +0100
> +++
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> Tue Jan 29 11:16:03 2019 +0000
> @@ -1,5 +1,5 @@
> #
> -# Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights
> reserved.
> +# Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights
> reserved.
> # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> #
> # This code is free software; you can redistribute it and/or modify it
> @@ -1051,6 +1051,10 @@
> compiler.err.proc.cant.find.class=\
> Could not find class file for ''{0}''.
>
> +# 0: string
> +compiler.err.proc.cant.load.class=\
> + Could not load processor class file due to ''{0}''.
> +
> # Print a client-generated error message; assumed to be localized, no
> translation required
> # 0: string
> compiler.err.proc.messager=\
>
>
> If this is deemed a bug and the change seems OK, please could someone
> sponsor this for me and I will create a proper webrev for the change for
> a full review.
>
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: _groeges at uk.ibm.com_ <mailto:groeges at uk.ibm.com>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
More information about the compiler-dev
mailing list