unbreak OS_Unix::get_executable_path() on OpenBSD

[omar-polo]

(and possibly also on NetBSD)

On the OpenBSD port, we've carried a local patch to hardcode the path to the godot binary for a long time. Recently, I had to revise it (since we're now building godot with and without tools enabled) and I'm wondering how the original code could have ever worked.

The current OS_Unix::get_executable_path re-uses OS::get_executable_path (which in turns is argv[0]), does a realpath(3) on it and return the resolved string. This basically only works when argv[0] is an absolute path and fails badly when Godot is invoked as godot, for e.g. from the shell. Why don't just re-use argv[0] as in the default implementation?

[akien-mga]

The aim of this method is to return an absolute path to the executable AFAIK.

Falling back to argv[0] should only be done if it's not possible to get the absolute path. Is there no way to retrieve that path on OpenBSD?

[bruvzg]

Seems like there's no API for it, so I guess we should check if realpath(argv[0]) exist and if it's not also check in the PATH?

[bruvzg]

For the reference: ziglang/zig#6718

[omar-polo]

OpenBSD doesn't provide a syscall to retrieve the path to the executable intentionally. I think that semarie@ reply in ziglang/zig#6718 (comment) explains better that I ever could why something like this is not wanted on OpenBSD.

I'd avoid doing realpath(argv[0]) since that is a real shootgun. I've tested a few shells (ksh, bash, csh) and they don't put an absolute path in argv[0], they simply set it to "godot" (which is really /usr/local/bin/godot here.) Now, if I happen to have a file in $PWD called "godot" (not at all surprising) realpath would succeed.

So no, I think that just exec(argv[0]) is the simplest, easiest and safest thing to do. it already handles for us the case of an absolute path to the executable or the $PATH handling.

[bruvzg]

So no, I think that just exec(argv[0]) is the simplest

get_executable_path() is used to detect self-contained mode and load GDExtension libraries, it's not just for running new instances of the engine (it's used in OS::create_instance for this purpose as well).

[omar-polo]

Ah! I failed to notice that it was used not only for OS::create_instance, sorry! I don't think i've ever managed to get GDNative/GDExtension running on OpenBSD (no real issues, just lack of time) and never tried the self-contained mode. It's not possible to set a default path for GDExtension libraries at compile time?

Anyway, for the scope of the OpenBSD port all of this it's not a real issue, i'll change the patch to hardcode the binary location as it was done before and, since it's a small patch, it's not a burden to maintain locally. I was a bit worried of folks that may want to hack on Godot on OpenBSD outside of the port tree however.

Otherwise, I could re-work it to inspect argv[0] and if it looks like a path and realpath likes it return that, walk $PATH otherwise. It's not pretty, but hey, it's not possible to do better on UNIX AFAIK :)

[bruvzg]

It's not possible to set a default path for GDExtension libraries at compile time?

It is possible to set absolute path in the GDExtenson config, but it's usually relative. Executable path is one of the locations the engine is searching for lib if path is relative (it's a current directory, executable path, and executable path + "../lib"). So non-working get_executable_path won't make it unusable.

Self-contained mode assumes the executable can be located anywhere, all configs, etc. are stored in the same folder as the executable (and this folder can be freely relocated).

[omar-polo]

OK, so I guess the quickest option is to look at argv[0] and see if we can deduct a working path from it. ugly, racy, but don't see other way around :)

will look into it tomorrow and update the PR :)

[omar-polo]

Sorry, it took a while to fix this because of other complications in building godot from the master branch on OpenBSD.

I've updated the PR to do what suggested, if argv[0] looks like a path and realpath(3) likes it, use that, otherwise search for argv[0] in the $PATH. Tested (in a branch with other misc tweaks so that it builds on OpenBSD) and it seems to work both when argv[0] is a path (./bin/godot-...) and when it's not (as when running godot from the shell)

[ghost]

Might I suggest this?

Get the executable of an arbitrary PID on OpenBSD? (Not sure if Godot actually needs this for anything):
https://github.com/time-killer-games/xproc/blob/9ff65440262a327115923f7c56e9d38f8921b1ad/process.cpp#L907-L1007

Or if you just need for the current executable, this might help things a bit as well for less failure cases:
https://github.com/time-killer-games/libfilesystem/blob/ea009efea523439455327552f12fc71fa30c1e50/filesystem.cpp#L565-L680

[hpvb]

I think the way to do this in a portable way is something like:

#include <errno.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <limits.h>

long get_pathmax(void) {
  long pathmax = -1;

  errno = 0;
  pathmax = pathconf("/", _PC_PATH_MAX);
  if (-1 == pathmax) {
    if (0 == errno) {
      pathmax = PATH_MAX;
    } else {
      fprintf (stderr, "pathconf() FAILED, %d, %s\n", errno, strerror(errno));
    }
  }

  return pathmax;
}

int main(int argc, char* argv[]) {
        char* fullpath = malloc(get_pathmax());
        char* cwd = getcwd(NULL, get_pathmax());

        if (!cwd) {
                fprintf (stderr, "getcwd() FAILED, %d, %s\n", errno, strerror(errno));
        }

        if (argv[0][0] == '/') {
                strncpy(fullpath, argv[0], get_pathmax());
        } else {
                strncpy(fullpath, cwd, get_pathmax());
                strncat(fullpath, "/", get_pathmax());
                strncat(fullpath, argv[0], get_pathmax());
        }

        char* fullpath_canonical = realpath(fullpath, NULL);

        printf("full path = %s\n", fullpath_canonical);
        free(cwd);
        free(fullpath);
        free(fullpath_canonical);
}

this would go in our unix main() function, and we'd have to store fullpath_canonical on the OS:: singleton somewhere I think.

Example output:

[hp@lilly tmp]$ ./test
full path = /home/hp/tmp/test
[hp@lilly tmp]$ /home/hp/tmp/./test 
full path = /home/hp/tmp/test
[hp@lilly tmp]$ ../tmp/./test 
full path = /home/hp/tmp/test

Note that this might still require us to do something to actually get a directory rather than the path to the full executable, but that would be a simple matter of adding appropriate dirname(3), pathname(3) and perhaps basename(3) calls to fullpath_canonical.

[bruvzg]

I think the way to do this in a portable way is something like

This seems reasonable, but it's possible that I might be missing some BSD specific aspects.

[omar-polo]

@hpvb nope, that's wrong. Ignoring the issues with strncpy which can overflow called like that (it's strlcpy and/or snprintf that you want to use), argv[0] is still not guaranteed to be a path. Let me show an example:

% cd /tmp
% cat > me.c
#include <stdio.h>
int
main(int argc, char **argv)
{
        puts(argv[0]);
        return (0);
}
% make me
cc -O2 -pipe    -o me me.c
% cp me ~/bin
% which me
/home/op/bin/me
% me
me
% ./me
./me

In the first case, where me was found on the path, argv[0] contains only the name of the utility and not its path. It's only when called by its path (as in ./me) that argv[0] may contain the actual path.

The shell in the example was OpenBSD' ksh, but with bash it's the same story:

bash-5.2$ which me
/home/op/bin/me
bash-5.2$ me
me
bash-5.2$ /tmp/me
/tmp/me

So, I still think that something like my PR where we walk $PATH in the general case it's the way to go.

[ghost]

@hpvb nope, that's wrong. Ignoring the issues with strncpy which can overflow called like that (it's strlcpy and/or snprintf that you want to use), argv[0] is still not guaranteed to be a path. Let me show an example:

% cd /tmp
% cat > me.c
#include <stdio.h>
int
main(int argc, char **argv)
{
        puts(argv[0]);
        return (0);
}
% make me
cc -O2 -pipe    -o me me.c
% cp me ~/bin
% which me
/home/op/bin/me
% me
me
% ./me
./me

In the first case, where me was found on the path, argv[0] contains only the name of the utility and not its path. It's only when called by its path (as in ./me) that argv[0] may contain the actual path.

The shell in the example was OpenBSD' ksh, but with bash it's the same story:

bash-5.2$ which me
/home/op/bin/me
bash-5.2$ me
me
bash-5.2$ /tmp/me
/tmp/me

So, I still think that something like my PR where we walk $PATH in the general case it's the way to go.

I think my example is still the most bullet proof solution, even if it still does have failure cases. It will allow you to check the exact ino_t/dev_t numbers and see if they match up with an existing hardlink to the executable, and this will give improper results if more than one hardlink to the exe exists, though very unlikely people would create multiple hardlinks to a game executable. If you really wanted to, you could check if the number of hardlinks is greater than one, in which case the function could either fail or print a warning to console. I gave a very robust solution which works with every running process of a vanilla install of OpenBSD + Xenocara/Xorg. Not sure why my comment has been seemingly disregarded/ignored.

I can provide a more stripped down reproducible for testing if you still aren't convinced.

i.e. run the following from your OpenBSD terminal and prepare to be amazed:

git clone https://github.com/time-killer-games/xproc $HOME/xproc
$HOME/xproc/example/example.sh

[omar-polo]

@time-killer-games I haven't ignored your suggestion, it's just that i filed the PR beforehand and would prefer if godot didn't link to libkvm just for this.

Also, please note the BUGS section of kvm_open(3):

There should not be two open calls. The ill-defined error semantics of the Sun library and the desire to have a backward-compatible library for BSD left little choice.

So, yeah, I wouldn't use libkvm for anything new, and especially in a program like godot where OS_Unix::get_executable_path() is called multiple times by different places (possibly by different threads too?)

[ghost]

There should not be two open calls. The ill-defined error semantics of the Sun library and the desire to have a backward-compatible library for BSD left little choice.

I got a second opinion from someone on the FreeBSD discord, and they said this:

I might misread it but I think the bugs section states that ideally the kvm_open() and kvm_openfiles() was one function, but they are both there for compatibility reasons

...meaning there really was meant to be only one kvm_open* call/function but with the desire to improve the function's error semantics as well as to keep backwards compatibility they had no choice but to split it into two different functions. Perhaps you could contact the OpenBSD mailing list to get clarity on the matter. Even then, libkvm is a very small library and it is very easy to link to it. I had it and the other system dependencies it relies on all statically linked in xproc, (in the past). It wasn't hard to do that. It barely increased the file size at all.