[PATCH] D29494: Mostly split the StackProtect Pass into an Analysis Pass and a Transformation Pass.

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls created this revision.
Herald added subscribers: igorb, mehdi_amini, aemerson.

This fixes PR30324.
This also enables writing tests with pipelines starting after the
StackProtector pass, end running until assembly output, see added test
case test/CodeGen/AArch64/GlobalISel/legalmir2asm.mir.

With this patch, SelectionDAGISel still uses the StackProtector
transformation pass as an analysis - that still needs to be cleaned up
in the future, but the clean-up here is already enough to fix PR30324
and writing tests with pass pipelines as described above.

Furthermore, IIUC, under the new pass manager, a pass cannot be both an
analysis and do transformations, so this is needed (but not enough) to
port the Stackprotect pass to the new pass manager.

Testing this patch on the test-suite, with cflags set to
"-fstack-protector", "-fstack-protector-strong", "-fstack-protector-all"
shows that for all 3 of those configurations, all tests keep on passing
on X86_64.
For -fstack-protector, 3 of the 499 programs have a slight change in
code generation.
For -fstack-protector-strong and -fstack-protector-all, code generation
changes for 7 out of 499 programs.
The changes happen in the case when 2 stack frames get merged that both
were analyzed to need stack protection. Before this patch, the tags for
object protection (SSPLK_LargeArray, SSPLK_SmallArray, SSPLK_AddrOf,
SSPLK_None) got merged so that priority was given according to that
order.
After this patch, the StackProtectorAnalysis gets run again after the
merging of frame slots, and only 1 type is available out of the original
2 on the original frame slot. Therefore, it's possible that in such a
merged frame slot, the tag describing the needed frame slot protection
is lower, e.g.:
 int32_t a;
 char b[2];
 &a;
if the frame slots for a and b get merged; before this slot would be
analysed as SSPLK_SmallArray; now it will be analysed as SSPLK_AddrOf.

I'm not entirely sure if this is acceptable or not. If not, I think an
obvious solution would be to not let the StackColoring pass merge stack
slots that the StackProtectorAnalysis analyzed as needing protection.

The patch needs more polishing, but I hope to get some review feedback
on the general direction before polishing more.


https://reviews.llvm.org/D29494

Files:
  include/llvm/CodeGen/StackProtector.h
  include/llvm/InitializePasses.h
  lib/CodeGen/CodeGen.cpp
  lib/CodeGen/LocalStackSlotAllocation.cpp
  lib/CodeGen/PrologEpilogInserter.cpp
  lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  lib/CodeGen/StackColoring.cpp
  lib/CodeGen/StackProtector.cpp
  test/CodeGen/AArch64/GlobalISel/legalmir2asm.mir
  test/CodeGen/X86/stack-protector.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29494.86961.patch
Type: text/x-patch
Size: 33404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170203/53a3d1e2/attachment-0001.bin>