This patch adds a function to get an exported function in a remote process.
We need this implementation to address Bug 1604008, Bug 1608645, and Bug 1610790.
When `WindowsDllInterceptor` detours a function in a remote process, we used the
native `GetProcAddress` locally, and then detours the returned address in the
target process. The problem is if the caller's export table was modified, the
address returned from `GetProcAddress` might be invalid in the target process,
which is Bug 1604008.
I implemented `GetProcAddress` depending on both local and remote process image,
but it caused two regressions Bug 1608645 and Bug 1610790 because multiple
applications modify firefox's export table in multiple ways, such as replacing
an entry of EAT, replacing an RVA to Export section, or etc.
With this patch, we can use `PEExportSection<MMPolicy>::GetProcAddress` to get
an exported function in a remote process without relying on any local data so
that it's not impacted by modification of the local export table.
Differential Revision: https://phabricator.services.mozilla.com//D62315
Depends on D62314
This patch adds a function to get an exported function in a remote process.
We need this implementation to address Bug 1604008, Bug 1608645, and Bug 1610790.
When `WindowsDllInterceptor` detours a function in a remote process, we used the
native `GetProcAddress` locally, and then detours the returned address in the
target process. The problem is if the caller's export table was modified, the
address returned from `GetProcAddress` might be invalid in the target process,
which is Bug 1604008.
I implemented `GetProcAddress` depending on both local and remote process image,
but it caused two regressions Bug 1608645 and Bug 1610790 because multiple
applications modify firefox's export table in multiple ways, such as replacing
an entry of EAT, replacing an RVA to Export section, or etc.
With this patch, we can use `PEExportSection<MMPolicy>::GetProcAddress` to get
an exported function in a remote process without relying on any local data so
that it's not impacted by modification of the local export table.
Differential Revision: https://phabricator.services.mozilla.com/D62315
Depends on D62314
--HG--
extra : rebase_source : 3088f5997a2097ef22ce8567783375e5f7866ab2
We had a thread-local varialbe `ModuleLoadFrame::sTopFrame` to track the topmost
stack frame of `LdrLoadDll`. However, our hook function `patched_LdrLoadDll` can
be called even before TLS is initialized. In such a case, accessing `sTopFrame`
causes AV.
This patch introduces `SafeThreadLocal` to safely access a thread-local varialbe.
If TLS is not initialized, it falls back to a global variable because in that
early stage there is only a single thread running.
Differential Revision: https://phabricator.services.mozilla.com/D55870
--HG--
extra : moz-landing-system : lando
We had a thread-local varialbe `ModuleLoadFrame::sTopFrame` to track the topmost
stack frame of `LdrLoadDll`. However, our hook function `patched_LdrLoadDll` can
be called even before TLS is initialized. In such a case, accessing `sTopFrame`
causes AV.
This patch introduces `SafeThreadLocal` to safely access a thread-local varialbe.
If TLS is not initialized, it falls back to a global variable because in that
early stage there is only a single thread running.
Differential Revision: https://phabricator.services.mozilla.com/D55870
--HG--
extra : moz-landing-system : lando
`GetProcessTimes` is based on QPC, while `GetSystemTime` is based on clock
interruption whose accuracy is lower than QPC. This means in a process's early
stage, `GetSystemTime` may return a timestamp earlier than creation timestamp.
If this happens we'll keep a negative process uptime which causes overflow in
telemetry processing.
Win8+ has a handy API `GetSystemTimePreciseAsFileTime` that solves everything.
On Win7, `GetSystemTimeAsFileTime` still solves this issue. In the worst case,
it returns the exact same timestamp as process creation, but it's ok.
Because the system time is stored as a `FILETIME` in `KUSER`, converting it to
a `SYSTEMTIME` with `GetSystemTime` drops accuracy. We should avoid it unless
needed.
This patch also moves the call to `GetProcessTimes` before getting the current
timestamp in case clock interruption happens in between those two function calls.
Differential Revision: https://phabricator.services.mozilla.com/D56273
--HG--
extra : moz-landing-system : lando
This is a pretty straightforward patch; we add `WindowsIATPatcher` to
implement the actual IAT patching, and use a partial specialization of
`FuncHook` to account for the underlying differences in implementation vs our
existing interceptor code.
Differential Revision: https://phabricator.services.mozilla.com/D57831
--HG--
extra : moz-landing-system : lando
This is a pretty straightforward patch; we add `WindowsIATPatcher` to
implement the actual IAT patching, and use a partial specialization of
`FuncHook` to account for the underlying differences in implementation vs our
existing interceptor code.
Differential Revision: https://phabricator.services.mozilla.com/D57831
--HG--
extra : moz-landing-system : lando
We have the `LauncherRegistryInfo` class to check the launcher process was
launched successfully on Windows by comparing the timestamps in the registry
when each process was launched.
The problem was when the process is launched from an elevated process, we
relaunch a new launcher process via shell after we updated the launcher's
timestamp. As a result, `LauncherRegistryInfo` unexpectedly disabled the
launcher process even though there was nothing wrong.
A proposed fix is to introduce delay-write to the `LauncherRegistryInfo`. With
this, `LauncherRegistryInfo::Check` modifies only the image timestamp. To update
the launcher/browser timestamps, we need to call `LauncherRegistryInfo::Commit`.
When we ask shell to relaunch a new process, we hold back commit, delegating it
to the new process.
There is another consideration needed. If something fails during `LauncherMain`,
we call `DisableDueToFailure()` to disable the launcher until the image timestamp
is changed. In such a case, we should not change the stored timestamps even
though commit is attempted. The problem is we use a different instance to call
`DisableDueToFailure()` in `HandleLauncherError`. To deal with this design,
`LauncherRegistryInfo` has a static boolean to indicate disablement happens or not.
Differential Revision: https://phabricator.services.mozilla.com/D44928
--HG--
extra : moz-landing-system : lando
We compare two file ids to check the current process is launched from the same
executable. However, our telemetry showed a number of Win7 users failed to open
a file handle of the parent process with STATUS_OBJECT_PATH_NOT_FOUND even
though we opened a process handle and retrieved a module path of the parent
process successfully. We don't have data to explain how this happens or why
this happens only on Win7, Win10 10240, and 10586.
To mitigate this situation, this patch introduces a logic to compare NT path
strings. The benefit from doing this is 1) we don't have to open a file handle
of a parent process executable and 2) when we get an NT path, a network drive
or a symbolic link is already solved.
This new logic is much faster, but we still compare file ids on the first
attempt to minimize the impact. We fall back to the new logic only if we
detect the STATUS_OBJECT_PATH_NOT_FOUND failure.
Differential Revision: https://phabricator.services.mozilla.com/D45476
--HG--
extra : moz-landing-system : lando
The previous commit 4eca0f08c43b73dc1dd908fad58bdfd7f6973119 mistakenly removed
`skip-if` from TestNativeNt. We need to add it back.
Differential Revision: https://phabricator.services.mozilla.com/D42961
--HG--
extra : moz-landing-system : lando
For launching with an external protocol handler on Windows, we validate a uri
before sending it to `ShellExecute`, by converting a string into `PIDL` using
`SHParseDisplayName` and extract a string back from PIDL using
`IShellFolder::GetDisplayNameOf`. The problem was that if a fragment, a
string following a hash mark (#), is always dropped after this validation.
This is caused by the intended design of Windows.
A proposed fix is to use `CreateUri` for validation, which is used behind
`IShellFolder::GetDisplayNameOf`. However, we also keep `SHParseDisplayName`
because there are cases where `CreateUri` succeeds while `SHParseDisplayName`
fails such as a non-existent `file:` uri and we want to keep the same
validation result for those cases.
Adding `CreateUri` broke MinGW build because of our toolkit issue. We use
dynamic linking for MinGW build in the meantime.
This patch adds a new unittest to make sure the new validation logic
behaves the same as the old one except the fragment issue.
Differential Revision: https://phabricator.services.mozilla.com/D42041
--HG--
extra : moz-landing-system : lando
For launching with an external protocol handler on Windows, we validate a uri
before sending it to `ShellExecute`, by converting a string into `PIDL` using
`SHParseDisplayName` and extract a string back from PIDL using
`IShellFolder::GetDisplayNameOf`. The problem was that if a fragment, a
string following a hash mark (#), is always dropped after this validation.
This is caused by the intended design of Windows.
A proposed fix is to use `CreateUri` for validation, which is used behind
`IShellFolder::GetDisplayNameOf`. However, we also keep `SHParseDisplayName`
because there are cases where `CreateUri` succeeds while `SHParseDisplayName`
fails such as a non-existent `file:` uri and we want to keep the same
validation result for those cases.
This patch adds a new unittest to make sure the new validation logic
behaves the same as the old one except the fragment issue.
Differential Revision: https://phabricator.services.mozilla.com/D42041
--HG--
extra : moz-landing-system : lando
We also s/mincore/version/ in OS_LIBS because the former breaks the test on
Windows 7.
Differential Revision: https://phabricator.services.mozilla.com/D34437
--HG--
extra : moz-landing-system : lando
Simple test program that exercises the most important APIs of BaseProfiler.
(Including checking that macros work even when BaseProfiler is not enabled.)
Differential Revision: https://phabricator.services.mozilla.com/D31926
--HG--
extra : moz-landing-system : lando
Simple test program that exercises the most important APIs of BaseProfiler.
(Including checking that macros work even when BaseProfiler is not enabled.)
Differential Revision: https://phabricator.services.mozilla.com/D31926
--HG--
extra : moz-landing-system : lando
Simple test program that exercises the most important APIs of BaseProfiler.
(Including checking that macros work even when BaseProfiler is not enabled.)
Differential Revision: https://phabricator.services.mozilla.com/D31926
--HG--
extra : moz-landing-system : lando
The logic in JSMath for generating cryptographically-secure
pseudorandom numbers without NSS is independently useful, and so
it's been moved to a common area.
It will eventually be used for generated random arena ids.
Differential Revision: https://phabricator.services.mozilla.com/D8597
--HG--
extra : moz-landing-system : lando
Also add a comment to mfbt/tests/moz.build to remind people that tests
need to be added to testing/cppunittest.py.
Differential Revision: https://phabricator.services.mozilla.com/D8664
--HG--
extra : moz-landing-system : lando
This relies on the fact that providing multiple --version-script
combines them all, so we effectively create a new symbol version
that has no global symbol, but hides the std:🧵:_M_start_thread
symbols.
This version script trick happens to work with BFD ld, gold, and lld.
The downside is that when providing multiple --version-script's, ld
doesn't want any of them to have no version at all. So for the libraries
that do already have a version script (through SYMBOLS_FILE), we use a
version where there used to be none, using the library name as the
version. Practically speaking, this binds the libraries a little closer
than they used to be, kind of non-flat namespace on OSX (which is the
default there), meaning the dynamic linker will actively want to use
symbols from those libraries instead of a system library that might
happen to have the same symbol name.
--HG--
extra : rebase_source : a7f672c35609d993849385ddb874ba791b34f929
This relies on the fact that providing multiple --version-script
combines them all, so we effectively create a new symbol version
that has no global symbol, but hides as much std::* stuff as possible.
The added symbol script could use `extern "C++"` syntax and demangled
symbols but there is no guarantee the demangled symbols won't change.
Plus, it's not possible to match demangled symbols that have a return
type: they contain a space, and the only way to match that is to use
double quotes, which doesn't allow globs at the same time.
This version script trick happens to work with BFD ld, gold, and lld.
The downside is that when providing multiple --version-script's, ld
doesn't want any of them to have no version at all. So for the libraries
that do already have a version script (through SYMBOLS_FILE), we use a
version where there used to be none, using the library name as the
version. Practically speaking, this binds the libraries a little closer
than they used to be, kind of non-flat namespace on OSX (which is the
default there), meaning the dynamic linker will actively want to use
symbols from those libraries instead of a system library that might
happen to have the same symbol name.
--HG--
extra : rebase_source : 78adb64b90e75ebad203b8a647b305c9d7198d16
This test turns the existing stand alone test for the FTP directory
listing parser into a gtest.
MozReview-Commit-ID: 7n60TfcTXTJ
--HG--
extra : rebase_source : 79c88708a9bf9bee6c27a82f2c93a95016e063dd
The macro simultaneously declares an enumeration and a count of its
enumerators.
A few variants of the macro are also provided to handle things like
enum classes, underlying types, and enumerations declared at class
scope.
MozReview-Commit-ID: 3z6yHnfXbLj
--HG--
extra : rebase_source : 92c333693e4bbf85b89cd3d7ac5b31f4b5434367
The test results were updated to match current behaviour. The
TestDummyAudioWithTransport and TestDummyVideoWithTransports are disabled due
to shutdown crashes and intermittent failures that show up in automation.
A follow up bug has been filed to fix these. The GMP test was removed
completely as it seems unlikely that it will be practical to test that from a
gtest.
MozReview-Commit-ID: 2pOb7u2Qp7v
--HG--
rename : media/webrtc/signaling/test/mediaconduit_unittests.cpp => media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
extra : rebase_source : 992330f83e0a6a57810f1c5f0b4ea77f2512cd92
* TestUDPSocket wasn't initializing it's members properly, fixup by mcmanus
* Scope the netwerk TestCommon waiting to be non-global, and add assertions so that waiting is deterministic. r=mcmanus
MozReview-Commit-ID: 7jLgNIujrbu
--HG--
extra : rebase_source : fabee29260f06686e874d0130cb00067c769ad6f
This change is mostly straightforward, except for the following.
- It removes all the printing from the do_check_* macros because gtest macros
do appropriate printing.
- test_StatementCache.cpp needs some special gtest magic for the type
parameterization.
- It merges the four tests in test_unlock_notify.cpp because they rely on being
executed in order, and so aren't independent.
- storage_test_harness_tail.h is no longer necessary because gtest provides the
test looping functionality.
- It uses #include and the preprocessor to remove the duplication between
test_deadlock_detector.cpp and xpcom/tests/DeadlockDetector.cpp.
- It makes the test in test_service_init_background_thread.cpp a death test to
force it to be the first storage gtest, because it fails otherwise.
- It adds code to undo the SQLite mutex hooking as necessary, so that tests
don't interfere with each other.
- It de-virtualizes Spinner's destructor (as identified in bug 1318282).
--HG--
rename : storage/test/storage_test_harness.h => storage/test/gtest/storage_test_harness.h
rename : storage/test/test_AsXXX_helpers.cpp => storage/test/gtest/test_AsXXX_helpers.cpp
rename : storage/test/test_StatementCache.cpp => storage/test/gtest/test_StatementCache.cpp
rename : storage/test/test_asyncStatementExecution_transaction.cpp => storage/test/gtest/test_asyncStatementExecution_transaction.cpp
rename : storage/test/test_async_callbacks_with_spun_event_loops.cpp => storage/test/gtest/test_async_callbacks_with_spun_event_loops.cpp
rename : storage/test/test_binding_params.cpp => storage/test/gtest/test_binding_params.cpp
rename : storage/test/test_deadlock_detector.cpp => storage/test/gtest/test_deadlock_detector.cpp
rename : storage/test/test_file_perms.cpp => storage/test/gtest/test_file_perms.cpp
rename : storage/test/test_mutex.cpp => storage/test/gtest/test_mutex.cpp
rename : storage/test/test_service_init_background_thread.cpp => storage/test/gtest/test_service_init_background_thread.cpp
rename : storage/test/test_statement_scoper.cpp => storage/test/gtest/test_statement_scoper.cpp
rename : storage/test/test_transaction_helper.cpp => storage/test/gtest/test_transaction_helper.cpp
rename : storage/test/test_true_async.cpp => storage/test/gtest/test_true_async.cpp
rename : storage/test/test_unlock_notify.cpp => storage/test/gtest/test_unlock_notify.cpp
extra : rebase_source : dbb695c112564efa1945116be1a8435988982e74
This is a most minimal gtest conversion possible. It leaves in place
significant amounts of non-typical-for-gtest code.
Notable changes:
- All the mock Link and URLSearchParams method definitions are no longer
needed.
- The changes adds a new constructor for Link that doesn't set mHistory.
Without that, leaked URLs occur at shutdown.
- The output printed by the test is slightly streamlined, mostly by omitting
the test filename.
- It disables TestMediaFormatReader.cpp, which was causing problems. That test
is slated for removal in bug 1318225 anyway.
--HG--
rename : toolkit/components/places/tests/cpp/mock_Link.h => toolkit/components/places/tests/gtest/mock_Link.h
rename : toolkit/components/places/tests/cpp/moz.build => toolkit/components/places/tests/gtest/moz.build
rename : toolkit/components/places/tests/cpp/places_test_harness.h => toolkit/components/places/tests/gtest/places_test_harness.h
rename : toolkit/components/places/tests/cpp/places_test_harness_tail.h => toolkit/components/places/tests/gtest/places_test_harness_tail.h
rename : toolkit/components/places/tests/cpp/test_IHistory.cpp => toolkit/components/places/tests/gtest/test_IHistory.cpp
extra : rebase_source : b7def3f9afce3a44e99f5ed35cb220f7814551cd
Toshihito Kikuchi
Cosmin Sabou
Narcis Beleuzu
Aaron Klotz
Gurzau Raul
Razvan Maries
Gerald Squelart
Noemi Erli
Bogdan Tara
Chris Martin
Botond Ballo
Andreea Pavel
Mike Hommey
Paul Adenot
Andrew McCreight
Matt Woodrow
Wes Kocher
Marco Castelluccio
Dan Minor
Benjamin Smedberg
Nathan Froyd
Nicholas Nethercote
Iris Hsiao
Matthew Gregan