Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FastUnwindStack infinite loops occasionally on Chromium net_unittests #162

Closed
ramosian-glider opened this issue Aug 31, 2015 · 9 comments
Closed

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 162

Blaming FastUnwindStack shows the initial commit plus the x32 patch from H.J. Lu.

I don't have debug info for the asan rtl in the execution that I caught looping, so
I haven't isolated it, but I'm filing anyway.

Trace:
(gdb) bt
#0  0x000000000046f7b4 in __sanitizer::StackTrace::FastUnwindStack(unsigned long, unsigned
long, unsigned long, unsigned long) ()
#1  0x0000000000465bb8 in operator new(unsigned long) ()
#2  0x00007fd5e4bb1a89 in std::string::_Rep::_S_create(unsigned long, unsigned long,
std::allocator<char> const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007fd5e4bb3495 in char* std::string::_S_construct<char const*>(char const*,
char const*, std::allocator<char> const&, std::forward_iterator_tag) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007fd5e4bb35e3 in std::basic_string<char, std::char_traits<char>, std::allocator<char>
>::basic_string(char const*, std::allocator<char> const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007fd5e631e0b7 in InitializeHistograms () at ../../net/cookies/cookie_monster.cc:2064
#6  0x00007fd5e631dc54 in CookieMonster () at ../../net/cookies/cookie_monster.cc:308
#7  0x0000000000b638de in Create () at ../../net/cookies/cookie_monster_unittest.cc:81
#8  GetCookieStore () at ../../net/cookies/cookie_store_unittest.h:215
#9  TestBody () at ../../net/cookies/cookie_store_unittest.h:444
#10 0x00000000026ee9b8 in Run () at ../../testing/gtest/src/gtest.cc:2067
#11 0x00000000026f06b6 in Run () at ../../testing/gtest/src/gtest.cc:2244
#12 0x00000000026f150d in Run () at ../../testing/gtest/src/gtest.cc:2351
#13 0x00000000026fe3d1 in RunAllTests () at ../../testing/gtest/src/gtest.cc:4177
#14 0x00000000026fd961 in impl () at ../../testing/gtest/src/gtest.cc:2051
#15 Run () at ../../testing/gtest/src/gtest.cc:3810
#16 0x0000000002715aeb in Run () at ../../base/test/test_suite.cc:159
#17 0x0000000000810c49 in main () at ../../net/base/run_all_unittests.cc:55

If I do "finish" from here, it never leaves the frame, which is my evidence for the
looping.

Here's the source:

  while (frame >= prev_frame &&
         frame < (uhwptr *)stack_top - 2 &&
         frame > (uhwptr *)stack_bottom &&
         size < max_size) {
    uhwptr pc1 = frame[1];
    if (pc1 != pc) {
      trace[size++] = (uptr) pc1;
    }
    prev_frame = frame;
    frame = (uhwptr *)frame[0];
  }

Here's the asm:

=> 0x000000000046f790 <+48>:    mov    (%rdi),%r9
   0x000000000046f793 <+51>:    mov    %rax,%rdx
   0x000000000046f796 <+54>:    cmp    0x8(%rdi),%r9
   0x000000000046f79a <+58>:    jae    0x46f7c3 <_ZN11__sanitizer10StackTrace15FastUnwindStackEmmmm+99>
   0x000000000046f79c <+60>:    mov    0x8(%rdx),%r10
   0x000000000046f7a0 <+64>:    cmp    %rsi,%r10
   0x000000000046f7a3 <+67>:    je     0x46f7b1 <_ZN11__sanitizer10StackTrace15FastUnwindStackEmmmm+81>
   0x000000000046f7a5 <+69>:    lea    0x1(%r9),%rax
   0x000000000046f7a9 <+73>:    mov    %rax,(%rdi)
   0x000000000046f7ac <+76>:    mov    %r10,0x10(%rdi,%r9,8)
   0x000000000046f7b1 <+81>:    mov    (%rdx),%rax
   0x000000000046f7b4 <+84>:    cmp    %rdx,%rax
   0x000000000046f7b7 <+87>:    jb     0x46f7c3 <_ZN11__sanitizer10StackTrace15FastUnwindStackEmmmm+99>
   0x000000000046f7b9 <+89>:    cmp    %rcx,%rax
   0x000000000046f7bc <+92>:    jae    0x46f7c3 <_ZN11__sanitizer10StackTrace15FastUnwindStackEmmmm+99>
   0x000000000046f7be <+94>:    cmp    %r8,%rax
   0x000000000046f7c1 <+97>:    ja     0x46f790 <_ZN11__sanitizer10StackTrace15FastUnwindStackEmmmm+48>

OK, so all those cmps use %rax, so that's frame from the while loop condition.

(gdb) p/x $rax
$1 = 0x7fff94a14000
(gdb) x/a $rax
0x7fff94a14000: 0x7fff94a14000

So, chasing frame pointer points to itself.

Reported by rnk@google.com on 2013-02-20 19:17:47

@ramosian-glider
Copy link
Member Author

Here's a small diff to try to fix the bug.

diff --git a/lib/sanitizer_common/sanitizer_stacktrace.cc b/lib/sanitizer_common/sanitizer_stacktrace.cc
index 639a69c..6309b23 100644
--- a/lib/sanitizer_common/sanitizer_stacktrace.cc
+++ b/lib/sanitizer_common/sanitizer_stacktrace.cc
@@ -131,8 +131,9 @@ void StackTrace::FastUnwindStack(uptr pc, uptr bp,
   CHECK(size == 0 && trace[0] == pc);
   size = 1;
   uhwptr *frame = (uhwptr *)bp;
-  uhwptr *prev_frame = frame;
-  while (frame >= prev_frame &&
+  uhwptr *prev_frame = frame - 1;
+  // Avoid infinite loop when frame == frame[0] by using frame > prev_frame.
+  while (frame > prev_frame &&
          frame < (uhwptr *)stack_top - 2 &&
          frame > (uhwptr *)stack_bottom &&
          size < max_size) {

I'll commit this upstream if I can satisfy myself that it worked.  I'm not familiar
with testing this code.

Reported by rnk@google.com on 2013-02-20 19:46:05


- _Attachment: [infloop.diff](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-162/comment-1/infloop.diff)_

@ramosian-glider
Copy link
Member Author

Tentatively fixed in r175661.  I used that version of the asan rtl with net_unittests
run in a loop and it hasn't hung again yet.  I could get it in 2 tries previously,
but each run is about 5min, so I don't have that much data.

We can reopen if it's still shown to be a problem.

Reported by rnk@google.com on 2013-02-20 20:41:29

  • Status changed: Fixed

@ramosian-glider
Copy link
Member Author

Any ideas how this can be tested?

Reported by ramosian.glider on 2013-02-21 11:04:24

@ramosian-glider
Copy link
Member Author

Other than by running every test we can (including chromium tests)?
No

Reported by konstantin.s.serebryany on 2013-02-21 11:06:19

@ramosian-glider
Copy link
Member Author

But finding the root cause and replicating that situation in a small test?

I mean, why did we get a frame pointer pointing to itself? Can it happen in valid code
(i.e. some kind of tail call between two functions with frame pointers)? Corrupted
stack?

Do we know which function (in net_unittests) our unwinder got stuck on?

Reported by eugenis@google.com on 2013-02-21 11:11:12

@ramosian-glider
Copy link
Member Author

Most likely it happened in libstdc++ with does not have frame pointers at all, so any
random thing is possible.
Still, it should not be too hard to cook a function that has a stack region filled
with self-pointing pointers and compile it without FP.

Reported by eugenis@google.com on 2013-02-21 11:16:22

@ramosian-glider
Copy link
Member Author

I will write a unit test with an array of pointers that are fp+retaddr pairs and call
the unwinder on it with the right fp.  One of the fp slots will point to itself.

Reported by rnk@google.com on 2013-02-21 13:50:01

@ramosian-glider
Copy link
Member Author

FYI I saw this issue when trying to land a fix in chromium and this bug was causing
some tests to timeout on ChromeOS ASan bots (https://codereview.chromium.org/12330144/).
I was able to reproduce the issue locally and see similar callstack in the hanging
thread:

(gdb) bt
#0  0x00000000005bc580 in __sanitizer::StackTrace::FastUnwindStack(unsigned long, unsigned
long, unsigned long, unsigned long) ()
#1  0x00000000005b2988 in operator new(unsigned long) ()
#2  0x00007f6c601d7a89 in std::string::_Rep::_S_create(unsigned long, unsigned long,
std::allocator<char> const&) ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007f6c601d9495 in char* std::string::_S_construct<char const*>(char const*,
char const*, std::allocator<char> const&, std::forward_iterator_tag) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007f6c601d95e3 in std::basic_string<char, std::char_traits<char>, std::allocator<char>
>::basic_string(char const*, std::allocator<char> const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00000000036cb11c in OnBeforeRequest () at ../../chrome/browser/extensions/api/web_request/web_request_api.cc:617
#6  0x0000000002982d5c in OnBeforeURLRequest () at ../../chrome/browser/net/chrome_network_delegate.cc:398
#7  0x0000000006413baa in NotifyBeforeURLRequest () at ../../net/base/network_delegate.cc:19
#8  0x00000000063c4ef4 in Start () at ../../net/url_request/url_request.cc:470
...

Reported by sergeyu@chromium.org on 2013-02-27 19:39:59

@ramosian-glider
Copy link
Member Author

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:13:40

  • Labels added: ProjectAddressSanitizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant