fbcode: __x__-protect all __attribute__ keywords, mechanically

Summary:
The first (and sometimes 2nd) argument to __attribute__
should begin and end with "__". If it does not, then it
is at risk of conflicting with user cpp-defined symbols.
Don't do that.  This change mechanically corrects most
such violations in fbcode.  A companion change (see parent task)
adds a lint rule to help keep things clean going forwards.

Define this function to filter the list of file names to which
we will apply the transformations. First, ensure we modify only
C and C++ sources.  The second process excludes a few directories
that appear to contain third-party code:

filter() {
grep -E '\.(c|h|cc|cpp|hpp)$' | grep -vE \
'^external/|/unbound/|/gearmand_|(3rd|third.)party|/external/libevent'
}

then, run this command,

git grep -l __attribute__ | filter \
| xargs perl -pi -e 's/(__attribute__ *)\(\(( *[^_]\w+)/$1((__${2}__/g'

inducing changes like this, for each such keyword:

-} __attribute__((packed));
+} __attribute__((__packed__));

That got all but the __format__ archetype arguments like this:
__attribute__((__format__(printf.  That final "printf"
keyword should have the "__" prefix and suffix, too.

Run this command to transform those remaining uses:

git grep -l '__attribute__.*__format' | filter | xargs perl -pi -e \
's/(__attribute__ *\(\(__format__ *)\( *(printf|scanf|strftime|strfmon)/$1(__${2}__/g'

That command induces changes like this:

-static inline int __attribute__ ((__format__ (printf, 3, 4)))
+static inline int __attribute__ ((__format__ (__printf__, 3, 4)))

This exercise is useful to avoid spurious name-space conflicts,
especially when the unprotected keyword is used in a header file.

Test Plan:
I've ensured tao builds and passes its tests.
I will watch the arc-run tests.

Reviewed By: njormrod@fb.com

Subscribers: rounak, trunkagent, hphp-diffs@, nli, ps, fbcode-common-diffs@, mcdonald, chaoyc, bill, search-fbcode-diffs@, lars, net-systems@, davejwatson, varunk, ruibalp, hero-diffs@, andrewcox, sorg, dfechete, dhruba, sdoroshenko, mcduff, marccelani, hitesh, mshneer, cold-storage-diffs@, omry, jcoens, unicorn-diffs@, ldbrandy, sumeet, abirchall, fugalh, atlas2-eng@, dcapra, mpawlowski, alandau, nkgupta, shilin, bmatheny, everstore-dev@, zhuohuang, wormhole-diffs@, vnalla, msk, maoy, mwa, jgehring, adsatlasreporting@, mconnor, oujin, bwester, micha, tulloch, ptc, logdevice-diffs, alikhtarov, shikong, fuegen

FB internal diff: D1508983

Tasks: 4947824
@override-unit-failures

diff --git a/folly/Arena.h b/folly/Arena.h
index 2b93aa344547fd658cb0a3477a3c3d7c826498ef..e08ca06454c323722a6f714068e85cb8cd7e9705 100644 (file)
--- a/folly/Arena.h
+++ b/folly/Arena.h
@@ -149,7 +149,7 @@ class Arena {
    private:
     Block() { }
     ~Block() { }
-  } __attribute__((aligned));
+  } __attribute__((__aligned__));
   // This should be alignas(std::max_align_t) but neither alignas nor
   // max_align_t are supported by gcc 4.6.2.
 

diff --git a/folly/Bits.cpp b/folly/Bits.cpp
index a2715a0b3f6b43914e9d5bad8c15ef6e426a6095..951e5cae51d1d55b5ff4a714a427c7cf30efb8ec 100644 (file)
--- a/folly/Bits.cpp
+++ b/folly/Bits.cpp
@@ -73,7 +73,7 @@ namespace detail {
 // or popcount_builtin
 int popcount(unsigned int x)
 #if FOLLY_HAVE_IFUNC && !defined(FOLLY_SANITIZE_ADDRESS)
-  __attribute__((ifunc("folly_popcount_ifunc")));
+  __attribute__((__ifunc__("folly_popcount_ifunc")));
 #else
 {  return popcount_builtin(x); }
 #endif
@@ -82,7 +82,7 @@ int popcount(unsigned int x)
 // or popcountll_builtin
 int popcountll(unsigned long long x)
 #if FOLLY_HAVE_IFUNC && !defined(FOLLY_SANITIZE_ADDRESS)
-  __attribute__((ifunc("folly_popcountll_ifunc")));
+  __attribute__((__ifunc__("folly_popcountll_ifunc")));
 #else
 {  return popcountll_builtin(x); }
 #endif

diff --git a/folly/Conv.h b/folly/Conv.h
index c624b1994cbb0899759640b470b3babb6f005579..76b9385cfe6c242872c21168c9b0a533abdd9de9 100644 (file)
--- a/folly/Conv.h
+++ b/folly/Conv.h
@@ -869,7 +869,7 @@ namespace detail {
 // still not overflow uint16_t.
 constexpr int32_t OOR = 10000;
 
-__attribute__((aligned(16))) constexpr uint16_t shift1[] = {
+__attribute__((__aligned__(16))) constexpr uint16_t shift1[] = {
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  // 0-9
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  10
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  20
@@ -898,7 +898,7 @@ __attribute__((aligned(16))) constexpr uint16_t shift1[] = {
   OOR, OOR, OOR, OOR, OOR, OOR                       // 250
 };
 
-__attribute__((aligned(16))) constexpr uint16_t shift10[] = {
+__attribute__((__aligned__(16))) constexpr uint16_t shift10[] = {
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  // 0-9
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  10
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  20
@@ -927,7 +927,7 @@ __attribute__((aligned(16))) constexpr uint16_t shift10[] = {
   OOR, OOR, OOR, OOR, OOR, OOR                       // 250
 };
 
-__attribute__((aligned(16))) constexpr uint16_t shift100[] = {
+__attribute__((__aligned__(16))) constexpr uint16_t shift100[] = {
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  // 0-9
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  10
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  20
@@ -956,7 +956,7 @@ __attribute__((aligned(16))) constexpr uint16_t shift100[] = {
   OOR, OOR, OOR, OOR, OOR, OOR                       // 250
 };
 
-__attribute__((aligned(16))) constexpr uint16_t shift1000[] = {
+__attribute__((__aligned__(16))) constexpr uint16_t shift1000[] = {
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  // 0-9
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  10
   OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR, OOR,  //  20

diff --git a/folly/Demangle.cpp b/folly/Demangle.cpp
index 765db5ac03fc52aa693e31469e1dfa067b0c018b..9438c391484a68cdf84797ecb9b0ec3b6a1c8ead 100644 (file)
--- a/folly/Demangle.cpp
+++ b/folly/Demangle.cpp
@@ -28,7 +28,7 @@
 //
 // TODO(tudorb): Detect this with autoconf for the open-source version.
 //
-// __attribute__((weak)) doesn't work, because cplus_demangle_v3_callback
+// __attribute__((__weak__)) doesn't work, because cplus_demangle_v3_callback
 // is exported by an object file in libiberty.a, and the ELF spec says
 // "The link editor does not extract archive members to resolve undefined weak
 // symbols" (but, interestingly enough, will resolve undefined weak symbols

diff --git a/folly/Malloc.h b/folly/Malloc.h
index 59745b5fd0c47615a72a6739f40000697c59b864..a3b6bccc6e1847e9e714f2a965e1ea83f8ecc433 100644 (file)
--- a/folly/Malloc.h
+++ b/folly/Malloc.h
@@ -51,11 +51,11 @@ namespace folly {
  * using another malloc implementation.
  */
 extern "C" int rallocm(void**, size_t*, size_t, size_t, int)
-__attribute__((weak));
+__attribute__((__weak__));
 extern "C" int allocm(void**, size_t*, size_t, int)
-__attribute__((weak));
+__attribute__((__weak__));
 extern "C" int mallctl(const char*, void*, size_t*, void*, size_t)
-__attribute__((weak));
+__attribute__((__weak__));
 
 #include <bits/functexcept.h>
 #define FOLLY_HAVE_MALLOC_H 1

diff --git a/folly/Portability.h b/folly/Portability.h
index 61b9eb22ff30234b47d969fffddf01ad6e6409b7..6ce3d51dfb67c7def26d835e1e928267003d6d8c 100644 (file)
--- a/folly/Portability.h
+++ b/folly/Portability.h
@@ -51,7 +51,7 @@
 
 // MaxAlign: max_align_t isn't supported by gcc
 #ifdef __GNUC__
-struct MaxAlign { char c; } __attribute__((aligned));
+struct MaxAlign { char c; } __attribute__((__aligned__));
 #else /* !__GNUC__ */
 # error Cannot define MaxAlign on this platform
 #endif
@@ -71,14 +71,14 @@ struct MaxAlign { char c; } __attribute__((aligned));
 #else
 # define FOLLY_PRINTF_FORMAT /**/
 # define FOLLY_PRINTF_FORMAT_ATTR(format_param, dots_param) \
-  __attribute__((format(printf, format_param, dots_param)))
+  __attribute__((__format__(__printf__, format_param, dots_param)))
 #endif
 
 // noreturn
 #if defined(_MSC_VER)
 # define FOLLY_NORETURN __declspec(noreturn)
 #elif defined(__clang__) || defined(__GNUC__)
-# define FOLLY_NORETURN __attribute__((noreturn))
+# define FOLLY_NORETURN __attribute__((__noreturn__))
 #else
 # define FOLLY_NORETURN
 #endif
@@ -87,7 +87,7 @@ struct MaxAlign { char c; } __attribute__((aligned));
 #ifdef _MSC_VER
 # define FOLLY_NOINLINE __declspec(noinline)
 #elif defined(__clang__) || defined(__GNUC__)
-# define FOLLY_NOINLINE __attribute__((noinline))
+# define FOLLY_NOINLINE __attribute__((__noinline__))
 #else
 # define FOLLY_NOINLINE
 #endif
@@ -96,7 +96,7 @@ struct MaxAlign { char c; } __attribute__((aligned));
 #ifdef _MSC_VER
 # define FOLLY_ALWAYS_INLINE __forceinline
 #elif defined(__clang__) || defined(__GNUC__)
-# define FOLLY_ALWAYS_INLINE inline __attribute__((always_inline))
+# define FOLLY_ALWAYS_INLINE inline __attribute__((__always_inline__))
 #else
 # define FOLLY_ALWAYS_INLINE
 #endif
@@ -114,7 +114,7 @@ struct MaxAlign { char c; } __attribute__((aligned));
 # define FOLLY_PACK_PUSH __pragma(pack(push, 1))
 # define FOLLY_PACK_POP __pragma(pack(pop))
 #elif defined(__clang__) || defined(__GNUC__)
-# define FOLLY_PACK_ATTR __attribute__((packed))
+# define FOLLY_PACK_ATTR __attribute__((__packed__))
 # define FOLLY_PACK_PUSH /**/
 # define FOLLY_PACK_POP /**/
 #else

diff --git a/folly/SmallLocks.h b/folly/SmallLocks.h
index 1e2915d5de127c5ffab8627c34e08cf9273277f3..6d06cff335cd6ff7555ef3df5df7e4d081f92820 100644 (file)
--- a/folly/SmallLocks.h
+++ b/folly/SmallLocks.h
@@ -96,7 +96,7 @@ namespace detail {
  * init(), since the free state is guaranteed to be all-bits zero.
  *
  * This class should be kept a POD, so we can used it in other packed
- * structs (gcc does not allow __attribute__((packed)) on structs that
+ * structs (gcc does not allow __attribute__((__packed__)) on structs that
  * contain non-POD data).  This means avoid adding a constructor, or
  * making some members private, etc.
  */
@@ -318,7 +318,7 @@ struct SpinLockArray {
 
   char padding_[FOLLY_CACHE_LINE_SIZE];
   std::array<PaddedSpinLock, N> data_;
-} __attribute__((aligned));
+} __attribute__((__aligned__));
 
 //////////////////////////////////////////////////////////////////////
 

diff --git a/folly/VersionCheck.h b/folly/VersionCheck.h
index 2eda84d01d93a9ad14464174f0e483ae8effac14..94be9e56be5cf398b7ec77ba1132f5836e796fa5 100644 (file)
--- a/folly/VersionCheck.h
+++ b/folly/VersionCheck.h
@@ -70,9 +70,9 @@
 
 #ifdef __APPLE__
 // OS X doesn't support constructor priorities. Just pray it works, I guess.
-#define FOLLY_VERSION_CHECK_PRIORITY __attribute__((constructor))
+#define FOLLY_VERSION_CHECK_PRIORITY __attribute__((__constructor__))
 #else
-#define FOLLY_VERSION_CHECK_PRIORITY __attribute__((constructor(101)))
+#define FOLLY_VERSION_CHECK_PRIORITY __attribute__((__constructor__(101)))
 #endif
 
 // Note that this is carefully crafted: PRODUCT##Version must have external

diff --git a/folly/detail/BitsDetail.h b/folly/detail/BitsDetail.h
index 96887081cb704f6e5c98a9086724a343756eac9f..db713061256fbce04bc8453a978e29d6c2c76d69 100644 (file)
--- a/folly/detail/BitsDetail.h
+++ b/folly/detail/BitsDetail.h
@@ -22,7 +22,7 @@ namespace detail {
 
 // If we're targeting an architecture with popcnt support, use
 // __builtin_popcount directly, as it's presumably inlined.
-// If not, use runtime detection using __attribute__((ifunc))
+// If not, use runtime detection using __attribute__((__ifunc__))
 // (see Bits.cpp)
 #ifdef _MSC_VER
 inline int popcount(unsigned int x) {

diff --git a/folly/detail/CacheLocality.h b/folly/detail/CacheLocality.h
index e35703a0d55cbb69abc18d36d475a57b54e78266..37f6e9607ee9a34bf78f60402109fc33bfc5d7ae 100644 (file)
--- a/folly/detail/CacheLocality.h
+++ b/folly/detail/CacheLocality.h
@@ -127,7 +127,7 @@ struct CacheLocality {
 
 /// An attribute that will cause a variable or field to be aligned so that
 /// it doesn't have false sharing with anything at a smaller memory address.
-#define FOLLY_ALIGN_TO_AVOID_FALSE_SHARING __attribute__((aligned(128)))
+#define FOLLY_ALIGN_TO_AVOID_FALSE_SHARING __attribute__((__aligned__(128)))
 
 /// Holds a function pointer to the VDSO implementation of getcpu(2),
 /// if available

diff --git a/folly/detail/Malloc.h b/folly/detail/Malloc.h
index 6a6298a7bd1b93336a26bceb280cab091fe77081..075c9efca114dcdab86f5fb571ede89a5f62d48a 100644 (file)
--- a/folly/detail/Malloc.h
+++ b/folly/detail/Malloc.h
@@ -24,9 +24,9 @@
 extern "C" {
 
 #if FOLLY_HAVE_WEAK_SYMBOLS
-int rallocm(void**, size_t*, size_t, size_t, int) __attribute__((weak));
-int allocm(void**, size_t*, size_t, int) __attribute__((weak));
-int mallctl(const char*, void*, size_t*, void*, size_t) __attribute__((weak));
+int rallocm(void**, size_t*, size_t, size_t, int) __attribute__((__weak__));
+int allocm(void**, size_t*, size_t, int) __attribute__((__weak__));
+int mallctl(const char*, void*, size_t*, void*, size_t) __attribute__((__weak__));
 #else
 extern int (*rallocm)(void**, size_t*, size_t, size_t, int);
 extern int (*allocm)(void**, size_t*, size_t, int);

diff --git a/folly/detail/MemoryIdler.cpp b/folly/detail/MemoryIdler.cpp
index 3e01ee448ae219f7d9877f18907b91e7e9fa825b..fdda4ca98da3a3d87e6f01d16021b7e452c51ca5 100644 (file)
--- a/folly/detail/MemoryIdler.cpp
+++ b/folly/detail/MemoryIdler.cpp
@@ -32,7 +32,7 @@
 // of a link failure
 extern "C" int mallctl(const char *name, void *oldp, size_t *oldlenp,
                        void *newp, size_t newlen)
-    __attribute__((weak));
+    __attribute__((__weak__));
 
 
 namespace folly { namespace detail {

diff --git a/folly/experimental/exception_tracer/ExceptionTracer.cpp b/folly/experimental/exception_tracer/ExceptionTracer.cpp
index 78b60ea0fc8d8236c48b9fad6351be397cd43429..81030dbfd853940c1b20eded0e1f436e4270d71a 100644 (file)
--- a/folly/experimental/exception_tracer/ExceptionTracer.cpp
+++ b/folly/experimental/exception_tracer/ExceptionTracer.cpp
@@ -33,7 +33,7 @@ using namespace ::folly::symbolizer;
 using namespace __cxxabiv1;
 
 extern "C" {
-StackTraceStack* getExceptionStackTraceStack(void) __attribute__((weak));
+StackTraceStack* getExceptionStackTraceStack(void) __attribute__((__weak__));
 typedef StackTraceStack* (*GetExceptionStackTraceStackType)(void);
 GetExceptionStackTraceStackType getExceptionStackTraceStackFn;
 }

diff --git a/folly/io/test/IOBufTest.cpp b/folly/io/test/IOBufTest.cpp
index b0169370cb8359ddb333d0d6184929a4cc8b20c3..b11859ecef6f7ae229b2dccc789f3d44df04e129 100644 (file)
--- a/folly/io/test/IOBufTest.cpp
+++ b/folly/io/test/IOBufTest.cpp
@@ -795,7 +795,7 @@ TEST(IOBuf, Alignment) {
   // max_align_t doesn't exist in gcc 4.6.2
   struct MaxAlign {
     char c;
-  } __attribute__((aligned));
+  } __attribute__((__aligned__));
   size_t alignment = alignof(MaxAlign);
 
   std::vector<size_t> sizes {0, 1, 64, 256, 1024, 1 << 10};

diff --git a/folly/test/ThreadCachedArenaTest.cpp b/folly/test/ThreadCachedArenaTest.cpp
index 1c68dfe277b17961e81ad2cd8fbed946b18b3ff3..63ae4c9c7d88194f77540df30b71bc38b75f2fa1 100644 (file)
--- a/folly/test/ThreadCachedArenaTest.cpp
+++ b/folly/test/ThreadCachedArenaTest.cpp
@@ -94,7 +94,7 @@ void ArenaTester::merge(ArenaTester&& other) {
 }  // namespace
 
 TEST(ThreadCachedArena, BlockSize) {
-  struct Align { char c; } __attribute__((aligned));
+  struct Align { char c; } __attribute__((__aligned__));
   static const size_t alignment = alignof(Align);
   static const size_t requestedBlockSize = 64;
 

diff --git a/folly/wangle/Later.h b/folly/wangle/Later.h
index 354f7d5a5614110af9bd64c6d605071458f9297e..4680aa8d7036957be85cb1344ae5fa18c867db0c 100644 (file)
--- a/folly/wangle/Later.h
+++ b/folly/wangle/Later.h
@@ -192,7 +192,7 @@ class Later {
   /*
    * Deprecated. Use launch()
    */
-  void fireAndForget() __attribute__ ((deprecated)) { launch(); }
+  void fireAndForget() __attribute__ ((__deprecated__)) { launch(); }
 
  private:
   Promise<void> starter_;