bazelbuild/bazel

Logic error in GetEnv()

Open

#15.364 geöffnet am 28. Apr. 2022

Auf GitHub ansehen
 (4 Kommentare) (1 Reaktion) (1 zugewiesene Person)Java (4.465 Forks)batch import
P2area-Windowshelp wantedteam-OSStype: bug

Repository-Metriken

Stars
 (25.384 Stars)
PR-Merge-Metriken
 (Durchschn. Merge 22T 20h) (77 gemergte PRs in 30 T)

Beschreibung

Description of the bug:

If you force Bazel to use an empty PATH environment variable (e.g. build --action_env=PATH= in .bazelrc) and run tests, you will get an error:

ERROR(tools/test/windows/tw.cc:358) value: 0 (0x00000000), arg: PATH: Failed to read envvar

Looking at the implementation, it uses GetEnvironmentVariableW, which has following possible results:

  • The var was not defined.
  • The var was defined and fits into the buffer.
  • The var was defined and does not fit into the buffer.
  • Some error happened.

The bug is in the handling of a zero returnvalue, which can mean three things:

  • The var is undefined. In that case, GetLastError() == ERROR_ENVVAR_NOT_FOUND.
  • The var is defined and has zero length. In that case, GetLastError() == 0. This is the case that is not handled correctly in the code.
  • Some error happened. GetLastError() gives you further info on the error.

Suggested fix for the condition:

-  if (size == 0 && err == ERROR_ENVVAR_NOT_FOUND) {
+  if (size == 0 && ((err == 0) || (err == ERROR_ENVVAR_NOT_FOUND))) {

...plus of course extending the existing unit tests to permanently ban that bug from the codebase.

Cheers & happy hacking!

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

As mentioned above, it's the PATH setting plus running tests (C++ or C test code based on Google Test). I haven't extracted a proper minimal example yet. I would like to avoid that but I can, if above info doesn't suffice.

Which operating system are you running Bazel on?

MS Windows

What is the output of bazel info release?

5.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

n/a

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No similar issues were reported yet.

Any other information, logs, or outputs that you want to share?

There is a workaround: Don't use an empty PATH. In order to get an effectively empty path, you can set it to ; (single semicolon) instead.

Contributor Guide