196472 2019-04-01 17:07:09 -0700 CFI base-to-derived cast error in SentinelLinkedList.h 2019-05-15 12:19:05 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified NEW P2 Normal --- 196533 1 chris.reid webkit-unassigned ap don.olmstead fpizlo fujii mcatanzaro saam stephan.szabo ysuzuki oldest_to_newest 1523143 0 chris.reid 2019-04-01 17:07:09 -0700 We're trying to get CFI running with JSC and one of the errors we're seeing is: `SentinelLinkedList.h:63:24: runtime error: control flow integrity check for type 'JSC::Watchpoint' failed during base-to-derived cast (vtable address 0x000000000000)` (lldb) bt * thread #6, name = ' via C++ thread', stop reason = Cfi bad type frame #0: 0x0000000000877810 testapi`__ubsan_on_report at ubsan_monitor.cc:39:40 frame #1: 0x0000000000871d54 testapi`::~Diag() at ubsan_diag.cc:354:29 frame #2: 0x00000000008789b4 testapi`::__ubsan_handle_cfi_bad_type() at ubsan_handlers_cxx.cc:133:3 frame #3: 0x00000000008771b2 testapi`__ubsan_handle_cfi_check_fail at ubsan_handlers.cc:846:5 * frame #4: 0x00000000009339a1 testapi`WTF::BasicRawSentinelNode<JSC::Watchpoint>::next(this=0x00007ffff0df93a0) at SentinelLinkedList.h:63:24 frame #5: 0x0000000000a34bc5 testapi`WTF::SentinelLinkedList<JSC::Watchpoint, WTF::BasicRawSentinelNode<JSC::Watchpoint> >::begin(this=0x00007ffff0df93a0) at SentinelLinkedList.h:151:43 frame #6: 0x0000000000a34b8b testapi`WTF::SentinelLinkedList<JSC::Watchpoint, WTF::BasicRawSentinelNode<JSC::Watchpoint> >::isEmpty(this=0x00007ffff0df93a0) at SentinelLinkedList.h:102:29 frame #7: 0x0000000000a34b3d testapi`JSC::WatchpointSet::~WatchpointSet(this=0x00007ffff0df9398) at Watchpoint.cpp:70:19 ... This looks like a valid base-to-derived error because the SentinelLinkedList head and tail nodes are constructed as the base class BasicRawSentinelNode<JSC::Watchpoint>. m_headSentinel.next() tries to cast the object constructed as BasicRawSentinelNode<JSC::Watchpoint>* to a JSC::Watchpoint*. Those two objects are also different sizes. It doesn't seem like the head and tail nodes are dereferenced after a prev/next call, but it will probably be unsafe if they do end up being dereferenced. 1523798 1 fpizlo 2019-04-03 11:12:22 -0700 This looks like it’s not a bug at all. 1523943 2 ysuzuki 2019-04-03 14:37:59 -0700 I think this use case exists everywhere in WebKit, and introducing some annotation / blacklisting this is not worth doing for this case. The best way here would be just disabling this check in CFI side. 1536521 3 mcatanzaro 2019-05-15 11:54:33 -0700 How is this OK? Why would you do this intentionally? 1536537 4 fpizlo 2019-05-15 12:19:05 -0700 (In reply to Michael Catanzaro from comment #3) > How is this OK? Why would you do this intentionally? Because it's valid C. We're only accessing fields that exist. We're just using the wrong enclosing type to do it.