From: Jim Meyering Date: Sun, 17 Aug 2014 21:34:32 +0000 (-0700) Subject: fbcode: __x__-protect all __attribute__ keywords, mechanically X-Git-Tag: v0.22.0~388 X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=a42878e365ad97ef653a823ac5d5b62f91f63663;p=folly.git 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 2b93aa34..e08ca064 100644 --- 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 a2715a0b..951e5cae 100644 --- 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 c624b199..76b9385c 100644 --- 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 765db5ac..9438c391 100644 --- 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 59745b5f..a3b6bccc 100644 --- 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 #define FOLLY_HAVE_MALLOC_H 1 diff --git a/folly/Portability.h b/folly/Portability.h index 61b9eb22..6ce3d51d 100644 --- 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 1e2915d5..6d06cff3 100644 --- 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 data_; -} __attribute__((aligned)); +} __attribute__((__aligned__)); ////////////////////////////////////////////////////////////////////// diff --git a/folly/VersionCheck.h b/folly/VersionCheck.h index 2eda84d0..94be9e56 100644 --- 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 96887081..db713061 100644 --- 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 e35703a0..37f6e960 100644 --- 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 6a6298a7..075c9efc 100644 --- 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 3e01ee44..fdda4ca9 100644 --- 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 78b60ea0..81030dbf 100644 --- 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 b0169370..b11859ec 100644 --- 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 sizes {0, 1, 64, 256, 1024, 1 << 10}; diff --git a/folly/test/ThreadCachedArenaTest.cpp b/folly/test/ThreadCachedArenaTest.cpp index 1c68dfe2..63ae4c9c 100644 --- 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 354f7d5a..4680aa8d 100644 --- 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 starter_;