-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add shadow propagation through memory-to-memory moves that use xmm registers #1453
Comments
From bruen...@google.com on February 25, 2014 07:39:57 Quoting from issue #243 : "It turns out that most (if not all) of these are simply memory-to-memory moves that use the xmm registers. This is very similar to many floating-point cases ( issue #471 ) and may be amenable to a similar solution as a workaround until we implement full xmm shadowing, which I split as issue #1453 . Full shadowing will require mirroring all the crazy data movements within these registers performed by all the many sets of SSE_, AVX_, etc. instruction sets and will be non-trivial." Here are some typical VS2013 uninits: http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%282%29/builds/486/steps/memory%20test%3A%20media/logs/stdio Error 0 base.dll!std::deque<>::emplace_back<> [e:\b\depot_tools\win_toolchain\vs2013_files\vc\include\deque:1161]1 base.dll!base::internal::IncomingTaskQueue::PostPendingTask [base\message_loop\incoming_task_queue.cc:138]2 base.dll!base::internal::IncomingTaskQueue::AddToIncomingQueue [base\message_loop\incoming_task_queue.cc:28]3 base.dll!base::internal::MessageLoopProxyImpl::PostDelayedTask [base\message_loop\message_loop_proxy_impl.cc:26]4 base.dll!base::TaskRunner::PostTask [base\task_runner.cc:45]5 media.dll!media::AudioManagerWin::AudioManagerWin [media\audio\win\audio_manager_win.cc:146]6 media.dll!media::CreateAudioManager [media\audio\win\audio_manager_win.cc:513]7 media.dll!media::AudioManager::Create [media\audio\audio_manager.cc:32]8 media.dll!media::AudioManager::CreateForTesting [media\audio\audio_manager.cc:40]9 testing::internal::HandleExceptionsInMethodIfSupported<> [testing\gtest\src\gtest.cc:2045]Note: @0:00:06.475 in thread 4088 0 CRNSS.dll!SEC_QuickDERDecodeItem_Util [third_party\nss\nss\lib\util\quickder.c:886]1 CRNSS.dll!sftk_GetPubKey [third_party\nss\nss\lib\softoken\pkcs11.c:1759]2 CRNSS.dll!sftk_handlePublicKeyObject [third_party\nss\nss\lib\softoken\pkcs11.c:967]3 CRNSS.dll!sftk_handleKeyObject [third_party\nss\nss\lib\softoken\pkcs11.c:1352]4 CRNSS.dll!sftk_handleObject [third_party\nss\nss\lib\softoken\pkcs11.c:1606]5 CRNSS.dll!NSC_GenerateKeyPair [third_party\nss\nss\lib\softoken\pkcs11c.c:4914]6 CRNSS.dll!PK11_GenerateKeyPairWithOpFlags [third_party\nss\nss\lib\pk11wrap\pk11akey.c:1388]7 CRNSS.dll!PK11_GenerateKeyPairWithFlags [third_party\nss\nss\lib\pk11wrap\pk11akey.c:1471]8 CRNSS.dll!PK11_GenerateKeyPair [third_party\nss\nss\lib\pk11wrap\pk11akey.c:1495]9 crcrypto.dll!crypto::ECPrivateKey::CreateWithParams [crypto\ec_private_key_nss.cc:313]#10 crcrypto.dll!crypto::ECPrivateKey::Create [crypto\ec_private_key_nss.cc:96] *** TODO breakdown of most frequent errors I grabbed some stdio results from the bots:
grep -h 'Note: instruction:' * | sed 's/^.*instruction: //;s/</span>//' | wc67972 271913 2003443 grep -h 'Note: instruction:' * | sed 's/^.*instruction: //;s/</span>//' | sort | uniq -c | sort -n... grep -h 'Note: instruction:' * | sed 's/^.*instruction: //;s/</span>//' | sort | uniq -c | grep -v '; %xmm'
grep -A 1 'Error #.*UNINITIALIZED' *.txt | grep '# 0' | awk '{print $4}' | sort | uniq -c | sort -n
1022 CRNSS.dll!DecodeSequence *** TODO get surrounding code for xmm ones cd /e/derek/chromium/src/outVS2013/out/Release(master)~/drmemory/git/build_x86_dbg/bin/drmemory.exe -pause_at_error -pause_at_assert $DRMEM_CHROME_ARGS -dr d:/derek/dr/git/exports -batch -- ./media_unittests.exe --ui-test-action-timeout=12000000 --ui-test-action-max-timeout=28000000 --ui-test-terminate-timeout=12000000 --gtest_filter=AudioInputControllerTest.CreateAndCloseNearly all are memory-to-memory copies that just use xmm0, like this: Both 64-bit and 128-bit in size -- so will be in slowpath. media: crypto ECPrivateKeyUnitTest.InitRandomTest: so while the deque one may be amenable to a heuristic, the crypto one has crypto ECPrivateKeyUnitTest.InitRandomTest also: net HttpBasicStateTest.ConstructsProperly: |
From bruen...@google.com on February 25, 2014 07:39:57 ...mium\src\net\http\http_response_info.cc @ 150]: net QuicCryptoClientStreamTest.ConnectedAfterSHLO crypto ECPrivateKeyUnitTest.InitRandomTest also: unit CaptivePortalTabHelperTest.HttpTimeoutLinkDoctor unit CaptivePortalTabHelperTest.HttpTimeoutLinkDoctor also unit CaptivePortalTabHelperTest.HttpTimeoutLinkDoctor also |
From bruen...@google.com on February 25, 2014 09:27:27 Labels: -Priority-Medium Priority-High Hotlist-Chrome |
From bruen...@google.com on February 26, 2014 08:10:54 After implementing a simple check modeled on the issue #471 check, most of the errors go away. *** DONE gather stats: big slowpath perf hit? nontrivial but livable conclusions: while there are a bunch of movdqu in the slowpath, they're crypto ECPrivateKeyUnitTest.InitRandomTest: unit CaptivePortalTabHelperTest.HttpTimeoutLinkDoctor *** TODO still seeing uninits due to more complex sequences % grep -A 1 'Error #.*UNINIT' /e/derek/chromium/src/OUT-i1453 | grep -vE 'UNINIT|^--' | awk '{print $4}' | sort | uniq -c icuuc and CmpHelperEq are not xmm0, so we really just have 2 here: [ RUN ] FileStreamTest.AsyncOpenExplicitClose 2 problems here: need to store 2 at once! and store addr depends on reg net!base::internal::ReturnAsParamAdapternet::FileStream::Context::OpenResult+0x17 [e:\derek\chromium\src\base\task_runner_util.h @ 23]: This one would need more complex determination of the store address: net!base::internal::Invoker<1,base::internal::BindState<base::internal::RunnableAdapter<net::FileStream::Context::IOResult (__thiscall net::FileStream::Context::*)(void)>,net::FileStream::Context::IOResult __cdecl(net::FileStream::Context *),void __cdecl(base::internal::UnretainedWrappernet::FileStream::Context)>,net::FileStream::Context::IOResult __cdecl(net::FileStream::Context *)>::Run+0x36 [e:\derek\chromium\src\base\bind_internal.h @ 1169]: |
From derek.br...@gmail.com on February 26, 2014 18:18:31 This issue was closed by revision r1746 . Status: Fixed |
From bruen...@google.com on February 27, 2014 13:56:29 Uh-oh, here are some more complex ones. They are passing a 16-byte struct to a caller: Error 0 extensions::WebNavigationTabObserver::DidStartProvisionalLoadForFrame [e:\derek\chromium\src\chrome\browser\extensions\api\web_navigation\web_navigation_api.cc:361](0x0338aa93 <unit_tests.exe+0x249aa93) modid:11 content.dll!content::WebContentsImpl::DidStartProvisionalLoad [e:\derek\chromium\src\content\browser\web_contents\web_contents_impl.cc:2024](0x633de435 <content.dll+0x2de435) modid:30338aa93 f30f6f856cffffff movdqu xmm0,xmmword ptr [ebp-94h] FrameNavigationState::FrameID frame_id(frame_num, render_view_host); 0:010> dt -v unit_tests!extensions::FrameNavigationState::FrameID Several more like that: Error 0 extensions::WebNavigationTabObserver::DidStartProvisionalLoadForFrame [e:\derek\chromium\src\chrome\browser\extensions\api\web_navigation\web_navigation_api.cc:359](0x0338aa60 <unit_tests.exe+0x249aa60) modid:11 content.dll!content::WebContentsImpl::DidStartProvisionalLoad [e:\derek\chromium\src\content\browser\web_contents\web_contents_impl.cc:2024](0x633de435 <content.dll+0x2de435) modid:3unit_tests!extensions::WebNavigationTabObserver::DidStartProvisionalLoadForFrame+0x90 [e:\derek\chromium\src\chrome\browser\extensions\api\web_navigation\web_navigation_api.cc @ 359]: Error 0 extensions::WebNavigationTabObserver::DidCommitProvisionalLoadForFrame [e:\derek\chromium\src\chrome\browser\extensions\api\web_navigation\web_navigation_api.cc:391](0x03389e75 <unit_tests.exe+0x2499e75) modid:11 content.dll!content::WebContentsImpl::DidCommitProvisionalLoad [e:\derek\chromium\src\content\browser\web_contents\web_contents_impl.cc:2117](0x633dd1a4 <content.dll+0x2dd1a4) modid:3unit_tests!extensions::WebNavigationTabObserver::DidCommitProvisionalLoadForFrame+0x75 [e:\derek\chromium\src\chrome\browser\extensions\api\web_navigation\web_navigation_api.cc @ 391]: etc. |
From bruen...@google.com on February 27, 2014 15:36:34 Re-opening. One observation is that 16-byte memrefs currently always go to slowpath: so I can solve this w/o needing the bitlevel hack, and thus w/o needing to know the store's address. Status: Started |
From bruen...@google.com on February 27, 2014 18:02:02 Strike that: aligned 16-byte memrefs stay on fastpath. Here are 2 possible solutions: ***** TODO re-order load to be next to store in app2app How about re-ordering the load to be next to the store? There's no **** TODO add static shadowing of xmm We could add shadowing of xmm* but no handling of intra-xmm operations. An added benefit is that this is a step toward full propagation. We would need to figure out where to put the shadow memory, and if we have |
From bruen...@google.com on March 02, 2014 10:18:25 I went with the static shadowing as follows: issue #1453, issue #243: add shadowing of xmm registers and propagation to and from All xmm registers are shadowed, and we propagate to and from them on loads The shadowing is indirected, with a pointer to the memory in one new field Puts in place basic OR-combining for inter-xmm copies, but we need more Generalizes the shadow register layer to handle 16-byte registers (and thus Generalizes the slowpath to handle 16-byte registers. Starts generalizing the fastpath to handle src size != dst size for Does not handle ymm registers yet. Cleans up the incorrect use of mi->need_nonoffs_reg3 for cases that |
From bruen...@google.com on March 02, 2014 10:18:43 VS2013 uses sequences like this to zero data structures:
so we need to handle xor as well |
From derek.br...@gmail.com on March 02, 2014 17:53:42 This issue was closed by revision r1755 . Status: Fixed |
From bruen...@google.com on February 25, 2014 10:35:03
Split from issue #243 which covers the general solution of shadow propagation through xmm registers and operations.
Original issue: http://code.google.com/p/drmemory/issues/detail?id=1453
The text was updated successfully, but these errors were encountered: