[Analyzer] StackAddrEscapeChecker & BugReport::addRange()

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[Analyzer] StackAddrEscapeChecker & BugReport::addRange()

Kristof Beyls via cfe-dev

Hello,

 

We experienced some strange anomaly between Clang 8.0 and 8.1: when analyzing the same project StackAddrEscapeChecker reported a bug “Address of stack memory associated with local variable '<name of local>' is still referred to by the global variable '<name of global>' upon returning to the caller.  This will be a dangling reference” to the declaration of the stack variable in 8.0 instead of the exit point of the function. In 8.1 the location was correct, thus the end of the function. When checking the source code of the checker, the tests and also BugReporter.h and .cpp I did not find any change between these versions.

 

However I found something strange in StackAddrEscapeChecker which is still there: the checker adds the source range of the variable declaration to the bugreport which seems to be wrong. The documentation of BugReport::addRange() states that “They should be at the same source code line as the BugReport location.” The declaration is definitely not part of the function exit point (return statement or closing bracket).

 

Should I try to fix this? I thing we should skip adding range in this particular case. Also I think we should add some assertion to BugReport::addRange() to avoid such cases. Do you also think that it is a good idea?

 

Regards,

 

Ádám

 


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Analyzer] StackAddrEscapeChecker & BugReport::addRange()

Kristof Beyls via cfe-dev
This wasn't an intentional change (at least i don't remember ever intending it) and i can't reproduce the problem on a recent clang. I didn't try 8.1 but both on 8.0 and on a recent "10.0" master i see the warning placed at the end of function and the stack variable location highlighted; there are no obvious changes in the visual output, neither in the plist output nor in html output).

I don't see a problem with stack variable location highlighting as such. It won't affect console output where range highlighting is the most useful as it causes ~~~~~~s to appear, but nonetheless it's pretty harmless.

Path sensitive reports generally don't end up anywhere except the bug node location unless you mess real hard with your bug report (eg., inherit from BugReport to override the virtual method getLocation()).

I guess your observation is at least worth debugging in order to figure out what exactly is going on and what else is broken (you should probably try bisecting).


On 06.11.2019 05:31, Ádám Balogh via cfe-dev wrote:

Hello,

 

We experienced some strange anomaly between Clang 8.0 and 8.1: when analyzing the same project StackAddrEscapeChecker reported a bug “Address of stack memory associated with local variable '<name of local>' is still referred to by the global variable '<name of global>' upon returning to the caller.  This will be a dangling reference” to the declaration of the stack variable in 8.0 instead of the exit point of the function. In 8.1 the location was correct, thus the end of the function. When checking the source code of the checker, the tests and also BugReporter.h and .cpp I did not find any change between these versions.

 

However I found something strange in StackAddrEscapeChecker which is still there: the checker adds the source range of the variable declaration to the bugreport which seems to be wrong. The documentation of BugReport::addRange() states that “They should be at the same source code line as the BugReport location.” The declaration is definitely not part of the function exit point (return statement or closing bracket).

 

Should I try to fix this? I thing we should skip adding range in this particular case. Also I think we should add some assertion to BugReport::addRange() to avoid such cases. Do you also think that it is a good idea?

 

Regards,

 

Ádám

 


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev