[LLVMbugs] [Bug 21308] New: clang/LLVM 3.5.0 skips checking the for loop's condition when entering the loop

[bugzilla-daemon at llvm.org]

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

            Bug ID: 21308
           Summary: clang/LLVM 3.5.0 skips checking the for loop's
                    condition when entering the loop
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: Loop Optimizer
          Assignee: unassignedbugs at nondot.org
          Reporter: Kaorukun at gmx.net
                CC: llvmbugs at cs.uiuc.edu
    Classification: Unclassified

Created attachment 13214
  --> http://llvm.org/bugs/attachment.cgi?id=13214&action=edit
nsCSSValue.cpp

Platform: Gentoo Linux x86_64, kernel 3.17.0
Packages: sys-devel/clang-3.5.0-r100 , sys-devel/llvm-3.5.0

Under some circumstances, optimized (-Os,-O2) code generation in clang/LLVM
3.5.0 for the for-loop causes the resulting code to not check the for-loop's
condition when entering the loop for the first time.

Background: 
I have discovered this bug after I compiled Firefox 33.0 using clang. There
were no issues with the compilation, and the application worked perfectly fine
as it seemed. But then I noticed that it reproducibly segfaults when visiting
Wikipedia's search page. This however does not happen if Firefox was compiled
with LLVM 3.4.2. I disassembled the part of the code where segfault occurs and
found out that LLVM 3.4.2 and 3.5.0 produce quite different x86 code. Further,
3.5.0 version did not properly check the for-loop condition, which allowed a
NULL pointer to slip through and be passed to the next function, causing a
segfault. Bug report on Mozilla's bug tracker :
https://bugzilla.mozilla.org/show_bug.cgi?id=1083461

I took the affected part of code from Firefox source (in
mozilla-release/layout/style/nsCSSValue.cpp) and reduced it as much I could to
make a demo reproducing this bug. I'm attaching it to this post, followed by
another nsGlue.cpp file.

Steps to compile/reproduce:
1. Save the attachments as nsCSSValue.cpp and nsGlue.cpp respectively
2. Compile and link with:
  clang++ -g -o nsCSSValue.o -c -Os nsCSSValue.cpp
  clang++ -g -o nsGlue.o -c -Os nsGlue.cpp
  clang++ -o test nsCSSValue.o nsGlue.o
3. Run the resulting binary:
  ./test

Expected results:
Application prints "equal=0"

Actual results:
LLVM 3.4.2 : Application prints "equal=0"
LLVM 3.5.0 : Application segfaults

The affected part of the code is the method nsCSSValueList::operator== which
compares two linked lists:

bool nsCSSValueList::operator==(const nsCSSValueList& aOther) const
{
  if (this == &aOther)
    return true;

  const nsCSSValueList *p1 = this, *p2 = &aOther;
  for ( ; p1 && p2; p1 = p1->mNext, p2 = p2->mNext) {
    if (p1->mValue != p2->mValue)
      return false;
  }
  return !p1 && !p2; // true if same length, false otherwise
}

In my example, this method is called with this!=NULL and &aOther==NULL, i.e.
the result should be 'false'. The for loop should not be entered at all,
because of the p1&&p2 condition, which will be false in this case. Therefore,
the code of the nsCSSValue::operator== (which is called through 'p1->mValue !=
p2->mValue') should never be called.

If you disassemble the nsCSSValue.o with: 
objdump -S -d nsCSSValue.o
and look at the assembly code for nsCSSValueList::operator== :

bool nsCSSValueList::operator==(const nsCSSValueList& aOther) const
{
  2a:   41 56                   push   %r14
  2c:   53                      push   %rbx
  2d:   50                      push   %rax
  2e:   49 89 f6                mov    %rsi,%r14
  31:   48 89 fb                mov    %rdi,%rbx
  34:   b0 01                   mov    $0x1,%al
  if (this == &aOther)
  36:   4c 39 f3                cmp    %r14,%rbx
  39:   74 3a                   je     75 <_ZNK14nsCSSValueListeqERKS_+0x4b>
  3b:   b1 01                   mov    $0x1,%cl
    return true;

  const nsCSSValueList *p1 = this, *p2 = &aOther;
  for ( ; p1 && p2; p1 = p1->mNext, p2 = p2->mNext) {
  3d:   48 85 db                test   %rbx,%rbx
  40:   74 27                   je     69 <_ZNK14nsCSSValueListeqERKS_+0x3f>
public:
  nsCSSValue() : mUnit(eCSSUnit_Null) {}

  bool operator==(const nsCSSValue& aOther) const;
  bool operator!=(const nsCSSValue& aOther) const
    { return !(*this == aOther); }
  42:   48 89 df                mov    %rbx,%rdi
  45:   4c 89 f6                mov    %r14,%rsi
  48:   e8 b3 ff ff ff          callq  0 <_ZNK10nsCSSValueeqERKS_>
  if (this == &aOther)
    return true;
(snip)

Here, %rbx=%rdi=this and %r14=%rsi=&aOther=0
The code checks for:
off 36 : cmp %r14,%rbx => (this==&aOther) => false, no jump
off 3d : test %rbx,%rbx => (this==NULL) => false, no jump
and forgets to check if (&aOther==NULL), so it calls the nsCSSValue::operator==
(_ZNK10nsCSSValueeqERKS_) with a NULL pointer that leads to a segfault.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20141018/fe16efb8/attachment.html>