In order to implement Rooted<struct> and WeakCache, we expect to be able to call a function with the wrong type. This is undefined, obviously, but relatively safe as long as everything is a pointer. When I tried to architect things on virtual before, I ran into heinous dependency cycles. I think for now at least we want to suppress these sites instead of reworking our rooting architecture.

Comment on attachment 8768593 [details] [diff] [review] ubsan-blacklist-generic-indirection-v0.diff Review of attachment 8768593 [details] [diff] [review]: ----------------------------------------------------------------- ::: mfbt/Attributes.h @@ +292,5 @@ > + */ > +#if defined(__clang__) > +# define MOZ_UBSAN_BLACKLIST MOZ_NEVER_INLINE __attribute__((no_sanitize("function"))) > +#else > +# define MOZ_UBSAN_BLACKLIST /* nothing */ I know it's messy, but... First, since this is blacklisting only mistyped function pointer calls, this should be MOZ_UBSAN_BLACKLIST_FUNCTION (to match the attribute name, though it really makes it sound like you're blacklisting an entire function. Which you are, but that has nothing to do with the "function" part.) Second, I'd like this to work in both clang and gcc. Annoyingly, they don't seem to support the same syntax (or functionality, even). So I think this'll have to be #if defined(__clang__) # define MOZ_UBSAN_BLACKLIST_FUNCTION MOZ_NEVER_INLINE __attribute__((no_sanitize("function"))) #elif defined(__GNUC__) # define MOZ_UBSAN_BLACKLIST_FUNCTION MOZ_NEVER_INLINE __attribute__((no_sanitize_undefined)) #else # define MOZ_UBSAN_BLACKLIST_FUNCTION /* nothing */ #endif Third, and don't do anything about this, but calling it a "blacklist" seems like a double-negative. You're blacklisting it from the list of things to check for problems. I would've called it a whitelist, But there are other flags that use the term blacklist for this, so this is just me complaining, not a request for a change. ;-)

Hurm, in all the GCC builds I'm getting: obj-spider/dist/include/js/SweepingAPI.h:60:45: error: 'no_sanitize_undefined' attribute directive ignored [-Werror=attributes]

GCC 4.8 is too old for UBSan, so I guess we'll need to just support clang for now.

Pushed by tcole@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d85334f696eb Blacklist UBSan detection of the GC's generic interfaces; r=sfink

The patch doesn't compile for me. Mac 10.10.5, Xcode 7.2.1

What are the failures?

/usr/local/bin/ccache /usr/bin/clang++ -std=gnu++11 -o Unified_cpp_dom_presentation0.o -c -fvisibility=hidden -fvisibility-inlines-hidden -DDEBUG=1 -DTRACING=1 -DOS_POSIX=1 -DOS_MACOSX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/Users/varga/Sources/Mozilla/dom/presentation -I/Users/varga/Sources/Mozilla/obj-ff-dbg/dom/presentation -I/Users/varga/Sources/Mozilla/dom/base -I/Users/varga/Sources/Mozilla/obj-ff-dbg/ipc/ipdl/_ipdlheaders -I/Users/varga/Sources/Mozilla/ipc/chromium/src -I/Users/varga/Sources/Mozilla/ipc/glue -I/Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include -I/Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/nspr -I/Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /Users/varga/Sources/Mozilla/obj-ff-dbg/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_dom_presentation0.o.pp -Qunused-arguments -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wthread-safety -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fno-omit-frame-pointer -Werror /Users/varga/Sources/Mozilla/obj-ff-dbg/dom/presentation/Unified_cpp_dom_presentation0.cpp In file included from /Users/varga/Sources/Mozilla/obj-ff-dbg/dom/presentation/Unified_cpp_dom_presentation0.cpp:2: In file included from /Users/varga/Sources/Mozilla/dom/presentation/AvailabilityCollection.cpp:10: In file included from /Users/varga/Sources/Mozilla/dom/presentation/PresentationAvailability.h:10: In file included from /Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/mozilla/DOMEventTargetHelper.h:12: In file included from /Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/nsCycleCollectionParticipant.h:13: /Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/js/RootingAPI.h:649:5: error: unknown attribute 'no_sanitize' ignored [-Werror,-Wunknown-attributes] MOZ_UBSAN_BLACKLIST_FUNCTION static void TraceWrapped(JSTracer* trc, T* thingp, ^ /Users/varga/Sources/Mozilla/obj-ff-dbg/dist/include/mozilla/Attributes.h:219:71: note: expanded from macro 'MOZ_UBSAN_BLACKLIST_FUNCTION' # define MOZ_UBSAN_BLACKLIST_FUNCTION MOZ_NEVER_INLINE __attribute__((no_sanitize("function"))) ^ 1 error generated. make[1]: *** [Unified_cpp_dom_presentation0.o] Error 1 make: *** [default] Error 2

The no_sanitize flag was added in 3.8 and is not present in 3.6. In XCode 7.2.1 on OSX 10.10, the LLVM version number is "Apple LLVM version 7.0.2 (clang-700.1.81)", which, sadly, is not particularly useful. Presumably it's some version of 3.6 that does not include the new flag. Looking at "Mac OS X Build Prerequisites" on MDN, it's really not clear what our minimum compiler version is, if we even have one. I guess since xcode makes it impossible to even know what version of clang you're running, much less control what version you're running, there's little point specifying. So I guess maybe we should fix the wiki? Or I can backout, figure out how to feature gate this feature gate? I'll ask around to see if anyone actually knows what our minimum version is here and why.

I guess we are still on 3.6, so here is a backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/16daeb9b4e17

reopen because of backout as mentioned in #13

You just need my patch in bug 1305345.