[LLVMbugs] [Bug 10610] New: PVS-Studio vs Clang
bugzilla-daemon at llvm.org
bugzilla-daemon at llvm.org
Mon Aug 8 09:34:58 PDT 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 llvm-bugs
mailing list