[LLVMbugs] [Bug 10610] New: PVS-Studio vs Clang

bugzilla-daemon at llvm.org bugzilla-daemon at llvm.org
Mon Aug 8 11:34:58 CDT 2011


http://llvm.org/bugs/show_bug.cgi?id=10610

           Summary: PVS-Studio vs Clang
           Product: clang
           Version: trunk
          Platform: PC
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: -New Bugs
        AssignedTo: unassignedclangbugs at nondot.org
        ReportedBy: karpov at viva64.com
                CC: llvmbugs at cs.uiuc.edu


I checked the Clang project using the PVS-Studio static code analyzer.
I just glanced through the code but managed to find a few obviously odd
fragments. Below I will cite the analyzer-generated messages I have
studied and the corresponding code fragments. I hope this will help to
improve the project a bit.

P.S.
And also see my blog post: http://www.viva64.com/en/b/0108/

---------------------------------------
V501 There are identical sub-expressions 'LBO->hasNoUnsignedWrap ()' to the
left and to the right of the '&&' operator. LLVMAnalysis
instructionsimplify.cpp 1891

static Value *SimplifyICmpInst(...) {
  ...
  case Instruction::Shl: {
    bool NUW = LBO->hasNoUnsignedWrap() && LBO->hasNoUnsignedWrap();
    bool NSW = LBO->hasNoSignedWrap() && RBO->hasNoSignedWrap();
  ...
}

Need:
bool NUW = LBO->hasNoUnsignedWrap() && RBO->hasNoUnsignedWrap();
---------------------------------------
V501 There are identical sub-expressions 'FoldMskICmp_Mask_AllZeroes' to the
left and to the right of the '|' operator. LLVMInstCombine
instcombineandorxor.cpp 505
V501 There are identical sub-expressions 'FoldMskICmp_Mask_NotAllZeroes' to the
left and to the right of the '|' operator. LLVMInstCombine
instcombineandorxor.cpp 509

static unsigned getTypeOfMaskedICmp(...)
{
  ...
  result |= (icmp_eq ? (FoldMskICmp_Mask_AllZeroes |
                        FoldMskICmp_Mask_AllZeroes |
                        FoldMskICmp_AMask_Mixed |
                        FoldMskICmp_BMask_Mixed)
                     : (FoldMskICmp_Mask_NotAllZeroes |
                        FoldMskICmp_Mask_NotAllZeroes |
                        FoldMskICmp_AMask_NotMixed |
                        FoldMskICmp_BMask_NotMixed));
  ...
}
---------------------------------------
V523 The 'then' statement is equivalent to the 'else' statement.
LLVMInstCombine instcombineandorxor.cpp 1387

static bool CollectBSwapParts(...) {
  ...
  // 2) The input and ultimate destinations must line up: if byte 3 of an i32
  // is demanded, it needs to go into byte 0 of the result.  This means that
the
  // byte needs to be shifted until it lands in the right byte bucket.  The
  // shift amount depends on the position: if the byte is coming from the high
  // part of the value (e.g. byte 3) then it must be shifted right.  If from
the
  // low part, it must be shifted left.
  unsigned DestByteNo = InputByteNo + OverallLeftShift;
  if (InputByteNo < ByteValues.size()/2) {
    if (ByteValues.size()-1-DestByteNo != InputByteNo)
      return true;
  } else {
    if (ByteValues.size()-1-DestByteNo != InputByteNo)
      return true;
  }
  ...
}
---------------------------------------
V501 There are identical sub-expressions to the left and to the right of the
'||' operator: Op1I->hasOneUse () || Op1I->hasOneUse () LLVMInstCombine
instcombineandorxor.cpp 2246

Instruction *InstCombiner::visitXor(BinaryOperator &I) {
  ...
  if (Op0I && Op1I && Op0I->isShift() && 
      Op0I->getOpcode() == Op1I->getOpcode() && 
      Op0I->getOperand(1) == Op1I->getOperand(1) &&
      (Op1I->hasOneUse() || Op1I->hasOneUse())) {
  ...
}

Need:
(Op0I->hasOneUse() || Op1I->hasOneUse())) {
---------------------------------------
V501 There are identical sub-expressions to the left and to the right of the
'==' operator: Src1.FloatVal == Src1.FloatVal LLVMInterpreter execution.cpp 425
V501 There are identical sub-expressions to the left and to the right of the
'==' operator: Src2.FloatVal == Src2.FloatVal LLVMInterpreter execution.cpp 426
V501 There are identical sub-expressions to the left and to the right of the
'==' operator: Src1.DoubleVal == Src1.DoubleVal LLVMInterpreter execution.cpp
428
V501 There are identical sub-expressions to the left and to the right of the
'==' operator: Src2.DoubleVal == Src2.DoubleVal LLVMInterpreter execution.cpp
429

static GenericValue executeFCMP_ORD(GenericValue Src1, GenericValue Src2,
                                     Type *Ty) {
  GenericValue Dest;
  if (Ty->isFloatTy())
    Dest.IntVal = APInt(1,(Src1.FloatVal == Src1.FloatVal && 
                           Src2.FloatVal == Src2.FloatVal));
  else
    Dest.IntVal = APInt(1,(Src1.DoubleVal == Src1.DoubleVal && 
                           Src2.DoubleVal == Src2.DoubleVal));
  return Dest;
}

Strange...
---------------------------------------
V501 There are identical sub-expressions to the left and to the right of the
'!=' operator: Src1.FloatVal != Src1.FloatVal LLVMInterpreter execution.cpp 437
V501 There are identical sub-expressions to the left and to the right of the
'!=' operator: Src2.FloatVal != Src2.FloatVal LLVMInterpreter execution.cpp 438
V501 There are identical sub-expressions to the left and to the right of the
'!=' operator: Src1.DoubleVal != Src1.DoubleVal LLVMInterpreter execution.cpp
440
V501 There are identical sub-expressions to the left and to the right of the
'!=' operator: Src2.DoubleVal != Src2.DoubleVal LLVMInterpreter execution.cpp
441

static GenericValue executeFCMP_UNO(GenericValue Src1, GenericValue Src2,
                                     Type *Ty) {
  GenericValue Dest;
  if (Ty->isFloatTy())
    Dest.IntVal = APInt(1,(Src1.FloatVal != Src1.FloatVal || 
                           Src2.FloatVal != Src2.FloatVal));
  else
    Dest.IntVal = APInt(1,(Src1.DoubleVal != Src1.DoubleVal || 
                           Src2.DoubleVal != Src2.DoubleVal));
  return Dest;
}
---------------------------------------
V501 There are identical sub-expressions to the left and to the right of the
'&&' operator: CurChar != '\n' && CurChar != '\n' LLVMMCParser asmlexer.cpp 149

AsmToken AsmLexer::LexLineComment() {
  // FIXME: This is broken if we happen to a comment at the end of a file,
which
  // was .included, and which doesn't end with a newline.
  int CurChar = getNextChar();
  while (CurChar != '\n' && CurChar != '\n' && CurChar != EOF)
    CurChar = getNextChar();
  ...
}

Need:
while (CurChar != '\n' && CurChar != EOF)
or
while (CurChar != '\n' && CurChar != '\r' && CurChar != EOF)
---------------------------------------
V501 There are identical sub-expressions to the left and to the right of the
'||' operator. LLVMSelectionDAG dagcombiner.cpp 7340

bool DAGCombiner::SimplifySelectOps(...) {
  ...
  LoadSDNode *LLD = cast<LoadSDNode>(LHS);
  LoadSDNode *RLD = cast<LoadSDNode>(RHS);
  ...
  if ((LLD->hasAnyUseOfValue(1) &&
       (LLD->isPredecessorOf(CondLHS) || LLD->isPredecessorOf(CondRHS))) ||
      (LLD->hasAnyUseOfValue(1) &&
       (LLD->isPredecessorOf(CondLHS) || LLD->isPredecessorOf(CondRHS))))
    return false;
  ...
}

Can be:
if ((LLD->hasAnyUseOfValue(1) &&
     (LLD->isPredecessorOf(CondLHS) || LLD->isPredecessorOf(CondRHS))) ||
    (RLD->hasAnyUseOfValue(1) &&
     (RLD->isPredecessorOf(CondLHS) || RLD->isPredecessorOf(CondRHS))))
  return false;
---------------------------------------
V501 There are identical sub-expressions '!DAG.isKnownNeverZero (LHS)' to the
left and to the right of the '&&' operator. LLVMX86CodeGen x86isellowering.cpp
11635

static SDValue PerformSELECTCombine(...)
{
  ...
  SDValue LHS = N->getOperand(1);
  SDValue RHS = N->getOperand(2);

  ...
  if (!UnsafeFPMath &&
      !DAG.isKnownNeverZero(LHS) && !DAG.isKnownNeverZero(RHS))
  ...
  if (!UnsafeFPMath &&
      !DAG.isKnownNeverZero(LHS) && !DAG.isKnownNeverZero(LHS))
  ...
}

Can be:
!DAG.isKnownNeverZero(LHS) && !DAG.isKnownNeverZero(RHS)
---------------------------------------
V523 The 'then' statement is equivalent to the 'else' statement. clangRewrite
rewriteobjc.cpp 4181

std::string RewriteObjC::SynthesizeBlockFunc(...)
{
  ...
  if (convertBlockPointerToFunctionPointer(QT))
    QT.getAsStringInternal(ParamStr, Context->PrintingPolicy);
  else
    QT.getAsStringInternal(ParamStr, Context->PrintingPolicy);      
  ...
}

Strange...
---------------------------------------
V501 There are identical sub-expressions 'CondResult.isInvalid ()' to the left
and to the right of the '||' operator. clangSema semastmt.cpp 899

StmtResult
Sema::ActOnDoStmt(...)
{
  ...
  ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc);
  if (CondResult.isInvalid() || CondResult.isInvalid())
    return StmtError();
  ...
}
---------------------------------------
V501 There are identical sub-expressions 'P->isMemberPointerType ()' to the
left and to the right of the '&&' operator. clangSema sematemplatededuction.cpp
3240

Sema::DeduceTemplateArguments(...)
{
  ...
  if ((P->isPointerType() && A->isPointerType()) ||
      (P->isMemberPointerType() && P->isMemberPointerType()))
  ...
}

Need:
(P->isMemberPointerType() && A->isMemberPointerType())
---------------------------------------
V519 The 'SignedOverflowBehavior' variable is assigned values twice
successively. Perhaps this is a mistake. Check lines: 207, 213. FrontendTests
langoptions.h 213

LangOptions() {
  ...
  SignedOverflowBehavior = SOB_Undefined;

  AssumeSaneOperatorNew = 1;
  AccessControl = 1;
  ElideConstructors = 1;

  SignedOverflowBehavior = 0;
  ...
}

It's not a bug. But ...
---------------------------------------
V556 The values of different enum types are compared: switch(ENUM_TYPE_A) {
case ENUM_TYPE_B: ... }. LLVMAsmPrinter targetlowering.h 268
V556 The values of different enum types are compared: switch(ENUM_TYPE_A) {
case ENUM_TYPE_B: ... }. LLVMAsmPrinter targetlowering.h 270

enum LegalizeAction {
  Legal,      // The target natively supports this operation.
  Promote,    // This operation should be executed in a larger type.
  Expand,     // Try to expand this to other ops, otherwise use a libcall.
  Custom      // Use the LowerOperation hook to implement custom lowering.
};

enum LegalizeTypeAction {
  TypeLegal,           // The target natively supports this type.
  TypePromoteInteger,  // Replace this integer with a larger one.
  TypeExpandInteger,   // Split this integer into two of half the size.
  TypeSoftenFloat,     // Convert this float to a same size integer type.
  TypeExpandFloat,     // Split this float into two of half the size.
  TypeScalarizeVector, // Replace this one-element vector with its element.
  TypeSplitVector,     // Split this vector into two of half the size.
  TypeWidenVector      // This vector should be widened into a larger vector.
};

LegalizeTypeAction getTypeAction(LLVMContext &Context, EVT VT) const;

EVT getTypeToExpandTo(LLVMContext &Context, EVT VT) const {
  ...
  switch (getTypeAction(Context, VT)) {
  case Legal:
    return VT;
  case Expand:
  ...
}
---------------------------------------
V524 It is odd that the body of 'clearTopDownPointers' function is fully
equivalent to the body of 'clearBottomUpPointers' function (ObjCARC.cpp, line
1318). LLVMScalarOpts objcarc.cpp 1322

MapTy PerPtrTopDown;
MapTy PerPtrBottomUp;

void clearBottomUpPointers() {
  PerPtrTopDown.clear();
}

void clearTopDownPointers() {
  PerPtrTopDown.clear();
}

Can be:
void clearBottomUpPointers() {
  PerPtrBottomUp.clear();
}
---------------------------------------
V519 The 'Src1Name' variable is assigned values twice successively. Perhaps
this is a mistake. Check lines: 211, 215. LLVMX86AsmPrinter x86instcomments.cpp
215

void llvm::EmitAnyX86InstComments(...) {
  ...
  case X86::VPERMILPSri:
    DecodeVPERMILPSMask(4, MI->getOperand(2).getImm(),
                        ShuffleMask);
    Src1Name = getRegName(MI->getOperand(0).getReg());
  case X86::VPERMILPSYri:
    DecodeVPERMILPSMask(8, MI->getOperand(2).getImm(),
                        ShuffleMask);
    Src1Name = getRegName(MI->getOperand(0).getReg());
    break;
  ... 
}

Need break operator.
---------------------------------------
V519 The 'ParmOffset' variable is assigned values twice successively. Perhaps
this is a mistake. Check lines: 3953, 3956. clangAST astcontext.cpp 3956

std::string ASTContext::getObjCEncodingForBlock(const BlockExpr *Expr) const {
  ...
  ParmOffset = PtrSize;
  // Argument types.
  ParmOffset = PtrSize;
  ...
}

It's not a bug. But ...
---------------------------------------

-- 
Configure bugmail: http://llvm.org/bugs/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


More information about the LLVMbugs mailing list