commit 76144aaa6397fe9e16893882cf59c5c9c0684a66
author Yabin Cui <yabinc@google.com> Thu Nov 19 13:52:16 2015 -0800
committer Yabin Cui <yabinc@google.com> Fri Nov 20 17:44:26 2015 -0800
tree 366d104ea020fb5d9138d296aac1fcceda383c61
parent 0ebe2f07c35d4b764bc5d8b5226004e3db46da91

Change _stdio_handles_locking into _caller_handles_locking.

It is reported by tsan that funlockfile() can unlock an unlocked mutex.
It happens when printf() is called before fopen() or other stdio stuff.
As FLOCKFILE(fp) is called before __sinit(), _stdio_handles_locking is false,
and _FLOCK(fp) will not be locked. But then cantwrite(fp) in __vfprintf()
calls__sinit(), which makes _stdio_handles_locking become true, and
FUNLOCKFILE(fp) unlocks _FLOCK(fp).

Change _stdio_handles_locking into _caller_handles_locking,
so __sinit() won't change its value. Add test due to my previous fault.

Bug: 25392375
Change-Id: I483e3c3cdb28da65e62f1fd9615bf58c5403b4dd

libc/bionic/flockfile.cpp

diff --git a/libc/bionic/flockfile.cpp b/libc/bionic/flockfile.cpp
index b73907cb..db68801 100644
--- a/libc/bionic/flockfile.cpp
+++ b/libc/bionic/flockfile.cpp
@@ -40,7 +40,7 @@
     __sinit();
   }
 
-  if (fp != NULL) {
+  if (fp != nullptr) {
     pthread_mutex_lock(&_FLOCK(fp));
   }
 }
@@ -52,7 +52,7 @@
 
   // The specification for ftrylockfile() says it returns 0 on success,
   // or non-zero on error. So return an errno code directly on error.
-  if (fp == NULL) {
+  if (fp == nullptr) {
     return EINVAL;
   }
 
@@ -64,7 +64,7 @@
     __sinit();
   }
 
-  if (fp != NULL) {
+  if (fp != nullptr) {
     pthread_mutex_unlock(&_FLOCK(fp));
   }
 }

libc/stdio/local.h

diff --git a/libc/stdio/local.h b/libc/stdio/local.h
index ced94e3..3ae7059 100644
--- a/libc/stdio/local.h
+++ b/libc/stdio/local.h
@@ -109,7 +109,7 @@
   pthread_mutex_t _lock;
 
   /* __fsetlocking support */
-  bool _stdio_handles_locking;
+  bool _caller_handles_locking;
 };
 
 #if defined(__cplusplus)
@@ -131,7 +131,7 @@
 	pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); \
 	pthread_mutex_init(&_FLOCK(fp), &attr); \
 	pthread_mutexattr_destroy(&attr); \
-	_EXT(fp)->_stdio_handles_locking = true; \
+	_EXT(fp)->_caller_handles_locking = false; \
 } while (0)
 
 #define _FILEEXT_SETUP(f, fext) \
@@ -208,8 +208,8 @@
 	(fp)->_lb._base = NULL; \
 }
 
-#define FLOCKFILE(fp)   if (_EXT(fp)->_stdio_handles_locking) flockfile(fp)
-#define FUNLOCKFILE(fp) if (_EXT(fp)->_stdio_handles_locking) funlockfile(fp)
+#define FLOCKFILE(fp)   if (!_EXT(fp)->_caller_handles_locking) flockfile(fp)
+#define FUNLOCKFILE(fp) if (!_EXT(fp)->_caller_handles_locking) funlockfile(fp)
 
 #define FLOATING_POINT
 #define PRINTF_WIDE_CHAR

libc/stdio/stdio_ext.cpp

diff --git a/libc/stdio/stdio_ext.cpp b/libc/stdio/stdio_ext.cpp
index 310076a..f273d45 100644
--- a/libc/stdio/stdio_ext.cpp
+++ b/libc/stdio/stdio_ext.cpp
@@ -74,7 +74,7 @@
 }
 
 int __fsetlocking(FILE* fp, int type) {
-  int old_state = _EXT(fp)->_stdio_handles_locking ? FSETLOCKING_INTERNAL : FSETLOCKING_BYCALLER;
+  int old_state = _EXT(fp)->_caller_handles_locking ? FSETLOCKING_BYCALLER : FSETLOCKING_INTERNAL;
   if (type == FSETLOCKING_QUERY) {
     return old_state;
   }
@@ -84,7 +84,7 @@
     __libc_fatal("Bad type (%d) passed to __fsetlocking", type);
   }
 
-  _EXT(fp)->_stdio_handles_locking = (type == FSETLOCKING_INTERNAL);
+  _EXT(fp)->_caller_handles_locking = (type == FSETLOCKING_BYCALLER);
   return old_state;
 }