[XRay][AArch64] Attempt to fix unstable test XRay-aarch64-linux::patching-unpatching.cc

[rSerge]

Summary

Currently test XRay-aarch64-linux::patching-unpatching.cc sometimes passes, sometimes fails. This is an attempt to fix it by handling better the situations when both __arm__ and __aarch64__ are defined.

[rengolin]

NFC. If this "fixes" the problem, then we have to understand why we're getting the arm macro set.

Regardless, this is "safer" anyway. LGTM. Thanks!

[dberris]

Has this been committed?

[rSerge]

In D27421#613925, @dberris wrote:

Has this been committed?

Yes, sorry I haven't waited for your review and just committed after Renato accepted, because I was hurrying to try to fix the test.
Do you have any objections?

Committed in https://reviews.llvm.org/rL288729

[dberris]

No objections, just wondering what the revision number was -- using 'arc land' would update the differential review thread appropriately. :)

[rSerge]

I just followed the steps from http://llvm.org/docs/Phabricator.html#subversion-and-arcanist .
Thanks for the suggestion. However, it seems I have to do it manually, because arc land for me says "is only supported under git, hg." . I'm using SVN.

[dberris]

Ah, okay. What I typically do is make sure the description has a Differential Revision: https://... in the commit description, so that Phabricator can update these review threads properly.

[rSerge]

In D27421#613938, @dberris wrote:

Ah, okay. What I typically do is make sure the description has a Differential Revision: https://... in the commit description, so that Phabricator can update these review threads properly.

I can see it in the SVN log for my commit. In the bottom there is a line:
Differential Revision: https://reviews.llvm.org/D27421

[rSerge]

By the way, if I scroll the current page up, I can see field "Commits", and there rL288729 is listed. So it just didn't do it the way it does for your commits, with stating "Closed by commit" and then the commit number.

[dberris]

In D27421#613951, @rSerge wrote:

By the way, if I scroll the current page up, I can see field "Commits", and there rL288729 is listed. So it just didn't do it the way it does for your commits, with stating "Closed by commit" and then the commit number.

Cool -- right, I was looking for that "Closed by commit" message, because it just looked like you closed the revision, without an indication that it indeed was committed.

It might be something silly like, if that message 'Differential Revision: https://...' didn't have a newline character in the end then Phabricator didn't quite parse it properly.

Anyway, something to try for another patch. :)

[rengolin]

Yup

lib/xray/xray_interface.cc

 #include "xray_defs.h"
 
 namespace __xray {
 
 #if defined(__x86_64__)
 // FIXME: The actual length is 11 bytes. Why was length 12 passed to mprotect()
 // ?
 static const int16_t cSledLength = 12;
-#elif defined(__arm__)
-static const int16_t cSledLength = 28;
 #elif defined(__aarch64__)
 static const int16_t cSledLength = 32;
+#elif defined(__arm__)
+static const int16_t cSledLength = 28;
 #else
 #error "Unsupported CPU Architecture"
 #endif /* CPU architecture */
 
 // This is the function to call when we encounter the entry or exit sleds.
 std::atomic<void (*)(int32_t, XRayEntryType)> XRayPatchedFunction{nullptr};
 
 // MProtectHelper is an RAII wrapper for calls to mprotect(...) that will undo