unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / Atom feed
* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
@ 2020-10-14 22:25 Ludovic Courtès
  2020-10-15  7:42 ` Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2020-10-14 22:25 UTC (permalink / raw)
  To: 44000

Hello!

You might have seen that ‘guix pull’ doesn’t work in your childhurd:

--8<---------------cut here---------------start------------->8---
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
guix pull: error: Git error: invalid version 0 on git_proxy_options
--8<---------------cut here---------------end--------------->8---

This is due to an ABI breakage whereby the Guile-Git code
(cross-compiled from x86_64-linux) fills in the ‘fetch_opts’ member of
‘git_clone options’ offset by one word.

On closer inspection, the problem is:

--8<---------------cut here---------------start------------->8---
scheme@(git structs)> (bytestructure-descriptor-size (bs:struct `(("x" ,(bs:pointer uint8)) ("y" ,size_t))))
$20 = 12
scheme@(git structs)> %host-type
$21 = "i586-pc-gnu"
--8<---------------cut here---------------end--------------->8---

Compare with the correct answer:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc -C -s i686-linux guile guile-bytestructures  -- guile

[...]

scheme@(guile-user)> ,use(bytestructures guile)
scheme@(guile-user)> %host-type
$1 = "i686-unknown-linux-gnu"
scheme@(guile-user)> (bytestructure-descriptor-size (bs:struct `(("x" ,(bs:pointer uint8))("y" ,size_t))))
$2 = 8
--8<---------------cut here---------------end--------------->8---

More specifically, the size of ‘size_t’ is wrong, but pointer size is
right:

--8<---------------cut here---------------start------------->8---
scheme@(git structs)>  (bytestructure-descriptor-size size_t)
$27 = 8
scheme@(git structs)>  (bytestructure-descriptor-size uintptr_t )
$28 = 8
scheme@(git structs)>  (bytestructure-descriptor-size (bs:pointer uint8))
$29 = 4
--8<---------------cut here---------------end--------------->8---

‘numeric.scm’ in bytestructures reads:

--8<---------------cut here---------------start------------->8---
(define arch32bit? (cond-expand
                    (lp32  #t)
                    (ilp32 #t)
                    (else  #f)))

;; …

(define uintptr_t (if arch32bit?
                      uint32
                      uint64))

(define size_t uintptr_t)
--8<---------------cut here---------------end--------------->8---

But (bytestructures guile numeric-data-model) has this:

--8<---------------cut here---------------start------------->8---
(define data-model
  (if (= 4 (sizeof '*))
      (if (= 2 (sizeof int))
          'lp32
          'ilp32)
      (cond
       ((= 8 (sizeof int))  'ilp64)
       ((= 4 (sizeof long)) 'llp64)
       (else                'lp64))))

(cond-expand-provide
 (current-module)
 (list architecture data-model))
--8<---------------cut here---------------end--------------->8---

The problem is that the ‘cond-expand’ used to define ‘arch32bit?’ is a
expansion-time thing when (cross-)building Bytestructures itself, so
it’s incorrect from cross-building from 64-bit to 32-bit.

I believe that changing it to:

  (define arch32bit? (memq data-model '(ilp32 lp32)))

would fix it because then the test would happen at run time.

(Note that Guile-Git uses only the procedural layer of Bytestructures.
The syntactic layer is likely not cross-compilation-capable for similar
reasons.)

To be continued…

Thanks,
Ludo’.




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-14 22:25 bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong Ludovic Courtès
@ 2020-10-15  7:42 ` Ludovic Courtès
  2020-10-15  8:09   ` Ludovic Courtès
                     ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Ludovic Courtès @ 2020-10-15  7:42 UTC (permalink / raw)
  To: 44000

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

Ludovic Courtès <ludo@gnu.org> skribis:

> The problem is that the ‘cond-expand’ used to define ‘arch32bit?’ is a
> expansion-time thing when (cross-)building Bytestructures itself, so
> it’s incorrect from cross-building from 64-bit to 32-bit.
>
> I believe that changing it to:
>
>   (define arch32bit? (memq data-model '(ilp32 lp32)))
>
> would fix it because then the test would happen at run time.

I confirm that the attached patch for Bytestructures solves the problem
(running ‘guix pull’ in my childhurd :-)).

Let’s see whether it needs to be adapted for inclusion upstream.

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Correctly-define-intptr_t-co.-when-cross-compiling.patch --]
[-- Type: text/x-patch, Size: 1696 bytes --]

From a44d04ed3f7031b4ab95b2f0da81c7ab020304d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 15 Oct 2020 08:47:06 +0200
Subject: [PATCH] Correctly define 'intptr_t' & co. when cross-compiling.

Fixes <https://issues.guix.gnu.org/44000>.

* bytestructures/guile/numeric-data-model.scm (data-model): Export.
* bytestructures/body/numeric.scm (arch32bit?): Check the value of
DATA-MODEL rather than use 'cond-expand'.
---
 bytestructures/body/numeric.scm             | 8 ++++----
 bytestructures/guile/numeric-data-model.scm | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/bytestructures/body/numeric.scm b/bytestructures/body/numeric.scm
index 842575d..e23cc24 100644
--- a/bytestructures/body/numeric.scm
+++ b/bytestructures/body/numeric.scm
@@ -282,10 +282,10 @@
 (define long-long int64)
 (define unsigned-long-long uint64)
 
-(define arch32bit? (cond-expand
-                    (lp32  #t)
-                    (ilp32 #t)
-                    (else  #f)))
+(define arch32bit? (case data-model
+                    ((lp32)  #t)
+                    ((ilp32) #t)
+                    (else    #f)))
 
 (define intptr_t (if arch32bit?
                      int32
diff --git a/bytestructures/guile/numeric-data-model.scm b/bytestructures/guile/numeric-data-model.scm
index 773b6cd..bd40c69 100644
--- a/bytestructures/guile/numeric-data-model.scm
+++ b/bytestructures/guile/numeric-data-model.scm
@@ -1,4 +1,5 @@
-(define-module (bytestructures guile numeric-data-model))
+(define-module (bytestructures guile numeric-data-model)
+  #:export (data-model))
 
 (import (system foreign))
 (import (system base target))
-- 
2.28.0


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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-15  7:42 ` Ludovic Courtès
@ 2020-10-15  8:09   ` Ludovic Courtès
  2020-10-15  8:48   ` Mathieu Othacehe
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2020-10-15  8:09 UTC (permalink / raw)
  To: 44000

Reported here:
<https://github.com/TaylanUB/scheme-bytestructures/issues/40>.




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-15  7:42 ` Ludovic Courtès
  2020-10-15  8:09   ` Ludovic Courtès
@ 2020-10-15  8:48   ` Mathieu Othacehe
  2020-10-15 10:06   ` Jan Nieuwenhuizen
  2020-10-16 15:58   ` Taylan Kammer
  3 siblings, 0 replies; 9+ messages in thread
From: Mathieu Othacehe @ 2020-10-15  8:48 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44000


Hey Ludo,

> I confirm that the attached patch for Bytestructures solves the problem
> (running ‘guix pull’ in my childhurd :-)).

Nice catch! I guess we had the same issue when cross-compiling for armhf
from x86_64.

Thanks,

Mathieu




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-15  7:42 ` Ludovic Courtès
  2020-10-15  8:09   ` Ludovic Courtès
  2020-10-15  8:48   ` Mathieu Othacehe
@ 2020-10-15 10:06   ` Jan Nieuwenhuizen
  2020-10-16 15:58   ` Taylan Kammer
  3 siblings, 0 replies; 9+ messages in thread
From: Jan Nieuwenhuizen @ 2020-10-15 10:06 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44000

Ludovic Courtès writes:

Hi,

> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> The problem is that the ‘cond-expand’ used to define ‘arch32bit?’ is a
>> expansion-time thing when (cross-)building Bytestructures itself, so
>> it’s incorrect from cross-building from 64-bit to 32-bit.
>>
>> I believe that changing it to:
>>
>>   (define arch32bit? (memq data-model '(ilp32 lp32)))
>>
>> would fix it because then the test would happen at run time.
>
> I confirm that the attached patch for Bytestructures solves the problem
> (running ‘guix pull’ in my childhurd :-)).

Wow, beautiful!

> Let’s see whether it needs to be adapted for inclusion upstream.

Yes, sure.

Greetings,
Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-15  7:42 ` Ludovic Courtès
                     ` (2 preceding siblings ...)
  2020-10-15 10:06   ` Jan Nieuwenhuizen
@ 2020-10-16 15:58   ` Taylan Kammer
  2020-10-17 17:44     ` Taylan Kammer
  3 siblings, 1 reply; 9+ messages in thread
From: Taylan Kammer @ 2020-10-16 15:58 UTC (permalink / raw)
  To: Ludovic Courtès, 44000

On 15.10.2020 09:42, Ludovic Courtès wrote:
> Ludovic Courtès <ludo@gnu.org> skribis:
> 
>> The problem is that the ‘cond-expand’ used to define ‘arch32bit?’ is a
>> expansion-time thing when (cross-)building Bytestructures itself, so
>> it’s incorrect from cross-building from 64-bit to 32-bit.
>>
>> I believe that changing it to:
>>
>>   (define arch32bit? (memq data-model '(ilp32 lp32)))
>>
>> would fix it because then the test would happen at run time.
> 
> I confirm that the attached patch for Bytestructures solves the problem
> (running ‘guix pull’ in my childhurd :-)).
> 
> Let’s see whether it needs to be adapted for inclusion upstream.

Hi Ludo,

Thanks for finding finding this bug and for the patch.

I'll try to include a portable version of the fix ASAP.  I should have
time for it tomorrow.

> 
> Ludo’.
> 


- Taylan




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-16 15:58   ` Taylan Kammer
@ 2020-10-17 17:44     ` Taylan Kammer
  2020-10-19  8:23       ` Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Taylan Kammer @ 2020-10-17 17:44 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44000

Taylan Kammer <taylan.kammer@gmail.com> writes:

> On 15.10.2020 09:42, Ludovic Courtès wrote:
>> Ludovic Courtès <ludo@gnu.org> skribis:
>> 
>>> The problem is that the ‘cond-expand’ used to define ‘arch32bit?’ is a
>>> expansion-time thing when (cross-)building Bytestructures itself, so
>>> it’s incorrect from cross-building from 64-bit to 32-bit.
>>>
>>> I believe that changing it to:
>>>
>>>   (define arch32bit? (memq data-model '(ilp32 lp32)))
>>>
>>> would fix it because then the test would happen at run time.
>> 
>> I confirm that the attached patch for Bytestructures solves the problem
>> (running ‘guix pull’ in my childhurd :-)).
>> 
>> Let’s see whether it needs to be adapted for inclusion upstream.
>
> Hi Ludo,
>
> Thanks for finding finding this bug and for the patch.
>
> I'll try to include a portable version of the fix ASAP.  I should have
> time for it tomorrow.

Hi again,

Could you please test whether bytestructures 1.0.8 fixes the issue?

Thank you!


- Taylan




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-17 17:44     ` Taylan Kammer
@ 2020-10-19  8:23       ` Ludovic Courtès
  2020-10-22 18:47         ` Taylan Kammer
  0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2020-10-19  8:23 UTC (permalink / raw)
  To: Taylan Kammer; +Cc: 44000

Hi,

Taylan Kammer <taylan.kammer@gmail.com> skribis:

> Could you please test whether bytestructures 1.0.8 fixes the issue?

Thanks for the prompt reply!  I tested 1.0.8 and it does not fix the
problem.

I think the problem might be that the ‘cond-expand-provide’ call might
affect a module that’s not the one tested in (eval '(cond-expand …) …).

Does that make sense?

Ludo’.




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

* bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong
  2020-10-19  8:23       ` Ludovic Courtès
@ 2020-10-22 18:47         ` Taylan Kammer
  0 siblings, 0 replies; 9+ messages in thread
From: Taylan Kammer @ 2020-10-22 18:47 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44000

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Taylan Kammer <taylan.kammer@gmail.com> skribis:
>
>> Could you please test whether bytestructures 1.0.8 fixes the issue?
>
> Thanks for the prompt reply!  I tested 1.0.8 and it does not fix the
> problem.
>
> I think the problem might be that the ‘cond-expand-provide’ call might
> affect a module that’s not the one tested in (eval '(cond-expand …) …).
>
> Does that make sense?

Yes, you're right.  I've made another release (1.0.9), where I use

  (environment '(guile) '(bytestructures guile numeric-data-model))

for the 'base-environment' binding in case we're running on Guile.

It now gives me correct results locally (woe on me for not having
properly tested the previous one) so I think it should definitely work
when cross-compiling too, since the 'eval' is sure to be executed at
run-time and not compile-time...

Fingers crossed, I hope I don't waste your time this time!

- Taylan




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

end of thread, other threads:[~2020-10-22 18:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 22:25 bug#44000: Guile-Git cross-compiled to i586-pc-gnu gets bytestructures wrong Ludovic Courtès
2020-10-15  7:42 ` Ludovic Courtès
2020-10-15  8:09   ` Ludovic Courtès
2020-10-15  8:48   ` Mathieu Othacehe
2020-10-15 10:06   ` Jan Nieuwenhuizen
2020-10-16 15:58   ` Taylan Kammer
2020-10-17 17:44     ` Taylan Kammer
2020-10-19  8:23       ` Ludovic Courtès
2020-10-22 18:47         ` Taylan Kammer

unofficial mirror of bug-guix@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/guix-bugs/0 guix-bugs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 guix-bugs guix-bugs/ https://yhetil.org/guix-bugs \
		bug-guix@gnu.org
	public-inbox-index guix-bugs

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.gnu.guix.bugs
	nntp://news.gmane.io/gmane.comp.gnu.guix.bugs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git