libgcc and libstdc++ not linking statically with incompatible Bazel flag

[oquenchil]

Hello,

I'm trying to flip the Bazel flag --incompatible_do_not_split_linking_cmdline:
bazelbuild/bazel#7687

bazel test --incompatible_do_not_split_linking_cmdline //test/exe:envoy_static_test
will start failing with:
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //test/exe:envoy_static_test

-----------------------------------------------------------------------------
libstdc++ and/or libgcc are dynamically linked:
	linux-vdso.so.1 (0x00007ffcdc7ee000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f7162a6b000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f7162867000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f7162648000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f71622aa000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f7161f21000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f7161b30000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f7165999000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f7161918000)

You have a script to ensure that libstdc++ and libgcc are linked statically.

The issue is that the link tool is not being invoked that way anymore. It used to be:
/bin/linktool -someflag -someflag2 -Wl,@parameter_file_with_more_flags

but now it's
/bin/linktool @all_flags

The script is very dependent on how the arguments to the tool were passed. I ask you to please make that script compatible with the new way so that I can flip this Bazel flag for 0.27. You can apply most of the same logic after reading every line from the all_flags parameter file. I don't think that you can still rely on ["-E", "-xc++", "-", "-v"] being the first arguments in the file but there must be an equivalent way of doing that.

The script will have to be compatible with current release too. So you could detect if one of the argv parameters starts with @ and take the new code path, if it starts with -Wl,@ the old one.

Please let me know if I can help.

Thank you.

[htuch]

@dnoe do you think you could take a look as Envoy import on-call?

[oquenchil]

Hi,

Replacing cc_wrapper.py with this should work. Every test passed now with the -- incompatible_do_not_split_linking_cmdline flag. I'm going on vacation tomorrow and unfortunately don't have time for the PR and code review. The flip will be in 0.27, so you are safe. But as soon as I flip the flag, envoy will appear red in our downstream pipeline and we may not catch other errors.

#!/usr/bin/python
import contextlib
import os
import shlex
import sys
import tempfile

envoy_real_cc = {ENVOY_REAL_CC}
envoy_real_cxx = {ENVOY_REAL_CXX}
envoy_cflags = {ENVOY_CFLAGS}
envoy_cxxflags = {ENVOY_CXXFLAGS}


@contextlib.contextmanager
def closing_fd(fd):
  try:
    yield fd
  finally:
    os.close(fd)


def sanitize_flagfile(in_path, out_fd):
  with open(in_path, "rb") as in_fp:
    for line in in_fp:
      if line != "-lstdc++\n":
        os.write(out_fd, line)
      elif "-stdlib=libc++" in envoy_cxxflags:
        os.write(out_fd, "-lc++\n")


# Is the arg a flag indicating that we're building for C++ (rather than C)?
def old_is_cpp_flag(arg):
  return arg in ["-static-libstdc++", "-stdlib=libc++", "-lstdc++", "-lc++"
                ] or arg.startswith("-std=c++") or arg.startswith("-std=gnu++")

# Is the arg a flag indicating that we're building for C++ (rather than C)?
def new_is_cpp_flag(arg):
  return arg in ["-static-libstdc++\n", "-stdlib=libc++\n", "-lstdc++\n", "-lc++\n"
                ] or arg.startswith("-std=c++") or arg.startswith("-std=gnu++")

def old_modify_driver_command_line_and_execute(sys_argv):
  # Detect if we're building for C++ or vanilla C.
  if any(map(old_is_cpp_flag, sys_argv[1:])):
    compiler = envoy_real_cxx
    # Append CXXFLAGS to all C++ targets (this is mostly for dependencies).
    argv = shlex.split(envoy_cxxflags)
  else:
    compiler = envoy_real_cc
    # Append CFLAGS to all C targets (this is mostly for dependencies).
    argv = shlex.split(envoy_cflags)

  # Either:
  # a) remove all occurrences of -lstdc++ (when statically linking against libstdc++),
  # b) replace all occurrences of -lstdc++ with -lc++ (when linking against libc++).
  if "-static-libstdc++" in sys_argv[1:] or "-stdlib=libc++" in envoy_cxxflags:
    for arg in sys_argv[1:]:
      if arg == "-lstdc++":
        if "-stdlib=libc++" in envoy_cxxflags:
          argv.append("-lc++")
      elif arg.startswith("-Wl,@"):
        # tempfile.mkstemp will write to the out-of-sandbox tempdir
        # unless the user has explicitly set environment variables
        # before starting Bazel. But here in $PWD is the Bazel sandbox,
        # which will be deleted automatically after the compiler exits.
        (flagfile_fd, flagfile_path) = tempfile.mkstemp(dir="./", suffix=".linker-params")
        with closing_fd(flagfile_fd):
          sanitize_flagfile(arg[len("-Wl,@"):], flagfile_fd)
        argv.append("-Wl,@" + flagfile_path)
      else:
        argv.append(arg)
  else:
    argv += sys_argv[1:]

  # Bazel will add -fuse-ld=gold in some cases, gcc/clang will take the last -fuse-ld argument,
  # so whenever we see lld once, add it to the end.
  if "-fuse-ld=lld" in argv:
    argv.append("-fuse-ld=lld")

  # Add compiler-specific options
  if "clang" in compiler:
    # This ensures that STL symbols are included.
    # See https://github.com/envoyproxy/envoy/issues/1341
    argv.append("-fno-limit-debug-info")
    argv.append("-Wthread-safety")
    argv.append("-Wgnu-conditional-omitted-operand")
  elif "gcc" in compiler or "g++" in compiler:
    # -Wmaybe-initialized is warning about many uses of absl::optional. Disable
    # to prevent build breakage. This option does not exist in clang, so setting
    # it in clang builds causes a build error because of unknown command line
    # flag.
    # See https://github.com/envoyproxy/envoy/issues/2987
    argv.append("-Wno-maybe-uninitialized")

  os.execv(compiler, [compiler] + argv)

def new_modify_driver_command_line_and_execute(flagfile_path, new_flagfile_path, new_flagfile_fd):
  flagfile_contents = open(flagfile_path, "r").readlines()

  # Detect if we're building for C++ or vanilla C.
  if any(map(new_is_cpp_flag, flagfile_contents)):
    compiler = envoy_real_cxx
    # Append CXXFLAGS to all C++ targets (this is mostly for dependencies).
    argv = shlex.split(envoy_cxxflags)
  else:
    compiler = envoy_real_cc
    # Append CFLAGS to all C targets (this is mostly for dependencies).
    argv = shlex.split(envoy_cflags)

  # Either:
  # a) remove all occurrences of -lstdc++ (when statically linking against libstdc++),
  # b) replace all occurrences of -lstdc++ with -lc++ (when linking against libc++).
  if "-static-libstdc++\n" in flagfile_contents or "-stdlib=libc++" in envoy_cxxflags:
    for arg in flagfile_contents:
      if arg == "-lstdc++\n":
        if "-stdlib=libc++" in envoy_cxxflags:
          argv.append("-lc++\n")
      else:
        argv.append(arg)
  else:
    argv += flagfile_contents

  # Bazel will add -fuse-ld=gold in some cases, gcc/clang will take the last -fuse-ld argument,
  # so whenever we see lld once, add it to the end.
  if "-fuse-ld=lld\n" in argv:
    argv.append("-fuse-ld=lld\n")

  # Add compiler-specific options
  if "clang" in compiler:
    # This ensures that STL symbols are included.
    # See https://github.com/envoyproxy/envoy/issues/1341
    argv.append("-fno-limit-debug-info\n")
    argv.append("-Wthread-safety\n")
    argv.append("-Wgnu-conditional-omitted-operand\n")
  elif "gcc" in compiler or "g++" in compiler:
    # -Wmaybe-initialized is warning about many uses of absl::optional. Disable
    # to prevent build breakage. This option does not exist in clang, so setting
    # it in clang builds causes a build error because of unknown command line
    # flag.
    # See https://github.com/envoyproxy/envoy/issues/2987
    argv.append("-Wno-maybe-uninitialized\n")

  for arg in argv:
    os.write(new_flagfile_fd, arg)
  os.execv(compiler, [compiler] + ["@" + new_flagfile_path])

def main():
  # Append CXXFLAGS to correctly detect include paths for either libstdc++ or libc++.
  if sys.argv[1:5] == ["-E", "-xc++", "-", "-v"]:
    os.execv(envoy_real_cxx, [envoy_real_cxx] + sys.argv[1:] + shlex.split(envoy_cxxflags))

  if sys.argv[1].startswith("@"):
    (new_flagfile_fd, new_flagfile_path) = tempfile.mkstemp(dir="./", suffix=".linker-params")
    flagfile_path = sys.argv[1][1:]
    with closing_fd(new_flagfile_fd):
      new_modify_driver_command_line_and_execute(flagfile_path, new_flagfile_path, new_flagfile_fd)
  else:
    # TODO(https://github.com/bazelbuild/bazel/issues/7687): Remove this branch
    # when Bazel 0.27 is released.
    old_modify_driver_command_line_and_execute(sys.argv)


if __name__ == "__main__":
  main()

[htuch]

@oquenchil thanks for doing the heavy lifting here. I think this looks roughly like what we should be doing, but I continue to be terrified by this script and its machinations (this is non-actionable). We can probably take it from here and try and get it merged in the coming week or two.

[hlopko]

Hi all, I'm updating all Bazel incompatible flags related issues - the Bazel team is in increased pressure because 0.27 will start a 3 month window of no incompatible changes). We will have to potentially temporarily disable envoy on our downstream pipeline if we cannot get it fixed this week.