teavm icon indicating copy to clipboard operation
teavm copied to clipboard

Some Potential NPD bugs

Open JulyChen728 opened this issue 6 years ago • 6 comments

Hi all, Our bug scanner has reported some potential NPD bugs.

The first one is at BufferedCodeWriter.java#L135 . Since the variable pendingFileName is compared with null on the previous line, it can be null and may be passed to the first parameter of the function printLineDirective() .This may raise a NullPointerException at the function escape()(when it invokes string.length) , which is invoked by printLineDirective .(printLineDirective passed its first paramater that can be null to the second parameter of the functionescape)

The second one is caused by the return null in method getClassLoader(). For example, when the return value loader of getClassLoader() invokes its method getResources , a NullPointerException may be raised.

The third one is caused by the return null in method get(). One possible call chain is getDecomposition() => matches() .When the return value decCurCodePoint invokes its method length, a NullPointerException may take place. The following call chain is similar : group() in TMatchResultImpl.java => group() in TMatcher.java => processReplacement() in TMatcher.java => group.length().

The forth is that the variable leaf is initialized as null at TPattern.java#L756 , and the assignment may be skipped, which produces a NullPointerException at TPattern.java#L765, when it invokes its setNext() method. The same statement occurs at #L773 , #L781 .

The fifth is at ExceptionHandling.java#L47 , the variable methodLocation may be null , and a NullPointerException may occur at #L50 .

Thanks.

JulyChen728 avatar Nov 12 '19 13:11 JulyChen728

The second one is caused by the return null in method getClassLoader(). For example, when the return value loader of getClassLoader() invokes its method getResources , a NullPointerException may be raised.

This is not an issue. This particular implementation of getClassLoader is never actually executed, instead it's replaced by classloader magic with proper implementation. Note that this implementation throws exception before returning null, so you never get NPE here.

The third one is caused by the return null in method get(). The forth is that the variable leaf is initialized as null

I won't fix anything in this code. I did not write it, I copied it from Apache Harmony. This particular code related to regular expressions. Unfortunately, this code is completely unclear and I don't understand how it works. May be some day I'll find time to rewrite regular expression engine from scratch, but for now I'll just leave it as it is.

konsoletyper avatar Nov 18 '19 08:11 konsoletyper

How about other bugs? Would you please confirm? Thanks

ITWOI avatar Nov 21 '19 09:11 ITWOI

I fixed other potential bugs, however I believe it was impossible to reproduce the first one. There's convention in TeaVM code for locations: either both fileName != null and lineNumber >= 0 or fileName == 0 and lineNumber == -1, so having something that does not fit into these conditions is a bug (and I could not find any case when this rule is violated). I think static analyzers can't infer such rules.

konsoletyper avatar Nov 21 '19 09:11 konsoletyper

Thanks!

ITWOI avatar Nov 21 '19 09:11 ITWOI

To me it seems to summarize as

  1. not an issue
  2. not an issue
  3. wontfix
  4. wontfix
  5. fixed

Is this correct (esp. 4. and 5.)? So can the issue be closed then? p.s.: For the record and future - it is sometimes better to rewite code so it is clear not to have a bug-pattern and make less smart static code analyzers happy if this can be done with no bigger drawbacks.

hohwille avatar Jan 16 '20 23:01 hohwille

@hohwille well, your comment is so educational. I learnt a great deal. B.T.W why don't try a smarter static code analyzer (or the smartest) by your standard (whatever that is) and show me the bug reports they produce. I would be very happy to hear if they are producing less false positives. If that's not the case, please genius programmers continue your fantastic coding practices.

HermioneSW avatar Apr 16 '20 01:04 HermioneSW