Bug 248160 - databases/mariadb105-server and databases/mariadb104-server: Fix SIGILL from __builtin_readcyclecounter on ARM platforms
Summary: databases/mariadb105-server and databases/mariadb104-server: Fix SIGILL from ...
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: arm64 Any
: --- Affects Some People
Assignee: Bernard Spil
Keywords: crash, needs-qa
Depends on:
Reported: 2020-07-22 01:38 UTC by Naram Qashat
Modified: 2020-10-24 11:35 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (brnrd)
koobs: merge-quarterly?

patch-MDEV-23249 (741 bytes, patch)
2020-07-22 03:41 UTC, Naram Qashat
no flags Details | Diff
patch-MDEV-23249 (1.54 KB, patch)
2020-07-22 16:12 UTC, Naram Qashat
cyberbotx: maintainer-approval? (brnrd)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Naram Qashat 2020-07-22 01:38:42 UTC
Currently, MariaDB's 10.4 and 10.5 branches both suffer from this problem. Since 10.4.7, a change to use __builtin_readcyclecounter causes a SIGILL on ARM platforms. The potential for this problem is noted in the commit to MariaDB that added this.

I have filed a bug upstream already: https://jira.mariadb.org/browse/MDEV-23249

But I'd like to get it fixed quicker in FreeBSD. I have not actually made any patches for either port, but the patch is just two lines of adding && !defined(__arm__) as shown here: https://raw.githubusercontent.com/its-pointless/its-pointless.github.io/master/my_rdtsc.h.patch
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2020-07-22 02:49:19 UTC
Thank you Naram. Could you:

 - Clarify if any other mariadb port versions are affected?
 - Include your patch as an attachment
 - Confirm the change passes QA (portlint, poudriere)
Comment 2 Naram Qashat 2020-07-22 03:41:33 UTC
Created attachment 216648 [details]

Only the ports listed in the subject are affected. As mentioned in the initial comment, the problem was introduced in 10.4.7, and 10.5 was created sometime after that. The MariaDB JIRA report lists every affected version.

The attached patch can be cleanly applied to both ports as the code for include/my_rdtsc.h is identical in both.

I have only tested compiling maria105-server in poudriere for 12-aarch64. As I have made no changes to the Makefile I did not check portlint nor do I feel I need to. The port still compiles even with the given patch.
Comment 3 Naram Qashat 2020-07-22 16:12:50 UTC
Created attachment 216668 [details]

I'm replacing the patch after some information from some MariaDB devs in the JIRA report.

The new patch combines what I did in the previous patch with the pull request here: https://github.com/MariaDB/server/pull/1620

I did change the conditional in the previous patch from __arm__ to __aarch64__, as the former was not defined on the RPi3 which I'm running aarch64 on. I do not have access to a 32-bit ARM system to know if __build_readcyclecounter causes a SIGILL on that architecture or not.
Comment 4 Naram Qashat 2020-07-23 13:38:55 UTC
One other note, this patch will only be needed temporarily, until MariaDB 10.4.14 and 10.5.5 land, as this will be fixed upstream in those versions.
Comment 5 Vincent Milum Jr 2020-08-11 15:27:24 UTC
MariaDB 10.4.14 and 10.5.5 both came out yesterday, and appear to include this fix upstream.

Considering this is a 100% blocker from running either version of MariaDB on some ARM platforms at all, it would be nice to see these new versions land in ports soon.
Comment 6 Naram Qashat 2020-08-11 15:47:21 UTC
I'd say that once the ports have been updated to those versions, this patch will be entirely unnecessary then. Perhaps it should be kept open until the ports are updated, however.
Comment 7 Vincent Milum Jr 2020-08-12 15:26:28 UTC
I just finished building mariadb105-server 10.5.5 locally, and can confirm that it is at least up and running on my Pi3b+ and I can do basic read/writing (have not tried anything more complex yet) - but this is certainly a lot better than crashing on load! :)
Comment 8 VVD 2020-08-15 22:29:42 UTC
(In reply to Vincent Milum Jr from comment #7)
databases/mariadb105-{server,client}: update to 10.5.5 + optimization:
Comment 9 Bernard Spil freebsd_committer 2020-10-24 11:35:23 UTC
This is merged in 10.5.5. Though this hunk seems to be missing?

@@ -26,7 +26,7 @@
 # ifndef __has_builtin
 #  define __has_builtin(x) 0 /* Compatibility with non-clang compilers */
 # endif
-# if __has_builtin(__builtin_readcyclecounter)
+# if __has_builtin(__builtin_readcyclecounter) && !defined(__aarch64__)
 # elif defined _WIN32
 #  include <intrin.h>
 # elif defined __i386__ || defined __x86_64__

closing for now, let me know if further work needs to be done.