[PATCH] D19498: [scan-build] fix logic error warnings emitted on llvm code base

[Apelete Seketeli via llvm-commits]

apelete added inline comments.

================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:917
@@ -916,2 +916,3 @@
     MachineInstr *MI = std::prev(MII);
-    if (MI && DbgMI) {
+    assert(MI && "MachineInstr is null, cannot iterate through MachineBasicBlock");
+
----------------
MatzeB wrote:
> I think this is so obvious that we don't even need the assert. We will get a crash in MI->isDebugValue() anyway when MI is nullptr.
In that case, not checking MI as I did below should be enough to suppress the analyzer warning.
Would that be a good solution (since MI->isDebugValue() will induce  a crash if MI is nullptr anyway) ?

================
Comment at: lib/Target/ARM/Thumb2SizeReduction.cpp:976-977
@@ -975,2 +975,4 @@
 
+    assert(BundleMI && "MachineInstr is null, abort reducing width of instructions");
+
     LiveCPSR = UpdateCPSRUse(*MI, LiveCPSR);
----------------
MatzeB wrote:
> This looks like it needs to go inside the "if (MI->isInsideBundle())" case below. I must admit I also don't understand why the analyzer would warn here, there is nothing obvious that indicates that BundleMI may be a nullptr below...
Will move the assert inside the "if (MI->isInsideBundle())" case.

It seems the analyzer is warning because of:

942.   MachineInstr *BundleMI = nullptr;

969.    if (MI->isBundle()) {
970.      BundleMI = MI;
971.     continue;
972.    }

If the condition is false, then BundleMI is nullptr at this point.


http://reviews.llvm.org/D19498