[cfe-dev] -Wsign-compare results on LLVM/clang

[John McCall]

I've made an audit of the -Wsign-compare warnings that clang is emitting on LLVM/Clang.  And here it is!

lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5382:37: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

  This is legitimate; AFAICT there's an actual bug here where the code uses -1 as a sentinel value and then that gets converted to unsigned and potentially used in arithmetic.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:189:55: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

      unsigned NumParts;
      ...
      unsigned RoundParts = NumParts & (NumParts - 1) ?
        1 << Log2_32(NumParts) : NumParts;

  False positive.  clang can avoid this warning by not diagnosing if the left operand is a bitshift of a literal.

include/llvm/CodeGen/MachORelocation.h:44:54: warning: operands of ? are integers of different signs: 'int32_t const' (aka 'int const') and 'uint32_t const' (aka 'unsigned int const') [-Wsign-compare]

    uint32_t getAddress() const { return r_scattered ? r_value : r_address; }

  If this is noise it's good noise.  Can r_value ever be negative?  If not, why is it signed?

lib/Target/X86/X86FastISel.cpp:306:45: warning: operands of ? are integers of different signs: 'int64_t' (aka 'long long') and 'uint64_t' (aka 'unsigned long long') [-Wsign-compare]

      addFullAddress(BuildMI(MBB, DL, TII.get(Opc)), AM)
                             .addImm(Signed ? CI->getSExtValue() :
                                              CI->getZExtValue());

  False positive because addImm just cares about its operand as a bit-pattern.  Damned if I know how to avoid diagnosing this.

lib/Target/MSP430/AsmPrinter/MSP430AsmPrinter.cpp:127:49: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

      O << ',' << (MAI->getAlignmentIsInBytes() ? (1 << Align) : Align);

  False positive, same resolution as earlier.

lib/Target/X86/X86ISelLowering.cpp:3245:22: warning: operands of ? are integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]

    int Val = isLeft ? (i - NumZeros) : i;

  False positive.  Could easily be worked around by using unsigned instead of int.

lib/Target/X86/X86ISelLowering.cpp:3246:39: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

    int Idx = SVOp->getMaskElt(isLeft ? i : (i - NumZeros));

  Same thing.

lib/Target/ARM/ARMLoadStoreOptimizer.cpp:793:5: warning: operands of ? are integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]

  int Offset = isAM2
    ? ARM_AM::getAM2Offset(OffField)
    : (isAM3 ? ARM_AM::getAM3Offset(OffField)
             : ARM_AM::getAM5Offset(OffField) * 4);

  Might be a false positive; probably worth complaining about, though.

lib/Target/SystemZ/SystemZRegisterInfo.cpp:203:44: warning: operands of ? are integers of different signs: 'int64_t' (aka 'long long') and 'uint64_t' (aka 'unsigned long long') [-Wsign-compare]

    uint64_t ThisVal = (Offset > Chunk) ? Chunk : Offset;
    ...                                                   
      .addReg(SystemZ::R15D).addImm((isSub ? -(int64_t)ThisVal : ThisVal));

  False positive, addImm just wants a bit-pattern.  No idea how to avoid warning about this while still being useful.

lib/Target/SystemZ/AsmPrinter/SystemZAsmPrinter.cpp:340:49: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

      O << ',' << (MAI->getAlignmentIsInBytes() ? (1 << Align) : Align);

  False positive, will be fixed.

lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp:1254:55: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

            O << ',' << (MAI->getAlignmentIsInBytes() ? (1 << Align) : Align);

  False positive, will be fixed.

lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp:1261:53: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

          O << "," << (MAI->getAlignmentIsInBytes() ? (1 << Align) : Align);

  False positive, will be fixed.

lib/Target/X86/AsmPrinter/X86AsmPrinter.cpp:734:55: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
lib/Target/X86/AsmPrinter/X86AsmPrinter.cpp:743:53: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

  I need to look into these.

lib/CodeGen/VirtRegRewriter.cpp:870:11: warning: operands of ? are integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]

        int SSorRMId = DoReMat
          ? VRM.getReMatId(NewOp.VirtReg) : NewOp.StackSlotOrReMat;

  False positive, I think.

tools/clang/lib/Basic/SourceManager.cpp:46:17: warning: operands of ? are integers of different signs: 'size_t' (aka 'unsigned long') and 'off_t' (aka 'long long') [-Wsign-compare]

  return Buffer ? Buffer->getBufferSize() : Entry->getSize();

  False positive: getBufferSize() is always positive.  It's possible this could be suppressed by noting that the signed operand is of higher rank than the unsigned operand, even though they're the same size.  I need to look into whether that will bury too many actual problems.

tools/clang/lib/Driver/ArgList.cpp:83:18: warning: operands of ? are integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]
tools/clang/lib/Driver/ArgList.cpp:84:18: warning: operands of ? are integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]
tools/clang/lib/Driver/ArgList.cpp:85:18: warning: operands of ? are integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]

  int A0Idx = A0 ? A0->getIndex() : -1;
  int A1Idx = A1 ? A1->getIndex() : -1;
  int A2Idx = A2 ? A2->getIndex() : -1;

  These are false positives (assuming fewer than 2^31 arguments) because of the implicit conversion back to signed int.  I don't really have a problem with warning about them, though.

tools/clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp:456:25: warning: comparison of integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]

  The only diagnostic arising from comparisons rather than the conditional operator.  This is a false positive which I can fix pretty easily.

John.