commit 9af13d24b16141d3f931b3b3768dfe08d6584153
author Chih-Hung Hsieh <chh@google.com> Thu Jun 02 14:40:09 2016 -0700
committer Chih-Hung Hsieh <chh@google.com> Thu Jun 02 15:02:25 2016 -0700
tree 844775e0cba6d52104fa4a22372a95e392a7b1bf
parent 676f6e3c7f29d4b77de8400f3b9a0113d6bb8a3b

Do not return with stack address in global variable.

Clang static analyzer gives warning when address of
local variable 'attr' is saved in a global variable.
This change passes required values down to signal handler
instead of saving local variable address in a signal handler.

Change-Id: I7955939487a5afdf7b1f47eb74a92eb5aa76cfc9

tests/pthread_test.cpp

diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 52a3252..b0c95fe 100755
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -1347,7 +1347,8 @@
 
 struct GetStackSignalHandlerArg {
   volatile bool done;
-  void* signal_handler_sp;
+  void* signal_stack_base;
+  size_t signal_stack_size;
   void* main_stack_base;
   size_t main_stack_size;
 };
@@ -1363,9 +1364,17 @@
   void* stack_base;
   size_t stack_size;
   ASSERT_EQ(0, pthread_attr_getstack(&attr, &stack_base, &stack_size));
-  getstack_signal_handler_arg.signal_handler_sp = &attr;
-  getstack_signal_handler_arg.main_stack_base = stack_base;
-  getstack_signal_handler_arg.main_stack_size = stack_size;
+
+  // Verify if the stack used by the signal handler is the alternate stack just registered.
+  ASSERT_LE(getstack_signal_handler_arg.signal_stack_base, &attr);
+  ASSERT_LT(static_cast<void*>(&attr),
+            static_cast<char*>(getstack_signal_handler_arg.signal_stack_base) +
+            getstack_signal_handler_arg.signal_stack_size);
+
+  // Verify if the main thread's stack got in the signal handler is correct.
+  ASSERT_EQ(getstack_signal_handler_arg.main_stack_base, stack_base);
+  ASSERT_LE(getstack_signal_handler_arg.main_stack_size, stack_size);
+
   getstack_signal_handler_arg.done = true;
 }
 
@@ -1398,18 +1407,13 @@
 
   ScopedSignalHandler handler(SIGUSR1, getstack_signal_handler, SA_ONSTACK);
   getstack_signal_handler_arg.done = false;
+  getstack_signal_handler_arg.signal_stack_base = sig_stack;
+  getstack_signal_handler_arg.signal_stack_size = sig_stack_size;
+  getstack_signal_handler_arg.main_stack_base = main_stack_base;
+  getstack_signal_handler_arg.main_stack_size = main_stack_size;
   kill(getpid(), SIGUSR1);
   ASSERT_EQ(true, getstack_signal_handler_arg.done);
 
-  // Verify if the stack used by the signal handler is the alternate stack just registered.
-  ASSERT_LE(sig_stack, getstack_signal_handler_arg.signal_handler_sp);
-  ASSERT_GE(reinterpret_cast<char*>(sig_stack) + sig_stack_size,
-            getstack_signal_handler_arg.signal_handler_sp);
-
-  // Verify if the main thread's stack got in the signal handler is correct.
-  ASSERT_EQ(main_stack_base, getstack_signal_handler_arg.main_stack_base);
-  ASSERT_LE(main_stack_size, getstack_signal_handler_arg.main_stack_size);
-
   ASSERT_EQ(0, sigaltstack(&oss, nullptr));
   ASSERT_EQ(0, munmap(sig_stack, sig_stack_size));
 }