[PATCH] D29023: [Stack Protection] Add diagnostic information for why stack protection was applied to a function

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

It turns out that by default, when no handlers are provided, all diagnostic messages are produced, regardless of their severity. Optimization remarks are a special case that have explicit handling. I can think of five different ways to fix this for my case:

1. Make DiagnosticInfoSSP derive from DiagnosticInfoOptimizationBase, and then provide a new switch to explicitly enable it.
2. Remove DiagnosticInfoSSP entirely and use emitOptimizationRemark. The remarks would then be emitted by specifying something like -pass-remarks=stack-protector. I haven't investigated this, but this might make the clang-side changes unnecessary.
3. Move the isEnabled() virtual function of DiagnosticInfoOptimizationBase up the inheritance hierarchy so that a greater range of subclasses can use the relevant behaviour. Potentially, move it all the way to the top level DiagnosticInfo class and default it to always be enabled, allowing subclasses to override it as desired.
4. Disable all diagnostics with remark severity by default, either via the implementation in 3) or by querying the severity in "isDiagnosticEnabled" in LLVMContext.cpp, and explicitly enable them only as requested. This would then need to be paired up with a new option to enable all remarks, or at least a switch to enable the new diagnostic itself. This would be similar to what clang does.
5. Always emitting this diagnostic is actually fine and so the failing tests need fixing.

Stack Protection is a pass, but it isn't really an optimization (in fact the motivation behind the change is to enable users to analyse where it has been applied and find ways to suppress it), so using something with Optimization in the name to emit it doesn't feel right to me. To me, 3) or 4) seem like the overall correct approach. As far as I can see, the only DiagnosticInfo users with DS_Remark severity all go through the DiagnosticInfoOptimizationBase class, so there wouldn't be any unintentional side effects of disabling remarks by default, since these are disabled already.

I guess the question is, should remarks be disabled by default? What do people think?


Repository:
  rL LLVM

https://reviews.llvm.org/D29023