Ability to skip class file parse errors #173
Comments
|
Hi @vlsi, You may not see the error when you run the program (although Java's bytecode verifier should complain about this), but all analysis code will have to work around it, so the @nonnull should really be removed in your code. The problem of forbiddenapis is only, that you can't see the class which causes the problem :-) I can for sure fix this, by catching exception and rethroing it with class name of parsed class appended. In general the exception is not typed, so it's a bad idea to just "ignore" that class. Because AIOOBE can be thrown anywhere and generally is a bug in the code, so catching and swallowing is a bad idea. My proposal: Give me a short snippet to trigger that bug (tell me what to checkout and what to run), so I can figure out for you which class file on your side causes the bug. In a later verison of forbiddenapis I will add a better error reporting when parsing with ASM fails with RuntilException subclasses. The correct way to handle this on your side is:
|
|
Background:
So I will also close the underlying issue as "won't fix, because the class files are broken and manual care has to be taken to either fix the class files - they should never be released in this broken state on Maven Central!!! - or exclude them manually. I will just create a pull request to give better information. To quickly help you and get out of the issue and figure out which class is causing this, it might me a good idea to run AsmVerifier to check all class files. |
forbidden-apis could catch the exception from parsing the class and convert it to a warning. |
No, that wont happen. Excactly for that case you are free to put classes on exclude list. The problem is that ASM does not return proper exceptions, so you can't for sure differentiate between a bug in forbiddenapis code and a broken class file. I tried my best in the given PR #174, but converting it to a warning should not be done. Pull request is here: #174 |
|
In short: The ability is there to ignore classes, but that must be done explicit: Use Gradle |
|
My strong recommendation is: Remove the @nonnull in your code for the cases that produce broken class files! Otherwise this may cause horrible bugs for frameworks using Calcite (like Servlet Containers parsing class files). |
|
Hi, I will add it to exclude list and check the others. |
|
Hi, You can use this as a workaround for now. I have checked the source code file and your changes, it's not clear which of the above listed bugs cause this. But be warned, any consumer of Calcite using this class with some framewroks may fail in same way! |
|
The bug is a well known Java 8: The problem are @nullable used inside a So unless you want to take the risk of producing broken class files, I'd recommend to change line 100 and 439 of RusltSetEnumerable to not use @nullable. The bug is described here: https://gitlab.ow2.org/asm/asm/-/issues/317886 (not sure which one is the correct javac bug) |
Ah, the same class causes checkerframework crash: typetools/checker-framework#3612 :) I guess the class begs refactoring to become more compiler-friendly :) |
|
I assume checkerframework uses ASM internally, too. |
It seems to crash for a different reason: it works OK for annotated version, and it crashes when annotations are missing. If I have both checkerframework and forbidden-apis, it crashes in any case :) |
|
See my comments in your pull request, I have a fix to maybe also fix both issues. Problem seems "lambda inside other lamda" f*cking up javac and maybe also checkerframework. |
|
One question out of interest: Is forbiddenapis also checking your kotlin classes? This would really be interesting. Actually, it should do this, if the SourceSet contains all classes, but I haven't verified. |
|
I verified - it does! COOL :-) (I just injected a bad class file into build/test/kotlin). |
|
We don't have much Kotlin yet: it is configured only for test classes. Good to know it parses Kotlin-generated bytecode as well :-) |
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
Add better reporting if ASM fails to parse a class with an unspecified RuntimeException
|
Hi, |
|
About the comments to print some more information when forbiddenapis crushes with a RuntimeException is a separate issue. It will then provide classfile and/or version number as a log entry. The issue here was more about ASM not able to parse error, which is a bug in input data so needs to be handled differently. |
|
I opened an issue about the additional "parsing state" printed to error logger in case of exception: #175 |
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
This reduces the likelihood of javac issues. See policeman-tools/forbidden-apis#173
I've tried
de.thetaphi.forbiddenapis.version=3.0.1and the error is still there, so I guess the's ASM issue.I've a case when forbidden-apis task fails with
ArrayIndexOutOfBoundsException, and it looks like there's no way to proceed :-/CI failure: https://github.com/apache/calcite/runs/1058616825?check_suite_focus=true#step:6:430
Code: apache/calcite#1929
Unfortunately, the exception does not tell which of the classes failed to parse, and there's no way to skip the failure.
Could you please print "unparseable" class file names in the warning messages and provide users the ability to skip unparseable class files?
I think it would be OK to skip the analysis by default in case ASM fails to parse the file.
Here's the stacktrace:
The text was updated successfully, but these errors were encountered: