|
|
DescriptionSplit the testing class into two.
The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest
The test cases are currently instantiated only with SYZYGY not with CLANG cause some fixes need to be done for the test cases to work with CLANG.
BUG=
Review-Url: https://codereview.chromium.org/2984303002
Committed: https://github.com/google/syzygy/commit/4c7cb860467e960396dc24e5146c57802ce3669b
Patch Set 1 #Patch Set 2 : Removed parametrization with Clang until the necessary fixes are done. #
Total comments: 22
Patch Set 3 : Response to reviewers comments. #
Total comments: 5
Patch Set 4 : Response to reviewers comments. #Patch Set 5 : Remove any CLANG code. #
Total comments: 10
Patch Set 6 : Response to reviewers comments #
Total comments: 2
Patch Set 7 : Response to reviewers comments #Patch Set 8 : Response to reviewers comments #Patch Set 9 : Rebase allocator shim #Patch Set 10 : Fix merging issue. #Messages
Total messages: 56 (39 generated)
Description was changed from ========== Split the testing class into two. Integration tests BUG= ========== to ========== Split the testing class into two. Integration tests BUG= ==========
njanevsk@google.com changed reviewers: + chrisha@chromium.org, sebmarchand@chromium.org
Description was changed from ========== Split the testing class into two. Integration tests BUG= ========== to ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest There are two test I commented out cause they are timing out whne run with the Clang instrumented dll. I will work on examining why that is happening. I need feedback on the splitting of the class. Is the approach OK? Any suggestions to improve it. BUG= ==========
Description was changed from ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest There are two test I commented out cause they are timing out whne run with the Clang instrumented dll. I will work on examining why that is happening. I need feedback on the splitting of the class. Is the approach OK? Any suggestions to improve it. BUG= ========== to ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest There are two test I commented out cause they are timing out whne run with the Clang instrumented dll. I will work on examining why that is happening. In the meanwhile, I need feedback on the splitting of the class. Is the approach OK? Any suggestions to improve it. BUG= ==========
Chris since Seb is OOO these two days I need your feedback on this CL.
Description was changed from ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest There are two test I commented out cause they are timing out whne run with the Clang instrumented dll. I will work on examining why that is happening. In the meanwhile, I need feedback on the splitting of the class. Is the approach OK? Any suggestions to improve it. BUG= ========== to ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest The test cases are currently instantiated only with SYZYGY not with CLANG cause some fixes need to be done for the test cases to work with CLANG. BUG= ==========
PTAL
The CQ bit was checked by sebmarchand@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); Don't remove this function. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:387: test_dll_path_ = temp_dir_.Append(input_dll_path_.BaseName()); Don't set this here, as it'll be invalid for the Clang test cases. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:439: } else if (instrumentation_mode == CLANG) { You don't need this yet as it's not instantiated. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:442: printf("%s\n", test_dll_path_.AsUTF8Unsafe().c_str()); Remove the debugging code. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:590: if (instrumentation_mode == SYZYGY) { I don't sure that you need this check for now? You're not using the CLANG type in your instantiation here so there's no need to add the SYZYGY checks until it breaks. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1250: void AsanSymbolizerTest(testing::EndToEndTestId test_id, This could probably be moved to the parent class? https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1590: // The following two tests are timing out when instrumented and run with Clang. Remove this comment as you're not instantiating the Clang test cases yet. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1694: ASSERT_NO_FATAL_FAILURE(EndToEndTest("bbentry", SYZYGY)); I'm not such a fan of having all these extra "SYZYGY" arguments. Could we derive another class from InstrumentAppIntegrationTest that default to SYZYGY? https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util.h File syzygy/pe/unittest_util.h (right): https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util... syzygy/pe/unittest_util.h:45: // Name of the clang instrumented integrations tests DLL You don't need this yet.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I improved the design based on your comments. PTAL. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/01 19:27:36, Sébastien Marchand wrote: > Don't remove this function. I haven't removed it. I moved it to the Parametrized test class so that the test cases associated with it can be ran with SYZYGY and CLANG. If those test cases should be ran only with SYZYGY I can put it back here. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:387: test_dll_path_ = temp_dir_.Append(input_dll_path_.BaseName()); On 2017/08/01 19:27:36, Sébastien Marchand wrote: > Don't set this here, as it'll be invalid for the Clang test cases. Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:439: } else if (instrumentation_mode == CLANG) { On 2017/08/01 19:27:36, Sébastien Marchand wrote: > You don't need this yet as it's not instantiated. Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:442: printf("%s\n", test_dll_path_.AsUTF8Unsafe().c_str()); On 2017/08/01 19:27:36, Sébastien Marchand wrote: > Remove the debugging code. Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:590: if (instrumentation_mode == SYZYGY) { On 2017/08/01 19:27:36, Sébastien Marchand wrote: > I don't sure that you need this check for now? You're not using the CLANG type > in your instantiation here so there's no need to add the SYZYGY checks until it > breaks. Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1250: void AsanSymbolizerTest(testing::EndToEndTestId test_id, On 2017/08/01 19:27:36, Sébastien Marchand wrote: > This could probably be moved to the parent class? Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1590: // The following two tests are timing out when instrumented and run with Clang. On 2017/08/01 19:27:36, Sébastien Marchand wrote: > Remove this comment as you're not instantiating the Clang test cases yet. Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1694: ASSERT_NO_FATAL_FAILURE(EndToEndTest("bbentry", SYZYGY)); On 2017/08/01 19:27:36, Sébastien Marchand wrote: > I'm not such a fan of having all these extra "SYZYGY" arguments. Could we derive > another class from InstrumentAppIntegrationTest that default to SYZYGY? Done. https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util.h File syzygy/pe/unittest_util.h (right): https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util... syzygy/pe/unittest_util.h:45: // Name of the clang instrumented integrations tests DLL On 2017/08/01 19:27:36, Sébastien Marchand wrote: > You don't need this yet. Is there a problem if it's left here? Cause it is declared in the associated .cc file and is used in instrument_integration_test.cc
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 01:23:21, njanevsk wrote: > On 2017/08/01 19:27:36, Sébastien Marchand wrote: > > Don't remove this function. > > I haven't removed it. I moved it to the Parametrized test class so that the test > cases associated with it can be ran with SYZYGY and CLANG. If those test cases > should be ran only with SYZYGY I can put it back here. Why just this one and not the other Asan*Test functions then? The Parameterized class derives from this one, so it can use any of the function here. https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util.h File syzygy/pe/unittest_util.h (right): https://codereview.chromium.org/2984303002/diff/20001/syzygy/pe/unittest_util... syzygy/pe/unittest_util.h:45: // Name of the clang instrumented integrations tests DLL On 2017/08/03 01:23:21, njanevsk wrote: > On 2017/08/01 19:27:36, Sébastien Marchand wrote: > > You don't need this yet. > > Is there a problem if it's left here? Cause it is declared in the associated .cc > file and is used in instrument_integration_test.cc You don't need this in this CL at all, here or in the cc, and instrument_integration_test.cc doesn't use this yet, all you're doing is splitting the class in 2 in preparation for Clang but you're not using the Clang instrumented binaries yet. https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:352: CLANG, I'd remove CLANG for now. https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1292: class ParametrizedLenientInstrumentAppIntegrationTest Add a comment to describe the purpose of this class. https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1298: if (GetParam() == CLANG) { Remove this, you'll add this once we do want to run the Clang tests? (i.e. add it to the INSTANTIANTE_TEST_CASES_P call) https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1307: } else if (GetParam() == CLANG) { Ditto https://codereview.chromium.org/2984303002/diff/40001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1314: if (GetParam() == SYZYGY) { Ditto
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 18:12:59, Sébastien Marchand wrote: > On 2017/08/03 01:23:21, njanevsk wrote: > > On 2017/08/01 19:27:36, Sébastien Marchand wrote: > > > Don't remove this function. > > > > I haven't removed it. I moved it to the Parametrized test class so that the > test > > cases associated with it can be ran with SYZYGY and CLANG. If those test cases > > should be ran only with SYZYGY I can put it back here. > > Why just this one and not the other Asan*Test functions then? The Parameterized > class derives from this one, so it can use any of the function here. Ha, I just realized that this depends on some things done at instrumentation time, and so it won't work with Clang yet. I'd just wrap the body of this function into a "if (GetParam() == SYZYGY)" block then.
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 20:44:31, Sébastien Marchand wrote: > On 2017/08/03 18:12:59, Sébastien Marchand wrote: > > On 2017/08/03 01:23:21, njanevsk wrote: > > > On 2017/08/01 19:27:36, Sébastien Marchand wrote: > > > > Don't remove this function. > > > > > > I haven't removed it. I moved it to the Parametrized test class so that the > > test > > > cases associated with it can be ran with SYZYGY and CLANG. If those test > cases > > > should be ran only with SYZYGY I can put it back here. > > > > Why just this one and not the other Asan*Test functions then? The > Parameterized > > class derives from this one, so it can use any of the function here. > > Ha, I just realized that this depends on some things done at instrumentation > time, and so it won't work with Clang yet. I'd just wrap the body of this > function into a "if (GetParam() == SYZYGY)" block then. (the body of the function or the tests, it just feels weird to have only one function moved out of this class).
PTAL.
https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1292: class ParametrizedLenientInstrumentAppIntegrationTest Add a comment explaining the purpose of this class. Say that it doesn't do much yet (it mostly forward the calls to the parent class) but will be extended for Clang. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1296: void SetUp() { LenientInstrumentAppIntegrationTest::SetUp(); } You don't need this? https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1298: void EndToEndTest(const std::string& mode) { Add an "override" keyword. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1303: void AsanZebraHeapTest(bool enabled); As mentioned in the previous comment, leave this in the parent class. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1305: void EndToEndCheckTestDll() { Add the override keyword.
PTAL https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1292: class ParametrizedLenientInstrumentAppIntegrationTest On 2017/08/07 16:46:59, Sébastien Marchand wrote: > Add a comment explaining the purpose of this class. Say that it doesn't do much > yet (it mostly forward the calls to the parent class) but will be extended for > Clang. Done. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1296: void SetUp() { LenientInstrumentAppIntegrationTest::SetUp(); } On 2017/08/07 16:46:59, Sébastien Marchand wrote: > You don't need this? I need this when adding CLANG to set the path: if (GetParam() == CLANG) { test_dll_path_ = testing::GetExeRelativePath(testing::kIntegrationTestsClangDllName); } I'll remove it for now. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1298: void EndToEndTest(const std::string& mode) { On 2017/08/07 16:46:59, Sébastien Marchand wrote: > Add an "override" keyword. Done. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1303: void AsanZebraHeapTest(bool enabled); On 2017/08/07 16:46:59, Sébastien Marchand wrote: > As mentioned in the previous comment, leave this in the parent class. Done. https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_test... syzygy/integration_tests/instrument_integration_test.cc:1305: void EndToEndCheckTestDll() { On 2017/08/07 16:46:59, Sébastien Marchand wrote: > Add the override keyword. Done.
lgtm https://codereview.chromium.org/2984303002/diff/100001/syzygy/integration_tes... File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/100001/syzygy/integration_tes... syzygy/integration_tests/instrument_integration_test.cc:1294: // Class created to enable parametrization of the tests with clang. Clang. https://codereview.chromium.org/2984303002/diff/100001/syzygy/integration_tes... syzygy/integration_tests/instrument_integration_test.cc:1296: // Rigth now it just forwards the calls to the parent class. right.
The CQ bit was checked by njanevsk@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sebmarchand@chromium.org Link to the patchset: https://codereview.chromium.org/2984303002/#ps120001 (title: "Response to reviewers comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/bui...)
The CQ bit was checked by njanevsk@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sebmarchand@chromium.org Link to the patchset: https://codereview.chromium.org/2984303002/#ps140001 (title: "Response to reviewers comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/bui...) win_rel_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_rel_try/bui...) win_x64_rel_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_x64_rel_try...)
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/bui...)
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/bui...)
The CQ bit was checked by njanevsk@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by njanevsk@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sebmarchand@chromium.org Link to the patchset: https://codereview.chromium.org/2984303002/#ps180001 (title: "Fix merging issue.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 180001, "attempt_start_ts": 1502313067847240, "parent_rev": "d0372eb9d3bbf7451a908762babbe8e9c08cccef", "commit_rev": "4c7cb860467e960396dc24e5146c57802ce3669b"}
Message was sent while issue was closed.
Description was changed from ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest The test cases are currently instantiated only with SYZYGY not with CLANG cause some fixes need to be done for the test cases to work with CLANG. BUG= ========== to ========== Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest The test cases are currently instantiated only with SYZYGY not with CLANG cause some fixes need to be done for the test cases to work with CLANG. BUG= Review-Url: https://codereview.chromium.org/2984303002 Committed: https://github.com/google/syzygy/commit/4c7cb860467e960396dc24e5146c57802ce3669b ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as https://github.com/google/syzygy/commit/4c7cb860467e960396dc24e5146c57802ce3669b |