[PATCH] gnu: hugs: Ignore integer overflow.

  • Open
  • quality assurance status badge
Details
3 participants
  • Homo
  • Lars-Dominik Braun
  • Lennart Augustsson
Owner
unassigned
Submitted by
Homo
Severity
normal

Debbugs page

(address . guix-patches@gnu.org)(name . Homo)(address . gay@disroot.org)
20250122054216.30430-2-gay@disroot.org
This is required to bootstrap MicroHs.

* gnu/packages/patches/hugs-ignore-integer-overflow.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/hugs.scm (hugs)[source]: Use it.

Change-Id: Ie13085d356f98d4dbb0fd1108b7179ad5226720f
---

It's a strong requirement from Lennart to not remove usage of pattern guards in MicroHs's code, so need help from someone experienced with YACC to add pattern guards support to Hugs, a nice to have, but not required, cherry on top of that: adding bang patterns and "instance forall" support in Hugs.

gnu/local.mk | 1 +
gnu/packages/hugs.scm | 2 +-
.../hugs-ignore-integer-overflow.patch | 55 +++++++++++++++++++
3 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/hugs-ignore-integer-overflow.patch

Toggle diff (88 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index f118fe4442..ecea6ae9c7 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1569,6 +1569,7 @@ dist_patch_DATA = \
%D%/packages/patches/hubbub-sort-entities.patch \
%D%/packages/patches/hueplusplus-mbedtls.patch \
%D%/packages/patches/hugs-fix-build.patch \
+ %D%/packages/patches/hugs-ignore-integer-overflow.patch \
%D%/packages/patches/hurd-64bit.patch \
%D%/packages/patches/hurd-refcounts-assert.patch \
%D%/packages/patches/hurd-rumpdisk-no-hd.patch \
diff --git a/gnu/packages/hugs.scm b/gnu/packages/hugs.scm
index b6a97ea78c..ae7acbd511 100644
--- a/gnu/packages/hugs.scm
+++ b/gnu/packages/hugs.scm
@@ -37,7 +37,7 @@ (define-public hugs
(sha256
(base32
"1mdy4aq4campgmnpc2qwq7bsbfhaxfsqdghbyyz2wms4lnfcmyma"))
- (patches (search-patches "hugs-fix-build.patch"))))
+ (patches (search-patches "hugs-fix-build.patch" "hugs-ignore-integer-overflow.patch"))))
(build-system gnu-build-system)
(arguments
`(#:phases
diff --git a/gnu/packages/patches/hugs-ignore-integer-overflow.patch b/gnu/packages/patches/hugs-ignore-integer-overflow.patch
new file mode 100644
index 0000000000..ef0b7de824
--- /dev/null
+++ b/gnu/packages/patches/hugs-ignore-integer-overflow.patch
@@ -0,0 +1,55 @@
+From a87d3a15194e4d7724627e43a94ac1d12ab78f9c Mon Sep 17 00:00:00 2001
+From: Lennart Augustsson <lennart@augustsson.net>
+Date: Tue, 21 Jan 2025 12:26:15 -0700
+Subject: [PATCH] Ignore overflow in conversion from Integer to Int.
+
+---
+ src/bignums.c | 24 ++++++++++++++++++++----
+ 1 file changed, 20 insertions(+), 4 deletions(-)
+
+diff --git a/src/bignums.c b/src/bignums.c
+index 32ce528..38ec43f 100644
+--- a/src/bignums.c
++++ b/src/bignums.c
+@@ -196,10 +196,18 @@ Bignum n; {
+ while (nonNull(ds=tl(ds))) {
+ Int d = digitOf(hd(ds));
+ if (b > (Int)(MAXHUGSWORD/BIGBASE))
+- return NIL;
++#if 0
++ return NIL;
++#else
++ fprintf(stderr, "Warning: bigToInt overflow ignored\n");
++#endif
+ b *= BIGBASE;
+ if (d > (Int)((MAXHUGSWORD - m)/b))
+- return NIL;
++#if 0
++ return NIL;
++#else
++ fprintf(stderr, "Warning: bigToInt overflow ignored\n");
++#endif
+ m += b*d;
+ }
+ } else { /* fst(n)==NEGNUM */
+@@ -207,10 +215,18 @@ Bignum n; {
+ while (nonNull(ds=tl(ds))) {
+ Int d = - digitOf(hd(ds));
+ if (b > (MAXPOSINT/BIGBASE))
+- return NIL;
++#if 0
++ return NIL;
++#else
++ fprintf(stderr, "Warning: bigToInt overflow ignored\n");
++#endif
+ b *= BIGBASE;
+ if (d < (MINNEGINT - m)/b)
+- return NIL;
++#if 0
++ return NIL;
++#else
++ fprintf(stderr, "Warning: bigToInt overflow ignored\n");
++#endif
+ m += b*d;
+ }
+ }
--
2.47.1
Lars-Dominik Braun wrote on 24 Jan 15:00 +0100
(name . Homo)(address . gay@disroot.org)(address . 75745@debbugs.gnu.org)
Z5Oc_0VCpwMKfNRd@noor.fritz.box
Hi,

Toggle quote (2 lines)
> This is required to bootstrap MicroHs.

at first glance and without much background knowledge this looks like
a really bad idea(tm). Do you know why this patch is required (i.e. why
MicroHs relies on this specific overflow behavior)?

Lars
0aecaf67eebfe070b9144e18860178c6@disroot.org
Lars-Dominik Braun kirjoitti 2025-01-24 16:00:
Toggle quote (10 lines)
> Hi,
>
>> This is required to bootstrap MicroHs.
>
> at first glance and without much background knowledge this looks like
> a really bad idea(tm). Do you know why this patch is required (i.e. why
> MicroHs relies on this specific overflow behavior)?
>
> Lars

Hi,

No idea, I tried grepping both large numbers and fromInteger usage, as
well as grep -i overflow, however not crashing in conversion from
Integer to Int is consistent behavior and might be assumed by a lot of
packages, like if you try:
ghci> fromInteger (9 ^ 100) :: Int
6627890308811632801

I remember several years ago seeing (unrelated to MicroHs) code that for
performance reasons uses integer overflow with division by some power of
2 (i++ / (1 << N)) for circular array (queue) to pass messages between
threads, so having it overflow or underflow is not always a bug.

This message is sent to 75745@debbugs.gnu.org and so is visible to
public at https://issues.guix.gnu.org/75745or if it doesn't work at
6b0d7e7991e366ee1a4d861c9775ebef@disroot.org
Toggle quote (6 lines)
> I remember several years ago seeing (unrelated to MicroHs) code that
> for performance reasons uses integer overflow with division by some
> power of 2 (i++ / (1 << N)) for circular array (queue) to pass messages
> between threads, so having it overflow or underflow is not always a
> bug.

Correction, it was: i++ & (1 << N)

But you get the idea.
Lars-Dominik Braun wrote on 25 Jan 15:28 +0100
(name . Lennart Augustsson)(address . lennart@augustsson.net)
Z5T0_dkuStz2fGTg@noor.fritz.box
Hi Lennart,

Toggle quote (4 lines)
> If someone fixes all the warnings when compiling Hugs, then I bet this
> problem goes away. But I am not going to do that. In its current state Hugs
> can compile a fully functional MicroHs, and that's enough for me.

actually, I can build the MicroHs package from
https://issues.guix.gnu.org/75778just fine with our default, unpatched
Hugs. It can also compile/run the example from

Can we therefore just drop this patch? It causes Hugs to die with a
SIGFPE while evaluating the example expression `fromInteger (9 ^ 100)
:: Int` from above, which is rather ugly behavior.

And on an unrelated note: Would it be possible to bootstrap (a possibly
very, very old version of) GHC with MicroHs? (As far as I see our GHC
7 is currently bootstrapped via binaries.)

Lars
Lennart Augustsson wrote on 25 Jan 00:14 +0100
(name . Homo)(address . gay@disroot.org)
CAJogeqfAbW=OiHn_FZCpbPpsXFBFWOb_fyocebjamxhRne1BnQ@mail.gmail.com
I actually checked carefully. MicroHs does not rely on any overflow
behaviour. There are no large Integers being used.
I believe this is a Hugs bug caused by Hugs assuming that Int is 32 bits,
but being compiled on a 64 bit platform.

Another example of such a bug is that it looks like the StablePtr
implementation casts 64 bit pointers to and from 32 bit int.

If someone fixes all the warnings when compiling Hugs, then I bet this
problem goes away. But I am not going to do that. In its current state Hugs
can compile a fully functional MicroHs, and that's enough for me.

Lennart

On Fri, Jan 24, 2025, 22:06 <gay@disroot.org> wrote:

Toggle quote (10 lines)
> > I remember several years ago seeing (unrelated to MicroHs) code that
> > for performance reasons uses integer overflow with division by some
> > power of 2 (i++ / (1 << N)) for circular array (queue) to pass messages
> > between threads, so having it overflow or underflow is not always a
> > bug.
>
> Correction, it was: i++ & (1 << N)
>
> But you get the idea.
>
Attachment: file
Lennart Augustsson wrote on 25 Jan 15:37 +0100
(name . Lars-Dominik Braun)(address . lars@6xq.net)
CAJogeqer01h+75EMD0Kh7O74m0_ir2o_SdkonUV1dduP59N8sA@mail.gmail.com
So, it seems random if Hugs need the patch or not. Which suggests some
undefined behaviour.
But if it works for you without the patch, so much the better.

I have no idea what it would take to compile GHC with MicroHs. I've not
tried.
I think it might not be too far away. MicroHs will probably need to get
unboxed types (or at least fake them). GHC is written fairly
conservatively and is not using all the latest features.
If somebody wants to try, I'm willing to add needed features to MicroHs if
they are not too crazy.


On Sat, Jan 25, 2025 at 3:28 PM Lars-Dominik Braun <lars@6xq.net> wrote:

Toggle quote (23 lines)
> Hi Lennart,
>
> > If someone fixes all the warnings when compiling Hugs, then I bet this
> > problem goes away. But I am not going to do that. In its current state
> Hugs
> > can compile a fully functional MicroHs, and that's enough for me.
>
> actually, I can build the MicroHs package from
> https://issues.guix.gnu.org/75778 just fine with our default, unpatched
> Hugs. It can also compile/run the example from
> https://github.com/augustss/MicroHs?tab=readme-ov-file#example.
>
> Can we therefore just drop this patch? It causes Hugs to die with a
> SIGFPE while evaluating the example expression `fromInteger (9 ^ 100)
> :: Int` from above, which is rather ugly behavior.
>
> And on an unrelated note: Would it be possible to bootstrap (a possibly
> very, very old version of) GHC with MicroHs? (As far as I see our GHC
> 7 is currently bootstrapped via binaries.)
>
> Lars
>
>
Attachment: file
f14a2461bc878b6a9274ebd7dfc150a1@disroot.org
I wouldn't hurry to close this thread, as it seems to be
architecture-specific problem as today macOS runs on ARM, so Guix on
non-x86 architectures might also be affected.

By the way, I just tested that this code doesn't crash Hugs: fromInteger
(9^20) :: Int

Honestly I am extremely surprised by your willingness to help to
bootstrap GHC, as it's hard to find anyone interested in Haskell
community, and it might be impossible to convince GHC developers to
cooperate.

However I am content with what I already have as I only wanted minimal
bootstrappable Haskell compiler which I can flexibly change to port to
Plan 9 and all architectures it supports, and someone else already
ported Hugs there.

Having GHC on that minimal and portable system is just too much work and
once ported won't even be able to build 99% of packages on Stackage
because Plan 9 is anti-ISO and anti-POSIX, Plan 9 is research OS that
from the beginning opposes existing standards and so maintaining GHC
there would make it harder for me to experiment.
Lennart Augustsson wrote on 26 Jan 01:28 +0100
(name . Homo)(address . gay@disroot.org)
CAJogeqcKtrKVF3f-Cm6iedWh5v95Vz-F9iwG+RhqeUjXpQi=Hw@mail.gmail.com
I think bootstrapping GHC is a fun goal, but I'm only willing to put in
limited effort.


On Sat, Jan 25, 2025, 20:45 <gay@disroot.org> wrote:

Toggle quote (23 lines)
> I wouldn't hurry to close this thread, as it seems to be
> architecture-specific problem as today macOS runs on ARM, so Guix on
> non-x86 architectures might also be affected.
>
> By the way, I just tested that this code doesn't crash Hugs: fromInteger
> (9^20) :: Int
>
> Honestly I am extremely surprised by your willingness to help to
> bootstrap GHC, as it's hard to find anyone interested in Haskell
> community, and it might be impossible to convince GHC developers to
> cooperate.
>
> However I am content with what I already have as I only wanted minimal
> bootstrappable Haskell compiler which I can flexibly change to port to
> Plan 9 and all architectures it supports, and someone else already
> ported Hugs there.
>
> Having GHC on that minimal and portable system is just too much work and
> once ported won't even be able to build 99% of packages on Stackage
> because Plan 9 is anti-ISO and anti-POSIX, Plan 9 is research OS that
> from the beginning opposes existing standards and so maintaining GHC
> there would make it harder for me to experiment.
>
Attachment: file
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 75745@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 75745
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch