[PATCH] Fix for crash during SjLj preparation step

Bill Wendling isanbard at gmail.com
Wed Jul 2 12:21:46 PDT 2014


This is crying out for a testcase. Can you supply one?

================
Comment at: lib/CodeGen/SjLjEHPrepare.cpp:259
@@ +258,3 @@
+    // for example, a struct with no fields which is used extensively in Rust
+    // In this case there is no need to transfer anything
+    if (ST || AT) {      
----------------
This comment should go inside of the 'if (ST || AT) {' block. Also, no need to mention specific languages. Something like this:

// An aggregate type with no fields (e.g., an empty struct) cannot have
// extract/insert instructions applied to it. In this case, there is no need to
// transfer anything.


================
Comment at: lib/CodeGen/SjLjEHPrepare.cpp:262
@@ +261,3 @@
+      if ((ST && ST->getNumElements() != 0) 
+        || (AT && AT->getNumElements() != 0)) {
+        Instruction *EI = ExtractValueInst::Create(AI, 0, "", AfterAllocaInsPt);
----------------
I'm worried about doing this. The purpose of this function is to lower the arguments so that we don't have to handle them with special code. If we do this, we are now allowing the empty aggregate to be live outside of the entry block. Actually, this whole code seems suspect to me. One way around both the original problem and your problem would be to use another instruction that would satisfy both. This one is off the top of my head:

define @foo([i8, i32] %a) {
  %new_a = select i1 true, [i8, i32] %a, [i8, i32] null
  ...
}

Could you convert this block of code to do that and see if it still works?

http://reviews.llvm.org/D4256






More information about the llvm-commits mailing list