RFR: [8218152] javac fails and exits with no error if a bad annotation processor is on the classpath
Steve Groeger
GROEGES at uk.ibm.com
Wed Feb 13 09:58:05 UTC 2019
Hi Jan,
Thanks for the response.
With regards to your comments re:
>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);
>> }
>> }
>---
I can do it the waty you sugegsted but I had done it this way because
Jonathan Gibbons had stated:
>> 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.
Either way is OK with me. We just need to make a decision on which is the
preferred way - then I can update the webrevs.
>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
I will take yopur advice on the tests and see if I can do this
programtically rather than havng the binary files in the repository.
Will post again in the mailing list when I have updated webrevs that are
suitable for being reviewed.
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: Jan Lahoda <jan.lahoda at oracle.com>
To: Steve Groeger <GROEGES at uk.ibm.com>, compiler-dev at openjdk.java.net
Date: 11/02/2019 12:45
Subject: Re: RFR: [8218152] javac fails and exits with no error if
a bad annotation processor is on the classpath
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://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8196182&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=mtVljMuzQaGWDLF3MWIp9x8aotlYKRR0DpRI9BjDwuI&e=
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://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8218152-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=GXmb6NEDC3BhXyEw8uaNod2uN_hYX-9HpHkoiGhEujY&e=
> [2]
>
_https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openjdk.java.net_pipermail_compiler-2Ddev_2019-2DJanuary_012920.html-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=AhWE854aM6al5na1Eo9sxF4IoWBZC3jfvLHRo2a2ETo&e=
> [3]
_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk8u_webrev.01_-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=OM__1ZIlBCUXiHNLfltEv9c_6a8k8nspR0KnKjRT8II&e=
> [4]
_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk_webrev.01_-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=HrpL5tvveJrGX0ABN9uGRn0bZV1nsiNxzuLOAVem3Z0&e=
>
> 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]
_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk8u_webrev.00_-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=JoEgW12V0tuoZvuG9GDF7g5YmXhjGolWyibAYlifWME&e=
> [2]
_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk_webrev.00_-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=NeM6uUrEq-vcqXQ5uHonCor_ChwsY20notpASJS4is8&e=
>
[3_https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8218152-5F&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=3eyXaufRP0Xc78gxZMprIOVTveYQ-jSaXZglJ3ZxTMo&s=GXmb6NEDC3BhXyEw8uaNod2uN_hYX-9HpHkoiGhEujY&e=
>
> 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
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190213/0cdbc40c/attachment-0001.html>
More information about the compiler-dev
mailing list