[clang-tidy] ensure run-clang-tidy reports children killed by signals

[ijc]

Summary

If a clang-tidy child process exits with a signal then run-clang-tidy will exit
with an error but there is no hint why in the output, since the clang-tidy
doesn't log anything and may not even have had the opportunity to do so
depending on the signal used.

subprocess.CompletedProcess.returncode is the negative signal number in this
case.

I hit this in a CI system where the parallelism used exceeded the RAM assigned
to the container causing the OOM killer to SIGKILL clang-tidy processes.

[ijc]

I sent this to cfe-commits a while back but it didn't gain any notice (perhaps that was the wrong place).

[sylvestre.ledru]

Do we have tests for this?
If so, it would be nice to add some :)

[ijc]

I may be looking in the wrong places but all I can see is tests which are using run-clang-tidy as part of the mechanism for testing clang-tidy itself, nothing which is testing run-clang-tidy itself AFAICT.

If I've missed some existing suite and it's just a case of adding a new test case to cover this case based on existing test infra then I'd be happy to do so given some pointers where to look, but if it's a case of creating the infrastructure to test run-clang-tidy in order to do so then I'm afraid I don't have the bandwidth for that right now.

[sylvestre.ledru]

OK, no worries.
I don't think it will regress the situation anyway

[ijc]

Thanks Sylvestre!

Now that you've approved AIUI things are now out of my hands and some process will result in this eventually being merged, but please let me know if I'm wrong and there is something further I'm meant to do.

[ijc]

@sylvestre.ledru do I need to do anything more for this to get merged?

[sylvestre.ledru]

Oh, I missed your message. I can land it for you, ok?

[ijc]

@sylvestre.ledru yes please that would be great.

clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
                                      tmpdir, build_path, args.header_filter,
                                      args.allow_enabling_alpha_checkers,
                                      args.extra_arg, args.extra_arg_before,
                                      args.quiet, args.config, args.line_filter)
 
     proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     output, err = proc.communicate()
     if proc.returncode != 0:
+      if proc.returncode < 0:
+        msg = "%s: terminated by signal %d\n" % (name, -proc.returncode)
+        err += msg.encode('utf-8')
       failed_files.append(name)
     with lock:
       sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
       if len(err) > 0:
         sys.stdout.flush()
         sys.stderr.write(err.decode('utf-8'))
     queue.task_done()