Compiling grub2 on *BSD?

Compiling grub2 on *BSD?

[walt]

As a test of my tolerance for frustration I'm building grub2 on FreeBSD,
DragonFlyBSD, NetBSD, and OpenBSD, and (naturally) each build fails in a
different way, but all the problems seem related to mutations of gcc.

NetBSD:

The problem is the -nostdlib flag when linking kernel.exec.  Adding -lc
and -lgcc solves part of the problem, but then libc wants crt0.o:

# nm --defined-only /usr/lib/crt0.o
00000018 T ___start
00000000 D __progname  <---- Needed by libc
00000000 B __ps_strings
00000000 T __start
000000f0 T _rtld_setup
00000000 T _start   <----- Conflicts with i386/pc/startup.S
00000004 C environ  <----- Needed by libc

OpenBSD:

OBSD is off on another planet, as usual.  They use a non-standard
definition of memcpy in <string.h>:
void    *memcpy(void *, const void *, size_t)
                 __attribute__ ((__bounded__(__buffer__,1,3)))
                 __attribute__ ((__bounded__(__buffer__,2,3)));

FreeBSD:

lnxboot.S: Assembler messages:
lnxboot.S:49: Error: `0x200+data_start - data_next(%ebx,%eax)' is not a valid 16 bit base/index expression
lnxboot.S:264: Error: `(%esi,%eax)' is not a valid 16 bit base/index expression
lnxboot.S:265: Error: `(%edi,%eax)' is not a valid 16 bit base/index expression

DragonFlyBSD:

This was easy.  All I did was add -lc and -lgcc to kernel_img_LDFLAGS in
i386-pc.mk.  I have no idea why the same trick doesn't work for NetBSD.

Anyone have any ideas for elegant ways to fix these four problems?

Thanks!

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Sat, Jan 26, 2008 at 09:08:59AM -0800, walt wrote:
> As a test of my tolerance for frustration I'm building grub2 on FreeBSD,
> DragonFlyBSD, NetBSD, and OpenBSD, and (naturally) each build fails in a
> different way, but all the problems seem related to mutations of gcc.
> 
> NetBSD:
> 
> The problem is the -nostdlib flag when linking kernel.exec.

What is the problem initially?

> Adding -lc
> and -lgcc solves part of the problem, but then libc wants crt0.o:
> 
> # nm --defined-only /usr/lib/crt0.o
> 00000018 T ___start
> 00000000 D __progname  <---- Needed by libc
> 00000000 B __ps_strings
> 00000000 T __start
> 000000f0 T _rtld_setup
> 00000000 T _start   <----- Conflicts with i386/pc/startup.S
> 00000004 C environ  <----- Needed by libc

You don't want -lc, adding that is wrong.

> OpenBSD:
> 
> OBSD is off on another planet, as usual.  They use a non-standard
> definition of memcpy in <string.h>:
> void    *memcpy(void *, const void *, size_t)
>                 __attribute__ ((__bounded__(__buffer__,1,3)))
>                 __attribute__ ((__bounded__(__buffer__,2,3)));

What's the error?

> FreeBSD:
> 
> lnxboot.S: Assembler messages:
> lnxboot.S:49: Error: `0x200+data_start - data_next(%ebx,%eax)' is not a 
> valid 16 bit base/index expression
> lnxboot.S:264: Error: `(%esi,%eax)' is not a valid 16 bit base/index 
> expression
> lnxboot.S:265: Error: `(%edi,%eax)' is not a valid 16 bit base/index 
> expression

I recall having seen this before.  Please, can you check the list archives?
I think we solved it for other files, but it might have been respawned in
lnxboot.S.

> DragonFlyBSD:
> 
> This was easy.  All I did was add -lc and -lgcc to kernel_img_LDFLAGS in
> i386-pc.mk.  I have no idea why the same trick doesn't work for NetBSD.

I'm surprised -lc works.  Maybe gcc/ld skips it.  Anyway, you should tell
us the initial error.

Thanks!

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[Bean]

On Jan 27, 2008 1:21 AM, Robert Millan <rmh@aybabtu.com> wrote:
> > FreeBSD:
> >
> > lnxboot.S: Assembler messages:
> > lnxboot.S:49: Error: `0x200+data_start - data_next(%ebx,%eax)' is not a
> > valid 16 bit base/index expression
> > lnxboot.S:264: Error: `(%esi,%eax)' is not a valid 16 bit base/index
> > expression
> > lnxboot.S:265: Error: `(%edi,%eax)' is not a valid 16 bit base/index
> > expression
>
> I recall having seen this before.  Please, can you check the list archives?
> I think we solved it for other files, but it might have been respawned in
> lnxboot.S.

yes, this is caused by a trick to use 32-bit address mode for fast
arithmetic. here is the patch, along with some code cleanup for
lnxboot.S.

diff --git a/boot/i386/pc/lnxboot.S b/boot/i386/pc/lnxboot.S
index f1a4ded..81d8e40 100644
--- a/boot/i386/pc/lnxboot.S
+++ b/boot/i386/pc/lnxboot.S
@@ -36,22 +36,7 @@
         .globl  start, _start

 data_start:
-	pushw	%cs
-	popw	%ds
-	xorl	%eax, %eax
-	xorl	%ebx, %ebx
-	call	data_next
-
-data_next:
-	popw	%bx
-	movw	%cs, %ax
-	shll	$4, %eax
-	leal	0x200 + data_start - data_next(%ebx,%eax), %eax
-	movzbl	setup_sects - data_next(%bx), %ecx
-	shll	$9, %ecx
-	addl	%ecx, %eax
-	movl	%eax, code32_start - data_next(%bx)
-
+	xorl	%ebp, %ebp
 	jmp	linux_next

 	. = data_start + 0x1F1
@@ -76,7 +61,7 @@ boot_flag:
 start:
 _start:

-	jmp linux_code
+	jmp linux_init

 	.ascii	"HdrS"			// Header signature
 	.word	0x0203			// Header version number
@@ -134,9 +119,10 @@ reg_edx:
 data_leng:
 	.long	0

-linux_code:
+linux_init:

 	movw	%cs:(reg_edx - start), %dx
+	movl	%cs:(code32_start - start), %ebp

 linux_next:

@@ -164,9 +150,6 @@ real_code:
 	movw	%si, %ss
 	movw	$(CODE_ADDR), %sp

-	pushl	%esi
-	pushl	%edi
-
 	// Move itself to 0:CODE_ADDR

 	cld
@@ -183,34 +166,45 @@ real_code:

 real_code_2:

-	pushw	%es
-	popw	%ds
-
-	movl	(ramdisk_image - start), %esi
-	or	%esi, %esi
+	xchgl	%ebp, %esi
+	orl	%esi, %esi
 	jnz	1f
-	movl	(code32_start - start), %esi
+	movw	%ds, %si
+	shll	$4, %esi
+	addl	%ebp, %esi
 1:

+	pushw	%es
+	popw	%ds
+
 	movl	$0x200, %ecx
 	addl	%ecx, %esi
 	movl	$DATA_ADDR, %edi

 	call	move_memory

-	movsbl	%dh, %eax
-	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
+	// Check for the multiboot signature
+	cmpl	$0x1badb002, %ss:(DATA_ADDR + 0x50)
+	jz	1f

-	movsbl	(reg_edx + 2 - start), %eax
-	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)
+	movl	(ramdisk_image - start), %esi
+	movl	(ramdisk_size - start), %ecx
+	movl	$(DATA_ADDR - 0x200), %edi
+	jmp	2f
+
+1:

 	movl	%ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_COMPRESSED_SIZE), %ecx
 	addl	$(GRUB_KERNEL_MACHINE_RAW_SIZE - 0x200), %ecx

+2:
 	call	move_memory

-	popl	%edi
-	popl	%esi
+	movsbl	%dh, %eax
+	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
+
+	movsbl	(reg_edx + 2 - start), %eax
+	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)

 	ljmp	$(DATA_ADDR >> 4), $0

@@ -261,8 +255,8 @@ move_memory:

 2:

-	leal	(%esi, %eax), %esi
-	leal	(%edi, %eax), %edi
+	addl	%eax, %esi
+	addl	%eax, %edi
 	subl	%eax, %ecx
 	jnz	1b




-- 
Bean

Re: Compiling grub2 on *BSD?

[Bean]

On Jan 27, 2008 2:15 AM, Bean <bean123ch@gmail.com> wrote:
> On Jan 27, 2008 1:21 AM, Robert Millan <rmh@aybabtu.com> wrote:
> > > FreeBSD:
> > >
> > > lnxboot.S: Assembler messages:
> > > lnxboot.S:49: Error: `0x200+data_start - data_next(%ebx,%eax)' is not a
> > > valid 16 bit base/index expression
> > > lnxboot.S:264: Error: `(%esi,%eax)' is not a valid 16 bit base/index
> > > expression
> > > lnxboot.S:265: Error: `(%edi,%eax)' is not a valid 16 bit base/index
> > > expression
> >
> > I recall having seen this before.  Please, can you check the list archives?
> > I think we solved it for other files, but it might have been respawned in
> > lnxboot.S.
>
> yes, this is caused by a trick to use 32-bit address mode for fast
> arithmetic. here is the patch, along with some code cleanup for
> lnxboot.S.
>
> diff --git a/boot/i386/pc/lnxboot.S b/boot/i386/pc/lnxboot.S
> index f1a4ded..81d8e40 100644
> --- a/boot/i386/pc/lnxboot.S
> +++ b/boot/i386/pc/lnxboot.S
> @@ -36,22 +36,7 @@
>          .globl  start, _start
>
>  data_start:
> -       pushw   %cs
> -       popw    %ds
> -       xorl    %eax, %eax
> -       xorl    %ebx, %ebx
> -       call    data_next
> -
> -data_next:
> -       popw    %bx
> -       movw    %cs, %ax
> -       shll    $4, %eax
> -       leal    0x200 + data_start - data_next(%ebx,%eax), %eax
> -       movzbl  setup_sects - data_next(%bx), %ecx
> -       shll    $9, %ecx
> -       addl    %ecx, %eax
> -       movl    %eax, code32_start - data_next(%bx)
> -
> +       xorl    %ebp, %ebp
>         jmp     linux_next
>
>         . = data_start + 0x1F1
> @@ -76,7 +61,7 @@ boot_flag:
>  start:
>  _start:
>
> -       jmp linux_code
> +       jmp linux_init
>
>         .ascii  "HdrS"                  // Header signature
>         .word   0x0203                  // Header version number
> @@ -134,9 +119,10 @@ reg_edx:
>  data_leng:
>         .long   0
>
> -linux_code:
> +linux_init:
>
>         movw    %cs:(reg_edx - start), %dx
> +       movl    %cs:(code32_start - start), %ebp
>
>  linux_next:
>
> @@ -164,9 +150,6 @@ real_code:
>         movw    %si, %ss
>         movw    $(CODE_ADDR), %sp
>
> -       pushl   %esi
> -       pushl   %edi
> -
>         // Move itself to 0:CODE_ADDR
>
>         cld
> @@ -183,34 +166,45 @@ real_code:
>
>  real_code_2:
>
> -       pushw   %es
> -       popw    %ds
> -
> -       movl    (ramdisk_image - start), %esi
> -       or      %esi, %esi
> +       xchgl   %ebp, %esi
> +       orl     %esi, %esi
>         jnz     1f
> -       movl    (code32_start - start), %esi
> +       movw    %ds, %si
> +       shll    $4, %esi
> +       addl    %ebp, %esi
>  1:
>
> +       pushw   %es
> +       popw    %ds
> +
>         movl    $0x200, %ecx
>         addl    %ecx, %esi
>         movl    $DATA_ADDR, %edi
>
>         call    move_memory
>
> -       movsbl  %dh, %eax
> -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
> +       // Check for the multiboot signature
> +       cmpl    $0x1badb002, %ss:(DATA_ADDR + 0x50)
> +       jz      1f
>
> -       movsbl  (reg_edx + 2 - start), %eax
> -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)
> +       movl    (ramdisk_image - start), %esi
> +       movl    (ramdisk_size - start), %ecx
> +       movl    $(DATA_ADDR - 0x200), %edi
> +       jmp     2f
> +
> +1:
>
>         movl    %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_COMPRESSED_SIZE), %ecx
>         addl    $(GRUB_KERNEL_MACHINE_RAW_SIZE - 0x200), %ecx
>
> +2:
>         call    move_memory
>
> -       popl    %edi
> -       popl    %esi
> +       movsbl  %dh, %eax
> +       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
> +
> +       movsbl  (reg_edx + 2 - start), %eax
> +       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)
>
>         ljmp    $(DATA_ADDR >> 4), $0
>
> @@ -261,8 +255,8 @@ move_memory:
>
>  2:
>
> -       leal    (%esi, %eax), %esi
> -       leal    (%edi, %eax), %edi
> +       addl    %eax, %esi
> +       addl    %eax, %edi
>         subl    %eax, %ecx
>         jnz     1b
>
>
>
>
> --
> Bean
>

if there is no objection, I'd like to check in this patch.

-- 
Bean

Re: Compiling grub2 on *BSD?

[Robert Millan]

> > -       movsbl  %dh, %eax
> > -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
> > +       // Check for the multiboot signature
> > +       cmpl    $0x1badb002, %ss:(DATA_ADDR + 0x50)
> > +       jz      1f
> [...]
> 
> if there is no objection, I'd like to check in this patch.

I don't like that you hardcode 0x1badb002.  Could you use the macro from
multiboot.h ?

Also for the comments, I think /* */ is preferred (at least, it is consistent
with the rest of GRUB).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[Bean]

On Jan 28, 2008 6:47 PM, Robert Millan <rmh@aybabtu.com> wrote:
> > > -       movsbl  %dh, %eax
> > > -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
> > > +       // Check for the multiboot signature
> > > +       cmpl    $0x1badb002, %ss:(DATA_ADDR + 0x50)
> > > +       jz      1f
> > [...]
> >
> > if there is no objection, I'd like to check in this patch.
>
> I don't like that you hardcode 0x1badb002.  Could you use the macro from
> multiboot.h ?
>
> Also for the comments, I think /* */ is preferred (at least, it is consistent
> with the rest of GRUB).

fixed.

diff --git a/boot/i386/pc/lnxboot.S b/boot/i386/pc/lnxboot.S
index f1a4ded..6c63af4 100644
--- a/boot/i386/pc/lnxboot.S
+++ b/boot/i386/pc/lnxboot.S
@@ -1,7 +1,7 @@
 /* -*-Asm-*- */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 1999,2000,2001,2002,2005,2006,2007  Free Software
Foundation, Inc.
+ *  Copyright (C) 2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -20,6 +20,7 @@
 #include <grub/boot.h>
 #include <grub/machine/boot.h>
 #include <grub/machine/kernel.h>
+#include <multiboot.h>

         .file   "lnxboot.S"

@@ -36,22 +37,7 @@
         .globl  start, _start

 data_start:
-	pushw	%cs
-	popw	%ds
-	xorl	%eax, %eax
-	xorl	%ebx, %ebx
-	call	data_next
-
-data_next:
-	popw	%bx
-	movw	%cs, %ax
-	shll	$4, %eax
-	leal	0x200 + data_start - data_next(%ebx,%eax), %eax
-	movzbl	setup_sects - data_next(%bx), %ecx
-	shll	$9, %ecx
-	addl	%ecx, %eax
-	movl	%eax, code32_start - data_next(%bx)
-
+	xorl	%ebp, %ebp
 	jmp	linux_next

 	. = data_start + 0x1F1
@@ -76,29 +62,29 @@ boot_flag:
 start:
 _start:

-	jmp linux_code
+	jmp linux_init

-	.ascii	"HdrS"			// Header signature
-	.word	0x0203			// Header version number
+	.ascii	"HdrS"			/* Header signature  */
+	.word	0x0203			/* Header version number  */

 realmode_swtch:
-	.word	0, 0			// default_switch, SETUPSEG
+	.word	0, 0			/* default_switch, SETUPSEG  */
 start_sys_seg:
-	.word	0x1000			// obsolete
+	.word	0x1000			/* Obsolete  */
 version_ptr:
-	.word	0			// version string ptr
+	.word	0			/* Version string ptr  */
 type_of_loader:
-	.byte	0			// Filled in by boot loader
+	.byte	0			/* Filled in by boot loader  */
 loadflags:
-	.byte	1			// Please load high
+	.byte	1			/* Please load high  */
 setup_move_size:
-	.word	0			// Unused
+	.word	0			/* Unused  */
 code32_start:
-	.long	0x100000		// 32-bit start address
+	.long	0x100000		/* 32-bit start address  */
 ramdisk_image:
-	.long	0			// Loaded ramdisk image address
+	.long	0			/* Loaded ramdisk image address  */
 ramdisk_size:
-	.long	0			// Size of loaded ramdisk
+	.long	0			/* Size of loaded ramdisk  */
 bootsect_kludge:
 	.word	0, 0
 heap_end_ptr:
@@ -106,27 +92,27 @@ heap_end_ptr:
 pad1:
 	.word	0
 cmd_line_ptr:
-	.long	0			// Command line
+	.long	0			/* Command line  */
 ramdisk_max:
-	.long	0xffffffff		// Highest allowed ramdisk address
+	.long	0xffffffff		/* Highest allowed ramdisk address  */

 gdt:
-	.long	0, 0, 0, 0		// Must be zero
-	.word	0xffff			// 64 K segment size
+	.long	0, 0, 0, 0		/* Must be zero  */
+	.word	0xffff			/* 64 K segment size  */
 gdt_src1:
-	.byte	0, 0 ,0			// Low 24 bits of source addy
-	.byte	0x93			// Access rights
-	.byte	0			// Extended access rights
+	.byte	0, 0 ,0			/* Low 24 bits of source addy  */
+	.byte	0x93			/* Access rights  */
+	.byte	0			/* Extended access rights  */
 gdt_src2:
-	.byte	0			// High 8 bits of source addy
-	.word	0xffff			// 64 K segment size
+	.byte	0			/* High 8 bits of source addy  */
+	.word	0xffff			/* 64 K segment size  */
 gdt_dst1:
-	.byte	0, 0, 0			// Low 24 bits of target addy
-	.byte	0x93			// Access rights
-	.byte	0			// Extended access rights
+	.byte	0, 0, 0			/* Low 24 bits of target addy  */
+	.byte	0x93			/* Access rights  */
+	.byte	0			/* Extended access rights  */
 gdt_dst2:
-	.byte	0			// High 8 bits of source addy
-	.long	0, 0, 0, 0		// More space for the BIOS
+	.byte	0			/* High 8 bits of source addy  */
+	.long	0, 0, 0, 0		/* More space for the BIOS  */

 reg_edx:
 	.byte	0x80,0,0xFF,0xFF
@@ -134,9 +120,10 @@ reg_edx:
 data_leng:
 	.long	0

-linux_code:
+linux_init:

 	movw	%cs:(reg_edx - start), %dx
+	movl	%cs:(code32_start - start), %ebp

 linux_next:

@@ -150,7 +137,7 @@ normalize:
 	addw	%bx, %ax
 	pushw	%ax
 	pushw	$(real_code - start)
-	lret				// jump to real_code
+	lret				/* Jump to real_code  */

 real_code:
 	subw	$0x20, %ax
@@ -158,16 +145,13 @@ real_code:
 	movw	(setup_sects - data_start), %cx
 	shlw	$7, %cx

-	// Setup stack
+	/* Setup stack  */

 	xorw	%si, %si
 	movw	%si, %ss
 	movw	$(CODE_ADDR), %sp

-	pushl	%esi
-	pushl	%edi
-
-	// Move itself to 0:CODE_ADDR
+	/* Move itself to 0:CODE_ADDR  */

 	cld
 	movw	%cs, %ax
@@ -183,41 +167,55 @@ real_code:

 real_code_2:

-	pushw	%es
-	popw	%ds
-
-	movl	(ramdisk_image - start), %esi
-	or	%esi, %esi
+	xchgl	%ebp, %esi
+	orl	%esi, %esi
 	jnz	1f
-	movl	(code32_start - start), %esi
+	movw	%ds, %si
+	shll	$4, %esi
+	addl	%ebp, %esi
 1:

+	pushw	%es
+	popw	%ds
+
 	movl	$0x200, %ecx
 	addl	%ecx, %esi
 	movl	$DATA_ADDR, %edi

 	call	move_memory

-	movsbl	%dh, %eax
-	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
+	/* Check for multiboot signature  */
+	cmpl	$MULTIBOOT_MAGIC, %ss:(DATA_ADDR + 0x50)
+	jz	1f

-	movsbl	(reg_edx + 2 - start), %eax
-	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)
+	movl	(ramdisk_image - start), %esi
+	movl	(ramdisk_size - start), %ecx
+	movl	$(DATA_ADDR - 0x200), %edi
+	jmp	2f
+
+1:

 	movl	%ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_COMPRESSED_SIZE), %ecx
 	addl	$(GRUB_KERNEL_MACHINE_RAW_SIZE - 0x200), %ecx

+2:
 	call	move_memory

-	popl	%edi
-	popl	%esi
+	movsbl	%dh, %eax
+	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
+
+	movsbl	(reg_edx + 2 - start), %eax
+	movl	%eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART)

 	ljmp	$(DATA_ADDR >> 4), $0

-// Parameters:
-//   esi: source address
-//   edi: target address
-//   ecx: number of bytes
+/*
+ * Parameters:
+ *   esi: source address
+ *   edi: target address
+ *   ecx: number of bytes
+ */
+
 move_memory:
 	incl	%ecx
 	andb	$0xFE, %cl
@@ -261,8 +259,8 @@ move_memory:

 2:

-	leal	(%esi, %eax), %esi
-	leal	(%edi, %eax), %edi
+	addl	%eax, %esi
+	addl	%eax, %edi
 	subl	%eax, %ecx
 	jnz	1b

@@ -270,8 +268,11 @@ move_memory:
 	popw	%dx
 	ret

-// Parameters:
-//   si: message
+/*
+ * Parameters:
+ *   si: message
+ */
+
 fail:
 	movb	$0x0e, %ah
 	xorw	%bx, %bx


-- 
Bean

Re: Compiling grub2 on *BSD?

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

>> > -       movsbl  %dh, %eax
>> > -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
>> > +       // Check for the multiboot signature
>> > +       cmpl    $0x1badb002, %ss:(DATA_ADDR + 0x50)
>> > +       jz      1f
>> [...]
>> 
>> if there is no objection, I'd like to check in this patch.
>
> I don't like that you hardcode 0x1badb002.  Could you use the macro from
> multiboot.h ?
>
> Also for the comments, I think /* */ is preferred (at least, it is consistent
> with the rest of GRUB).

And do not forget "." :-)

--
Marco

Re: Compiling grub2 on *BSD?

[Bean]

On Jan 28, 2008 8:56 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> Robert Millan <rmh@aybabtu.com> writes:
>
> >> > -       movsbl  %dh, %eax
> >> > -       movl    %eax, %ss:(DATA_ADDR + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART)
> >> > +       // Check for the multiboot signature
> >> > +       cmpl    $0x1badb002, %ss:(DATA_ADDR + 0x50)
> >> > +       jz      1f
> >> [...]
> >>
> >> if there is no objection, I'd like to check in this patch.
> >
> > I don't like that you hardcode 0x1badb002.  Could you use the macro from
> > multiboot.h ?
> >
> > Also for the comments, I think /* */ is preferred (at least, it is consistent
> > with the rest of GRUB).
>
> And do not forget "." :-)

Fixed and committed.

-- 
Bean

Re: Compiling grub2 on *BSD?

[walt]

On Sat, 2008-01-26 at 18:21 +0100, Robert Millan wrote:
> On Sat, Jan 26, 2008 at 09:08:59AM -0800, walt wrote:
> > As a test of my tolerance for frustration I'm building grub2 on FreeBSD,
> > DragonFlyBSD, NetBSD, and OpenBSD, and (naturally) each build fails in a
> > different way, but all the problems seem related to mutations of gcc.
> > 
> > NetBSD:
> > 
> > The problem is the -nostdlib flag when linking kernel.exec.
> 
> What is the problem initially?

kernel_img-kern_device.o: In function `grub_device_iterate':
../kern/device.c:84: undefined reference to `__enable_execute_stack'

That's defined in libgcc so I add -lgcc to the link flags:

/usr/lib/libgcc.a(_enable_execute_stack.o): In function
`__enable_execute_stack':
_enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
_enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'

Both of those symbols are defined in libc.a, so I included that, too.
Any ideas?

> > FreeBSD:

Bean's lnxboot.S patch fixed this.  Thanks Bean!

> > DragonFlyBSD:

Correction:  grub2 builds correctly right out of the box.  Dunno why
I had problems the first time.  Black Magic!

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Sat, Jan 26, 2008 at 12:11:48PM -0800, walt wrote:
> > What is the problem initially?
> 
> kernel_img-kern_device.o: In function `grub_device_iterate':
> ../kern/device.c:84: undefined reference to `__enable_execute_stack'
> 
> That's defined in libgcc so I add -lgcc to the link flags:
> 
> /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> `__enable_execute_stack':
> _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> 
> Both of those symbols are defined in libc.a, so I included that, too.
> Any ideas?

Do you see this line in ./configure ?

  checking whether `gcc' accepts `-fstack-protector'... yes

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[jakllsch]

On Sun, Jan 27, 2008 at 09:56:50AM +0100, Robert Millan wrote:
> On Sat, Jan 26, 2008 at 12:11:48PM -0800, walt wrote:
> > > What is the problem initially?
> > 
> > kernel_img-kern_device.o: In function `grub_device_iterate':
> > ../kern/device.c:84: undefined reference to `__enable_execute_stack'
> > 
> > That's defined in libgcc so I add -lgcc to the link flags:
> > 
> > /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> > `__enable_execute_stack':
> > _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> > _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> > 
> > Both of those symbols are defined in libc.a, so I included that, too.
> > Any ideas?
> 
> Do you see this line in ./configure ?
> 
>   checking whether `gcc' accepts `-fstack-protector'... yes

Yes.

Apparently -fno-stack-protector doesn't actually work the way
I/we would expect.

But, I'm surprised that this doesn't affect the building
of NetBSD's own 'stand' code.

FWIW, the pkgsrc-wip package just patches in a dummy
function.  patch-ag and patch-ah.

Aside from those patches it built pretty much
out-of-the-checkout for me.

	Jonathan Kollasch

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Mon, Jan 28, 2008 at 11:30:51PM -0600, jakllsch@kollasch.net wrote:
> On Sun, Jan 27, 2008 at 09:56:50AM +0100, Robert Millan wrote:
> > On Sat, Jan 26, 2008 at 12:11:48PM -0800, walt wrote:
> > > > What is the problem initially?
> > > 
> > > kernel_img-kern_device.o: In function `grub_device_iterate':
> > > ../kern/device.c:84: undefined reference to `__enable_execute_stack'
> > > 
> > > That's defined in libgcc so I add -lgcc to the link flags:
> > > 
> > > /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> > > `__enable_execute_stack':
> > > _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> > > _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> > > 
> > > Both of those symbols are defined in libc.a, so I included that, too.
> > > Any ideas?
> > 
> > Do you see this line in ./configure ?
> > 
> >   checking whether `gcc' accepts `-fstack-protector'... yes
> 
> Yes.
> 
> Apparently -fno-stack-protector doesn't actually work the way
> I/we would expect.
> 
> But, I'm surprised that this doesn't affect the building
> of NetBSD's own 'stand' code.

Maybe we need another flag?  Could you check the link command in NetBSD's
kernel ?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[jakllsch]

On Tue, Jan 29, 2008 at 02:47:07PM +0100, Robert Millan wrote:
> On Mon, Jan 28, 2008 at 11:30:51PM -0600, jakllsch@kollasch.net wrote:
> > On Sun, Jan 27, 2008 at 09:56:50AM +0100, Robert Millan wrote:
> > > On Sat, Jan 26, 2008 at 12:11:48PM -0800, walt wrote:
> > > > > What is the problem initially?
> > > > 
> > > > kernel_img-kern_device.o: In function `grub_device_iterate':
> > > > ../kern/device.c:84: undefined reference to `__enable_execute_stack'
> > > > 
> > > > That's defined in libgcc so I add -lgcc to the link flags:
> > > > 
> > > > /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> > > > `__enable_execute_stack':
> > > > _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> > > > _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> > > > 
> > > > Both of those symbols are defined in libc.a, so I included that, too.
> > > > Any ideas?
> > > 
> > > Do you see this line in ./configure ?
> > > 
> > >   checking whether `gcc' accepts `-fstack-protector'... yes
> > 
> > Yes.
> > 
> > Apparently -fno-stack-protector doesn't actually work the way
> > I/we would expect.
> > 
> > But, I'm surprised that this doesn't affect the building
> > of NetBSD's own 'stand' code.
> 
> Maybe we need another flag?  Could you check the link command in NetBSD's
> kernel ?

I couldn't find anything that looked like it would fix this.

I believe the reason that NetBSD's own code isn't affected by this is
because they (apparently) do not use nested functions in
their stand-alone code.

I did see OpenBSD had a -ftrampolines flag, I'm not quite sure what
it exactly does, but it may be related to this.

	Jonathan Kollasch

Re: Compiling grub2 on *BSD?

[Marco Gerards]

jakllsch@kollasch.net writes:

[...]

>> Maybe we need another flag?  Could you check the link command in NetBSD's
>> kernel ?
>
> I couldn't find anything that looked like it would fix this.
>
> I believe the reason that NetBSD's own code isn't affected by this is
> because they (apparently) do not use nested functions in
> their stand-alone code.
>
> I did see OpenBSD had a -ftrampolines flag, I'm not quite sure what
> it exactly does, but it may be related to this.

It is, trampolines are used to implement nested functions in cases
like ours.

--
Marco

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Tue, Jan 29, 2008 at 06:37:50PM -0600, jakllsch@kollasch.net wrote:
> > > > > /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> > > > > `__enable_execute_stack':
> > > > > _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> > > > > _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> > > > > 
> > > > > Both of those symbols are defined in libc.a, so I included that, too.
> > > > > Any ideas?
> > > > 
> > > > Do you see this line in ./configure ?
> > > > 
> > > >   checking whether `gcc' accepts `-fstack-protector'... yes
> > > 
> > > Yes.
> > > 
> > > Apparently -fno-stack-protector doesn't actually work the way
> > > I/we would expect.
> > > 
> > > But, I'm surprised that this doesn't affect the building
> > > of NetBSD's own 'stand' code.
> > 
> > Maybe we need another flag?  Could you check the link command in NetBSD's
> > kernel ?
> 
> I couldn't find anything that looked like it would fix this.
> 
> I believe the reason that NetBSD's own code isn't affected by this is
> because they (apparently) do not use nested functions in
> their stand-alone code.

Ok, so we need __enable_execute_stack then, I guess.  I find it strange that
some parts of libgcc can not be used in freestanding code.  Does this happen
with the unpatched version of GCC too?

Maybe we'll have add some dummy symbols to make it happy.  I don't think
any of this will be actually needed when kernel isn't present.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[walt]

On Wed, 2008-01-30 at 14:23 +0100, Robert Millan wrote:
> On Tue, Jan 29, 2008 at 06:37:50PM -0600, jakllsch@kollasch.net wrote:
> > > > > > /usr/lib/libgcc.a(_enable_execute_stack.o): In function
> > > > > > `__enable_execute_stack':
> > > > > > _enable_execute_stack.c:(.text+0x40): undefined reference to `__sysctl'
> > > > > > _enable_execute_stack.c:(.text+0x6f): undefined reference to `mprotect'
> > > > > > 
> > > > > > Both of those symbols are defined in libc.a, so I included that, too.
> > > > > > Any ideas?
> > > > > 
> > > > > Do you see this line in ./configure ?
> > > > > 
> > > > >   checking whether `gcc' accepts `-fstack-protector'... yes
> > > > 
> > > > Yes.
> > > > 
> > > > Apparently -fno-stack-protector doesn't actually work the way
> > > > I/we would expect.
> > > > 
> > > > But, I'm surprised that this doesn't affect the building
> > > > of NetBSD's own 'stand' code.
> > > 
> > > Maybe we need another flag?  Could you check the link command in NetBSD's
> > > kernel ?
> > 
> > I couldn't find anything that looked like it would fix this.
> > 
> > I believe the reason that NetBSD's own code isn't affected by this is
> > because they (apparently) do not use nested functions in
> > their stand-alone code.
> 
> Ok, so we need __enable_execute_stack then, I guess.  I find it strange that
> some parts of libgcc can not be used in freestanding code.  Does this happen
> with the unpatched version of GCC too?
> 
> Maybe we'll have add some dummy symbols to make it happy.  I don't think
> any of this will be actually needed when kernel isn't present.

I don't understand much of the above, but I did find this comment in the
NetBSD manpage for gcc:

In traditional UNIX systems, the signal trampoline, also referred to as
the ``sigcode'', is provided by the kernel and copied to the top of the
user's stack when a new process is created or a new program image is
exec'd.

Starting in NetBSD 2.0, the signal trampoline is provided by the system
C library.  This allows for more flexibility when the signal facility 
is extended, makes dealing with signals easier in debuggers, such as 
gdb(1), and may also enhance system security by allowing the kernel to 
disallow execution of code on the stack.

The signal trampoline is specified on a per-signal basis.  The correct
trampoline is selected automatically by the C library when a signal
handler is registered by a process.

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Wed, Jan 30, 2008 at 05:51:43AM -0800, walt wrote:
> > 
> > Ok, so we need __enable_execute_stack then, I guess.  I find it strange that
> > some parts of libgcc can not be used in freestanding code.  Does this happen
> > with the unpatched version of GCC too?
> > 
> > Maybe we'll have add some dummy symbols to make it happy.  I don't think
> > any of this will be actually needed when kernel isn't present.
> 
> I don't understand much of the above, but I did find this comment in the
> NetBSD manpage for gcc:
> 
> In traditional UNIX systems, the signal trampoline, also referred to as
> the ``sigcode'', is provided by the kernel and copied to the top of the
> user's stack when a new process is created or a new program image is
> exec'd.
> 
> Starting in NetBSD 2.0, the signal trampoline is provided by the system
> C library.  This allows for more flexibility when the signal facility 
> is extended, makes dealing with signals easier in debuggers, such as 
> gdb(1), and may also enhance system security by allowing the kernel to 
> disallow execution of code on the stack.
> 
> The signal trampoline is specified on a per-signal basis.  The correct
> trampoline is selected automatically by the C library when a signal
> handler is registered by a process.

This seems to be related to kernel<->user interaction.  Not much of our
concern I think.

Our problem, AFAICT, is that gcc/libgcc in NetBSD wants to rely on system
facilities to enable executable stack, but that operation is unnecessary
for freestanding code (unless someone setup paging and the NX bit?).

What happens if you define the missing symbols somewhere?  Any part of
the GRUB kernel will do (e.g. kern/main.c).  Just add them as empty
functions:

void
__sysctl (void)
{
}

void
mprotect (void)
{
}

and check if that works.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[walt]

Robert Millan wrote:
>>> Ok, so we need __enable_execute_stack then, I guess.  I find it strange that
>>> some parts of libgcc can not be used in freestanding code.  Does this happen
>>> with the unpatched version of GCC too?
>>>
>>> Maybe we'll have add some dummy symbols to make it happy....
> What happens if you define the missing symbols somewhere?  Any part of
> the GRUB kernel will do (e.g. kern/main.c).  Just add them as empty
> functions:
>
> void
> __sysctl (void)
> {
> }
>
> void
> mprotect (void)
> {
> }
>
> and check if that works.

BTW, I just discovered that this was discussed in a thread beginning
11-30-2005 but never finished.

Anyway, your suggestion helps, but not quite enough.  The c code now
compiles and links okay into *.mod files, but now __enable_execute_stack
shows up in several und-*.lst files, so that moddep.lst won't build.

I cheated by copying moddep.lst from my linux machine, and that allowed
the build of grub to finish, but now I get that same missing symbol when
I *run* grub. <sigh>

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Wed, Jan 30, 2008 at 08:38:23AM -0800, walt wrote:
> 
> BTW, I just discovered that this was discussed in a thread beginning
> 11-30-2005 but never finished.
> 
> Anyway, your suggestion helps, but not quite enough.  The c code now
> compiles and links okay into *.mod files, but now __enable_execute_stack
> shows up in several und-*.lst files, so that moddep.lst won't build.
> 
> I cheated by copying moddep.lst from my linux machine, and that allowed
> the build of grub to finish, but now I get that same missing symbol when
> I *run* grub. <sigh>

Try adding EXPORT_FUNC declarations in grub/kernel.h:

  void EXPORT_FUNC (foo) (void);

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[walt]

On Wed, 2008-01-30 at 08:38 -0800, walt wrote:
> Robert Millan wrote:
> ...

> > What happens if you define the missing symbols somewhere?  Any part of
> > the GRUB kernel will do (e.g. kern/main.c).  Just add them as empty
> > functions:
> >
> > void
> > __sysctl (void)
> > {
> > }
> >
> > void
> > mprotect (void)
> > {
> > }
> >
> > and check if that works.

> Anyway, your suggestion helps, but not quite enough.  The c code now
> compiles and links okay into *.mod files, but now __enable_execute_stack
> shows up in several und-*.lst files...

I've been hacking the various flags for elf.mod just to get *any* module
to link properly but no luck.  Here is the actual text printed by gcc 
as it links elf.mod:

gcc -m32 -nostdlib -lgcc -L/usr/lib  -Wl,-r,-d,-lgcc,-L/usr/lib -o
elf.mod pre-elf.o mod-elf.o

You can tell I'm desperate because I'm putting -lgcc everywhere I can
think of.  Here is the symbol in libgcc.a:

_enable_execute_stack.o:
00000000 T __enable_execute_stack

And here is the symbol table from the resulting elf.mod:

# nm elf.mod 
         U __enable_execute_stack   <======= WTF?
0000038a T grub_elf32_load
000003fe T grub_elf32_size

Okay, I found one *more* place to put libgcc:

gcc -m32 -nostdlib -lgcc -L/usr/lib  -Wl,-r,-d,-lgcc,-L/usr/lib -o
elf.mod pre-elf.o mod-elf.o /usr/lib/libgcc.a

Here is the puzzling (to me) result:

# nm elf.mod 
00000784 T __enable_execute_stack
         U __sysctl
0000038a T grub_elf32_load
000003fe T grub_elf32_size

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Wed, Jan 30, 2008 at 11:06:50AM -0800, walt wrote:
> 
> > Anyway, your suggestion helps, but not quite enough.  The c code now
> > compiles and links okay into *.mod files, but now __enable_execute_stack
> > shows up in several und-*.lst files...
> 
> I've been hacking the various flags for elf.mod just to get *any* module
> to link properly but no luck.  Here is the actual text printed by gcc 
> as it links elf.mod:

You don't need any of that.  Just make __enable_execute_stack appear in
def-kernel.lst.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[jakllsch]

On Wed, Jan 30, 2008 at 11:06:50AM -0800, walt wrote:
> 
> You can tell I'm desperate because I'm putting -lgcc everywhere I can
> think of.  Here is the symbol in libgcc.a:

I (like I found in pkgsrc-wip/grub2) am using a fake __enable_execute_stack(),
-lgcc isn't necessary.

	Jonathan Kollasch

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Wed, Jan 30, 2008 at 03:55:59PM -0600, jakllsch@kollasch.net wrote:
> On Wed, Jan 30, 2008 at 11:06:50AM -0800, walt wrote:
> > 
> > You can tell I'm desperate because I'm putting -lgcc everywhere I can
> > think of.  Here is the symbol in libgcc.a:
> 
> I (like I found in pkgsrc-wip/grub2) am using a fake __enable_execute_stack(),

Ok, we could add that dummy __enable_execute_stack() but ...

> -lgcc isn't necessary.

... if we remove -lgcc, what happens if it becomes necessary for other versions
of GCC?  Or does gcc already add libgcc per demand?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[walt]

On Thu, 2008-01-31 at 12:03 +0100, Robert Millan wrote:
> On Wed, Jan 30, 2008 at 03:55:59PM -0600, jakllsch@kollasch.net wrote:
> > On Wed, Jan 30, 2008 at 11:06:50AM -0800, walt wrote:
> > > 
> > > You can tell I'm desperate because I'm putting -lgcc everywhere I can
> > > think of.  Here is the symbol in libgcc.a:
> > 
> > I (like I found in pkgsrc-wip/grub2) am using a fake __enable_execute_stack(),
> 
> Ok, we could add that dummy __enable_execute_stack() but ...
> 
> > -lgcc isn't necessary.
> 
> ... if we remove -lgcc, what happens if it becomes necessary for other versions
> of GCC?  Or does gcc already add libgcc per demand?

I got grub2 compiled on NetBSD using the fake in those pkgsrc patches.
I regenerated the patches below against grub2 cvs.  They are the same
ideas you suggested earlier, I just didn't use them all at the same
time:

Index: include/grub/mm.h
===================================================================
RCS file: /sources/grub/grub2/include/grub/mm.h,v
retrieving revision 1.8
diff -u -r1.8 mm.h
--- include/grub/mm.h   21 Jul 2007 23:32:22 -0000      1.8
+++ include/grub/mm.h   1 Feb 2008 21:04:27 -0000
@@ -33,6 +33,7 @@
 void EXPORT_FUNC(grub_free) (void *ptr);
 void *EXPORT_FUNC(grub_realloc) (void *ptr, grub_size_t size);
 void *EXPORT_FUNC(grub_memalign) (grub_size_t align, grub_size_t size);
+void EXPORT_FUNC(__enable_execute_stack) (void);
 
 /* For debugging.  */
 #if defined(MM_DEBUG) && !defined(GRUB_UTIL)
Index: kern/mm.c
===================================================================
RCS file: /sources/grub/grub2/kern/mm.c,v
retrieving revision 1.19
diff -u -r1.19 mm.c
--- kern/mm.c   23 Jan 2008 14:57:40 -0000      1.19
+++ kern/mm.c   1 Feb 2008 21:04:28 -0000
@@ -66,6 +66,9 @@
 #include <grub/disk.h>
 #include <grub/dl.h>
 
+void __enable_execute_stack(void);
+void __enable_execute_stack(void) {}
+
 #ifdef MM_DEBUG
 # undef grub_malloc
 # undef grub_realloc

Re: Compiling grub2 on *BSD?

[Robert Millan]

On Fri, Feb 01, 2008 at 01:11:06PM -0800, walt wrote:
> > ... if we remove -lgcc, what happens if it becomes necessary for other versions
> > of GCC?  Or does gcc already add libgcc per demand?
> 
> I got grub2 compiled on NetBSD using the fake in those pkgsrc patches.
> I regenerated the patches below against grub2 cvs.  They are the same
> ideas you suggested earlier, I just didn't use them all at the same
> time:

Nice.  Some comments:

> --- include/grub/mm.h   21 Jul 2007 23:32:22 -0000      1.8
> +++ include/grub/mm.h   1 Feb 2008 21:04:27 -0000
> @@ -33,6 +33,7 @@
>  void EXPORT_FUNC(grub_free) (void *ptr);
>  void *EXPORT_FUNC(grub_realloc) (void *ptr, grub_size_t size);
>  void *EXPORT_FUNC(grub_memalign) (grub_size_t align, grub_size_t size);
> +void EXPORT_FUNC(__enable_execute_stack) (void);
>  
>  /* For debugging.  */
>  #if defined(MM_DEBUG) && !defined(GRUB_UTIL)
> Index: kern/mm.c
> ===================================================================
> RCS file: /sources/grub/grub2/kern/mm.c,v
> retrieving revision 1.19
> diff -u -r1.19 mm.c
> --- kern/mm.c   23 Jan 2008 14:57:40 -0000      1.19
> +++ kern/mm.c   1 Feb 2008 21:04:28 -0000
> @@ -66,6 +66,9 @@
>  #include <grub/disk.h>
>  #include <grub/dl.h>
>  
> +void __enable_execute_stack(void);

Why this declaration; doesn't it duplicate the one in mm.h?

> +void __enable_execute_stack(void) {}

We need a comment explaining why we have this "weird" function in GRUB.
Also, please follow grub code style:

void
foo (void)
{
}

And, could you provide a ChangeLog entry with your patch?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: Compiling grub2 on *BSD?

[Marco Gerards]

walt <wa1ter@myrealbox.com> writes:

> On Thu, 2008-01-31 at 12:03 +0100, Robert Millan wrote:
>> On Wed, Jan 30, 2008 at 03:55:59PM -0600, jakllsch@kollasch.net wrote:
>> > On Wed, Jan 30, 2008 at 11:06:50AM -0800, walt wrote:
>> > > 
>> > > You can tell I'm desperate because I'm putting -lgcc everywhere I can
>> > > think of.  Here is the symbol in libgcc.a:
>> > 
>> > I (like I found in pkgsrc-wip/grub2) am using a fake __enable_execute_stack(),
>> 
>> Ok, we could add that dummy __enable_execute_stack() but ...
>> 
>> > -lgcc isn't necessary.
>> 
>> ... if we remove -lgcc, what happens if it becomes necessary for other versions
>> of GCC?  Or does gcc already add libgcc per demand?
>
> I got grub2 compiled on NetBSD using the fake in those pkgsrc patches.
> I regenerated the patches below against grub2 cvs.  They are the same
> ideas you suggested earlier, I just didn't use them all at the same
> time:
>
> Index: include/grub/mm.h
> ===================================================================
> RCS file: /sources/grub/grub2/include/grub/mm.h,v
> retrieving revision 1.8
> diff -u -r1.8 mm.h
> --- include/grub/mm.h   21 Jul 2007 23:32:22 -0000      1.8
> +++ include/grub/mm.h   1 Feb 2008 21:04:27 -0000
> @@ -33,6 +33,7 @@
>  void EXPORT_FUNC(grub_free) (void *ptr);
>  void *EXPORT_FUNC(grub_realloc) (void *ptr, grub_size_t size);
>  void *EXPORT_FUNC(grub_memalign) (grub_size_t align, grub_size_t size);
> +void EXPORT_FUNC(__enable_execute_stack) (void);

Is this arch specific?
  
>  /* For debugging.  */
>  #if defined(MM_DEBUG) && !defined(GRUB_UTIL)
> Index: kern/mm.c
> ===================================================================
> RCS file: /sources/grub/grub2/kern/mm.c,v
> retrieving revision 1.19
> diff -u -r1.19 mm.c
> --- kern/mm.c   23 Jan 2008 14:57:40 -0000      1.19
> +++ kern/mm.c   1 Feb 2008 21:04:28 -0000
> @@ -66,6 +66,9 @@
>  #include <grub/disk.h>
>  #include <grub/dl.h>
>  
> +void __enable_execute_stack(void);
> +void __enable_execute_stack(void) {}
> +

Is this arch specific?  Please put braces on separate lines.

Won't this break on GNU/Linux?  Or on other architectures?  Did you
test this?

--
Marco