Static analysis to find heap allocations that could be stack allocations

NEW
Assigned to

Status

()

Core
Rewriting and Analysis
--
enhancement
8 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: Sunita)

Tracking

(Blocks: 1 bug, {perf})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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

8 years ago
Don't we already have problems with blowing the stack?
(Reporter)

Comment 2

8 years ago
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.
(Reporter)

Updated

8 years ago
Keywords: perf

Comment 3

8 years ago
(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?
(Reporter)

Comment 4

8 years ago
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.
> 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.
(Assignee)

Comment 6

8 years ago
I, Sunita E, am a student of SJCE,Mysore ,India and would like to take up this bug.
(Reporter)

Comment 7

8 years ago
Thanks, Sunita.  I suggest reading https://developer.mozilla.org/en/Dehydra and asking any questions you have in irc.mozilla.org #static.
Assignee: nobody → sunsai16
(Assignee)

Comment 8

7 years ago
Created attachment 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.

Updated

7 years ago
Attachment #441706 - Attachment mime type: application/octet-stream → text/plain

Comment 9

7 years ago
(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.
(Assignee)

Comment 10

7 years ago
Created attachment 443297 [details] [diff] [review]
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.
(Assignee)

Comment 11

7 years ago
Could you please go through the latest attachment.
If its acceptable, I shall add the necessary warning statements.
Sunita, you need to request review/feedback from a particular person, probably dwitte or taras.
(Assignee)

Comment 13

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

Sure.Thank you.
(Assignee)

Updated

7 years ago
Attachment #441706 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #443297 - Attachment is patch: true
Attachment #443297 - Attachment mime type: application/javascript → text/plain
Attachment #443297 - Flags: feedback?(tglek)

Updated

7 years ago
Attachment #443297 - Flags: feedback?(tglek)

Comment 14

7 years ago
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.
(Assignee)

Comment 15

7 years ago
Created attachment 458281 [details]
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.
Attachment #458281 - Flags: review?(tglek)

Updated

7 years ago
Attachment #458281 - Flags: review?(tglek)
You need to log in before you can comment on or make changes to this bug.