Community
Participate
Working Groups
NullPointerException thrown when creating messages generated by declare error. Test in tests/bug/systemouts. When run from the command-line, it produces an NPE in CompilationResult.computePriority(CompilationResult.java:110). When run from the harness, it produces an NPE in BcelShadow.findHandlerParamName(BcelShadow.java:441). (That they are different is not good, of course.) Aside from checking in the test, I will attach the stack traces and commands. True of the current tree but not of AspectJ 1.1.1.
Created attachment 9210 [details] two traces and commands
Moving to P2/1.2 for investigation before 1.2rc1 b/c a common-case crash.
This appears to have been introduced with the fix for 50570. Based on the notes in 50570 I believe it is OK to simply guard the line that results in the NPE and return '<missing>' as handler param name.
wrt your note, there might be two bugs here. When I comment out the handler, I still get the second trace (computePriority). Sorry for not isolating better. (I should also have generated -g variants in the test cases.)
I'm looking at the second problem (computePriority) now. I can basically see why it is happening - it is related to determining the file against which to report the problem. In the case of a binary weave against something from a jar there are problems determining the originating file. Interestingly (I didn't know this), the default option to the javac task in an ANT script is no debug - this means javac is invoked with "-g:none" as an input flag. This appears to discard line number tables. On a binary weave this means for any shadow we have no idea which line number it originated from. Have we always had this problem? I guess it doesn't make too much difference when weaving in advice, but when weaving declare/error warning that need to name the offending lines - it is unfortunate!
Just fyi, I get the same results when weaving classes produced by Eclipse's JDT compiler. (For JDT, I have all the debug flags enabled.)
This bug is a right can of worms !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! I have fixed the NPE - but I'm still not happy with all the messages that come out. Right now I've switched to using classes with the debug attributes in (line number table), I'll try it without once I'm happy with what this case is producing. Here are 3 errors messages as of my system 2 minutes ago: Here is a typical message when we compile 'ajc X.java A.java' - i.e. all source: C:\Eclipse\212\eclipse\aspectj_clean_ws\tests\bugs\systemouts\X.java:22 error NPE System.out.print(" "); ^^^^^^^^^^^^^^^^^^^^^^ field-get(java.io.PrintStream java.lang.System.out) see also: C:\Eclipse\212 \eclipse\aspectj_clean_ws\tests\bugs\systemouts\A.java:5 Here is the message when we compiled using a binary form of X 'ajc -inpath p A.java' where directory p contains just X.class: pack\X.java:13 error NPE (no source information available) method-call(void java.lang.RuntimeException.printStackTrace (java.io.PrintStream)) see also: C:\Eclipse\212 \eclipse\aspectj_clean_ws\tests\bugs\systemouts\A.java:5 see also: C:\Eclipse\212 \eclipse\aspectj_clean_ws\tests\bugs\systemouts\p\pack\X.class And here is what we get when X.class is packaged into the jar file 'ajc - inpath injar.jar A.java': pack\X.java:27 error NPE (no source information available) field-get(java.io.PrintStream java.lang.System.out) see also: C:\Eclipse\212 \eclipse\aspectj_clean_ws\tests\bugs\systemouts\A.java:5 see also: C:\Eclipse\212 \eclipse\aspectj_clean_ws\tests\bugs\systemouts\injar.jar With this kind of output, the test that Wes created does pass, but I'm not happy with the message content. Anyone want to comment on those messages? I'm reasonably happy with the 'see also' lines, it is just the first line 'pack\X.java' when using binary inputs that I'm not so keen on...
I am going to check in 'some' fixes for this. The first NPE is fixed with a guard. The NPE behind it occurs because Wes' test case attaches 21 errors to a file that is being used as binary input. Up to 20 he would have been OK, but 21 is a problem. Wes, did you *know* that 21 would cause it to fail? :) Anyway, the failure is because if you add more problems to a resource than its 'maxProbs' limit then it attempts to sort the problems it has by priority and discard some of them - priority sorting causes the NPE because there is no reference context object for binary source with which to work. The fix I propose for 1.2 is as follows: 20 seems to have been chosen 'at random' - the correct setting should be determined by asking the compiler for it. This is what would happen if working with source and the default in this case is 100. If I fix the setting of the number then we will tolerate up to 100 errors per type (where the type is binary source) before blowing up with an NPE - this doesn't affect compilation from all source. This seems possibly reasonable? Although I have prototyped the larger fix of creating a binarySourceReferenceContext, it is much more far reaching considering we are trying to lock down for the AspectJ1.2 release. So, I would like to do the minimum for AspectJ1.2 and leave this bug open to put in the full fix later. The related issue of course is what the messages look like for declare error against files with no debug information. They are currently (with the NPEs fixed) like this: pack\<Unknown>:0 NPE see also: C:\Eclipse\212\eclipse\aspectj_ws2 \tests\bugs\systemouts\A.java:4 see also: C:\Eclipse\212\eclipse\aspectj_ws2 \tests\bugs\systemouts\injar-nodebug.jar Which is rather unhelpful - so I'm fixing it to this: Type 'pack.X' (no debug info available):0 NPE see also: C:\Eclipse\212\eclipse\aspectj_ws2 \tests\bugs\systemouts\A.java:4 see also: C:\Eclipse\212\eclipse\aspectj_ws2 \tests\bugs\systemouts\injar-nodebug.jar
I'm ok with a limit of 100-200, but would distinguish from compiler errors. If there are n>30 compiler errors, the user will likely fix a few and recompile (fix an entire error cascade). But declare warnings are actually sought, e.g., for spelunking and for finding problems in code. E.g., this test case was whittled down from > 150 messages over 500+ classes. As for workarounds, if it's possible to to avert or catch the NPE and emit one last message "other messages omitted", that would be great. As for message formatting, another option could be foo\bar\Bash.class:0 (no debug info...) which would preserve the parse rule "everything up to the first colon is a filename (of sorts)" (I use that rule in a plugin to search sourcepaths for the associated sourcefile.) This is a major thorn for me, so I'm happy you've taken the time to look at it.
Thanks for the comments Wes. - I had thought about keeping the stuff before the ':' as a filename, then I realised in the current case 'pack/<Unknown>:0' (which comes out without my changes) breaks that rule, so I'm not making things any worse. Without a valid source name from the debug info in the class file the only other pathname I have available with the current infrastructure is the *destination* where the class file will be put once weaving is complete - but if an error is reported, the class won't be put out and so that path is bogus too. Any manipulation of the type information 'pack.X' to create a valid class file location would be unlikely to work reliably. I would like to preserve the path used as input (pack/X.class) and use that - but even that is not quite right in the case where the class file originates from inside a jar file. The only path that is really valid is the path to the input jar that contained the class. - Also, I don't believe we currently distinguish at all between declare error/warning and normal error/warning and I agree with you that the declare forms ought not to be subject to a limit. I think it is a new enhancement request to get that feature though. In this particular bug we are binary weaving and so we know that the only kind of errors we'll see are due to declares. I hope it doesn't appear as if I'm trying to be awkward. I'm trying to do the best I can without messing with the codebase too much in order that we can build a solid 1.2 this week.
Moved this bug to a P3 enh for post 1.2. Main issue is solved in 1.2
*** Bug 58679 has been marked as a duplicate of this bug. ***
I was experiencing the NPE in computePriority with 1.2rc1, so I switched to 1.2 and the NPE disappeared, as the comments indicate they should. However, there seems to be another problem. All of my policies are warnings (i.e., declare warning), but they are all being reported as errors, which causes the build to fail (since they are errors rather than warnings). In addition, errors are reported in classes that I have declared to be excluded. That is, part of my declare warning declarations include !within (com.mypackage..*), but errors are being reported for classes that are within the specified package structure.
Chuck, you might want to open a new bug, if you haven't already. It seems like a separate issue. Our test suite would have caught the simple mistake of listing declare-warnings as errors, so if you could include in the bug what's necessary to reproduce, that would be great.
