Open Bug 1535523 Opened 5 years ago Updated 2 years ago

MOZ_CAN_RUN_SCRIPT analysis doesn't handle destructors very well

Categories

(Developer Infrastructure :: Source Code Analysis, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

I just annotated a destructor as MOZ_CAN_RUN_SCRIPT (because it's ~nsAutoMicroTask, which can do a microtask checkpoint, which can run script in all sorts of ways), but the analysis is not complaining at me about callers of the destructor needing the annotation.

It looks like calls to destructors are not present in the AST or something; when I look at the AST for a function that has a stack temporary in it I see a CXXConstructExpr but nothing for the destructor. And https://clang.llvm.org/docs/LibASTMatchersReference.html mentions nothing about matching a destructor call (unlike cxxConstructExpr() etc).

What we may be able to do is to require that if the destructor is MOZ_CAN_RUN_SCRIPT (should be detectable via cxxDestructorDecl) then all the constructors are also MOZ_CAN_RUN_SCRIPT. We can presumably getParent to get the CXXRecordDecl for the class, then iterate the constructors with ctor_iterator, etc. Since the ctor and dtor run in the same scope, that should give us kinda what we want.

Sort of, because it will place constraints on the ctor arguments which might not make sense. Maybe we need another annotation for "must have a MOZ_CAN_RUN_SCRIPT caller" which otherwise doesn't impose any constraints and require that the constructors have that or MOZ_CAN_RUN_SCRIPT.

The priority flag is not set for this bug.
:sylvestre, could you have a look please?

Flags: needinfo?(sledru)
Flags: needinfo?(sledru)
Priority: -- → P3
Product: Firefox Build System → Developer Infrastructure
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.