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

[James Henderson via Phabricator via llvm-commits]

jhenderson updated this revision to Diff 89655.
jhenderson marked 3 inline comments as done.
jhenderson added a comment.

Addressed comments.

I'm a little unsure about the comment I've added, as I am not familiar with those aspects of the compiler, being new to the scene. Can you confirm that it looks ok? I noticed in particular that there is some DominatorTree information being passed around elsewhere in the stack protector code, but I don't know if this is relevant at all.

Also, I'm not a particular fan of what I've had to do to emit the "function attribute or command-line switch" remark. In the event that there are no basic blocks, the remark is not emitted, but the rest of the stack protection code is still run (although I don't think it does much - there's perhaps a separate opportunity to bail out early, but I don't want to risk changing behaviour in this change). That particular version of the remark feels like a function-level remark, whereas from what I can see, the two constructors only really support basic block-level and instruction-level remarks. Would it make sense to have a third constructor in OptimizationRemark that takes a Function instead? It seems to be easy to add, but I don't know if it fits the wider architecture, from your point of view.


https://reviews.llvm.org/D29023

Files:
  include/llvm/IR/DiagnosticInfo.h
  lib/CodeGen/StackProtector.cpp
  lib/IR/DiagnosticInfo.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29023.89655.patch
Type: text/x-patch
Size: 5868 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170224/12a83090/attachment.bin>