500874 - Static analysis to find heap allocations that could be stack allocations

Status: NEW

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Jesse Ruderman]

new & delete in same function --> probably safe to use a stack variable or alloca()

(suggested by pav when i asked "what static analyses could we do to find patterns that are common performance issues?")

Comment 1

[(dormant account)]

Don't we already have problems with blowing the stack?

Comment 2

[Jesse Ruderman]

Dynamically-sized (alloca) stack allocations require case-by-case reasoning about whether it's safe.  Other than that, I believe there's plenty of room on the stack.

Comment 3

[(dormant account)]

(In reply to comment #2)
> Dynamically-sized (alloca) stack allocations require case-by-case reasoning
> about whether it's safe.  Other than that, I believe there's plenty of room on
> the stack.

is that portable? Or should this be about looking for malloc/free with a constant size?

Comment 4

[Jesse Ruderman]

We can start by just looking at constant-size mallocs.  If fixing those goes well, we can try variable-size mallocs and see how many of them are safe to convert to alloca.

Comment 5

[georgi - hopefully not receiving bugspam]

> safe to use a stack variable or alloca()

i don't think this is good idea. alloca(rand()) crashes. dynamic alloca is dangerous - no overflowing check.

> I believe there's plenty of room on the stack.

don't think so. i have seen crashes caused by recursion - to fix them i needed |ulimit -s BIG| <-- this sets the stack size.

Comment 6

[Sunita]

I, Sunita E, am a student of SJCE,Mysore ,India and would like to take up this bug.

Comment 7

[Jesse Ruderman]

Thanks, Sunita.  I suggest reading https://developer.mozilla.org/en/Dehydra and asking any questions you have in irc.mozilla.org #static.

Comment 8

[Sunita]

Attachment: script to match new and delete calls in a function

please take a look at the attachment.I need guidance to go further.

Comment 9

[(dormant account)]

(In reply to comment #8)
> Created an attachment (id=441706) [details]
> script to match new and delete calls in a function
> 
> please take a look at the attachment.I need guidance to go further.

Look for new* calls that take a constant as an argument.

Comment 10

[Sunita]

Attachment: Looks for new calls that have constant argument

Changes:
Added check for arguments:
if(rhs[0].arguments[0].value) { 
//...
}
This makes sure that the values of arguments are constant or in other words available at compile time.

Comment 11

[Sunita]

Could you please go through the latest attachment.
If its acceptable, I shall add the necessary warning statements.

Comment 12

[Benjamin Smedberg]

Sunita, you need to request review/feedback from a particular person, probably dwitte or taras.

Comment 13

[Sunita]

(In reply to comment #12)
> Sunita, you need to request review/feedback from a particular person, probably
> dwitte or taras.

Sure.Thank you.

Comment 14

[(dormant account)]

Comment on attachment 443297 [details] [diff] [review]
Looks for new calls that have constant argument

>var allocs = []
>
>function process_function(decl, body) {
>
>    for (var i in body) {
>        for (var j in body[i].statements) {

Instead of looping directly over body, use iterate_vars() like in bug 522776. 

Otherwise the code doesn't matter, it's just a tool to produce a report. Run it on mozilla, see what sort of code it flags and refine the code until it seems to flag reasonable problems.

To proceed with the bug I recommend that you run this on mozilla, produce a report. Then inspect it to see if the analysis produces too many false positives, if so refine the script. Once you are sure that the analysis might be showing up likely candidates, attach a report to this bug.

Comment 15

[Sunita]

Attachment: list of warnings generated on mozilla code base

Almost all the warnings have been generated in cases with conditions i.e within if statements.I would like to know what should/can be done.