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

[Daniel Dunbar]

Meta question, what rule is gcc following that we don't get these
warnings with it?

 - Daniel

On Thu, Dec 24, 2009 at 5:11 PM, John McCall <rjmccall at apple.com> wrote:
> 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.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>