Skip to content

Commit

Permalink
Revert "Ensure using the given executable path. (dotnet/coreclr#10525)…
Browse files Browse the repository at this point in the history
…" (dotnet#31848)

Reverts dotnet/coreclr#10525.

This broke probing for .ni.exe images. If we run `corerun foo.exe` for something that has a `foo.ni.exe`, we wouldn't pick up the native image because foo.exe is considered trusted and foo.ni.exe is not. This means we would fall back to JIT. But only if foo.exe is not in CORE_ROOT. If it is, it works...

The pull request broke crossgen testing that was eventually worked around in dotnet/coreclr#11272, but it's also breaking the tests in tests\src\readytorun\tests (we just didn't find out because they're falling back to JIT). We could work around there too, but this took half a day for me to investigate, so I don't want to leave the landmine there for someone else to lose a leg over (the fact that ni probing works if you put the exe and ni.exe into CORE_ROOT was a major timesink when troubleshooting).

I don't see much point in this change. It's fixing the case when someone copied something.exe to CORE_ROOT and then tries to `corerun some\other\version\of\something.exe`, but it doesn't fix the situation when the something.exe depends on dependency.dll - we would still pick up dependency.dll from CORE_ROOT, which is likely the wrong version. The only way to win this game is not to play it - don't put stuff in CORE_ROOT.
  • Loading branch information
MichalStrehovsky authored Feb 7, 2020
1 parent ffcb4b4 commit 3790f0d
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 23 deletions.
14 changes: 1 addition & 13 deletions src/coreclr/src/hosts/corerun/corerun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,18 +582,6 @@ bool TryRun(const int argc, const wchar_t* argv[], Logger &log, const bool verbo
return false;
}

StackSString tpaList;
if (!managedAssemblyFullName.IsEmpty())
{
// Target assembly should be added to the tpa list. Otherwise corerun.exe
// may find wrong assembly to execute.
// Details can be found at https://github.com/dotnet/coreclr/issues/5631
tpaList = managedAssemblyFullName;
tpaList.Append(W(';'));
}

tpaList.Append(hostEnvironment.GetTpaList());

//-------------------------------------------------------------

// Create an AppDomain
Expand Down Expand Up @@ -623,7 +611,7 @@ bool TryRun(const int argc, const wchar_t* argv[], Logger &log, const bool verbo
};
const wchar_t *property_values[] = {
// TRUSTED_PLATFORM_ASSEMBLIES
tpaList,
hostEnvironment.GetTpaList(),
// APP_PATHS
appPath,
// APP_NI_PATHS
Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/src/hosts/unixcoreruncommon/coreruncommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,6 @@ int ExecuteManagedAssembly(
GetDirectory(managedAssemblyAbsolutePath, appPath);

std::string tpaList;
if (strlen(managedAssemblyAbsolutePath) > 0)
{
// Target assembly should be added to the tpa list. Otherwise corerun.exe
// may find wrong assembly to execute.
// Details can be found at https://github.com/dotnet/coreclr/issues/5631
tpaList = managedAssemblyAbsolutePath;
tpaList.append(":");
}

// Construct native search directory paths
std::string nativeDllSearchDirs(appPath);
char *coreLibraries = getenv("CORE_LIBRARIES");
Expand All @@ -371,7 +362,6 @@ int ExecuteManagedAssembly(
AddFilesFromDirectoryToTpaList(coreLibraries, tpaList);
}
}

nativeDllSearchDirs.append(":");
nativeDllSearchDirs.append(clrFilesAbsolutePath);

Expand Down

0 comments on commit 3790f0d

Please sign in to comment.