1771996 - Flag stack-based container types as uninitialized

Status: NEW

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Alex Thayer [:alexical] (she/her)]

Note: this will do nothing without the -ftrivial-auto-var-init flag tracked in bug 1515213.

I noticed when looking at a binary produced when compiling with -ftrivial-auto-var-init that there are a lot of places where we memset big chunks of memory to 0xaa. These correspond to locations where we use stack-based containers like AutoTArray and then pass those to other functions to be filled up, or otherwise use them in such a way that the compiler isn't smart enough to figure out that the uninitialized buffer is always being initialized prior to use.

Given that these stack-based container types have a higher degree of scrutiny than your average uninitialized local variable, it seems like we could get a win by flagging them as uninitialized. Indeed this appears to be the case: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=3e1dd04a29bf7f50bda7deed006b347a52858979&newProject=try&newRevision=1c7ea403f74aeadc2a3d772bf215f9823421fb10&framework=1&page=1&showOnlyConfident=1. This is a try run where I flagged just AutoTArray and nsTAutoStringN as uninitialized, but we could see a bigger win by finding other container types which use stack storage, such as mozilla::Vector, JS::AutoStableStringChars, js::InlineCharBuffer.

Anyway, the patch is pretty trivial, however we would have to upstream either my changes or a similar patch to Clang to get it to work, as currently clang only supports the uninitialized attribute for local variables. My clang patch is pretty simple, but I've never contributed to anything llvm before so I don't know how much of a rigamarole it will be from start to finish.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Happy to help or to connect you to the right person in the llvm/clang community.
It is usually easy to contribute!

Comment 2

[Alex Thayer [:alexical] (she/her)]

(In reply to Sylvestre Ledru [:Sylvestre] from comment #1)

Happy to help or to connect you to the right person in the llvm/clang community.
It is usually easy to contribute!

That would be nice! I have a patch ready with a test and everything per the contributing guide, but I think it's something that needs an RFC? I would be extending the uninitialized attribute to be applicable to structs.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Depends on the nature of the change. A review might be enough. :)