Closed Bug 1605130 Opened 6 years ago Closed 6 years ago

Disallow calling `forget` on `OwningNonNull` except from CC unlink

Categories

(Core :: XPCOM, task)

Product:
Core
Component:
XPCOM
Type:
task
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla73
Tracking Flags:
Tracking Status
firefox73 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

Bug 1605130. Make it harder to misuse OwningNonNull::forget. r=froydnj
47 bytes, text/x-phabricator-request
Details | Review

forget on an OwningNonNull puts it into an invalid state. People shouldn't be doing that, in general. We need to do it from CC unlink, but no one else should be doing it.

Thanks to Simon Giesecke for catching this.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eeb7c0aec59c Make it harder to misuse OwningNonNull::forget. r=froydnj

Backed out changeset eeb7c0aec59c (bug 1605130) for build bustage at build/src/obj-firefox/dist/include/mozilla/OwningNonNull.

Backout: https://hg.mozilla.org/integration/autoland/rev/583bdbe6e51ae2f63b0413ccad14ddd19e3e38fd

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=eeb7c0aec59cc3121656f54957f274c0d5b1648c

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=281930001&repo=autoland&lineNumber=15084

[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/BindingDeclarations.h:20,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/ClientsBinding.h:12,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/ClientBinding.h:6,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/ClientInfo.h:11,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIGlobalObject.h:12,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/xpcpublic.h:21,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:24,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from /builds/worker/workspace/build/src/security/sandbox/linux/reporter/SandboxReporter.cpp:20,
[task 2019-12-19T15:51:06.050Z] 15:51:06 INFO - from Unified_cpp_linux_reporter0.cpp:2:
[task 2019-12-19T15:51:06.058Z] 15:51:06 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/OwningNonNull.h:109:65: error: friend declaration 'void mozilla::ImplCycleCollectionUnlink(mozilla::OwningNonNull<T>&)' declares a non-template function [-Werror=non-template-friend]
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - friend void ImplCycleCollectionUnlink(OwningNonNull<T>& aField);
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - ^
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/OwningNonNull.h:109:65: note: (if this is not what you intended, make sure the function template has already been declared and add <> after the function name here)
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - cc1plus: all warnings being treated as errors
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - /builds/worker/workspace/build/src/config/rules.mk:736: recipe for target 'Unified_cpp_linux_reporter0.o' failed
[task 2019-12-19T15:51:06.058Z] 15:51:06 ERROR - make[4]: *** [Unified_cpp_linux_reporter0.o] Error 1
[task 2019-12-19T15:51:06.058Z] 15:51:06 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/security/sandbox/linux/reporter'
[task 2019-12-19T15:51:06.059Z] 15:51:06 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'security/sandbox/linux/reporter/target-objects' failed
[task 2019-12-19T15:51:06.059Z] 15:51:06 ERROR - make[3]: *** [security/sandbox/linux/reporter/target-objects] Error 2
[task 2019-12-19T15:51:06.059Z] 15:51:06 INFO - make[3]: *** Waiting for unfinished jobs....
[task 2019-12-19T15:51:06.074Z] 15:51:06 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/config/external/icu/common'
[task 2019-12-19T15:51:06.074Z] 15:51:06 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ -std=gnu++17 -o unistr_titlecase_brkiter.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DU_COMMON_IMPLEMENTATION -DUCONFIG_NO_TRANSLITERATION -DUCONFIG_NO_REGULAR_EXPRESSIONS -DUCONFIG_NO_LEGACY_CONVERSION -DU_USING_ICU_NAMESPACE=0 -DU_HIDE_OBSOLETE_UTF_OLD_H=1 -DU_NO_DEFAULT_INCLUDE_UTF_HEADERS=1 -DUNISTR_FROM_CHAR_EXPLICIT=explicit -DUNISTR_FROM_STRING_EXPLICIT=explicit -DU_CHARSET_IS_UTF8 -DU_HAVE_NL_LANGINFO_CODESET=0 -I/builds/worker/workspace/build/src/config/external/icu/common -I/builds/worker/workspace/build/src/obj-firefox/config/external/icu/common -I/builds/worker/workspace/build/src/intl/icu/source/i18n -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -Wformat-overflow=2 -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Wno-deprecated-declarations -Wno-type-limits -Wno-unused-but-set-variable -Wno-unused-function -Wno-sign-compare -Wno-maybe-uninitialized -frtti -MD -MP -MF .deps/unistr_titlecase_brkiter.o.pp /builds/worker/workspace/build/src/intl/icu/source/common/unistr_titlecase_brkiter.cpp
[task 2019-12-19T15:51:06.074Z] 15:51:06 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/config/external/icu/common'
[task 2019-12-19T15:51:06.095Z] 15:51:06 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/config/external/icu/common'
[task 2019-12-19T15:51:06.095Z] 15:51:06 INFO - config/external/icu/common/unormcmp.o

Flags: needinfo?(bzbarsky)

As mentioned in the other bug, this method doesn't actually need to return anything. It just needs to clear the pointer. You could even call it unlinkForCC or something. It is also fine to leave it as is.

Looks like gcc is unhappy, ok. Will see how to make it happy.

Flags: needinfo?(bzbarsky)
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/31e65944a484 Make it harder to misuse OwningNonNull::forget. r=froydnj

https://hg.mozilla.org/mozilla-central/rev/31e65944a484

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox73: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: