Backed out changeset f74adc43b654 (bug 1608645) as requested by tkiku…

…chi (toshi).

--HG--
extra : rebase_source : 83d53600fe057aca34128ac37b451120cb3337b5

mozglue/misc/NativeNt.h

@@ -531,16 +531,13 @@ class MOZ_RAII PEHeaders final {
    * entry of the Export Address Table i.e. a pointer to an RVA of a matched
    * function.  If the entry is forwarded, this function returns nullptr.
    */
-  LauncherResult<const DWORD*> FindExportAddressTableEntry(
-      const char* aFunctionNameASCII) {
+  const DWORD* FindExportAddressTableEntry(const char* aFunctionNameASCII) {
     struct NameTableComparator {
       NameTableComparator(PEHeaders& aPEHeader, const char* aTarget)
           : mPEHeader(aPEHeader), mTarget(aTarget) {}
 
       int operator()(DWORD aOther) const {
-        // Use RVAToPtrUnchecked because StrcmpASCII does not accept nullptr.
-        return StrcmpASCII(mTarget,
-                           mPEHeader.RVAToPtrUnchecked<const char*>(aOther));
+        return StrcmpASCII(mTarget, mPEHeader.RVAToPtr<const char*>(aOther));
       }
 
       PEHeaders& mPEHeader;
@@ -551,7 +548,7 @@ class MOZ_RAII PEHeaders final {
     const auto exportDir = GetImageDirectoryEntry<PIMAGE_EXPORT_DIRECTORY>(
         IMAGE_DIRECTORY_ENTRY_EXPORT, &rvaDirStart, &rvaDirEnd);
     if (!exportDir) {
-      return LAUNCHER_ERROR_FROM_WIN32(ERROR_BAD_EXE_FORMAT);
+      return nullptr;
     }
 
     const auto exportAddressTable =
@@ -561,24 +558,18 @@ class MOZ_RAII PEHeaders final {
     const auto exportOrdinalTable =
         RVAToPtr<const WORD*>(exportDir->AddressOfNameOrdinals);
 
-    // If any of these tables are modified and located outside the mapped image,
-    // we don't search and bail out.
-    if (!exportAddressTable || !exportNameTable || !exportOrdinalTable) {
-      return LAUNCHER_ERROR_FROM_WIN32(ERROR_BAD_EXE_FORMAT);
-    }
-
     const NameTableComparator comp(*this, aFunctionNameASCII);
 
     size_t match;
     if (!BinarySearchIf(exportNameTable, 0, exportDir->NumberOfNames, comp,
                         &match)) {
-      return LAUNCHER_ERROR_FROM_WIN32(ERROR_PROC_NOT_FOUND);
+      return nullptr;
     }
 
     WORD index = exportOrdinalTable[match];
 
     if (index >= exportDir->NumberOfFunctions) {
-      return LAUNCHER_ERROR_FROM_WIN32(ERROR_BAD_EXE_FORMAT);
+      return nullptr;
     }
 
     DWORD rvaFunction = exportAddressTable[index];

mozglue/misc/interceptor/MMPolicies.h

@@ -671,21 +671,16 @@ class MMPolicyOutOfProcess : public MMPolicyBase {
   }
 
   /**
-   * This searches the current process's export address table for a given name,
-   * but retrieves an RVA from a corresponding table entry in the target process
-   * because the table entry in the current process might have been replaced.
+   * This searches the target process's export address table for a given name
+   * instead of simply calling ::GetProcAddress because the local export table
+   * might be modified and the value of a table entry might be different from
+   * the target process.  If we fail to get an entry for some reason, we fall
+   * back to using ::GetProcAddress.
    */
   FARPROC GetProcAddress(HMODULE aModule, const char* aName) const {
     nt::PEHeaders moduleHeaders(aModule);
-    auto funcEntry = moduleHeaders.FindExportAddressTableEntry(aName);
-    if (funcEntry.isErr()) {
-      // FindExportAddressTableEntry fails if |aName| is invalid or the entire
-      // export table has been modified.  In the former case, ::GetProcAddress
-      // will return nullptr.  In the latter case, funcEntry may point to an
-      // invalid address in the target process.  We bail out in both cases.
-      return nullptr;
-    }
-    if (!funcEntry.inspect()) {
+    const DWORD* funcEntry = moduleHeaders.FindExportAddressTableEntry(aName);
+    if (!funcEntry) {
       // FindExportAddressTableEntry returns nullptr if a matched entry is
       // forwarded to another module.  Because a forwarder entry needs to point
       // a null-terminated string within the export section, it's less likely to
@@ -695,9 +690,8 @@ class MMPolicyOutOfProcess : public MMPolicyBase {
 
     SIZE_T numBytes = 0;
     DWORD rvaTargetFunction = 0;
-    BOOL ok =
-        ::ReadProcessMemory(mProcess, funcEntry.inspect(), &rvaTargetFunction,
-                            sizeof(rvaTargetFunction), &numBytes);
+    BOOL ok = ::ReadProcessMemory(mProcess, funcEntry, &rvaTargetFunction,
+                                  sizeof(rvaTargetFunction), &numBytes);
     if (!ok || numBytes != sizeof(rvaTargetFunction)) {
       // If we fail to read the table entry in the target process for unexpected
       // reason, we fall back to ::GetProcAddress.

mozglue/tests/TestNativeNt.cpp

@@ -223,10 +223,9 @@ int main(int argc, char* argv[]) {
   auto tableOfNames = ntdllHeaders.RVAToPtr<PDWORD>(exportDir->AddressOfNames);
   for (DWORD i = 0; i < exportDir->NumberOfNames; ++i) {
     const auto name = ntdllHeaders.RVAToPtr<const char*>(tableOfNames[i]);
-    auto funcEntry = ntdllHeaders.FindExportAddressTableEntry(name);
-    if (funcEntry.isErr() || !funcEntry.inspect() ||
-        ntdllHeaders.RVAToPtr<const void*>(*funcEntry.inspect()) !=
-            ::GetProcAddress(ntdllImageBase, name)) {
+    const DWORD* funcEntry = ntdllHeaders.FindExportAddressTableEntry(name);
+    if (ntdllHeaders.RVAToPtr<const void*>(*funcEntry) !=
+        ::GetProcAddress(ntdllImageBase, name)) {
       printf(
           "TEST-FAILED | NativeNt | FindExportAddressTableEntry returned "
           "a wrong value.\n");
@@ -235,20 +234,18 @@ int main(int argc, char* argv[]) {
   }
 
   // Test a known forwarder RVA.
-  auto funcEntry = k32headers.FindExportAddressTableEntry("HeapAlloc");
-  if (funcEntry.isErr() || funcEntry.inspect()) {
+  if (k32headers.FindExportAddressTableEntry("HeapAlloc")) {
     printf(
         "TEST-FAILED | NativeNt | kernel32!HeapAlloc should be forwarded to "
         "ntdll!RtlAllocateHeap.\n");
     return 1;
   }
 
   // Test an invalid name.
-  funcEntry = k32headers.FindExportAddressTableEntry("Invalid name");
-  if (funcEntry.isOk()) {
+  if (k32headers.FindExportAddressTableEntry("Invalid name")) {
     printf(
         "TEST-FAILED | NativeNt | FindExportAddressTableEntry should return "
-        "an error for a non-existent name.\n");
+        "null for an non-existent name.\n");
     return 1;
   }