[LLVMdev] Multiple one-line bugs in LLVM

[Lockal S]

Hi. There are few one-line bugs Andrey Karpov have found with static analisys.
He wrote a big article in russian on http://habrahabr.ru/blogs/compilers/125626/
for advertising purposes of static analyzer for Visual Studio his company
developed.

Most of the problems are easy to fix, so I list them in here for trunk version.
Also few problems in clang code were found, I don't list them in here.

----

lib/Target/X86/X86ISelLowering.cpp:11689
!DAG.isKnownNeverZero(LHS) && !DAG.isKnownNeverZero(LHS))

Note that there are identical subexpressions '!DAG.isKnownNeverZero (LHS)' to
the left and to the right of the '&&' operator.
The second subexpression should probably be !DAG.isKnownNeverZero(RHS)).

----

lib/Transforms/Scalar/ObjCARC.cpp:1138
void clearBottomUpPointers() {
    PerPtrTopDown.clear();
}

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

Note that the body of 'clearTopDownPointers' function is fully equivalent to
the body of 'clearBottomUpPointers' function. The advised solution is to change
this code into

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

----

lib/Analysis/InstructionSimplify.cpp:1891
    bool NUW = LBO->hasNoUnsignedWrap() && LBO->hasNoUnsignedWrap();

There are identical sub-expressions 'LBO->hasNoUnsignedWrap()' to the left and
to the right of the '&&' operator. Looks like the correct code is

    bool NUW = LBO->hasNoUnsignedWrap() && RBO->hasNoUnsignedWrap();

----

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1384
  if (InputByteNo < ByteValues.size()/2) {
    if (ByteValues.size()-1-DestByteNo != InputByteNo)
      return true;
  } else {
    if (ByteValues.size()-1-DestByteNo != InputByteNo)
      return true;
  }

Note that 'then' and 'else' are the same. It can be a problem or can not.

----

lib/Target/X86/InstPrinter/X86InstComments.cpp:208
  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());

The 'Src1Name' variable is assigned values twice successively. So break
instruction should be at line 212.

----

lib/MC/MCParser/AsmLexer.cpp:149
  while (CurChar != '\n' && CurChar != '\n' && CurChar != EOF)

There are identical sub-expressions to the left and to the right of the '&&'
operator: CurChar != '\n' && CurChar != '\n'. The second expression should
probably be CurChar != '\n'?

----
lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:505
    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));

There are identical sub-expressions 'FoldMskICmp_Mask_AllZeroes' and
'FoldMskICmp_Mask_NotAllZeroes' to the left and to the right of the
'|'
operator. Probably not a bug, just some duplicated code.

----

include/llvm/Target/TargetLowering.h:267
      switch (getTypeAction(Context, VT)) {
      case Legal:
        return VT;
      case Expand:

Note that the values of different enum types (LegalizeAction and
LegalizeTypeAction) are compared. This code works well because of the same order
for enums, but it would be better to change this into

      switch (getTypeAction(Context, VT)) {
      case TypeLegal:
        return VT;
      case TypeExpand:

----

Thanks a lots for any fixes and answers.