corerun issue when loaded from PATH on Linux

[hadibrais]

corerun works correctly when the current directory is the same as the one in which it resides or when an explicit full path is specified. However, when loaded from PATH, an error is emitted. This is similar to dotnet/runtime#5128 and can be easily fixed in the same way. In fact, this issue was pointed out there but was only fixed for ildasm.

[janvorli]

@hadibrais the change dotnet/runtime#5128 modified code that's shared between ildasm and corerun, so it should have fixed corerun too.

[hadibrais]

The ildasm issue was fixed by using GetEntrypointExecutableAbsolutePath when GetAbsolutePath fails. GetEntrypointExecutableAbsolutePath is indeed shared but corerun doesn't use it when GetAbsolutePath fails. That's why the issue still occurs in corerun. Actually, GetEntrypointExecutableAbsolutePath can be called directly.

[janvorli]

@hadibrais ah, thank you for the info, I've missed the fact that the fix for ildasm also included the non-shared part.

[hadibrais]

@janvorli I can fix it and submit a pull request if you like.

[janvorli]

@hadibrais that would be great! I actually wonder now why the fix for ildasm changed the GetEntrypointExecutableAbsolutePath implementation instead of just using the same sequence of calls like the coreconsole does - GetEntrypointExecutableAbsolutePath followed by the call to GetAbsolutePath - without any change to GetEntrypointExecutableAbsolutePath. From what I can see, it should work. Anyways, if it works, I would prefer fixing the corerun like it is done in the coreconsole. That basically ignores argv[0], which I think is a right thing to do.

[hadibrais]

@janvorli I would add also that GetEntrypointExecutableAbsolutePath is sufficient to get an absolute path. There seems to be no need of following that with GetAbsolutePath.

[janvorli]

Yes, in its current form it is true. It was not that case before the fix in ilasm was done. So it seems we should just use the GetEntrypointExecutableAbsolutePath without adding the GetAbsolutePath after or before it. I would then suggest modifying the fallback case in the GetEntrypointExecutableAbsolutePath to call GetAbsolutePath on symlinkEntrypointExecutable instead of entrypointExecutable.assign(symlinkEntrypointExecutable); to make it get the real absolute path to the binary everywhere. And in that case, we could revert part of the previous fix and use the same code path for Linux too - realpath in the GetAbsolutePath and the readlink that was added in the fix should produce the same result. And less code that can achieve the same result is always better.

[hadibrais]

Why don't we just call GetAbsolutePath on symlinkEntrypointExecutable for all the cases?

[janvorli]

I don't think that it works on FreeBSD, OSX and NetBSD. The BSD's don't have proc file system mapped by default and OSX doesn't have it at all, so links like /proc/self/exe etc don't exist.

[hadibrais]

How about this:

bool GetEntrypointExecutableAbsolutePath(std::string& entrypointExecutable)
{
    bool result = false;

    entrypointExecutable.clear();

    // Get path to the executable for the current process using
    // platform specific means.
#if defined (__FreeBSD__)
    static const int name[] = {
        CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1
    };
    char path[PATH_MAX];
    size_t len;

    len = sizeof(path);
    if (sysctl(name, 4, path, &len, nullptr, 0) == 0)
    {
        entrypointExecutable.assign(path);
        result = true;
    }
    else
    {
        // ENOMEM
        result = false;
    }
#elif defined(__APPLE__)
    // On Mac, we ask the OS for the absolute path to the entrypoint executable
    uint32_t lenActualPath = 0;
    if (_NSGetExecutablePath(nullptr, &lenActualPath) == -1)
    {
        // OSX has placed the actual path length in lenActualPath,
        // so re-attempt the operation
        std::string resizedPath(lenActualPath, '\0');
        char *pResizedPath = const_cast<char *>(resizedPath.c_str());
        if (_NSGetExecutablePath(pResizedPath, &lenActualPath) == 0)
        {
            entrypointExecutable.assign(pResizedPath);
            result = true;
        }
    }
#elif defined(__NetBSD__) && defined(KERN_PROC_PATHNAME)
    static const int name[] = {
        CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME,
    };
    char path[MAXPATHLEN];
    size_t len;

    len = sizeof(path);
    if (sysctl(name, __arraycount(name), path, &len, NULL, 0) != -1)
    {
        entrypointExecutable.assign(path);
        result = true;
    }
    else
    {
        result = false;
    }
#else

    // Handle the following cases:
    // defined(__linux__) || (defined(__NetBSD__) && !defined(KERN_PROC_PATHNAME))
    // All others.

    result = GetAbsolutePath(symlinkEntrypointExecutable, entrypointExecutable);

#endif 

    return result;
}

Then in corerun and coreconsole, GetEntrypointExecutableAbsolutePath can be called to get the path to the host binary without calling GetAbsolutePath.

[janvorli]

@hadibrais looks good