Skip to content

Commit

Permalink
Postpone menu destruction
Browse files Browse the repository at this point in the history 
...to avoid use-after-free, as e.g. happens on macOS with -fsanitize=address in
CppunitTest_sw_mailmerge:

> ==29010==ERROR: AddressSanitizer: heap-use-after-free on address 0x60800088faf8 at pc 0x000118ebc153 bp 0x7fff52f81a40 sp 0x7fff52f81a38
> READ of size 8 at 0x60800088faf8 thread T0
> WARNING: failed decoding unknown ioctl 0x20007454
> WARNING: failed decoding unknown ioctl 0x20007452
>     #0 0x118ebc152 in AquaSalMenu::SetSubMenu(SalMenuItem*, SalMenu*, unsigned int) salmenu.cxx:597
>     #1 0x1177bbefc in Menu::SetPopupMenu(unsigned short, PopupMenu*) menu.cxx:803
>     #2 0x138dccb5a in framework::MenuBarManager::RemoveListener() menubarmanager.cxx:552
>     #3 0x138dcb452 in framework::MenuBarManager::Destroy() menubarmanager.cxx:237
>     #4 0x138dcd6d7 in framework::MenuBarManager::dispose() menubarmanager.cxx:267
>     #5 0x138dccfa2 in framework::MenuBarManager::RemoveListener() menubarmanager.cxx:587
>     #6 0x138dcb452 in framework::MenuBarManager::Destroy() menubarmanager.cxx:237
>     #7 0x138dcd6d7 in framework::MenuBarManager::dispose() menubarmanager.cxx:267
>     #8 0x138e06acf in framework::MenuBarWrapper::dispose() menubarwrapper.cxx:103
>     #9 0x1389c0f67 in framework::LayoutManager::impl_clearUpMenuBar() layoutmanager.cxx:226
>     #10 0x1389c497b in framework::LayoutManager::implts_destroyElements() layoutmanager.cxx:447
>     #11 0x1389c3777 in framework::LayoutManager::implts_reset(bool) layoutmanager.cxx:413
>     #12 0x1389edf4b in framework::LayoutManager::frameAction(com::sun::star::frame::FrameActionEvent const&) layoutmanager.cxx:2811
>     #13 0x138b9e1a8 in (anonymous namespace)::Frame::implts_sendFrameActionEvent(com::sun::star::frame::FrameAction const&) frame.cxx:3110
>     #14 0x138b8219b in (anonymous namespace)::Frame::setComponent(com::sun::star::uno::Reference<com::sun::star::awt::XWindow> const&, com::sun::star::uno::Reference<com::sun::star::frame::XController> const&) frame.cxx:1557
>     #15 0x138b88545 in (anonymous namespace)::Frame::close(unsigned char) frame.cxx:1801
>     #16 0x12078429a in SfxFrame::DoClose() frame.cxx:127
>     #17 0x120812990 in SfxViewFrame::Notify(SfxBroadcaster&, SfxHint const&) viewfrm.cxx:1234
>     #18 0x11ab542d5 in SfxBroadcaster::Broadcast(SfxHint const&) SfxBroadcaster.cxx:50
>     #19 0x1203a0682 in SfxModelListener_Impl::notifyClosing(com::sun::star::lang::EventObject const&) objxtor.cxx:171
>     #20 0x1204453d2 in SfxBaseModel::close(unsigned char) sfxbasemodel.cxx:1372
>     #21 0x1245130d5 in SwXTextDocument::close(unsigned char) unotxdoc.cxx:621
>     #22 0x1247af99b in CloseModelAndDocSh(com::sun::star::uno::Reference<com::sun::star::frame::XModel>&, tools::SvRef<SfxObjectShell>&) unomailmerge.cxx:115
>     #23 0x1247af4bf in DeleteTmpFile_Impl(com::sun::star::uno::Reference<com::sun::star::frame::XModel>&, tools::SvRef<SfxObjectShell>&, rtl::OUString const&) unomailmerge.cxx:342
>     #24 0x1247b6ad6 in SwXMailMerge::execute(com::sun::star::uno::Sequence<com::sun::star::beans::NamedValue> const&) unomailmerge.cxx:814
>     #25 0x1247b9c62 in non-virtual thunk to SwXMailMerge::execute(com::sun::star::uno::Sequence<com::sun::star::beans::NamedValue> const&) unomailmerge.cxx:434
>     #26 0x11eeab4fd in MMTest::executeMailMerge() mailmerge.cxx:179
>     #27 0x11eea2470 in testMultiPageAnchoredDraws::verify() mailmerge.cxx:336
>     #28 0x11ef3be50 in MMTest::executeMailMergeTest(char const*, char const*, char const*, bool, int, char const*) mailmerge.cxx:87
>     #29 0x11ef38af3 in testMultiPageAnchoredDraws::MailMerge() mailmerge.cxx:334
>     #30 0x11ef557b9 in CppUnit::TestCaller<testMultiPageAnchoredDraws>::runTest() TestCaller.h:166
>     #31 0x10cfff749 in CppUnit::TestCaseMethodFunctor::operator()() const TestCase.cpp:32
>     #32 0x110736b67 in (anonymous namespace)::Protector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) vclbootstrapprotector.cxx:36
>     #33 0x10cfc9c20 in CppUnit::ProtectorChain::ProtectFunctor::operator()() const ProtectorChain.cpp:20
>     #34 0x110498fa7 in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unobootstrapprotector.cxx:89
>     #35 0x10cfc9c20 in CppUnit::ProtectorChain::ProtectFunctor::operator()() const ProtectorChain.cpp:20
>     #36 0x10f776880 in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unoexceptionprotector.cxx:65
>     #37 0x10cfc9c20 in CppUnit::ProtectorChain::ProtectFunctor::operator()() const ProtectorChain.cpp:20
>     #38 0x10cf64042 in CppUnit::DefaultProtector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) DefaultProtector.cpp:15
>     #39 0x10cfc9c20 in CppUnit::ProtectorChain::ProtectFunctor::operator()() const ProtectorChain.cpp:20
>     #40 0x10cfc7200 in CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) ProtectorChain.cpp:77
>     #41 0x10d06f15a in CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) TestResult.cpp:181
>     #42 0x10cffd8cd in CppUnit::TestCase::run(CppUnit::TestResult*) TestCase.cpp:91
>     #43 0x10d00097a in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) TestComposite.cpp:64
>     #44 0x10d00045e in CppUnit::TestComposite::run(CppUnit::TestResult*) TestComposite.cpp:23
>     #45 0x10d00097a in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) TestComposite.cpp:64
>     #46 0x10d00045e in CppUnit::TestComposite::run(CppUnit::TestResult*) TestComposite.cpp:23
>     #47 0x10d0990ac in CppUnit::TestRunner::WrappingSuite::run(CppUnit::TestResult*) TestRunner.cpp:47
>     #48 0x10d06da55 in CppUnit::TestResult::runTest(CppUnit::Test*) TestResult.cpp:148
>     #49 0x10d099ebd in CppUnit::TestRunner::run(CppUnit::TestResult&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) TestRunner.cpp:96
>     #50 0x10cc6f509 in (anonymous namespace)::ProtectedFixtureFunctor::run() const cppunittester.cxx:305
>     #51 0x10cc6a0ac in sal_main() cppunittester.cxx:455
>     #52 0x10cc687a6 in main cppunittester.cxx:362
>     #53 0x7fffc9f36254 in start (libdyld.dylib+0x5254)
>
> 0x60800088faf8 is located 88 bytes inside of 96-byte region [0x60800088faa0,0x60800088fb00)
> freed by thread T0 here:
>     #0 0x10d327b6b in wrap__ZdlPv asan_new_delete.cc:179
>     #1 0x118eb3011 in AquaSalMenu::~AquaSalMenu() salmenu.cxx:279
>     #2 0x118eb10de in AquaSalInstance::DestroyMenu(SalMenu*) salmenu.cxx:238
>     #3 0x1177acd1d in Menu::ImplSetSalMenu(SalMenu*) menu.cxx:2342
>     #4 0x1177ab046 in Menu::dispose() menu.cxx:183
>     #5 0x1177d86f5 in PopupMenu::dispose() menu.cxx:2764
>     #6 0x117f99ee2 in VclReferenceBase::disposeOnce() vclreferencebase.cxx:42
>     #7 0x1177bf7d8 in VclPtr<Menu>::disposeAndClear() vclptr.hxx:208
>     #8 0x1177bbbd5 in Menu::SetPopupMenu(unsigned short, PopupMenu*) menu.cxx:788
>     #9 0x138dccb5a in framework::MenuBarManager::RemoveListener() menubarmanager.cxx:552
>     #10 0x138dcb452 in framework::MenuBarManager::Destroy() menubarmanager.cxx:237
>     #11 0x138dcd6d7 in framework::MenuBarManager::dispose() menubarmanager.cxx:267
>     #12 0x138dccfa2 in framework::MenuBarManager::RemoveListener() menubarmanager.cxx:587
>     #13 0x138dcb452 in framework::MenuBarManager::Destroy() menubarmanager.cxx:237
>     #14 0x138dcd6d7 in framework::MenuBarManager::dispose() menubarmanager.cxx:267
>     #15 0x138e06acf in framework::MenuBarWrapper::dispose() menubarwrapper.cxx:103
>     #16 0x1389c0f67 in framework::LayoutManager::impl_clearUpMenuBar() layoutmanager.cxx:226
>     #17 0x1389c497b in framework::LayoutManager::implts_destroyElements() layoutmanager.cxx:447
>     #18 0x1389c3777 in framework::LayoutManager::implts_reset(bool) layoutmanager.cxx:413
>     #19 0x1389edf4b in framework::LayoutManager::frameAction(com::sun::star::frame::FrameActionEvent const&) layoutmanager.cxx:2811
>     #20 0x138b9e1a8 in (anonymous namespace)::Frame::implts_sendFrameActionEvent(com::sun::star::frame::FrameAction const&) frame.cxx:3110
>     #21 0x138b8219b in (anonymous namespace)::Frame::setComponent(com::sun::star::uno::Reference<com::sun::star::awt::XWindow> const&, com::sun::star::uno::Reference<com::sun::star::frame::XController> const&) frame.cxx:1557
>     #22 0x138b88545 in (anonymous namespace)::Frame::close(unsigned char) frame.cxx:1801
>     #23 0x12078429a in SfxFrame::DoClose() frame.cxx:127
>     #24 0x120812990 in SfxViewFrame::Notify(SfxBroadcaster&, SfxHint const&) viewfrm.cxx:1234
>     #25 0x11ab542d5 in SfxBroadcaster::Broadcast(SfxHint const&) SfxBroadcaster.cxx:50
>     #26 0x1203a0682 in SfxModelListener_Impl::notifyClosing(com::sun::star::lang::EventObject const&) objxtor.cxx:171
>     #27 0x1204453d2 in SfxBaseModel::close(unsigned char) sfxbasemodel.cxx:1372
>     #28 0x1245130d5 in SwXTextDocument::close(unsigned char) unotxdoc.cxx:621
>     #29 0x1247af99b in CloseModelAndDocSh(com::sun::star::uno::Reference<com::sun::star::frame::XModel>&, tools::SvRef<SfxObjectShell>&) unomailmerge.cxx:115
>
> previously allocated by thread T0 here:
>     #0 0x10d32752b in wrap__Znwm asan_new_delete.cc:106
>     #1 0x118eafa18 in AquaSalInstance::CreateMenu(bool, Menu*) salmenu.cxx:230
>     #2 0x1177d75e0 in PopupMenu::PopupMenu() menu.cxx:2711
>     #3 0x1177d7664 in PopupMenu::PopupMenu() menu.cxx:2710
>     #4 0x129136557 in VclPtr<PopupMenu> VclPtr<PopupMenu>::Create<>() vclptr.hxx:131
>     #5 0x1291362de in VCLXMenu::ImplCreateMenu(bool) vclxmenu.cxx:73
>     #6 0x1291463ca in VCLXPopupMenu::VCLXPopupMenu() vclxmenu.cxx:901
>     #7 0x129146414 in VCLXPopupMenu::VCLXPopupMenu() vclxmenu.cxx:900
>     #8 0x138dc5e83 in framework::MenuBarManager::FillMenuManager(Menu*, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, bool) menubarmanager.cxx:1354
>     #9 0x138dc2316 in framework::MenuBarManager::MenuBarManager(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::util::XURLTransformer> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, Menu*, bool, bool) menubarmanager.cxx:140
>     #10 0x138dc72bd in framework::MenuBarManager::MenuBarManager(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::util::XURLTransformer> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, Menu*, bool, bool) menubarmanager.cxx:138
>     #11 0x138dc57bc in framework::MenuBarManager::FillMenuManager(Menu*, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, bool) menubarmanager.cxx:1304
>     #12 0x138dc2316 in framework::MenuBarManager::MenuBarManager(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::util::XURLTransformer> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, Menu*, bool, bool) menubarmanager.cxx:140
>     #13 0x138dc72bd in framework::MenuBarManager::MenuBarManager(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, com::sun::star::uno::Reference<com::sun::star::util::XURLTransformer> const&, com::sun::star::uno::Reference<com::sun::star::frame::XDispatchProvider> const&, rtl::OUString const&, Menu*, bool, bool) menubarmanager.cxx:138
>     #14 0x138e07ba5 in framework::MenuBarWrapper::initialize(com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) menubarwrapper.cxx:181
>     #15 0x138f32e6d in framework::MenuBarFactory::CreateUIElement(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, char const*, rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::ui::XUIElement> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&) menubarfactory.cxx:154
>     #16 0x138f31848 in framework::MenuBarFactory::createUIElement(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) menubarfactory.cxx:63
>     #17 0x138f3313a in non-virtual thunk to framework::MenuBarFactory::createUIElement(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) menubarfactory.cxx:56
>     #18 0x138f56f6e in (anonymous namespace)::UIElementFactoryManager::createUIElement(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) uielementfactorymanager.cxx:450
>     #19 0x138f5a94a in non-virtual thunk to (anonymous namespace)::UIElementFactoryManager::createUIElement(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) uielementfactorymanager.cxx:408
>     #20 0x1389ca5da in framework::LayoutManager::implts_createElement(rtl::OUString const&) layoutmanager.cxx:731
>     #21 0x1389d6fb4 in framework::LayoutManager::createElement(rtl::OUString const&) layoutmanager.cxx:1482
>     #22 0x11fbd36f1 in SfxDispatcher::SetMenu_Impl() dispatch.cxx:1216
>     #23 0x11fbc36dc in SfxDispatcher::Update_Impl(bool) dispatch.cxx:1290
>     #24 0x11fb7556d in SfxBindings::NextJob_Impl(Timer*) bindings.cxx:1459
>     #25 0x11fb8ad4c in SfxBindings::NextJob(Timer*) bindings.cxx:1441
>     #26 0x11fb61177 in SfxBindings::LinkStubNextJob(void*, Timer*) bindings.cxx:1439
>     #27 0x11898aea1 in Link<Timer*, void>::Call(Timer*) const link.hxx:84
>     #28 0x11898add6 in Timer::Invoke() timer.cxx:88
>     #29 0x1188dae6d in ImplSchedulerData::Invoke() scheduler.cxx:47

Change-Id: I16d5b11710ee46dbaa77afd94a09ba5f07a311b0
  • Loading branch information
@stbergmann
stbergmann committed Nov 7, 2016
1 parent 4e3e87e commit 3f7fc4e
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion vcl/source/window/menu.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ void Menu::SetPopupMenu( sal_uInt16 nItemId, PopupMenu* pMenu )
return;

// remove old menu
pData->pSubMenu.disposeAndClear();
auto oldSubMenu = pData->pSubMenu;

// data exchange
pData->pSubMenu = pMenu;
Expand All @@ -803,6 +803,8 @@ void Menu::SetPopupMenu( sal_uInt16 nItemId, PopupMenu* pMenu )
ImplGetSalMenu()->SetSubMenu( pData->pSalMenuItem, nullptr, nPos );
}

oldSubMenu.disposeAndClear();

ImplCallEventListeners( VCLEVENT_MENU_SUBMENUCHANGED, nPos );
}

Expand Down

0 comments on commit 3f7fc4e

Please sign in to comment.