universal-ctags / ctags

A maintained ctags implementation
https://ctags.io
GNU General Public License v2.0
6.59k stars 629 forks source link

SQL: fix the failure of fuzz-hitting-assertions/sql-fha-0 #399

Closed masatake closed 9 years ago

masatake commented 9 years ago

...Where are my sweet longjmp/setjmp and trashbox? I cannot find a solution for this issue without them.

vhda commented 9 years ago

Wouldn't simply deleting the Assert() call inside parseRecord() be enough?

masatake commented 9 years ago

It is a bad idea.

sql parsrs use the Assert() for rejecting bad input. Other than heavily crafted parser like c.c, a parser should stop parsing when detecting input is broken; and return the control to main. Just removing Assert() means the parser continues parsing after detecting broken input. It is the world I don't want to enter.

masatake commented 9 years ago

An exception is that if there is a good maintainer for SQL parser, one can consider the behavior of parser of broken input. This is ideal but we cannot expect it for every languages. The default behavior should be just stopping.

vhda commented 9 years ago

In my opinion we should not verify the input for correctness, but we should make a best effort to find as many tags as possible, and I don't think that anyone is expecting perfect results with wrong code. The Assert() call goes against premise because it aborts when it finds unexpected input. Using this SQL example, what if ctags is being called automatically whenever the file is saved and the user is in the middle of writing this record entry. Now assume we have 1000 lines of code after this one. Should we really abort and return an empty list of tags? Obviously, the tag list will be very short if there is some kind of skip function that skips too many lines because of wrong input. But again, who would expect a perfect list of tags with broken input?

I can tell you that I have a single Assert() call in the Verilog parser that verifies if vUngetc() is called when the "stack" is empty. Up until now I have had no problems with this solution and neither anyone reported any issues.