[clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

[fpetrogalli]

Summary

This is useful for retriving crash reports of LIT runs when the
temporary folder is not accessible.

[michaelplatings]

Looks useful, and matches the behaviour I would have expected anyway. The buildkite libcxx tests already rely on being able to pass this variable through, so the same seems desirable for the clang tests. LGTM

[leonardchan]

Hi. I think this led to some test failures on our builder at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:

Script:
--
: 'RUN: at line 4';   rm -rf /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir
: 'RUN: at line 5';   mkdir -p /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/i /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/m
: 'RUN: at line 7';   env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir TEMP=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir TMP=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir  not /b/s/w/ir/x/w/staging/llvm_build/bin/clang -fsyntax-only /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m -I /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/module -isysroot /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crash-report-modules.m.tmp/i/                  -fmodules -fmodules-cache-path=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/m/ -DFOO=BAR 2>&1 | /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m
: 'RUN: at line 11';   /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --check-prefix=CHECKSRC /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m -input-file /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.m
: 'RUN: at line 12';   /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --check-prefix=CHECKSH /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m -input-file /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.sh
--
Exit Code: 2

Command Output (stderr):
--
Could not open input file '/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.m': No such file or directory
--

I think perhaps since we define CLANG_CRASH_DIAGNOSTICS_DIR as an env, tests that produce crash reports place them there, but those tests also expect the crash report to be in a temp dir %T, so perhaps some tests need to be updated along side this? Would you be able to take a look and fix or revert?

[michaelplatings]

It looks like the failing tests already go to some effort to set each of TMP, TEMP & TMPDIR so perhaps they should be simplified to just set CLANG_CRASH_DIAGNOSTICS_DIR instead.

[fpetrogalli]

In D147209#4244615, @leonardchan wrote:

Hi. I think this led to some test failures on our builder at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:

Hi @leonardchan - thank you for the heads up!

I do not have access to this bot (the link you sent just shows me a blank page), and I have not seen this (clang?) test failing in my local build. I can revert the patch, but I'll need some more info to be able to fix those failures.

Francesco

[leonardchan]

In D147209#4245229, @fpetrogalli wrote:

In D147209#4244615, @leonardchan wrote:

Hi. I think this led to some test failures on our builder at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:

Hi @leonardchan - thank you for the heads up!

I do not have access to this bot (the link you sent just shows me a blank page), and I have not seen this (clang?) test failing in my local build. I can revert the patch, but I'll need some more info to be able to fix those failures.

Francesco

Hmm, that's interesting since the link should be public and normally doesn't just appear as blank. Does it still show up as blank if you click it again? Otherwise I can share the full repro. I think you should be able to reproduce this if you define CLANG_CRASH_DIAGNOSTICS_DIR as an environment variable before running ninja check-clang.

[fpetrogalli]

In D147209#4246622, @leonardchan wrote:

In D147209#4245229, @fpetrogalli wrote:

In D147209#4244615, @leonardchan wrote:

Hi. I think this led to some test failures on our builder at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:

Hi @leonardchan - thank you for the heads up!

I do not have access to this bot (the link you sent just shows me a blank page), and I have not seen this (clang?) test failing in my local build. I can revert the patch, but I'll need some more info to be able to fix those failures.

Francesco

Hmm, that's interesting since the link should be public and normally doesn't just appear as blank. Does it still show up as blank if you click it again? Otherwise I can share the full repro. I think you should be able to reproduce this if you define CLANG_CRASH_DIAGNOSTICS_DIR as an environment variable before running ninja check-clang.

yep - still a white page for me...

Anyway, thank you for the suggestion - I managed to reproduce the issue (for example, FAIL: Clang :: Driver/crash-report.cpp (93 of 17575)), I'll revert and submit a new patch with the fix.

Francesco

clang/test/lit.cfg.py

 # objects only. In order to not affect most test cases, which expect to support
 # 32-bit and 64-bit objects by default, set the environment variable
 # "OBJECT_MODE" to 'any' for llvm-nm on AIX OS.
 
 if 'system-aix' in config.available_features:
         config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
         config.substitutions.append(('llvm-ar', 'env OBJECT_MODE=any llvm-ar'))
 
+# Pass the crash diagnostic dir set in the os environment to LIT.
+if 'CLANG_CRASH_DIAGNOSTICS_DIR' in os.environ:
+    config.environment['CLANG_CRASH_DIAGNOSTICS_DIR'] = os.environ['CLANG_CRASH_DIAGNOSTICS_DIR']
+
 # It is not realistically possible to account for all options that could
 # possibly be present in system and user configuration files, so disable
 # default configs for the test runs.
 config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"