Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Launcher doesn't work if argv[0] isn't set to the executable filename #14343

Closed
phst opened this issue Nov 29, 2021 · 0 comments
Closed
Labels
team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website untriaged

Comments

@phst
Copy link
Contributor

phst commented Nov 29, 2021

Description of the problem / feature request:

https://github.com/bazelbuild/bazel/blob/6.0.0-pre.20211110.1/src/tools/launcher/launcher_main.cc#L39 uses argv[0] to read information from the launcher binary, but argv[0] can be set arbitrarily by callers.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Create a Bazel workspace on Windows with the following files:

  • WORKSPACE must be present, but can be empty
  • BUILD:
    py_binary(name='bin',srcs=['bin.py'])
    
  • bin.py:
    print('hello world')
    
  • launch.py:
    #!/usr/bin/env python3
    import subprocess
    subprocess.run(['something'], executable='./bazel-bin/bin.exe', check=True)
    

Then, build the Python binary with bazel build :bin and run the launcher script using python launch.py. It will print an error:

LAUNCHER ERROR: Cannot open the binary to read launch data
LAUNCHER ERROR: Failed to parse launch info.
Traceback (most recent call last):
File "C:\Users\p\bazel-launcher-argv\launch.py", line 3, in
subprocess.run(['something'], executable='./bazel-bin/bin.exe', check=True)
File "C:\Program Files\Python39\lib\subprocess.py", line 528, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['something']' returned non-zero exit status 1.

The problem is that https://github.com/bazelbuild/bazel/blob/6.0.0-pre.20211110.1/src/tools/launcher/launcher_main.cc#L39 uses argv[0], it should use GetModuleFileNameW instead to obtain the actual executable filename.

This may seem exotic, but setting argv[0] to an arbitrary value is valid and supported. Also see the discussion at https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#parameters.

A Bazel-specific problem is that https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub (implemented for the launcher in https://github.com/bazelbuild/bazel/pull/12650/files#diff-8632bef7ce05c706d0a109e19947e78a1ac4e029409fd4c32dc2ca1d3f1d7fdf) mandates checking argv[0] first for runfiles, and tools depending on tools (https://docs.bazel.build/versions/4.2.1/skylark/rules.html#tools-depending-on-tools) or binaries run as data dependencies at runtime/test time might inadvertedly pick the wrong runfile tree if argv[0] isn't set to the filename of the top-level tool. Following the recommendation at https://docs.bazel.build/versions/4.2.1/skylark/rules.html#tools-depending-on-tools ("it’s recommended that each subtool optionally accept its runfiles root as a parameter") addresses this, but it's somewhat awkward and requires all tools involved to cooperate.

What operating system are you running Bazel on?

Microsoft Windows [Version 10.0.17763.2300]

What's the output of bazel info release?

release 4.2.1

(I guess this issue is present in all versions.)

Have you found anything relevant by searching the web?

https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-getmodulefilenamew

Going forward, it might be better to use the resource functions (e.g. https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-loadstringw) instead.

phst added a commit to phst/rules_elisp that referenced this issue Nov 29, 2021
phst added a commit to phst/rules_elisp that referenced this issue Nov 29, 2021
phst added a commit to phst/rules_elisp that referenced this issue Nov 30, 2021
phst added a commit to phst/rules_elisp that referenced this issue Dec 1, 2021
@sventiffe sventiffe added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website untriaged labels Dec 8, 2021
phst added a commit to phst/rules_elisp that referenced this issue Dec 24, 2021
phst added a commit to phst/rules_elisp that referenced this issue Dec 25, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 25, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 25, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 25, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 26, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 26, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Dec 26, 2021
This removes a dependency on the non-builtin Go rules.

Because of bazelbuild/bazel#14343, we have to add an
intermediary C++ launcher on Windows.
phst added a commit to phst/rules_elisp that referenced this issue Mar 5, 2022
phst added a commit to phst/rules_elisp that referenced this issue Mar 5, 2022
This addresses bazelbuild/bazel#14343.

This is only possible because bazelbuild/bazel#14500
has been fixed in all supported Bazel versions.
phst added a commit to phst/rules_elisp that referenced this issue Mar 5, 2022
This addresses bazelbuild/bazel#14343.

This is only possible because bazelbuild/bazel#14500
has been fixed in all supported Bazel versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website untriaged
Projects
None yet
Development

No branches or pull requests

2 participants