Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 21:01:08 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1718825 - Treat strong pointers which are members of class/struct and marked as MOZ_KNOWN_LIVE as safe r=andi

Browse Source 
`MOZ_KNOWN_LIVE RefPtr<Foo> mFoo` is not treated as safe because its raw pointer
is referred with operators but they are not checked at handling `MOZ_KNOWN_LIVE`
annotation.

Additionally, when members marked as `MOZ_KNOWN_LIVE` are in the stack, they
are also not treated as safe, but they should be safe in most cases.

With these changes, `HTMLTableEditor.cpp` can get rid of a lot of
`MOZ_KnownLive` method calls.

Differential Revision: https://phabricator.services.mozilla.com/D136122
This commit is contained in:
Masayuki Nakano 2022-02-02 00:48:19 +00:00
parent ee4740158a
commit 8d88df10f6
3 changed files with 80 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
build/clang-plugin/CanRunScriptChecker.cpp
View File

@ -84,7 +84,9 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
auto KnownLiveMemberOfParam =
memberExpr(hasKnownLiveAnnotation(),
hasObjectExpression(ignoreTrivials(KnownLiveParam)));
hasObjectExpression(anyOf(
ignoreTrivials(KnownLiveParam),
declRefExpr(to(varDecl(hasAutomaticStorageDuration()))))));
// A matcher that matches various things that are known to be live directly,
// without making any assumptions about operators.
@ -111,11 +113,17 @@ void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
// example). For purposes of this analysis we are assuming the method
// calls on smart ptrs all just return the pointer inside,
cxxMemberCallExpr(
on(allOf(hasType(isSmartPtrToRefCounted()), KnownLiveBase))),
on(anyOf(allOf(hasType(isSmartPtrToRefCounted()), KnownLiveBase),
// Allow it if calling a member method which is marked as
// MOZ_KNOWN_LIVE
KnownLiveMemberOfParam))),
// operator* or operator-> on a thing that is already known to be live.
cxxOperatorCallExpr(anyOf(hasOverloadedOperatorName("*"),
hasOverloadedOperatorName("->")),
hasAnyArgument(KnownLiveBase), argumentCountIs(1)),
cxxOperatorCallExpr(
anyOf(hasOverloadedOperatorName("*"),
hasOverloadedOperatorName("->")),
hasAnyArgument(
anyOf(KnownLiveBase, ignoreTrivials(KnownLiveMemberOfParam))),
argumentCountIs(1)),
// A dereference on a thing that is known to be live. This is _not_
// caught by the "operator* or operator->" clause above, because
// cxxOperatorCallExpr() only catches cases when a class defines

45
build/clang-plugin/tests/TestCanRunScript.cpp
View File

@ -623,33 +623,64 @@ struct DisallowMemberArgsViaReferenceAlias2 {
struct AllowMozKnownLiveMember {
public:
MOZ_KNOWN_LIVE RefCountedBase* mWhatever;
MOZ_CAN_RUN_SCRIPT void foo(RefCountedBase* aWhatever) {}
MOZ_KNOWN_LIVE RefPtr<RefCountedBase> mRefCountedWhatever;
MOZ_CAN_RUN_SCRIPT void fooPtr(RefCountedBase* aWhatever) {}
MOZ_CAN_RUN_SCRIPT void fooRef(RefCountedBase& aWhatever) {}
MOZ_CAN_RUN_SCRIPT void bar() {
foo(mWhatever);
fooPtr(mWhatever);
fooRef(*mWhatever);
fooPtr(mRefCountedWhatever);
fooRef(*mRefCountedWhatever);
}
};
struct AllowMozKnownLiveMemberParent : AllowMozKnownLiveMember {
MOZ_CAN_RUN_SCRIPT void baz() {
foo(mWhatever);
fooPtr(mWhatever);
fooRef(*mWhatever);
fooPtr(mRefCountedWhatever);
fooRef(*mRefCountedWhatever);
}
};
struct AllowMozKnownLiveParamMember {
public:
MOZ_CAN_RUN_SCRIPT void foo(AllowMozKnownLiveMember& aAllow) {
aAllow.foo(aAllow.mWhatever);
aAllow.fooPtr(aAllow.mWhatever);
aAllow.fooRef(*aAllow.mWhatever);
aAllow.fooPtr(aAllow.mRefCountedWhatever);
aAllow.fooRef(*aAllow.mRefCountedWhatever);
}
MOZ_CAN_RUN_SCRIPT void bar(AllowMozKnownLiveMemberParent& aAllowParent) {
aAllowParent.foo(aAllowParent.mWhatever);
aAllowParent.fooPtr(aAllowParent.mWhatever);
aAllowParent.fooRef(*aAllowParent.mWhatever);
aAllowParent.fooPtr(aAllowParent.mRefCountedWhatever);
aAllowParent.fooRef(*aAllowParent.mRefCountedWhatever);
}
};
MOZ_CAN_RUN_SCRIPT void AllowMozKnownLiveMemberInAutoStorage() {
AllowMozKnownLiveMember inStack;
AllowMozKnownLiveMember* inHeap = new AllowMozKnownLiveMember();
inStack.fooPtr(inStack.mWhatever);
inStack.fooRef(*inStack.mWhatever);
inStack.fooPtr(inStack.mRefCountedWhatever);
inStack.fooRef(*inStack.mRefCountedWhatever);
inStack.fooPtr(inHeap->mWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'inHeap->mWhatever' is neither.}}
inStack.fooRef(*inHeap->mWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). '*inHeap->mWhatever' is neither.}}
inStack.fooPtr(inHeap->mRefCountedWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'inHeap->mRefCountedWhatever' is neither.}}
inStack.fooRef(*inHeap->mRefCountedWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). '*inHeap->mRefCountedWhatever' is neither.}}
}
struct DisallowMozKnownLiveMemberNotFromKnownLive {
AllowMozKnownLiveMember* mMember;
MOZ_CAN_RUN_SCRIPT void foo(RefCountedBase* aWhatever) {}
MOZ_CAN_RUN_SCRIPT void fooPtr(RefCountedBase* aWhatever) {}
MOZ_CAN_RUN_SCRIPT void fooRef(RefCountedBase& aWhatever) {}
MOZ_CAN_RUN_SCRIPT void bar() {
foo(mMember->mWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'mMember->mWhatever' is neither.}}
fooPtr(mMember->mWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'mMember->mWhatever' is neither.}}
fooRef(*mMember->mWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). '*mMember->mWhatever' is neither.}}
fooPtr(mMember->mRefCountedWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'mMember->mRefCountedWhatever' is neither.}}
fooRef(*mMember->mRefCountedWhatever); // expected-error {{arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). '*mMember->mRefCountedWhatever' is neither.}}
}
};

67
editor/libeditor/HTMLTableEditor.cpp
View File

@ -49,8 +49,8 @@ using EmptyCheckOption = HTMLEditUtils::EmptyCheckOption;
*/
class MOZ_STACK_CLASS AutoSelectionSetterAfterTableEdit final {
private:
RefPtr<HTMLEditor> mHTMLEditor;
RefPtr<Element> mTable;
MOZ_KNOWN_LIVE RefPtr<HTMLEditor> mHTMLEditor;
MOZ_KNOWN_LIVE RefPtr<Element> mTable;
int32_t mCol, mRow, mDirection, mSelected;
public:
@ -66,9 +66,8 @@ class MOZ_STACK_CLASS AutoSelectionSetterAfterTableEdit final {
MOZ_CAN_RUN_SCRIPT ~AutoSelectionSetterAfterTableEdit() {
if (mHTMLEditor) {
MOZ_KnownLive(mHTMLEditor)
->SetSelectionAfterTableEdit(MOZ_KnownLive(mTable), mRow, mCol,
mDirection, mSelected);
mHTMLEditor->SetSelectionAfterTableEdit(mTable, mRow, mCol, mDirection,
mSelected);
}
}
@ -511,7 +510,7 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
// Thus we set the colspan to its true value.
if (!cellDataAtSelection.mColSpan) {
DebugOnly<nsresult> rvIgnored =
SetColSpan(MOZ_KnownLive(cellDataAtSelection.mElement),
SetColSpan(cellDataAtSelection.mElement,
cellDataAtSelection.mEffectiveColSpan);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::SetColSpan() failed, but ignored");
@ -565,9 +564,8 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
// Note: we do nothing if colsspan=0, since it should automatically
// span the new column.
if (cellData.mColSpan > 0) {
DebugOnly<nsresult> rvIgnored =
SetColSpan(MOZ_KnownLive(cellData.mElement),
cellData.mColSpan + aNumberOfColumnsToInsert);
DebugOnly<nsresult> rvIgnored = SetColSpan(
cellData.mElement, cellData.mColSpan + aNumberOfColumnsToInsert);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::SetColSpan() failed, but ignored");
}
@ -577,8 +575,7 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
// Simply set selection to the current cell. So, we can let
// InsertTableCellsWithTransaction() do the work. Insert a new cell
// before current one.
CollapseSelectionToStartOf(MOZ_KnownLive(*cellData.mElement),
ignoredError);
CollapseSelectionToStartOf(*cellData.mElement, ignoredError);
if (NS_WARN_IF(ignoredError.ErrorCodeIs(NS_ERROR_EDITOR_DESTROYED))) {
return NS_ERROR_EDITOR_DESTROYED;
}
@ -745,7 +742,7 @@ nsresult HTMLEditor::InsertTableRowsWithTransaction(
// Thus we set the rowspan to its true value.
if (!cellDataAtSelection.mRowSpan) {
DebugOnly<nsresult> rvIgnored =
SetRowSpan(MOZ_KnownLive(cellDataAtSelection.mElement),
SetRowSpan(cellDataAtSelection.mElement,
cellDataAtSelection.mEffectiveRowSpan);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::SetRowSpan() failed, but ignored");
@ -786,9 +783,8 @@ nsresult HTMLEditor::InsertTableRowsWithTransaction(
// Note that if rowspan is 0, we do nothing since that cell should
// automatically extend into the new row.
if (cellData.mRowSpan > 0) {
DebugOnly<nsresult> rvIgnored =
SetRowSpan(MOZ_KnownLive(cellData.mElement),
cellData.mRowSpan + aNumberOfRowsToInsert);
DebugOnly<nsresult> rvIgnored = SetRowSpan(
cellData.mElement, cellData.mRowSpan + aNumberOfRowsToInsert);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::SetRowSpan() failed, but ignored");
}
@ -1549,7 +1545,7 @@ nsresult HTMLEditor::DeleteTableColumnWithTransaction(Element& aTableElement,
NS_WARNING_ASSERTION(cellData.mColSpan > 1,
"colspan should be 2 or larger");
DebugOnly<nsresult> rvIgnored =
SetColSpan(MOZ_KnownLive(cellData.mElement), cellData.mColSpan - 1);
SetColSpan(cellData.mElement, cellData.mColSpan - 1);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::SetColSpan() failed, but ignored");
}
@ -1558,7 +1554,7 @@ nsresult HTMLEditor::DeleteTableColumnWithTransaction(Element& aTableElement,
// so delete contents of cell instead of cell itself (We must have
// reset colspan above).
DebugOnly<nsresult> rvIgnored =
DeleteAllChildrenWithTransaction(MOZ_KnownLive(*cellData.mElement));
DeleteAllChildrenWithTransaction(*cellData.mElement);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
"HTMLEditor::DeleteAllChildrenWithTransaction() "
"failed, but ignored");
@ -1577,8 +1573,7 @@ nsresult HTMLEditor::DeleteTableColumnWithTransaction(Element& aTableElement,
if (numberOfCellsInRow != 1) {
// If removing cell is not the last cell of the row, we can just remove
// it.
nsresult rv =
DeleteNodeWithTransaction(MOZ_KnownLive(*cellData.mElement));
nsresult rv = DeleteNodeWithTransaction(*cellData.mElement);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::DeleteNodeWithTransaction() failed");
return rv;
@ -2044,8 +2039,7 @@ NS_IMETHODIMP HTMLEditor::SelectAllTableCells() {
// XXX So, we should distinguish whether CellData returns error or just
// not found later.
if (cellData.mElement && !cellData.IsSpannedFromOtherRowOrColumn()) {
nsresult rv =
AppendContentToSelectionAsRange(MOZ_KnownLive(*cellData.mElement));
nsresult rv = AppendContentToSelectionAsRange(*cellData.mElement);
if (rv == NS_ERROR_EDITOR_DESTROYED) {
NS_WARNING(
"HTMLEditor::AppendContentToSelectionAsRange() caused destroying "
@ -2159,7 +2153,7 @@ NS_IMETHODIMP HTMLEditor::SelectTableRow() {
// XXX So, we should distinguish whether CellData returns error or just
// not found later.
if (cellData.mElement && !cellData.IsSpannedFromOtherRowOrColumn()) {
rv = AppendContentToSelectionAsRange(MOZ_KnownLive(*cellData.mElement));
rv = AppendContentToSelectionAsRange(*cellData.mElement);
if (rv == NS_ERROR_EDITOR_DESTROYED) {
NS_WARNING(
"HTMLEditor::AppendContentToSelectionAsRange() caused destroying "
@ -2268,7 +2262,7 @@ NS_IMETHODIMP HTMLEditor::SelectTableColumn() {
// XXX So, we should distinguish whether CellData returns error or just
// not found later.
if (cellData.mElement && !cellData.IsSpannedFromOtherRowOrColumn()) {
rv = AppendContentToSelectionAsRange(MOZ_KnownLive(*cellData.mElement));
rv = AppendContentToSelectionAsRange(*cellData.mElement);
if (rv == NS_ERROR_EDITOR_DESTROYED) {
NS_WARNING(
"HTMLEditor::AppendContentToSelectionAsRange() caused destroying "
@ -2447,7 +2441,7 @@ nsresult HTMLEditor::SplitCellIntoColumns(Element* aTable, int32_t aRowIndex,
}
// Reduce colspan of cell to split
nsresult rv = SetColSpan(MOZ_KnownLive(cellData.mElement), aColSpanLeft);
nsresult rv = SetColSpan(cellData.mElement, aColSpanLeft);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetColSpan() failed");
return rv;
@ -2456,8 +2450,8 @@ nsresult HTMLEditor::SplitCellIntoColumns(Element* aTable, int32_t aRowIndex,
// Insert new cell after using the remaining span
// and always get the new cell so we can copy the background color;
RefPtr<Element> newCellElement;
rv = InsertCell(MOZ_KnownLive(cellData.mElement), cellData.mEffectiveRowSpan,
aColSpanRight, true, false, getter_AddRefs(newCellElement));
rv = InsertCell(cellData.mElement, cellData.mEffectiveRowSpan, aColSpanRight,
true, false, getter_AddRefs(newCellElement));
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::InsertCell() failed");
return rv;
@ -2468,8 +2462,7 @@ nsresult HTMLEditor::SplitCellIntoColumns(Element* aTable, int32_t aRowIndex,
if (aNewCell) {
*aNewCell = do_AddRef(newCellElement).take();
}
rv =
CopyCellBackgroundColor(newCellElement, MOZ_KnownLive(cellData.mElement));
rv = CopyCellBackgroundColor(newCellElement, cellData.mElement);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"HTMLEditor::CopyCellBackgroundColor() failed");
return rv;
@ -2573,7 +2566,7 @@ nsresult HTMLEditor::SplitCellIntoRows(Element* aTable, int32_t aRowIndex,
}
// Reduce rowspan of cell to split
nsresult rv = SetRowSpan(MOZ_KnownLive(cellData.mElement), aRowSpanAbove);
nsresult rv = SetRowSpan(cellData.mElement, aRowSpanAbove);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetRowSpan() failed");
return rv;
@ -3047,8 +3040,7 @@ NS_IMETHODIMP HTMLEditor::JoinTableCells(bool aMergeNonContiguousContents) {
if (spanAboveMergedCell > 0) {
// Cell we merged started in a row above the target cell
// Reduce rowspan to give room where target cell will extend its colspan
nsresult rv = SetRowSpan(MOZ_KnownLive(rightCellData.mElement),
spanAboveMergedCell);
nsresult rv = SetRowSpan(rightCellData.mElement, spanAboveMergedCell);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetRowSpan() failed");
return EditorBase::ToGenericNSResult(rv);
@ -3056,9 +3048,8 @@ NS_IMETHODIMP HTMLEditor::JoinTableCells(bool aMergeNonContiguousContents) {
}
// Reset target cell's colspan to encompass cell to the right
rv = SetColSpan(
MOZ_KnownLive(leftCellData.mElement),
leftCellData.mEffectiveColSpan + rightCellData.mEffectiveColSpan);
rv = SetColSpan(leftCellData.mElement, leftCellData.mEffectiveColSpan +
rightCellData.mEffectiveColSpan);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetColSpan() failed");
return EditorBase::ToGenericNSResult(rv);
@ -3205,8 +3196,8 @@ nsresult HTMLEditor::FixBadRowSpan(Element* aTable, int32_t aRowIndex,
// not found a cell. Fix this later.
if (cellData.mElement && cellData.mRowSpan > 0 &&
!cellData.IsSpannedFromOtherRowOrColumn()) {
nsresult rv = SetRowSpan(MOZ_KnownLive(cellData.mElement),
cellData.mRowSpan - rowsReduced);
nsresult rv =
SetRowSpan(cellData.mElement, cellData.mRowSpan - rowsReduced);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetRawSpan() failed");
return rv;
@ -3285,8 +3276,8 @@ nsresult HTMLEditor::FixBadColSpan(Element* aTable, int32_t aColIndex,
// not found a cell. Fix this later.
if (cellData.mElement && cellData.mColSpan > 0 &&
!cellData.IsSpannedFromOtherRowOrColumn()) {
nsresult rv = SetColSpan(MOZ_KnownLive(cellData.mElement),
cellData.mColSpan - colsReduced);
nsresult rv =
SetColSpan(cellData.mElement, cellData.mColSpan - colsReduced);
if (NS_FAILED(rv)) {
NS_WARNING("HTMLEditor::SetColSpan() failed");
return rv;