This is an archive of the discontinued LLVM Phabricator instance.

[clang] Ignore -fconserve-stack
AbandonedPublic

Authored by nathanchance on Jan 19 2022, 1:18 PM.

Details

Summary

This flag is used when building the Linux kernel with GCC. When the
kernel is built with scan-build (which defaults to GCC for the compiler
step), ccc-analyzer passes along all '-f' flags it sees from the
compiler step, which causes the clang invocation to fail because this
flag is completely unrecognized.

Add this flag to the clang_ignored_gcc_optimization_f_Group so that it
is recognized and does not cause the analyzer step of ccc-analyzer to
fail.

Link: https://lore.kernel.org/r/20220119135147.1859982-1-amadeuszx.slawinski@linux.intel.com

Diff Detail

Event Timeline

nathanchance created this revision.Jan 19 2022, 1:18 PM
nathanchance requested review of this revision.Jan 19 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2022, 1:18 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Clang has a long list of ignored options but most were added during the catch-up phase when Clang strove to be a competitor compiler.
I think for new ignored options there needs to be some higher standard.
From Debian Code Search https://codesearch.debian.net/search?q=fconserve-stack&literal=0&page=2 and internal code search, I believe this is a fairly obscure option.
Shall we fix the projects instead?

Clang has a long list of ignored options but most were added during the catch-up phase when Clang strove to be a competitor compiler.
I think for new ignored options there needs to be some higher standard.
From Debian Code Search https://codesearch.debian.net/search?q=fconserve-stack&literal=0&page=2 and internal code search, I believe this is a fairly obscure option.
Shall we fix the projects instead?

I'll give a little background:

The Linux kernel has a make macro called cc-option that invokes the compiler with a flag and if the flag is supported, it adds it to the kernel's CFLAGS variable. Nick recently went through and cleaned up the use of this macro, as it can slow down build times if it is used excessively, especially with incremental compiles because this macro runs during each invocation of make: https://git.kernel.org/linus/7d73c3e9c51400d3e0e755488050804e4d44737a. This flag was one of the ones that was cleaned up; it was moved into a block that is only built when GCC is used as the compiler because we know this flag is not supported with clang so there is no point in testing for it. However, this caused issues with scan-build, as GCC is used as the compiler but clang is invoked after it with all the -f options that GCC was.

The kernel can add back the cc-option call (and likely will to workaround this problem) but all that is going to do is drop this flag during the scan-build process, which may result in higher stack usage.

Should we just tell people to run scan-build only with clang? How ccc-analyzer works seems weird to me but I assume some of the flag machinery that is present here is for that reason?

MaskRay added a comment.EditedJan 19 2022, 2:18 PM

Assuming scan-build is clang/tools/scan-build, which is in tree and can be fixed instead.
I am not familiar with it, but from

However, this caused issues with scan-build, as GCC is used as the compiler but clang is invoked after it with all the -f options that GCC was.

if scan-build+GCC takes all GCC recognized options and forwards them to Clang, I think it is a generally infeasible model.
GCC has dozens of options not recognized by Clang...

Should we just tell people to run scan-build only with clang

I think so, or they restrict themselves to non-GCC-specific options.

Assuming scan-build is clang/tools/scan-build, which is in tree and can be fixed instead.

Yes, that is the one.

I am not familiar with it, but from

However, this caused issues with scan-build, as GCC is used as the compiler but clang is invoked after it with all the -f options that GCC was.

if scan-build+GCC takes all GCC recognized options and forwards them to Clang, I think it is a generally infeasible model.
GCC has dozens of options not recognized by Clang...

I can barely read Perl but it seems like that is what the script does:

https://github.com/llvm/llvm-project/blob/058d2123792da54ae7460fea0946d2c90a032e1c/clang/tools/scan-build/libexec/ccc-analyzer#L661-L664

I don't really know what the fix is for that on the scan-build side of things (how do you decide what flags should be passed through or not?). If there is someone who should be looped in, please do so.

I agree that this is an infeasible model to support so I have commented on the kernel patch that brought this up: https://lore.kernel.org/r/YeiaAgQ+gbZYTMwD@archlinux-ax161/

I'll abandon this depending on what others have to say.

Adding some reviewers from the Clang Static Analyzer side of things.

Should we just tell people to run scan-build only with clang? How ccc-analyzer works seems weird to me but I assume some of the flag machinery that is present here is for that reason?

FWIW, I don't have good impressions of using scan-build. It attempts to interpose the analysis compilation with the real execution of the compiler, but it has effectively no intelligence with that interposition and ends up passing along all the options to Clang. Other tools I've worked on in this realm do far more complicated modelling of compiler options to make that interposition work for arbitrary compilers. I'm not convinced scan-build works for anything but Clang or extremely simple GCC invocations (but doesn't work very well at all with other compilers like ICC, etc because the compiler options get farther away from what Clang understands).

I'm not certain whether that warrants telling people it only works with Clang or not, but I (personally) wouldn't be opposed to that. (I also wouldn't be opposed to ridding ourselves of this script -- it's the only Perl script we have in the project which is a significant pain for those of us on Windows.)

I'm not really familiar with scan-build, we at CodeChecker, we ignore this gcc specific flag explicitly:
https://github.com/Ericsson/codechecker/blob/85dd52dc8f4d47fcf26419fcb44de1ff2a58924f/analyzer/codechecker_analyzer/buildlog/log_parser.py#L72

So, IMO we should probably have something similar and ignore this in scan-build. WDYT @NoQ.

nathanchance abandoned this revision.Jan 27 2022, 7:44 PM

As discussed, this is not the right fix for the issue. I have just gone ahead and told the original reporter to use scan-build with the --use-cc=clang flag to avoid these issues.