1257982 - Detect hazards in eg unrooted HashSet<JSObject*> or GCHashSet<JSObject*>

Assignee

Description

•

8 years ago

Waldo was wondering why GCHashSet<object subclass*> did not bother the hazard analysis. I suppose HashSet<> allocates its own chunk of memory to store the pointers in, so it isn't strictly a problem of storing GC pointers on the stack. But it seems like we could do the same thing for hashtable storage that we do for UniquePtr -- especially now that you can do Rooted<GCHashSet<T>>, so you don't *need* to have these unrooted things lying around on the stack.

Marking s-s for now until I see what it turns up. I have some other analysis cleanups to finish up first.

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Group: core-security → javascript-core-security

Andrew McCreight [:mccr8]

Comment 1

•

8 years ago

Please clear the sec-audit flag or file separate bugs for any security issues you find so they can be triaged separately. Thanks.

Keywords: sec-audit

Steve Fink [:sfink] [:s:]

Assignee

Updated

•

5 years ago

Status: NEW → RESOLVED

Closed: 5 years ago

Resolution: --- → DUPLICATE

Steve Fink [:sfink] [:s:]

Assignee

Comment 3

•

5 years ago

On second thought, bug 1500247 only covers GCHashMap and GCHashSet. But as described in this bug, HashMap and HashSet have the same issues. I'm doing a try push now to see if that uncovers any problems, either with the hazard analysis or anything else that might use MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8fc234942ddd8f91a883a0d9c988d6e25ef6263

Status: RESOLVED → REOPENED

Resolution: DUPLICATE → ---

Steve Fink [:sfink] [:s:]

Assignee

Updated

•

5 years ago

Depends on: 1500247

Steve Fink [:sfink] [:s:]

Assignee

Updated

•

5 years ago

Priority: -- → P3

Steve Fink [:sfink] [:s:]

Assignee

Comment 4

•

5 years ago

Attached file hazards.txt — Details

The initial results are not too bad, 12 hazards.

Steve Fink [:sfink] [:s:]

Assignee

Comment 5

•

5 years ago

Attached file Bug 1257982 - [hazards] Inherit template attrs for plain HashMap/HashSet r=jonco — Details

Steve Fink [:sfink] [:s:]

Assignee

Comment 6

•

5 years ago

Attached file Bug 1257982 - root some hashtables r=jonco — Details

Depends on D23619

Steve Fink [:sfink] [:s:]

Assignee

Comment 7

•

5 years ago

Attached file Bug 1257982 - [hazards] Inherit template attrs for plain HashMap/HashSet r=jonco — Details

Phabricator Automation

Updated

•

5 years ago

Attachment #9051187 - Attachment is obsolete: true

Phabricator Automation

Updated

•

5 years ago

Attachment #9051187 - Attachment is obsolete: false

Steve Fink [:sfink] [:s:]

Assignee

Updated

•

5 years ago

Assignee: nobody → sphink

Status: REOPENED → ASSIGNED

Steve Fink [:sfink] [:s:]

Assignee

Comment 8

•

5 years ago

None of these issues are security problems, since the relevant atoms will be kept alive through other means.

Group: javascript-core-security

Keywords: sec-audit

Pulsebot

Comment 9

•

5 years ago

Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/13425f1eca22
[hazards] Inherit template attrs for plain HashMap/HashSet r=jonco
https://hg.mozilla.org/integration/autoland/rev/e73fdb7ff05d
root some hashtables r=jonco

Cristina Coroiu [:ccoroiu]

Comment 10

•

5 years ago

Backed out 2 changesets (bug 1257982) for build bustage at build/src/js/src/frontend/ParseContext.h

Backout: https://hg.mozilla.org/integration/autoland/rev/eff691bb6cf8cd3d2c74eecaa56062eb1c2f1e47

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

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

[task 2019-03-19T08:26:57.584Z] 08:26:57 [task 2019-03-19T08:26:57.584Z] 08:26:57 [task 2019-03-19T08:26:57.585Z] 08:26:57 [task 2019-03-19T08:26:57.585Z] 08:26:57 [task 2019-03-19T08:26:57.585Z] 08:26:57 [task 2019-03-19T08:26:57.615Z] 08:26:57 [task 2019-03-19T08:26:57.615Z] 08:26:57 [task 2019-03-19T08:26:57.615Z] 08:26:57 [task 2019-03-19T08:26:57.615Z] 08:26:57 [task 2019-03-19T08:26:57.661Z] 08:26:57 [task 2019-03-19T08:26:57.663Z] 08:26:57 [task 2019-03-19T08:26:57.663Z] 08:26:57 [task 2019-03-19T08:26:57.671Z] 08:26:57 [task 2019-03-19T08:26:57.673Z] 08:26:57 [task 2019-03-19T08:26:57.674Z] 08:26:57 [task 2019-03-19T08:26:57.674Z] 08:26:57 [task 2019-03-19T08:26:57.674Z] 08:26:57 [task 2019-03-19T08:26:57.675Z] 08:26:57 [task 2019-03-19T08:26:57.757Z] 08:26:57 [task 2019-03-19T08:26:57.759Z] 08:26:57 [task 2019-03-19T08:26:57.760Z] 08:26:57 [task 2019-03-19T08:26:57.760Z] 08:26:57 [task 2019-03-19T08:26:57.761Z] 08:26:57 [task 2019-03-19T08:26:57.761Z] 08:26:57 [task 2019-03-19T08:26:57.761Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.762Z] 08:26:57 [task 2019-03-19T08:26:57.763Z] 08:26:57 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o s_cbrt.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/workspace/build/src/modules/fdlibm/src -I/builds/worker/workspace/build/src/obj-firefox/modules/fdlibm/src -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 -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -Wno-parentheses -Wno-sign-compare -Wno-dangling-else -MD -MP -MF .deps/s_cbrt.o.pp /builds/worker/workspace/build/src/modules/fdlibm/src/s_cbrt.cpp
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/modules/fdlibm/src'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/modules/fdlibm/src'
INFO - modules/fdlibm/src/s_ceilf.o
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/modules/fdlibm/src'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/modules/zlib/src'
INFO - modules/zlib/src/inffast.o
INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang -std=gnu99 -m32 -o inffast.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -I/builds/worker/workspace/build/src/modules/zlib/src -I/builds/worker/workspace/build/src/obj-firefox/modules/zlib/src -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 -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -fPIC -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -MD -MP -MF .deps/inffast.o.pp /builds/worker/workspace/build/src/modules/zlib/src/inffast.c
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/modules/zlib/src'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/logalloc'
INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o LogAlloc.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DMOZ_REPLACE_MALLOC_PREFIX=logalloc -DMOZ_NO_MOZALLOC -DMOZ_HAS_MOZGLUE -I/builds/worker/workspace/build/src/memory/replace/logalloc -I/builds/worker/workspace/build/src/obj-firefox/memory/replace/logalloc -I/builds/worker/workspace/build/src/memory/build -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 -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -MD -MP -MF .deps/LogAlloc.o.pp /builds/worker/workspace/build/src/memory/replace/logalloc/LogAlloc.cpp
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/logalloc'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/dmd'
INFO - mkdir -p '.deps/'
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/dmd'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/dmd'
INFO - memory/replace/dmd/Unified_cpp_memory_replace_dmd0.o
INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/memory/replace/dmd'
INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/js/src/frontend'
INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o BinASTParser.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/workspace/build/src/js/src/frontend -I/builds/worker/workspace/build/src/obj-firefox/js/src/frontend -I/builds/worker/workspace/build/src/obj-firefox/js/src -I/builds/worker/workspace/build/src/js/src -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/js/src/js-confdefs.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-noexcept-type -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -fno-strict-aliasing -Werror=format -Wno-shadow -MD -MP -MF .deps/BinASTParser.o.pp /builds/worker/workspace/build/src/js/src/frontend/BinASTParser.cpp
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BinASTParser.cpp:12:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BinASTParser.h:17:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BCEParserHandle.h:12:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/Parser.h:186:
ERROR - /builds/worker/workspace/build/src/js/src/frontend/ParseContext.h:679:14: error: expected class name
INFO - : public IgnoreGCPolicy {};
INFO - ^
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BinASTParser.cpp:12:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BinASTParser.h:17:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/BCEParserHandle.h:11:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/FullParseHandler.h:16:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/ParseNode.h:12:
INFO - In file included from /builds/worker/workspace/build/src/js/src/frontend/TokenStream.h:211:
INFO - In file included from /builds/worker/workspace/build/src/js/src/util/Text.h:23:
INFO - In file included from /builds/worker/workspace/build/src/js/src/NamespaceImports.h:16:

Flags: needinfo?(sphink)

Pulsebot

Comment 11

•

5 years ago

Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ce0c7c9a1b6
[hazards] Inherit template attrs for plain HashMap/HashSet r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/a62af21dfc4b
root some hashtables r=jonco

Andrei Ciure[:aciure]

Comment 12

•

5 years ago

Backed out 2 changesets (bug 1257982) for causing multiple ProtectedData.cpp asertion failures

push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a62af21dfc4b03f04bc5d2cf1237c8187959545c

failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=234894566&searchStr=linux%2Cx64%2Cquantumrender%2Cdebug%2Creftests%2Cwith%2Ce10s%2Ctest-linux64-qr%2Fdebug-jsreftest-e10s-1%2Cr-e10s%28j1%29

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/58f85a29cee1e7ef51a860b683fb5c93366495e8

BMO Automation

Updated

•

2 years ago

Severity: normal → S3

hazards.txt 5 years ago Steve Fink [:sfink] [:s:] 27.38 KB, text/plain		Details
Bug 1257982 - [hazards] Inherit template attrs for plain HashMap/HashSet r=jonco 5 years ago Steve Fink [:sfink] [:s:] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1257982 - root some hashtables r=jonco 5 years ago Steve Fink [:sfink] [:s:] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1257982 - [hazards] Inherit template attrs for plain HashMap/HashSet r=jonco 5 years ago Steve Fink [:sfink] [:s:] 47 bytes, text/x-phabricator-request		Details \| Review