This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] [test] Handle missing ld.gold gracefully
ClosedPublic

Authored by mgorny on Sep 6 2022, 7:40 AM.

Details

Summary

Fix the is_binutils_lto_supported() function to handle missing
executables gracefully. Currently, the function does not catch
exceptions from subprocess.Popen() and therefore causes lit to crash
if config.gold_executable does not specify a valid executable:

lit: /usr/lib/python3.11/site-packages/lit/TestingConfig.py:136: fatal: unable to parse config file '/tmp/portage/sys-libs/compiler-rt-
15.0.0/work/compiler-rt/test/lit.common.cfg.py', traceback: Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/lit/TestingConfig.py", line 125, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/tmp/portage/sys-libs/compiler-rt-15.0.0/work/compiler-rt/test/lit.common.cfg.py", line 561, in <module>
    if is_binutils_lto_supported():
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/portage/sys-libs/compiler-rt-15.0.0/work/compiler-rt/test/lit.common.cfg.py", line 543, in is_binutils_lto_supported
    ld_cmd = subprocess.Popen([exe, '--help'], stdout=subprocess.PIPE, env={'LANG': 'C'})
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1022, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.11/subprocess.py", line 1899, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'GOLD_EXECUTABLE-NOTFOUND'

Diff Detail

Event Timeline

mgorny created this revision.Sep 6 2022, 7:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 7:40 AM
mgorny requested review of this revision.Sep 6 2022, 7:40 AM
mgorny added a comment.Sep 6 2022, 7:48 AM

After reviewing this, please also approve backport to 15.x in https://github.com/llvm/llvm-project/issues/57580.

MaskRay accepted this revision.Sep 6 2022, 3:25 PM

Python 3.7 can use subprocess.run with capture_output: https://docs.python.org/3/library/subprocess.html#:~:text=capture_output

But llvm-project requires Python >= 3.6 now, so we have to stay with Popen. Technically OSError can be FileNotFoundError if you want to be more specific.

This revision is now accepted and ready to land.Sep 6 2022, 3:25 PM
This revision was landed with ongoing or failed builds.Sep 6 2022, 10:15 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 10:15 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript

Python 3.7 can use subprocess.run with capture_output: https://docs.python.org/3/library/subprocess.html#:~:text=capture_output

Well, if you wanted to avoid calling subprocess.Popen(), then you could also use the "older" subprocess.check_output(). Not sure if using the "older" API should be a goal though.

Technically OSError can be FileNotFoundError if you want to be more specific.

I actually wanted a catch-all here. I don't think we want to crash if ld.gold e.g. isn't executable or something like that, we just care that we can't use it.