Kernel fixes for Cygwin

PATCH In review Christian Franke 4 participants 14 messages Jul 20, 2008 – Jul 22, 2008

Share & tools

Christian Franke

author

2008-07-20 20:52 UTC #0 lore raw

PATCH 1/1 Kernel fixes for Cygwin

This adds Cygwin support to kernel sources. It handles the issues introduced by PE->ELF conversion and adds support for HAVE_ASM_USCORE.

Christian

2007-07-20 Christian Franke <[email protected]>

* include/grub/dl.h: Remove .previous, gas supports this only for ELF format.

* include/grub/symbol.h [__CYGWIN__] (#define FUNCTION/VARIABLE):
Remove .type, gas supports this only for ELF format.

* kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE case for these symbols.
(grub_dl_resolve_dependencies): Add check for trailing nullbytes in symbol table. This fixes an infinite loop if table is zero filled.

* kern/i386/dl.c [__CYGWIN__] (fix_pc_rel_relocation): New function to fix bad PC relative relocation produced by objcopy. [__CYGWIN__] (grub_arch_dl_relocate_symbols): Add fix of PC relative relocation. (grub_arch_dl_relocate_symbols): Abort on unknown relocation type.

include/grub/dl.h +3 -2

index b630c6f..3029f95 100644
@@ -40,11 +40,12 @@ grub_##name##_fini (void) { grub_mod_fini (); } \
 static void \
 grub_mod_fini (void)
 
+/* Note: .previous not supported for non-ELF targets.  */
 #define GRUB_MOD_NAME(name)	\
-__asm__ (".section .modname,\"S\"\n.string \"" #name "\"\n.previous")
+__asm__ (".section .modname\n.string \"" #name "\"\n")
 
 #define GRUB_MOD_DEP(name)	\
-__asm__ (".section .moddeps,\"S\"\n.string \"" #name "\"\n.previous")
+__asm__ (".section .moddeps\n.string \"" #name "\"\n")
 
 struct grub_dl_segment
 {

include/grub/symbol.h +6 -0

index aa0ea5a..72209d1 100644
@@ -28,8 +28,14 @@
 # define EXT_C(sym)	sym
 #endif
 
+#ifndef __CYGWIN__
 #define FUNCTION(x)	.globl EXT_C(x) ; .type EXT_C(x), "function" ; EXT_C(x):
 #define VARIABLE(x)	.globl EXT_C(x) ; .type EXT_C(x), "object" ; EXT_C(x):
+#else
+/* .type not supported for non-ELF targets.  XXX: Check this in configure? */
+#define FUNCTION(x)	.globl EXT_C(x) ; EXT_C(x):
+#define VARIABLE(x)	.globl EXT_C(x) ; EXT_C(x):
+#endif
 
 /* Mark an exported symbol.  */
 #ifndef GRUB_SYMBOL_GENERATOR

kern/dl.c +39 ~~-16~~

89 lines not on this page · raw message

kern/i386/dl.c +30 -0

index e9e43e5..94d1510 100644
@@ -37,6 +37,28 @@ grub_arch_dl_check_header (void *ehdr)
   return GRUB_ERR_NONE;
 }
 
+#ifdef __CYGWIN__
+/* Fix PC relative relocation.  Objcopy does not adjust
+the addent when converting from pe-i386 to elf32-i386.  */
+static int
+fix_pc_rel_relocation (Elf32_Word *addr)
+{
+  /* To be safe, check instruction first.  */
+  const unsigned char * pc = (const unsigned char *)addr - 1;
+  if (!(*pc == 0xe8/*call*/ || *pc == 0xe9/*jmp*/))
+    return grub_error (GRUB_ERR_BAD_MODULE, "unknown pc-relative instruction %02x", *pc);
+  /* Check and adjust offset.  */
+  if (*addr != (Elf32_Word)-4)
+    {
+      if (*addr != 0)
+	return grub_error (GRUB_ERR_BAD_MODULE, "invalid pc-relative relocation base %lx",
+			   (long)(*addr));
+      *addr = (Elf32_Word)-4;
+    }
+  return GRUB_ERR_NONE;
+}
+#endif
+
 /* Relocate symbols.  */
 grub_err_t
 grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)

@@ -99,9 +121,17 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 		    break;
 
 		  case R_386_PC32:
+#ifdef __CYGWIN__
+		    if (fix_pc_rel_relocation (addr))
+		      return grub_errno;
+#endif
 		    *addr += (sym->st_value - (Elf32_Word) seg->addr
 			      - rel->r_offset);
 		    break;
+
+		  default:
+		    return grub_error (GRUB_ERR_BAD_MODULE, "unknown relocation type %x.",
+				       ELF32_R_TYPE (rel->r_info));
 		  }
 	      }
 	  }

Bean

2008-07-21 07:02 UTC #1 lore raw

Quoted 19 lines · Christian Franke

This adds Cygwin support to kernel sources. It handles the issues introduced by PE->ELF conversion and adds support for HAVE_ASM_USCORE.
Christian
2007-07-20 Christian Franke <[email protected]>
* include/grub/dl.h: Remove .previous, gas supports this only for ELF format.
* include/grub/symbol.h [__CYGWIN__] (#define FUNCTION/VARIABLE):
Remove .type, gas supports this only for ELF format.
* kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE case for these symbols.
(grub_dl_resolve_dependencies): Add check for trailing nullbytes in symbol table. This fixes an infinite loop if table is zero filled.

Quoted 5 lines

* kern/i386/dl.c [__CYGWIN__] (fix_pc_rel_relocation): New function to fix bad PC relative relocation produced by objcopy. [__CYGWIN__] (grub_arch_dl_relocate_symbols): Add fix of PC relative
relocation.
(grub_arch_dl_relocate_symbols): Abort on unknown relocation type.

Hi,

I'm not fond of fixing elf relocation bug in dl.c. First of all, this is an objcopy bug, it may be changed in the future, also, it makes modules compiled by cygwin not compatible with those compiled by elf gcc.

Perhaps you can write a small tool, which fix various bugs after using objcopy to generate the module file.

Signature · 1 line

Bean

Bean

2008-07-21 07:25 UTC #2 lore raw

Quoted 29 lines · Bean

Quoted 19 lines · Christian Franke
This adds Cygwin support to kernel sources. It handles the issues introduced by PE->ELF conversion and adds support for HAVE_ASM_USCORE.
Christian
2007-07-20 Christian Franke <[email protected]>
* include/grub/dl.h: Remove .previous, gas supports this only for ELF format.
* include/grub/symbol.h [__CYGWIN__] (#define FUNCTION/VARIABLE):
Remove .type, gas supports this only for ELF format.
* kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE case for these symbols.
(grub_dl_resolve_dependencies): Add check for trailing nullbytes in symbol table. This fixes an infinite loop if table is zero filled.
Quoted 5 lines
* kern/i386/dl.c [__CYGWIN__] (fix_pc_rel_relocation): New function to fix bad PC relative relocation produced by objcopy. [__CYGWIN__] (grub_arch_dl_relocate_symbols): Add fix of PC relative
relocation.
(grub_arch_dl_relocate_symbols): Abort on unknown relocation type.
Hi,

Quoted 7 lines

I'm not fond of fixing elf relocation bug in dl.c. First of all, this is an objcopy bug, it may be changed in the future, also, it makes modules compiled by cygwin not compatible with those compiled by elf gcc.
Perhaps you can write a small tool, which fix various bugs after using objcopy to generate the module file.

Hi,

BTW, if you have time, you can consider writing a tool that convert pe to elf directly, thus avoiding objcopy altogether. This shouldn't be too difficult, you can take a look at util/i386/efi/grub-mkimage.c, which does exactly the opposite, converting elf to pe32.

Signature · 1 line

Bean

Christian Franke

author

2008-07-21 10:34 UTC #3 lore raw

Quoted 9 lines · Bean

Hi,
BTW, if you have time, you can consider writing a tool that convert pe to elf directly, thus avoiding objcopy altogether. This shouldn't be too difficult, you can take a look at util/i386/efi/grub-mkimage.c, which does exactly the opposite, converting elf to pe32.

Hi,

due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here. It was rejected, see this thread:
http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00174.html

It should be possible to build a smaller tool that only fixes the relocation info in the generated ELF. But due to the C++ redesign of smartmontools, my open source time is somewhat limited in the moment :-)

To provide Cygwin support for the next release, I would suggest to accept the small hack in kern/i386/dl.c for now. It is already available it and it works. It can be removed when a tool is available. Or accept the converter mentioned above.

Christian

Bean

2008-07-21 10:57 UTC #4 lore raw

Quoted 7 lines · Christian Franke

Hi,
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here. It was rejected, see this thread:
http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00174.html

Hi,

I can't find the source code for your converter, perhaps you can repost it ?

IMO, it's better to use our own tool, if it's not very complicated. I learn this lesson when writing efi support for x86_64. The gnu-efi toolchain can produce efi image, but it turns out to have some limitation on relocation, and it's hard to fix. I end up modifying grub-mkimage to support converting elf to pe32+. Relying on external tool means we are at the mercy of them, it's annoying when they have issue, as we can't expect upstream to fix it soon.

Signature · 1 line

Bean

Christian Franke

author

2008-07-22 16:59 UTC #5 lore raw

Quoted 12 lines · Bean

Quoted 8 lines · Christian Franke
Hi,
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here. It was rejected, see this thread:
http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00174.html

Quoted 5 lines

Hi,
I can't find the source code for your converter, perhaps you can repost it ?

I did not post it in the original thread from 2007-11.

It is actually a reduced version of objcopy with
- stuff not necessary for object format conversion removed,
- some functions from shared modules added to make it self-contained,
- the fix for the pc-relative relocation added to

It was intended as starting point to get rid of the hack in kern/i386/dl.c.

The hard work is still done by BFD.

Quoted 9 lines

IMO, it's better to use our own tool, if it's not very complicated. I learn this lesson when writing efi support for x86_64. The gnu-efi toolchain can produce efi image, but it turns out to have some limitation on relocation, and it's hard to fix. I end up modifying grub-mkimage to support converting elf to pe32+. Relying on external tool means we are at the mercy of them, it's annoying when they have issue, as we can't expect upstream to fix it soon.

Yes, an own tool is probably best in the long term.

But such a tool will be likely not available for GRUB 1.97 tarball release.

Christian

Javier Martín

2008-07-21 11:02 UTC #6 lore raw

In reply to Christian Franke

Quoted 8 lines · Christian Franke

Quoted 4 lines · Bean
BTW, if you have time, you can consider writing a tool that convert pe to elf directly, thus avoiding objcopy altogether. This shouldn't be too difficult, you can take a look at util/i386/efi/grub-mkimage.c, which does exactly the opposite, converting elf to pe32.
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here.

Why do we even consider a PE->ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf "cross compiler" built first, then use that for the bootloader programs and the normal gcc for tools. Even a "naked" (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's "freestanding"). That way, we are free from "objcopy bugs" or "BFD design limitations".

This scheme is kinda like what's done with x86_64-pc-linux, with the difference that in that case we use the same gcc for both "host" and "target" by adding -m32 to the latter, but it's essentially the same concept.

Habbit

Christian Franke

author

2008-07-21 11:16 UTC #7 lore raw

Quoted 11 lines · Javier Martín

Quoted 10 lines · Christian Franke
Quoted 6 lines · Bean
BTW, if you have time, you can consider writing a tool that convert pe to elf directly, thus avoiding objcopy altogether. This shouldn't be too difficult, you can take a look at util/i386/efi/grub-mkimage.c, which does exactly the opposite, converting elf to pe32.
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here.

Quoted 9 lines

Why do we even consider a PE->ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf "cross compiler" built first, then use that for the bootloader programs and the normal gcc for tools. Even a "naked" (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's "freestanding").
That way, we are free from "objcopy bugs" or "BFD design limitations".

Yes, but this is not yet possible for the grub2 package in the Cygwin distro, see:
http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00263.html

Christian

Pavel Roskin

2008-07-21 15:51 UTC #8 lore raw

In reply to Javier Martín

Quoted 10 lines · Javier Martín

Quoted 2 lines · Christian Franke
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here.
Why do we even consider a PE->ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf "cross compiler" built first, then use that for the bootloader programs and the normal gcc for tools. Even a "naked" (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's "freestanding"). That way, we are free from "objcopy bugs" or "BFD design limitations".

Well, if we want users to recompile their toolchain first, it's too much to ask.

Maybe we could treat ELF header like a multiboot header? That means that we write the header fields in the assembly language, substitute the necessary variables and ask objcopy to make a raw binary that would actually be an ELF file?

We could actually do it for all platforms, so that we won't depend on the object file format.

Signature · 2 lines

Regards,
Pavel Roskin

Javier Martín

2008-07-21 16:50 UTC #9 lore raw

Quoted 12 lines · Pavel Roskin

Quoted 10 lines · Javier Martín
Quoted 2 lines · Christian Franke
due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here.
Why do we even consider a PE->ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf "cross compiler" built first, then use that for the bootloader programs and the normal gcc for tools. Even a "naked" (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's "freestanding"). That way, we are free from "objcopy bugs" or "BFD design limitations".

Quoted 2 lines

Well, if we want users to recompile their toolchain first, it's too much to ask.

I think it is not, since people building GRUB in Cygwin are not exactly newcomers to the land of compiling: this package requires that its files and modules be in ELF format; your compiler does not do it, so you need another compiler. End of the problem.

Of course, another way to go could be to allow the bootloader part of GRUB to be built in PE format: it would "just" be a matter of writing the PE counterparts to kern/elf.c and abstracting kern/dl.c "a bit" (i.e. a lot of work). The downside to this, apart from the unspecified work required, is that Windows-built i386-pc-pe modules are no longer compatible with Linux-built i386-pc-elf. Not a showstopper, but might require a sober thinking. As I have a lot of free time right now, I'll try to think whether it's possible or not.

Quoted 4 lines

Maybe we could treat ELF header like a multiboot header? That means that we write the header fields in the assembly language, substitute the necessary variables and ask objcopy to make a raw binary that would actually be an ELF file?

As far as I understand the ELF format, this would be too complex to get
right: there's a lot of info in there.

Quoted 4 lines

We could actually do it for all platforms, so that we won't depend on the object file format.

Pavel Roskin

2008-07-21 17:03 UTC #10 lore raw

Quoted 8 lines · Javier Martín

Of course, another way to go could be to allow the bootloader part of GRUB to be built in PE format: it would "just" be a matter of writing the PE counterparts to kern/elf.c and abstracting kern/dl.c "a bit" (i.e. a lot of work). The downside to this, apart from the unspecified work required, is that Windows-built i386-pc-pe modules are no longer compatible with Linux-built i386-pc-elf. Not a showstopper, but might require a sober thinking. As I have a lot of free time right now, I'll try to think whether it's possible or not.

I think it's important to have a consistent format for modules.

Quoted 6 lines

Quoted 4 lines
Maybe we could treat ELF header like a multiboot header? That means that we write the header fields in the assembly language, substitute the necessary variables and ask objcopy to make a raw binary that would actually be an ELF file?
As far as I understand the ELF format, this would be too complex to get
right: there's a lot of info in there.

If ELF is too complex to write manually, we can use another format everywhere. It could be something GRUB specific. But I think we should try to use ELF, as it's widespread and extensible.

Signature · 2 lines

Regards,
Pavel Roskin

Javier Martín

2008-07-21 17:23 UTC #11 lore raw

Quoted 11 lines · Pavel Roskin

Quoted 8 lines · Javier Martín
Of course, another way to go could be to allow the bootloader part of GRUB to be built in PE format: it would "just" be a matter of writing the PE counterparts to kern/elf.c and abstracting kern/dl.c "a bit" (i.e. a lot of work). The downside to this, apart from the unspecified work required, is that Windows-built i386-pc-pe modules are no longer compatible with Linux-built i386-pc-elf. Not a showstopper, but might require a sober thinking. As I have a lot of free time right now, I'll try to think whether it's possible or not.

Quoted 1 line

I think it's important to have a consistent format for modules.

I thought that two hours ago, but now I find no advantage other than the sharing of modules between similar computers.

Pavel Roskin

2008-07-21 20:00 UTC #12 lore raw

Quoted 5 lines · Javier Martín

Quoted 1 line · Pavel Roskin
I think it's important to have a consistent format for modules.
I thought that two hours ago, but now I find no advantage other than the sharing of modules between similar computers.

Sharing modules is unsafe.

My concern is sharing the code. If we use different formats, there are more place for bugs to hide or to appear. There is more work to test changes that can affect module loading.

Signature · 2 lines

Regards,
Pavel Roskin

Christian Franke

author

2008-07-21 10:22 UTC #13 lore raw

In reply to Bean

Quoted 29 lines · Bean

Quoted 20 lines · Christian Franke
This adds Cygwin support to kernel sources. It handles the issues introduced by PE->ELF conversion and adds support for HAVE_ASM_USCORE.
Christian
2007-07-20 Christian Franke <[email protected]>
* include/grub/dl.h: Remove .previous, gas supports this only for ELF format.
* include/grub/symbol.h [__CYGWIN__] (#define FUNCTION/VARIABLE):
Remove .type, gas supports this only for ELF format.
* kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE case for these symbols.
(grub_dl_resolve_dependencies): Add check for trailing nullbytes in symbol table. This fixes an infinite loop if table is zero filled.
Quoted 6 lines
* kern/i386/dl.c [__CYGWIN__] (fix_pc_rel_relocation): New function to fix bad PC relative relocation produced by objcopy. [__CYGWIN__] (grub_arch_dl_relocate_symbols): Add fix of PC relative relocation.
(grub_arch_dl_relocate_symbols): Abort on unknown relocation type.

Quoted 4 lines

Hi,
I'm not fond of fixing elf relocation bug in dl.c. ...

Hi,

I agree that kern/i386/dl.c:fix_pc_rel_relocation() is an ugly hack and shall be removed when a better solution is available.

Quoted 3 lines

... First of all, this
is an objcopy bug, it may be changed in the future, ...

My patch for objcopy was rejected upstream. According to binutils mailing list, this is not an objcopy bug, but a BFD design limitation:
http://sourceware.org/ml/binutils/2007-10/msg00306.html

So I don't expect a fix before time_t wraps around ...

Quoted 4 lines

... also, it makes
modules compiled by cygwin not compatible with those compiled by elf gcc.

The fix_pc_rel_relocation() works also with correct ELF files, because the (-4) fix is only applied if necessary.

Supporting Modules compiled with ELF gcc would also require to handle the different syntax of C-Symbols (HAVE_ASM_USCORE or NOT).

Actually my first version of supported this by aliasing symbols with/without underscore in kern/dl.c. It was tested with modules compiled with Linux gcc loaded by kernel.img compiled with Cygwin gcc and vice versa and all this *worked*.

But according to a comment from Robert, this is possibly useless or not desired, see:
http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00152.html

So I removed this (non?-)feature for now. Would be easy to re-add later.

Christian