[compiler-rt] r304598 - [asan] fix one more case where stack-use-after-return is not async-signal-safe (during thread startup). beef-up the test to give it a chance to catch regressions. Also relax the lint to make C++11 more usable.

[Hahnfeld, Jonas via llvm-commits]

Hi,

 

please have a look at https://reviews.llvm.org/D34876 where I reverted this assertion. I’m having ulimit –s unlimited. Can you please explain why having such a large stack is actually a bad thing?

 

Regards,

Jonas

 

From: Kostya Serebryany [mailto:kcc at google.com] 
Sent: Wednesday, July 12, 2017 12:20 AM
To: Hahnfeld, Jonas <Hahnfeld at itc.rwth-aachen.de>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [compiler-rt] r304598 - [asan] fix one more case where stack-use-after-return is not async-signal-safe (during thread startup). beef-up the test to give it a chance to catch regressions. Also relax the lint to make C++11 more usable.

 

sorry for delay... 

 

Yes, CHECK_LE(stack_size, 0x10000000); is intentional. 

Do you really have 1Gb stacks? 

 

On Mon, Jun 26, 2017 at 2:04 AM, Hahnfeld, Jonas <Hahnfeld at itc.rwth-aachen.de <mailto:Hahnfeld at itc.rwth-aachen.de> > wrote:

Hi,


-----Original Message-----
From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org <mailto:llvm-commits-bounces at lists.llvm.org> ] On Behalf Of
Kostya Serebryany via llvm-commits
Sent: Friday, June 2, 2017 11:32 PM
To: llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org> 
Subject: [compiler-rt] r304598 - [asan] fix one more case where
stack-use-after-return is not async-signal-safe (during thread startup).
beef-up the test to give it a chance to catch regressions. Also relax the lint
to make C++11 more usable.

Author: kcc
Date: Fri Jun  2 16:32:04 2017
New Revision: 304598

URL: http://llvm.org/viewvc/llvm-project?rev=304598 <http://llvm.org/viewvc/llvm-project?rev=304598&view=rev> &view=rev
Log:
[asan] fix one more case where stack-use-after-return is not async-signal-safe
(during thread startup). beef-up the test to give it a chance to catch
regressions. Also relax the lint to make C++11 more usable.

Modified:
    compiler-rt/trunk/lib/asan/asan_thread.cc
    compiler-rt/trunk/lib/sanitizer_common/scripts/check_lint.sh
    compiler-rt/trunk/test/asan/TestCases/Linux/uar_signals.cc

Modified: compiler-rt/trunk/lib/asan/asan_thread.cc
URL:
http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_thread.cc?rev=304598 <http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_thread.cc?rev=304598&r1=304597&r2=304598&view=diff> &r1=304597&r2=304598&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_thread.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_thread.cc Fri Jun  2 16:32:04 2017
@@ -166,16 +166,19 @@ void AsanThread::FinishSwitchFiber(FakeS
 }

 inline AsanThread::StackBounds AsanThread::GetStackBounds() const {
-  if (!atomic_load(&stack_switching_, memory_order_acquire))
-    return StackBounds{stack_bottom_, stack_top_};  // NOLINT
+  if (!atomic_load(&stack_switching_, memory_order_acquire)) {
+    // Make sure the stack bounds are fully initialized.
+    if (stack_bottom_ >= stack_top_) return {0, 0};
+    return {stack_bottom_, stack_top_};  }

   char local;
   const uptr cur_stack = (uptr)&local;
   // Note: need to check next stack first, because FinishSwitchFiber
   // may be in process of overwriting stack_top_/bottom_. But in such case
   // we are already on the next stack.
   if (cur_stack >= next_stack_bottom_ && cur_stack < next_stack_top_)
-    return StackBounds{next_stack_bottom_, next_stack_top_};  // NOLINT
-  return StackBounds{stack_bottom_, stack_top_};              // NOLINT
+    return {next_stack_bottom_, next_stack_top_};  return
+ {stack_bottom_, stack_top_};
 }

 uptr AsanThread::stack_top() {
@@ -197,6 +200,7 @@ FakeStack *AsanThread::AsyncSignalSafeLa
   uptr stack_size = this->stack_size();
   if (stack_size == 0)  // stack_size is not yet available, don't use
FakeStack.
     return nullptr;
+  CHECK_LE(stack_size, 0x10000000);

Was this change intended to be committed? I get quite some failures on my
CentOS 7 with this:
==31552==AddressSanitizer CHECK failed:
[...]/projects/compiler-rt/lib/asan/asan_thread.cc:203 "((stack_size)) <=
((0x10000000))" (0x40000000, 0x10000000)

Removing this check "fixes" the failures.

Cheers,
Jonas

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170712/f8bf2c7e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5792 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170712/f8bf2c7e/attachment.bin>