Closed Bug 1605130 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: XPCOM, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

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
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
You need to log in before you can comment on or make changes to this bug.