Permalink
Browse files

Fix more MacOS crashes due to non-portable local labels (#8278)

Summary:
Isolated repro of the problem: https://gist.github.com/fredemmott/e3b7f6f1b41be02e5980416677dd05a6

There's a few issues muddled together here:
 - the `.l%=` labels were never actually local labels, but were presumably meant to be (`.L`)
 - labels are case-sensitive, so fixing that made it so that we hit duplicates with other `.L%=` labels in some files, depending on how aggressive the compiler is at inlining (this is an issue with gcc5, but not apple clang)
 - the crashes on MacOS due to non-local labels weren't an issue on GCC5/Linux - despite them also being non-local labels there - because GCC5/Linux isn't inserting the incorrect `noexcept` frame metadata

fixes #8276
Pull Request resolved: #8278

Reviewed By: alexeyt

Differential Revision: D9226853

Pulled By: fredemmott
  • Loading branch information...
fredemmott committed Aug 10, 2018
1 parent 4089ed8 commit 05f97eb0388d84ba2e0685c2715582c3a41dd3d1
Showing with 16 additions and 13 deletions.
  1. +8 −6 hphp/runtime/base/hash-table-inl.h
  2. +1 −1 hphp/util/etch-helpers.h
  3. +6 −0 hphp/util/portability.h
  4. +1 −6 hphp/util/word-mem.h
@@ -14,6 +14,8 @@
+----------------------------------------------------------------------+
*/
#include "hphp/util/portability.h"
namespace HPHP {
namespace array {
@@ -44,10 +46,10 @@ void HashTableCommon::InitHash(int32_t* hash, uint32_t scale) {
uint64_t offset = scale * 16;
__asm__ __volatile__(
"pcmpeqd %%xmm0, %%xmm0\n" // xmm0 <- 11111....
".l%=:\n"
ASM_LOCAL_LABEL("HTCIH%=") ":\n"
"sub $0x10, %0\n"
"movdqu %%xmm0, (%1, %0)\n"
"ja .l%=\n"
"ja " ASM_LOCAL_LABEL("HTCIH%=") "\n"
: "+r"(offset) : "r"(hash) : "xmm0"
);
#elif defined(__aarch64__)
@@ -58,10 +60,10 @@ void HashTableCommon::InitHash(int32_t* hash, uint32_t scale) {
uint64_t ones = -1;
auto hash2 = hash;
__asm__ __volatile__(
".l%=:\n"
ASM_LOCAL_LABEL("HTCIH%=") ":\n"
"stp %x2, %x2, [%x1], #16\n"
"subs %x0, %x0, #16\n"
"bhi .l%=\n"
"bhi " ASM_LOCAL_LABEL("HTCIH%=") "\n"
: "+r"(offset), "+r"(hash2) : "r"(ones) : "cc"
);
#elif defined(__powerpc__)
@@ -71,10 +73,10 @@ void HashTableCommon::InitHash(int32_t* hash, uint32_t scale) {
uint64_t offset = scale * 16;
__asm__ __volatile__(
"vspltisw 0, -1 \n"
".l%=: \n"
ASM_LOCAL_LABEL("HTCIH%=") ":\n"
"subic. %0, %0, 0x10\n"
"stxvd2x 32, %1, %0 \n"
"bgt .l%= \n"
"bgt " ASM_LOCAL_LABEL("HTCIH%=") "\n"
: "+b"(offset) : "b"(hash) : "v0");
#else
static_assert(Empty == -1, "Cannot use wordfillones().");
View
@@ -36,7 +36,7 @@
#define ETCH_SECTION(x) .text
#define ETCH_SIZE(x) /* not used on OSX */
#define ETCH_NAME(x) _##x
#define ETCH_LABEL(x) .L##_##x
#define ETCH_LABEL(x) L##_##x /* not .L */
#define ETCH_TYPE(x, y) /* not used on OSX */
#define ETCH_NAME_REL(x) _##x@GOTPCREL(%rip)
#define ETCH_ARG1 %rdi
View
@@ -268,4 +268,10 @@
# define MSVC_NO_STD_CHRONO_DURATION_DOUBLE_ADD 1
#endif
#ifdef __APPLE__
#define ASM_LOCAL_LABEL(x) "L" x
#else
#define ASM_LOCAL_LABEL(x) ".L" x
#endif
#endif
View
@@ -20,18 +20,13 @@
#include <folly/Portability.h>
#include "hphp/util/assertions.h"
#include "hphp/util/portability.h"
extern "C" void* _memcpy8(void* dst, const void* src, size_t len);
extern "C" void* _memcpy16(void* dst, const void* src, size_t len);
extern "C" void _bcopy32(void* dst, const void* src, size_t len);
extern "C" void _bcopy_in_64(void* dst, const void* src, size_t lenIn64);
#ifdef __APPLE__
#define ASM_LOCAL_LABEL(x) "L" x
#else
#define ASM_LOCAL_LABEL(x) ".L" x
#endif
namespace HPHP {
/*

0 comments on commit 05f97eb

Please sign in to comment.