PruneEH pass incorrectly reports that a change was made

[apilipenko]

Summary

PruneEH incorrectly sets MadeChange while adding function attributes. To determine whether a change was made it compares function attributes set with new attributes set. Function attributes set might include new attributes set as well as some other attributes. In this case MadeChange is set, however no change is made.

[majnemer]

Test case?

[apilipenko]

In D14097#275726, @majnemer wrote:

Test case?

As far as I can see this value is not reported or logged anywhere, so I can't see an easy way to test it. I come across this issue while running PruneEH in a fixed point iteration loop.

[reames]

lib/Transforms/IPO/PruneEH.cpp

158 ↗

(On Diff #38478)

I think it would be much clearer to restructure this code as:
if (!SCCMightUnwind &&

 !F->hasFnAttribute(Attribute::NoUnwind)) {
F->addFnAttr(Attribute::NoUnwind);
MadeChange = true;

}
...

[reames]

LGTM

llvm/trunk/lib/Transforms/IPO/PruneEH.cpp

if (!F) {
           break;
       }
     }
   }
 
   // If the SCC doesn't unwind or doesn't throw, note this fact.
   if (!SCCMightUnwind || !SCCMightReturn)
     for (CallGraphSCC::iterator I = SCC.begin(), E = SCC.end(); I != E; ++I) {
-      AttrBuilder NewAttributes;
-
-      if (!SCCMightUnwind)
-        NewAttributes.addAttribute(Attribute::NoUnwind);
-      if (!SCCMightReturn)
-        NewAttributes.addAttribute(Attribute::NoReturn);
-
       Function *F = (*I)->getFunction();
-      const AttributeSet &PAL = F->getAttributes().getFnAttributes();
-      const AttributeSet &NPAL = AttributeSet::get(
-          F->getContext(), AttributeSet::FunctionIndex, NewAttributes);
 
-      if (PAL != NPAL) {
+      if (!SCCMightUnwind && !F->hasFnAttribute(Attribute::NoUnwind)) {
+        F->addFnAttr(Attribute::NoUnwind);
+        MadeChange = true;
+      }
+
+      if (!SCCMightReturn && !F->hasFnAttribute(Attribute::NoReturn)) {
+        F->addFnAttr(Attribute::NoReturn);
         MadeChange = true;
-        F->addAttributes(AttributeSet::FunctionIndex, NPAL);
       }
     }
 
   for (CallGraphSCC::iterator I = SCC.begin(), E = SCC.end(); I != E; ++I) {
     // Convert any invoke instructions to non-throwing functions in this node
     // into call instructions with a branch.  This makes the exception blocks
     // dead.
     if (Function *F = (*I)->getFunction())