New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TodoCommentCheck overflows the stack for files with many comments #4563

Closed
denizarsan opened this Issue Jun 30, 2017 · 2 comments

Comments

Projects
None yet
3 participants
@denizarsan

denizarsan commented Jun 30, 2017

$ { echo "class C {"; for i in {00000..30000}; do echo "// "$i; done; echo "}"; } > C.java

$ javac C.java

$ cat config.xml

<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN" "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="TodoComment"/>
  </module>
</module>

$ java -jar checkstyle-7.8.2-all.jar -c config.xml C.java 2>log

Starting audit...

$ head log

Exception in thread "main" java.lang.Error: Error was thrown while processing C.java
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:299)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:213)
        at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:425)
        at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:359)
        at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)
Caused by: java.lang.StackOverflowError
        at com.puppycrawl.tools.checkstyle.api.DetailAST.setParent(DetailAST.java:205)
        at com.puppycrawl.tools.checkstyle.api.DetailAST.setParent(DetailAST.java:205)
        at com.puppycrawl.tools.checkstyle.api.DetailAST.setParent(DetailAST.java:205)
# [...] there are 1021 more lines for the same recursive call

While the input file may look like an artificial example, it is actually simplified from the real file
https://github.com/robovm/robovm/blob/ef091902377c00dc0fb2db87e8d79c8afb5e9010/tests/robovm/src/test/java/org/robovm/rt/LineNumbersInStackTracesTestMethods.java

@romani romani added the approved label Jul 3, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Aug 1, 2017

Member

I can confirm issue still exists in 8.1 .
It is unrelated to module in configuration, but to our Java parser and for comment aware grammar.

$ { echo "class C {"; for i in {00000..30000}; do echo "// "$i; done; echo "}"; } > C.java

File's tree looks like:

CLASS_DEF -> CLASS_DEF [1:0]
|--MODIFIERS -> MODIFIERS [1:0]
|--LITERAL_CLASS -> class [1:0]
|--IDENT -> C [1:6]
`--OBJBLOCK -> OBJBLOCK [1:8]
    |--LCURLY -> { [1:8]
    |--SINGLE_LINE_COMMENT -> // [2:0]
    |  `--COMMENT_CONTENT ->  00000\n [2:2]
    |--SINGLE_LINE_COMMENT -> // [3:0]
    |  `--COMMENT_CONTENT ->  00001\n [3:2]
    |--SINGLE_LINE_COMMENT -> // [4:0]
    |  `--COMMENT_CONTENT ->  00002\n [4:2]

StackOverflow occurs at

.

this is always the comment like //[30000x0] and parent is OBJBLOCK[1x8].

Stack during this process is:

	DetailAST.setParent(DetailAST) line: 205
	DetailAST.setNextSibling(AST) line: 98	
	DetailAST.addPreviousSibling(DetailAST) line: 125	
	TreeWalker.appendHiddenCommentNodes(DetailAST) line: 593	
	TreeWalker.processFiltered(File, FileText) line: 208	
	TreeWalker(AbstractFileSetCheck).process(File, FileText) line: 79	
Member

rnveach commented Aug 1, 2017

I can confirm issue still exists in 8.1 .
It is unrelated to module in configuration, but to our Java parser and for comment aware grammar.

$ { echo "class C {"; for i in {00000..30000}; do echo "// "$i; done; echo "}"; } > C.java

File's tree looks like:

CLASS_DEF -> CLASS_DEF [1:0]
|--MODIFIERS -> MODIFIERS [1:0]
|--LITERAL_CLASS -> class [1:0]
|--IDENT -> C [1:6]
`--OBJBLOCK -> OBJBLOCK [1:8]
    |--LCURLY -> { [1:8]
    |--SINGLE_LINE_COMMENT -> // [2:0]
    |  `--COMMENT_CONTENT ->  00000\n [2:2]
    |--SINGLE_LINE_COMMENT -> // [3:0]
    |  `--COMMENT_CONTENT ->  00001\n [3:2]
    |--SINGLE_LINE_COMMENT -> // [4:0]
    |  `--COMMENT_CONTENT ->  00002\n [4:2]

StackOverflow occurs at

.

this is always the comment like //[30000x0] and parent is OBJBLOCK[1x8].

Stack during this process is:

	DetailAST.setParent(DetailAST) line: 205
	DetailAST.setNextSibling(AST) line: 98	
	DetailAST.addPreviousSibling(DetailAST) line: 125	
	TreeWalker.appendHiddenCommentNodes(DetailAST) line: 593	
	TreeWalker.processFiltered(File, FileText) line: 208	
	TreeWalker(AbstractFileSetCheck).process(File, FileText) line: 79	

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 3, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 3, 2017

@romani romani added the bug label Aug 4, 2017

@romani romani added this to the 8.2 milestone Aug 4, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 4, 2017

Member

fix is merged.

Member

romani commented Aug 4, 2017

fix is merged.

@romani romani closed this Aug 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Aug 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment