all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
@ 2021-01-26  6:37 Ahmed Khanzada
  2021-01-26  6:57 ` Ahmed Khanzada
  2021-01-26  9:36 ` Philipp Stephani
  0 siblings, 2 replies; 23+ messages in thread
From: Ahmed Khanzada @ 2021-01-26  6:37 UTC (permalink / raw)
  To: 46111

I'm not an expert on C, OpenBSD, or SPARC64, but I did notice
emacs-current was not compiling.

During a gmake, bootstrap-emacs would get a SIGBUS and fail. Running
gdb led me to the hash_string function where it seemed to be failing.

Reverting the hash_string function to a previous version seems to have
fixed the issue.

If I had to guess, it may have something to do with SPARC64 alignment.

I don't have an updated AMD64 OpenBSD box running at the moment, but if you
would like me to test there, I could find a way.

I understand that reverting this function might not be the best way
forward, but wrote a patch just in case that reverts hash_string to an
earlier version that compiles on OpenBSD/SPARC64.

I am already signed all my GNU contributor docs.

--[[text/plain; type=patch; name="0001-Reverting-hash_string-function-due-to-problem-on-Ope.patch"
Content-Disposition: attachment; filename="0001-Reverting-hasattach filh_string-function-due-to-problem-on-Ope.patch"][base64]]
RnJvbSBkOTM4NzQ1Y2U1NjA1OGJlMjQwNzhlYzUyZDk3Mzk4Njk3ZGI0Yjc2IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBaG1lZCBLaGFuemFkYSA8bWVAZW56dS5ydT4KRGF0ZTogTW9u
LCAyNSBKYW4gMjAyMSAyMToxNjo0OCAtMDgwMApTdWJqZWN0OiBbUEFUQ0hdIFJldmVydGluZyBo
YXNoX3N0cmluZyBmdW5jdGlvbiBkdWUgdG8gcHJvYmxlbSBvbgogT3BlbkJTRC9TUEFSQzY0Cgot
LS0KIHNyYy9mbnMuYyB8IDMyICsrKysrKystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZp
bGUgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCspLCAyNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQg
YS9zcmMvZm5zLmMgYi9zcmMvZm5zLmMKaW5kZXggN2FiMmU4ZjFhMC4uMDEyN2ZlM2Y0OCAxMDA2
NDQKLS0tIGEvc3JjL2Zucy5jCisrKyBiL3NyYy9mbnMuYwpAQCAtNDU5OSwzNCArNDU5OSwxNiBA
QCAjZGVmaW5lIFNYSEFTSF9NQVhfTEVOICAgNwogRU1BQ1NfVUlOVAogaGFzaF9zdHJpbmcgKGNo
YXIgY29uc3QgKnB0ciwgcHRyZGlmZl90IGxlbikKIHsKLSAgRU1BQ1NfVUlOVCBjb25zdCAqcCAg
ID0gKEVNQUNTX1VJTlQgY29uc3QgKikgcHRyOwotICBFTUFDU19VSU5UIGNvbnN0ICplbmQgPSAo
RU1BQ1NfVUlOVCBjb25zdCAqKSAocHRyICsgbGVuKTsKLSAgRU1BQ1NfVUlOVCBoYXNoID0gbGVu
OwotICAvKiBBdCBtb3N0IDggc3RlcHMuICBXZSBjb3VsZCByZXVzZSBTWEhBU0hfTUFYX0xFTiwg
b2YgY291cnNlLAotICAgKiBidXQgZGl2aWRpbmcgYnkgOCBpcyBjaGVhcGVyLiAgKi8KLSAgcHRy
ZGlmZl90IHN0ZXAgPSAxICsgKChlbmQgLSBwKSA+PiAzKTsKLQotICAvKiBCZXdhcmU6IGBlbmRg
IG1pZ2h0IGJlIHVuYWxpZ25lZCwgc28gYHAgPCBlbmRgIGlzIG5vdCBhbHdheXMgdGhlIHNhbWUK
LSAgICogYXMgYHAgPD0gZW5kIC0gMWAuICAqLwotICB3aGlsZSAocCA8PSBlbmQgLSAxKQorICBj
aGFyIGNvbnN0ICpwID0gcHRyOworICBjaGFyIGNvbnN0ICplbmQgPSBwICsgbGVuOworICB1bnNp
Z25lZCBjaGFyIGM7CisgIEVNQUNTX1VJTlQgaGFzaCA9IDA7CisKKyAgd2hpbGUgKHAgIT0gZW5k
KQogICAgIHsKLSAgICAgIEVNQUNTX1VJTlQgYyA9ICpwOwotICAgICAgcCArPSBzdGVwOworICAg
ICAgYyA9ICpwKys7CiAgICAgICBoYXNoID0gc3hoYXNoX2NvbWJpbmUgKGhhc2gsIGMpOwogICAg
IH0KLSAgaWYgKHAgPCBlbmQpCi0gICAgeyAvKiBBIGZldyBsYXN0IGJ5dGVzIHJlbWFpbiAoc21h
bGxlciB0aGFuIGFuIEVNQUNTX1VJTlQpLiAgKi8KLSAgICAgIC8qIEZJWE1FOiBXZSBjb3VsZCBk
byB0aGlzIHdpdGhvdXQgYSBsb29wLCBidXQgaXQnZCByZXF1aXJlCi0gICAgICAgICBlbmRpYW4t
ZGVwZW5kZW50IGNvZGUgOi0oICAqLwotICAgICAgY2hhciBjb25zdCAqcDEgPSAoY2hhciBjb25z
dCAqKXA7Ci0gICAgICBjaGFyIGNvbnN0ICplbmQxID0gKGNoYXIgY29uc3QgKillbmQ7Ci0gICAg
ICBkbwotICAgICAgICB7Ci0gICAgICAgICAgdW5zaWduZWQgY2hhciBjID0gKnAxKys7Ci0gICAg
ICAgICAgaGFzaCA9IHN4aGFzaF9jb21iaW5lIChoYXNoLCBjKTsKLSAgICAgICAgfQotICAgICAg
d2hpbGUgKHAxIDwgZW5kMSk7Ci0gICAgfQogCiAgIHJldHVybiBoYXNoOwogfQotLSAKMi4yOC4w
Cgo=





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26  6:37 bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking Ahmed Khanzada
@ 2021-01-26  6:57 ` Ahmed Khanzada
  2021-01-26 11:15   ` Eli Zaretskii
  2021-01-26  9:36 ` Philipp Stephani
  1 sibling, 1 reply; 23+ messages in thread
From: Ahmed Khanzada @ 2021-01-26  6:57 UTC (permalink / raw)
  To: 46111, me

[-- Attachment #1: Type: text/plain, Size: 3452 bytes --]

Not sure if the patch attached correctly. Trying again.

On Mon, Jan 25, 2021 at 10:37 PM Ahmed Khanzada <me@enzu.ru> wrote:
>
> I'm not an expert on C, OpenBSD, or SPARC64, but I did notice
> emacs-current was not compiling.
>
> During a gmake, bootstrap-emacs would get a SIGBUS and fail. Running
> gdb led me to the hash_string function where it seemed to be failing.
>
> Reverting the hash_string function to a previous version seems to have
> fixed the issue.
>
> If I had to guess, it may have something to do with SPARC64 alignment.
>
> I don't have an updated AMD64 OpenBSD box running at the moment, but if you
> would like me to test there, I could find a way.
>
> I understand that reverting this function might not be the best way
> forward, but wrote a patch just in case that reverts hash_string to an
> earlier version that compiles on OpenBSD/SPARC64.
>
> I am already signed all my GNU contributor docs.
>
> --[[text/plain; type=patch; name="0001-Reverting-hash_string-function-due-to-problem-on-Ope.patch"
> Content-Disposition: attachment; filename="0001-Reverting-hasattach filh_string-function-due-to-problem-on-Ope.patch"][base64]]
> RnJvbSBkOTM4NzQ1Y2U1NjA1OGJlMjQwNzhlYzUyZDk3Mzk4Njk3ZGI0Yjc2IE1vbiBTZXAgMTcg
> MDA6MDA6MDAgMjAwMQpGcm9tOiBBaG1lZCBLaGFuemFkYSA8bWVAZW56dS5ydT4KRGF0ZTogTW9u
> LCAyNSBKYW4gMjAyMSAyMToxNjo0OCAtMDgwMApTdWJqZWN0OiBbUEFUQ0hdIFJldmVydGluZyBo
> YXNoX3N0cmluZyBmdW5jdGlvbiBkdWUgdG8gcHJvYmxlbSBvbgogT3BlbkJTRC9TUEFSQzY0Cgot
> LS0KIHNyYy9mbnMuYyB8IDMyICsrKysrKystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZp
> bGUgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCspLCAyNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQg
> YS9zcmMvZm5zLmMgYi9zcmMvZm5zLmMKaW5kZXggN2FiMmU4ZjFhMC4uMDEyN2ZlM2Y0OCAxMDA2
> NDQKLS0tIGEvc3JjL2Zucy5jCisrKyBiL3NyYy9mbnMuYwpAQCAtNDU5OSwzNCArNDU5OSwxNiBA
> QCAjZGVmaW5lIFNYSEFTSF9NQVhfTEVOICAgNwogRU1BQ1NfVUlOVAogaGFzaF9zdHJpbmcgKGNo
> YXIgY29uc3QgKnB0ciwgcHRyZGlmZl90IGxlbikKIHsKLSAgRU1BQ1NfVUlOVCBjb25zdCAqcCAg
> ID0gKEVNQUNTX1VJTlQgY29uc3QgKikgcHRyOwotICBFTUFDU19VSU5UIGNvbnN0ICplbmQgPSAo
> RU1BQ1NfVUlOVCBjb25zdCAqKSAocHRyICsgbGVuKTsKLSAgRU1BQ1NfVUlOVCBoYXNoID0gbGVu
> OwotICAvKiBBdCBtb3N0IDggc3RlcHMuICBXZSBjb3VsZCByZXVzZSBTWEhBU0hfTUFYX0xFTiwg
> b2YgY291cnNlLAotICAgKiBidXQgZGl2aWRpbmcgYnkgOCBpcyBjaGVhcGVyLiAgKi8KLSAgcHRy
> ZGlmZl90IHN0ZXAgPSAxICsgKChlbmQgLSBwKSA+PiAzKTsKLQotICAvKiBCZXdhcmU6IGBlbmRg
> IG1pZ2h0IGJlIHVuYWxpZ25lZCwgc28gYHAgPCBlbmRgIGlzIG5vdCBhbHdheXMgdGhlIHNhbWUK
> LSAgICogYXMgYHAgPD0gZW5kIC0gMWAuICAqLwotICB3aGlsZSAocCA8PSBlbmQgLSAxKQorICBj
> aGFyIGNvbnN0ICpwID0gcHRyOworICBjaGFyIGNvbnN0ICplbmQgPSBwICsgbGVuOworICB1bnNp
> Z25lZCBjaGFyIGM7CisgIEVNQUNTX1VJTlQgaGFzaCA9IDA7CisKKyAgd2hpbGUgKHAgIT0gZW5k
> KQogICAgIHsKLSAgICAgIEVNQUNTX1VJTlQgYyA9ICpwOwotICAgICAgcCArPSBzdGVwOworICAg
> ICAgYyA9ICpwKys7CiAgICAgICBoYXNoID0gc3hoYXNoX2NvbWJpbmUgKGhhc2gsIGMpOwogICAg
> IH0KLSAgaWYgKHAgPCBlbmQpCi0gICAgeyAvKiBBIGZldyBsYXN0IGJ5dGVzIHJlbWFpbiAoc21h
> bGxlciB0aGFuIGFuIEVNQUNTX1VJTlQpLiAgKi8KLSAgICAgIC8qIEZJWE1FOiBXZSBjb3VsZCBk
> byB0aGlzIHdpdGhvdXQgYSBsb29wLCBidXQgaXQnZCByZXF1aXJlCi0gICAgICAgICBlbmRpYW4t
> ZGVwZW5kZW50IGNvZGUgOi0oICAqLwotICAgICAgY2hhciBjb25zdCAqcDEgPSAoY2hhciBjb25z
> dCAqKXA7Ci0gICAgICBjaGFyIGNvbnN0ICplbmQxID0gKGNoYXIgY29uc3QgKillbmQ7Ci0gICAg
> ICBkbwotICAgICAgICB7Ci0gICAgICAgICAgdW5zaWduZWQgY2hhciBjID0gKnAxKys7Ci0gICAg
> ICAgICAgaGFzaCA9IHN4aGFzaF9jb21iaW5lIChoYXNoLCBjKTsKLSAgICAgICAgfQotICAgICAg
> d2hpbGUgKHAxIDwgZW5kMSk7Ci0gICAgfQogCiAgIHJldHVybiBoYXNoOwogfQotLSAKMi4yOC4w
> Cgo=

[-- Attachment #2: 0001-Reverting-hash_string-function-due-to-problem-on-Ope.patch --]
[-- Type: application/octet-stream, Size: 1655 bytes --]

From d938745ce56058be24078ec52d97398697db4b76 Mon Sep 17 00:00:00 2001
From: Ahmed Khanzada <me@enzu.ru>
Date: Mon, 25 Jan 2021 21:16:48 -0800
Subject: [PATCH] Reverting hash_string function due to problem on
 OpenBSD/SPARC64

---
 src/fns.c | 32 +++++++-------------------------
 1 file changed, 7 insertions(+), 25 deletions(-)

diff --git a/src/fns.c b/src/fns.c
index 7ab2e8f1a0..0127fe3f48 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4599,34 +4599,16 @@ #define SXHASH_MAX_LEN   7
 EMACS_UINT
 hash_string (char const *ptr, ptrdiff_t len)
 {
-  EMACS_UINT const *p   = (EMACS_UINT const *) ptr;
-  EMACS_UINT const *end = (EMACS_UINT const *) (ptr + len);
-  EMACS_UINT hash = len;
-  /* At most 8 steps.  We could reuse SXHASH_MAX_LEN, of course,
-   * but dividing by 8 is cheaper.  */
-  ptrdiff_t step = 1 + ((end - p) >> 3);
-
-  /* Beware: `end` might be unaligned, so `p < end` is not always the same
-   * as `p <= end - 1`.  */
-  while (p <= end - 1)
+  char const *p = ptr;
+  char const *end = p + len;
+  unsigned char c;
+  EMACS_UINT hash = 0;
+
+  while (p != end)
     {
-      EMACS_UINT c = *p;
-      p += step;
+      c = *p++;
       hash = sxhash_combine (hash, c);
     }
-  if (p < end)
-    { /* A few last bytes remain (smaller than an EMACS_UINT).  */
-      /* FIXME: We could do this without a loop, but it'd require
-         endian-dependent code :-(  */
-      char const *p1 = (char const *)p;
-      char const *end1 = (char const *)end;
-      do
-        {
-          unsigned char c = *p1++;
-          hash = sxhash_combine (hash, c);
-        }
-      while (p1 < end1);
-    }
 
   return hash;
 }
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26  6:37 bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking Ahmed Khanzada
  2021-01-26  6:57 ` Ahmed Khanzada
@ 2021-01-26  9:36 ` Philipp Stephani
  2021-01-26 11:12   ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Philipp Stephani @ 2021-01-26  9:36 UTC (permalink / raw)
  To: Ahmed Khanzada; +Cc: 46111

Am Di., 26. Jan. 2021 um 10:07 Uhr schrieb Ahmed Khanzada <me@enzu.ru>:
>
> I'm not an expert on C, OpenBSD, or SPARC64, but I did notice
> emacs-current was not compiling.
>
> During a gmake, bootstrap-emacs would get a SIGBUS and fail. Running
> gdb led me to the hash_string function where it seemed to be failing.


The culprit might be commit be0f2de179235980b5409d5e77577207b93a4f12.
I don't think that something like
EMACS_UINT const *p   = (EMACS_UINT const *) ptr;
is legal on any platform; it just happens to work on forgiving
platforms such as x86-64.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26  9:36 ` Philipp Stephani
@ 2021-01-26 11:12   ` Eli Zaretskii
  2021-01-26 11:17     ` Andreas Schwab
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-26 11:12 UTC (permalink / raw)
  To: 46111, p.stephani2, me

On January 26, 2021 11:36:25 AM GMT+02:00, Philipp Stephani <p.stephani2@gmail.com> wrote:
> Am Di., 26. Jan. 2021 um 10:07 Uhr schrieb Ahmed Khanzada
> <me@enzu.ru>:
> >
> > I'm not an expert on C, OpenBSD, or SPARC64, but I did notice
> > emacs-current was not compiling.
> >
> > During a gmake, bootstrap-emacs would get a SIGBUS and fail. Running
> > gdb led me to the hash_string function where it seemed to be
> failing.
> 
> 
> The culprit might be commit be0f2de179235980b5409d5e77577207b93a4f12.
> I don't think that something like
> EMACS_UINT const *p   = (EMACS_UINT const *) ptr;
> is legal on any platform; it just happens to work on forgiving
> platforms such as x86-64.

It's definitely legal, as it doesn't violate any laws...





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26  6:57 ` Ahmed Khanzada
@ 2021-01-26 11:15   ` Eli Zaretskii
  2021-01-28  4:06     ` Ahmed Khanzada
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-26 11:15 UTC (permalink / raw)
  To: 46111, me, me

On January 26, 2021 8:57:18 AM GMT+02:00, Ahmed Khanzada <me@enzu.ru> wrote:
> Not sure if the patch attached correctly. Trying again.


Thanks.  However, could you please show the C-level backtrace from the SIGBUS crash, as displayed by GDB?  I think we'd like to know which string has its data unaligned to cause this.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:12   ` Eli Zaretskii
@ 2021-01-26 11:17     ` Andreas Schwab
  2021-01-26 11:43       ` Eli Zaretskii
  2021-01-27  7:42       ` Richard Stallman
  0 siblings, 2 replies; 23+ messages in thread
From: Andreas Schwab @ 2021-01-26 11:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46111, me, p.stephani2

On Jan 26 2021, Eli Zaretskii wrote:

> It's definitely legal, as it doesn't violate any laws...

It violates the laws of C.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:17     ` Andreas Schwab
@ 2021-01-26 11:43       ` Eli Zaretskii
  2021-01-26 11:49         ` Andreas Schwab
  2021-01-27  7:42       ` Richard Stallman
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-26 11:43 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 46111, me, p.stephani2

On January 26, 2021 1:17:06 PM GMT+02:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Jan 26 2021, Eli Zaretskii wrote:
> 
> > It's definitely legal, as it doesn't violate any laws...
> 
> It violates the laws of C.

We call that "invalid"  not "illegal".





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:43       ` Eli Zaretskii
@ 2021-01-26 11:49         ` Andreas Schwab
  2021-01-26 15:36           ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Andreas Schwab @ 2021-01-26 11:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46111, me, p.stephani2

On Jan 26 2021, Eli Zaretskii wrote:

> On January 26, 2021 1:17:06 PM GMT+02:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> On Jan 26 2021, Eli Zaretskii wrote:
>> 
>> > It's definitely legal, as it doesn't violate any laws...
>> 
>> It violates the laws of C.
>
> We call that "invalid"  not "illegal".

It's still a bug.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:49         ` Andreas Schwab
@ 2021-01-26 15:36           ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-26 15:36 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 46111, me, p.stephani2

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 46111@debbugs.gnu.org, p.stephani2@gmail.com, me@enzu.ru
> Date: Tue, 26 Jan 2021 12:49:44 +0100
> 
> On Jan 26 2021, Eli Zaretskii wrote:
> 
> > On January 26, 2021 1:17:06 PM GMT+02:00, Andreas Schwab <schwab@linux-m68k.org> wrote:
> >> On Jan 26 2021, Eli Zaretskii wrote:
> >> 
> >> > It's definitely legal, as it doesn't violate any laws...
> >> 
> >> It violates the laws of C.
> >
> > We call that "invalid"  not "illegal".
> 
> It's still a bug.

That goes without saying: a crash is always a bug.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:17     ` Andreas Schwab
  2021-01-26 11:43       ` Eli Zaretskii
@ 2021-01-27  7:42       ` Richard Stallman
  2021-01-27  8:24         ` Andreas Schwab
  1 sibling, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2021-01-27  7:42 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 46111, p.stephani2, me

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > It violates the laws of C.

C is not a government, and neither is the ISO C Committee.
Breaking the rules of C is not illegal.

Indeed, GNU C goes against the standard in some simple ways
unless you specify options such as --pedantic.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-27  7:42       ` Richard Stallman
@ 2021-01-27  8:24         ` Andreas Schwab
  0 siblings, 0 replies; 23+ messages in thread
From: Andreas Schwab @ 2021-01-27  8:24 UTC (permalink / raw)
  To: Richard Stallman; +Cc: 46111, p.stephani2, me

On Jan 27 2021, Richard Stallman wrote:

> Breaking the rules of C is not illegal.

If it breaks, you get to keep both pieces.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-26 11:15   ` Eli Zaretskii
@ 2021-01-28  4:06     ` Ahmed Khanzada
  2021-01-28 13:58       ` Eli Zaretskii
  2021-01-28 15:09       ` Stefan Monnier
  0 siblings, 2 replies; 23+ messages in thread
From: Ahmed Khanzada @ 2021-01-28  4:06 UTC (permalink / raw)
  To: eliz, 46111

Eli Zaretskii <eliz@gnu.org> writes:

> On January 26, 2021 8:57:18 AM GMT+02:00, Ahmed Khanzada <me@enzu.ru> wrote:
>> Not sure if the patch attached correctly. Trying again.
>
>
> Thanks.  However, could you please show the C-level backtrace from the SIGBUS crash, as displayed by GDB?  I think we'd like to know which string has its data unaligned to cause this.

Is the log below the information that you are looking for?

Starting program: /home/enzuru/src/emacs/src/bootstrap-emacs

Breakpoint 1, hash_string (ptr=0x47fa34d596 "DndProtocol", len=11) at
fns.c:4602
4602      EMACS_UINT const *p   = (EMACS_UINT const *) ptr;
(gdb) info args
ptr = 0x47fa34d596 "DndProtocol"
len = 11
(gdb) next
4603      EMACS_UINT const *end = (EMACS_UINT const *) (ptr + len);
(gdb) next
4604      EMACS_UINT hash = len;
(gdb) next
4607      ptrdiff_t step = 1 + ((end - p) >> 3);
(gdb) next
4611      while (p <= end - 1)
(gdb) next
4613          EMACS_UINT c = *p;
(gdb) next

Program received signal SIGBUS, Bus error.
0x000000455fe1dc6c in hash_string (ptr=0x47fa34d596 "DndProtocol",
len=11) at fns.c:4613
4613          EMACS_UINT c = *p;
(gdb) backtrace
#0  0x000000455fe1dc6c in hash_string (ptr=0x47fa34d596 "DndProtocol",
len=11) at fns.c:4613
#1  0x000000455fe1dd48 in sxhash_string (ptr=0x47fa34d596 "DndProtocol",
len=11) at fns.c:4640
#2  0x000000455fe1e36c in sxhash_obj (obj=0x47fa02f0bc, depth=0) at
fns.c:4759
#3  0x000000455fe1e270 in sxhash (obj=0x47fa02f0bc) at fns.c:4741
#4  0x000000455fe1c52c in hashfn_equal (key=0x47fa02f0bc,
h=0x47fa02eff0) at fns.c:4096
#5  0x000000455fe1cf44 in hash_table_rehash (hash=0x47fa02eff5) at
fns.c:4342
#6  0x000000455fdc5264 in hash_table_thaw (hash=0x47fa02eff5) at
pdumper.c:2652
#7  0x000000455fdcd184 in thaw_hash_tables () at pdumper.c:5477
#8  0x000000455fdcccf0 in pdumper_load (dump_filename=0x4832495d00
"/home/enzuru/src/emacs/src/bootstrap-emacs.pdmp") at pdumper.c:5405
#9  0x000000455fcea4ac in load_pdump (argc=1, argv=0xffffffffffff2968)
at emacs.c:859
#10 0x000000455fceabec in main (argc=1, argv=0xffffffffffff2968) at
emacs.c:1067






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28  4:06     ` Ahmed Khanzada
@ 2021-01-28 13:58       ` Eli Zaretskii
  2021-01-28 15:09       ` Stefan Monnier
  1 sibling, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-28 13:58 UTC (permalink / raw)
  To: Ahmed Khanzada, Stefan Monnier; +Cc: 46111

> From: Ahmed Khanzada <me@enzu.ru>
> Cc: 
> Date: Wed, 27 Jan 2021 20:06:01 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks.  However, could you please show the C-level backtrace from the SIGBUS crash, as displayed by GDB?  I think we'd like to know which string has its data unaligned to cause this.
> 
> Is the log below the information that you are looking for?

Yes, thanks.

> 4613          EMACS_UINT c = *p;
> (gdb) next
> 
> Program received signal SIGBUS, Bus error.
> 0x000000455fe1dc6c in hash_string (ptr=0x47fa34d596 "DndProtocol",
> len=11) at fns.c:4613
> 4613          EMACS_UINT c = *p;
> (gdb) backtrace
> #0  0x000000455fe1dc6c in hash_string (ptr=0x47fa34d596 "DndProtocol",
> len=11) at fns.c:4613
> #1  0x000000455fe1dd48 in sxhash_string (ptr=0x47fa34d596 "DndProtocol",
> len=11) at fns.c:4640
> #2  0x000000455fe1e36c in sxhash_obj (obj=0x47fa02f0bc, depth=0) at
> fns.c:4759
> #3  0x000000455fe1e270 in sxhash (obj=0x47fa02f0bc) at fns.c:4741
> #4  0x000000455fe1c52c in hashfn_equal (key=0x47fa02f0bc,
> h=0x47fa02eff0) at fns.c:4096
> #5  0x000000455fe1cf44 in hash_table_rehash (hash=0x47fa02eff5) at
> fns.c:4342
> #6  0x000000455fdc5264 in hash_table_thaw (hash=0x47fa02eff5) at
> pdumper.c:2652
> #7  0x000000455fdcd184 in thaw_hash_tables () at pdumper.c:5477

Stefan, any ideas?

One possibility is to use memcpy instead of dereferencing a pointer.
We could do that either unconditionally or when the original pointer
is not aligned on 4/8-byte boundary.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28  4:06     ` Ahmed Khanzada
  2021-01-28 13:58       ` Eli Zaretskii
@ 2021-01-28 15:09       ` Stefan Monnier
  2021-01-28 15:19         ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 15:09 UTC (permalink / raw)
  To: Ahmed Khanzada; +Cc: 46111

> Starting program: /home/enzuru/src/emacs/src/bootstrap-emacs
>
> Breakpoint 1, hash_string (ptr=0x47fa34d596 "DndProtocol", len=11) at
> fns.c:4602
> 4602      EMACS_UINT const *p   = (EMACS_UINT const *) ptr;
> (gdb) info args
> ptr = 0x47fa34d596 "DndProtocol"
> len = 11
> (gdb) next
> 4603      EMACS_UINT const *end = (EMACS_UINT const *) (ptr + len);
> (gdb) next
> 4604      EMACS_UINT hash = len;
> (gdb) next
> 4607      ptrdiff_t step = 1 + ((end - p) >> 3);
> (gdb) next
> 4611      while (p <= end - 1)
> (gdb) next
> 4613          EMACS_UINT c = *p;
> (gdb) next
>
> Program received signal SIGBUS, Bus error.

Hmm... so it's doing a dereference at address 0x47fa34d596 and getting
a bus error?

I have two questions here:

- I'd guess that the bus error is due to alignment restrictions.
  What hardware is this running on?  Last I checked, the computer
  architecture community had agreed (many years ago already) that
  (except for very small CPUs maybe, those not able to run Emacs) it's
  better to have the hardware support unaligned memory accesses (it took
  more time to get there than the consensus on branch delay slots,
  admittedly), so I'd be curious if there is still moderately recent
  hardware that insists on signaling an error.

- AFAICT from the backtrace, `ptr` points to a plain normal ELisp
  string's content, yet these are supposed to be aligned, so what's
  going on here (this question is not directed at you ;-)


        Stefan






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 15:09       ` Stefan Monnier
@ 2021-01-28 15:19         ` Eli Zaretskii
  2021-01-28 15:44           ` Stefan Monnier
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-28 15:19 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 46111, me

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: eliz@gnu.org,  46111@debbugs.gnu.org
> Date: Thu, 28 Jan 2021 10:09:21 -0500
> 
> Hmm... so it's doing a dereference at address 0x47fa34d596 and getting
> a bus error?

Yes.

> I have two questions here:
> 
> - I'd guess that the bus error is due to alignment restrictions.
>   What hardware is this running on?

See the Subject: it's SPARC64.

> - AFAICT from the backtrace, `ptr` points to a plain normal ELisp
>   string's content, yet these are supposed to be aligned, so what's
>   going on here

I wondered that myself.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 15:19         ` Eli Zaretskii
@ 2021-01-28 15:44           ` Stefan Monnier
  2021-01-28 15:52             ` Stefan Monnier
  2021-01-28 16:04             ` Andreas Schwab
  0 siblings, 2 replies; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 15:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46111, me

>> - I'd guess that the bus error is due to alignment restrictions.
>>   What hardware is this running on?
> See the Subject: it's SPARC64.

I mean the actual hardware, not the architecture.

[ I know most RISC processors started with a restriction that they only
  allowed aligned memory accesses, but AFAIK they've changed stance
  since (IIUC the extra hardware can be very little, sometimes even less
  than the hardware that would be needed to implement the ad-hoc
  "support instructions" used to do the unaligned access as a sequence
  of instructions).
  It's one of the RISC simplifications that just didn't pan out.  ]

>> - AFAICT from the backtrace, `ptr` points to a plain normal ELisp
>>   string's content, yet these are supposed to be aligned, so what's
>>   going on here
> I wondered that myself.

And what did you conclude?  ;-)


        Stef






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 15:44           ` Stefan Monnier
@ 2021-01-28 15:52             ` Stefan Monnier
  2021-01-28 16:17               ` Stefan Monnier
  2021-01-28 16:04             ` Andreas Schwab
  1 sibling, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 15:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46111, me

>>> - AFAICT from the backtrace, `ptr` points to a plain normal ELisp
>>>   string's content, yet these are supposed to be aligned, so what's
>>>   going on here
>> I wondered that myself.
> And what did you conclude?  ;-)

Hmm... now that I think about it, I think `make_pure_string` may get us
there, because the string's content is allocated from the end without
any alignment!

And the backtrace shows we're hashing the string `DndProtocol` which
comes from `lisp/x-dnd.el` which is indeed preloaded, so I think that's
what's going on.


        Stefan






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 15:44           ` Stefan Monnier
  2021-01-28 15:52             ` Stefan Monnier
@ 2021-01-28 16:04             ` Andreas Schwab
  2021-01-28 16:14               ` Stefan Monnier
  1 sibling, 1 reply; 23+ messages in thread
From: Andreas Schwab @ 2021-01-28 16:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: me, 46111

On Jan 28 2021, Stefan Monnier wrote:

>>> - I'd guess that the bus error is due to alignment restrictions.
>>>   What hardware is this running on?
>> See the Subject: it's SPARC64.
>
> I mean the actual hardware, not the architecture.

Strict alignment is a property of the architecture, not the hardware.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 16:04             ` Andreas Schwab
@ 2021-01-28 16:14               ` Stefan Monnier
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 16:14 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: me, 46111

Andreas Schwab [2021-01-28 17:04:19] wrote:

> On Jan 28 2021, Stefan Monnier wrote:
>
>>>> - I'd guess that the bus error is due to alignment restrictions.
>>>>   What hardware is this running on?
>>> See the Subject: it's SPARC64.
>>
>> I mean the actual hardware, not the architecture.
>
> Strict alignment is a property of the architecture, not the hardware.

IIRC some RISC architectures *allowed* errors on unaligned memory
accesses without requiring it, making it a property of the hardware.


        Stefan






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 15:52             ` Stefan Monnier
@ 2021-01-28 16:17               ` Stefan Monnier
  2021-01-28 16:22                 ` Andreas Schwab
  2021-01-28 17:22                 ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 16:17 UTC (permalink / raw)
  To: Ahmed Khanzada; +Cc: 46111

> And the backtrace shows we're hashing the string `DndProtocol` which
> comes from `lisp/x-dnd.el` which is indeed preloaded, so I think that's
> what's going on.

Could you try the patch below?


        Stefan


diff --git a/src/fns.c b/src/fns.c
index 7ab2e8f1a0..0c6bb770ef 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4610,7 +4610,20 @@ hash_string (char const *ptr, ptrdiff_t len)
    * as `p <= end - 1`.  */
   while (p <= end - 1)
     {
+      /* Here `p` is *almost* always be properly aligned, so we want to
+         optimize for the aligned case, but we still need to support the
+         non-aligned case.  */
+      /* FIXME: Could we just always use `memcpy` and rely on GCC optimizing
+         it to a single word-sized memory access on all-but-sparc64?  */
+#ifdef __sparc64__  /* Arch that still insists on aligned memory accesses. */
+      EMACS_UINT c;
+      if (!((unsigned long)p) % sizeof (c))
+        c = *p;
+      else
+        memcpy (&c, (char const *)p, sizeof (c)); /* `p` is unaligned!  */
+#else
       EMACS_UINT c = *p;
+#fi
       p += step;
       hash = sxhash_combine (hash, c);
     }






^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 16:17               ` Stefan Monnier
@ 2021-01-28 16:22                 ` Andreas Schwab
  2021-01-28 17:22                 ` Eli Zaretskii
  1 sibling, 0 replies; 23+ messages in thread
From: Andreas Schwab @ 2021-01-28 16:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 46111, Ahmed Khanzada

On Jan 28 2021, Stefan Monnier wrote:

> +#ifdef __sparc64__  /* Arch that still insists on aligned memory accesses. */

$ git grep -E '#define +STRICT_ALIGNMENT +1' -- gcc/config | wc -l
35

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 16:17               ` Stefan Monnier
  2021-01-28 16:22                 ` Andreas Schwab
@ 2021-01-28 17:22                 ` Eli Zaretskii
  2021-01-28 17:30                   ` Stefan Monnier
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2021-01-28 17:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 46111, me

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 28 Jan 2021 11:17:31 -0500
> Cc: 46111@debbugs.gnu.org
> 
> +      /* Here `p` is *almost* always be properly aligned, so we want to
> +         optimize for the aligned case, but we still need to support the
> +         non-aligned case.  */
> +      /* FIXME: Could we just always use `memcpy` and rely on GCC optimizing
> +         it to a single word-sized memory access on all-but-sparc64?  */
> +#ifdef __sparc64__  /* Arch that still insists on aligned memory accesses. */
> +      EMACS_UINT c;
> +      if (!((unsigned long)p) % sizeof (c))
> +        c = *p;
> +      else
> +        memcpy (&c, (char const *)p, sizeof (c)); /* `p` is unaligned!  */
> +#else
>        EMACS_UINT c = *p;
> +#fi

AFAIK, memcpy itself optimizes: once it gets to aligned address, it
starts copying words instead of bytes.  So I'm not sure we need either
#ifdef or the run-time condition.  I suggest to time both variants on
x86 architecture, to see whether there's any performance hit due to
use of memcpy, and if not, use memcpy always -- it will let us have
the cake and eat it, too.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking
  2021-01-28 17:22                 ` Eli Zaretskii
@ 2021-01-28 17:30                   ` Stefan Monnier
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2021-01-28 17:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46111-done, me

> AFAIK, memcpy itself optimizes: once it gets to aligned address, it
> starts copying words instead of bytes.

I don't know what this `memcpy` code expands to on sparc64, but since it
turns into a plain `mov` on x86, I simplified the code to always use
`memcpy`.


        Stefan






^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-01-28 17:30 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-26  6:37 bug#46111: Reverting fns.c hash function due to OpenBSD/SPARC64 compile breaking Ahmed Khanzada
2021-01-26  6:57 ` Ahmed Khanzada
2021-01-26 11:15   ` Eli Zaretskii
2021-01-28  4:06     ` Ahmed Khanzada
2021-01-28 13:58       ` Eli Zaretskii
2021-01-28 15:09       ` Stefan Monnier
2021-01-28 15:19         ` Eli Zaretskii
2021-01-28 15:44           ` Stefan Monnier
2021-01-28 15:52             ` Stefan Monnier
2021-01-28 16:17               ` Stefan Monnier
2021-01-28 16:22                 ` Andreas Schwab
2021-01-28 17:22                 ` Eli Zaretskii
2021-01-28 17:30                   ` Stefan Monnier
2021-01-28 16:04             ` Andreas Schwab
2021-01-28 16:14               ` Stefan Monnier
2021-01-26  9:36 ` Philipp Stephani
2021-01-26 11:12   ` Eli Zaretskii
2021-01-26 11:17     ` Andreas Schwab
2021-01-26 11:43       ` Eli Zaretskii
2021-01-26 11:49         ` Andreas Schwab
2021-01-26 15:36           ` Eli Zaretskii
2021-01-27  7:42       ` Richard Stallman
2021-01-27  8:24         ` Andreas Schwab

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.