Warnings in kern/dl.c

Warnings in kern/dl.c

[Pavel Roskin]

Hello, Bean!

I think this change from the x86_64 EFI patch should be reverted:

         * kern/dl.c (GRUB_CPU_SIZEOF_VOID_P): Changed to
          GRUB_TARGET_SIZEOF_VOID_P.

It causes warnings when compiling grub-emu on x64_64 for i386:

kern/dl.c: In function 'grub_dl_resolve_symbols':
kern/dl.c:357: warning: cast from pointer to integer of different size
kern/dl.c:368: warning: cast from pointer to integer of different size
kern/dl.c:370: warning: cast to pointer from integer of different size
kern/dl.c:376: warning: cast from pointer to integer of different size
kern/dl.c:378: warning: cast to pointer from integer of different size
kern/dl.c:382: warning: cast to pointer from integer of different size
kern/dl.c:384: warning: cast to pointer from integer of different size
kern/dl.c:389: warning: cast from pointer to integer of different size

I'm really close to eliminating all warnings, and such additions are  
not making me happy.

The warnings could be suppressed somehow, but I don't think it's the  
right thing to do.  64-bit grub-emu cannot load 32-bit modules (and  
vice versa).  64-bit grub-emu could load 64-bit modules in principle,  
although we don't compile them.   We could compile special grub-emu  
modules, but I would not spend any serious effort on it.

Anyway, I would prefer that we compile support for loading 64-bit  
modules and possibly catch some useful warnings relevant to 64-bit  
platforms (even if our target is 32-bit).  It's better than to  
"implement" cross-architecture loading of modules that would not work  
anyway by filling the code with casts.  It's better than to tolerate  
several warnings.  Either increases the risk that a real warning would  
be missed.

-- 
Regards,
Pavel Roskin

Re: Warnings in kern/dl.c

[Bean]

On Fri, Jul 18, 2008 at 1:38 PM, Pavel Roskin <proski@gnu.org> wrote:
> Hello, Bean!
>
> I think this change from the x86_64 EFI patch should be reverted:
>
>        * kern/dl.c (GRUB_CPU_SIZEOF_VOID_P): Changed to
>         GRUB_TARGET_SIZEOF_VOID_P.
>
> It causes warnings when compiling grub-emu on x64_64 for i386:
>
> kern/dl.c: In function 'grub_dl_resolve_symbols':
> kern/dl.c:357: warning: cast from pointer to integer of different size
> kern/dl.c:368: warning: cast from pointer to integer of different size
> kern/dl.c:370: warning: cast to pointer from integer of different size
> kern/dl.c:376: warning: cast from pointer to integer of different size
> kern/dl.c:378: warning: cast to pointer from integer of different size
> kern/dl.c:382: warning: cast to pointer from integer of different size
> kern/dl.c:384: warning: cast to pointer from integer of different size
> kern/dl.c:389: warning: cast from pointer to integer of different size
>
> I'm really close to eliminating all warnings, and such additions are not
> making me happy.
>
> The warnings could be suppressed somehow, but I don't think it's the right
> thing to do.  64-bit grub-emu cannot load 32-bit modules (and vice versa).
>  64-bit grub-emu could load 64-bit modules in principle, although we don't
> compile them.   We could compile special grub-emu modules, but I would not
> spend any serious effort on it.
>
> Anyway, I would prefer that we compile support for loading 64-bit modules
> and possibly catch some useful warnings relevant to 64-bit platforms (even
> if our target is 32-bit).  It's better than to "implement"
> cross-architecture loading of modules that would not work anyway by filling
> the code with casts.  It's better than to tolerate several warnings.  Either
> increases the risk that a real warning would be missed.

Hi,

This is used for cross complication. For example, when we compile
x86_64-efi in i386, the host system is i386, but target system is
64-bit. I have been compiling the efi image using this method, there
is no problem.

As for grub-emu, does it matter if we can't load external modules ?
The import module are all static linked. Besides, some of the modules
depend on real mode component, so we can't load them anyway.

-- 
Bean

Re: Warnings in kern/dl.c

[Bean]

On Fri, Jul 18, 2008 at 2:18 PM, Bean <bean123ch@gmail.com> wrote:
> On Fri, Jul 18, 2008 at 1:38 PM, Pavel Roskin <proski@gnu.org> wrote:
>> Hello, Bean!
>>
>> I think this change from the x86_64 EFI patch should be reverted:
>>
>>        * kern/dl.c (GRUB_CPU_SIZEOF_VOID_P): Changed to
>>         GRUB_TARGET_SIZEOF_VOID_P.
>>
>> It causes warnings when compiling grub-emu on x64_64 for i386:
>>
>> kern/dl.c: In function 'grub_dl_resolve_symbols':
>> kern/dl.c:357: warning: cast from pointer to integer of different size
>> kern/dl.c:368: warning: cast from pointer to integer of different size
>> kern/dl.c:370: warning: cast to pointer from integer of different size
>> kern/dl.c:376: warning: cast from pointer to integer of different size
>> kern/dl.c:378: warning: cast to pointer from integer of different size
>> kern/dl.c:382: warning: cast to pointer from integer of different size
>> kern/dl.c:384: warning: cast to pointer from integer of different size
>> kern/dl.c:389: warning: cast from pointer to integer of different size
>>
>> I'm really close to eliminating all warnings, and such additions are not
>> making me happy.
>>
>> The warnings could be suppressed somehow, but I don't think it's the right
>> thing to do.  64-bit grub-emu cannot load 32-bit modules (and vice versa).
>>  64-bit grub-emu could load 64-bit modules in principle, although we don't
>> compile them.   We could compile special grub-emu modules, but I would not
>> spend any serious effort on it.
>>
>> Anyway, I would prefer that we compile support for loading 64-bit modules
>> and possibly catch some useful warnings relevant to 64-bit platforms (even
>> if our target is 32-bit).  It's better than to "implement"
>> cross-architecture loading of modules that would not work anyway by filling
>> the code with casts.  It's better than to tolerate several warnings.  Either
>> increases the risk that a real warning would be missed.
>
> Hi,
>
> This is used for cross complication. For example, when we compile
> x86_64-efi in i386, the host system is i386, but target system is
> 64-bit. I have been compiling the efi image using this method, there
> is no problem.
>
> As for grub-emu, does it matter if we can't load external modules ?
> The import module are all static linked. Besides, some of the modules
> depend on real mode component, so we can't load them anyway.

Hi,

BTW, there is another way to solve this. For grub-emu, we define
GRUB_TARGET_SIZEOF_VOID_P the same as GRUB_CPU_SIZEOF_VOID_P, doesn't
that fix the problem ?

-- 
Bean

Re: Warnings in kern/dl.c

[Bean]

Hi,

I have another thought about this issue, it's actually caused by
different working environment for the same file. For kern/dl.c, there
are three modes:

1. Target mode
When it's used in boot environment, it's in target mode, for example, i386.

2. Host mode
When it's used in tools like grub-emu, it's in host mode, for example, x86_64.

3. Cross mode
When it's used in grub-mkimage, it's in cross mode. It runs in x86_64,
but produce image that will be used in i386.

For these three modes, we have only one controlling macro, GRUB_UTIL,
so it can't cover all cases. To solve this, we can either:

a. Add new macro, for example GRUB_CROSS or GRUB_MKIMAGE.

or

b. Drop one of the usage.

But actually, host mode is not useful, we can't load external modules
in grub-emu, so support 1 and 3 is enough.

BTW, kern/dl.c has issue as well. For example, grub_dl_resolve_symbol
returns void*. It should be Elf_Addr, so that it can expand to the
correct type according to GRUB_TARGET_SIZEOF_VOID_P.

-- 
Bean

Re: Warnings in kern/dl.c

[Bean]

Hi,

Oh, it seems kern/dl.c is not used by grub-mkimage, so it should be ok
to revert back to GRUB_CPU_SIZEOF_VOID_P.

--
Bean

Re: Warnings in kern/dl.c

[chaac]

Pavel Roskin [proski@gnu.org] kirjoitti: 
> The warnings could be suppressed somehow, but I don't think it's the  
> right thing to do.  64-bit grub-emu cannot load 32-bit modules (and  
> vice versa).  64-bit grub-emu could load 64-bit modules in principle,  
> although we don't compile them.   We could compile special grub-emu  
> modules, but I would not spend any serious effort on it.

Hi,

I think that using grub-emu might be a good idea for testing. I think there should be grub-emu architecture that could be compiled and in example there would be grub-emu specific disk layer, input layer and so on. So we could use grub-emu almost as same way we can do in real runtime environment. I know Marco will be using grub-emu like this on his GSoC project and I think it is a good thing too. But it needs some work of course.

We could do some special testing framework using this method. Eg. we could drive user input from special user input module that can be controlled from outside.

So please, do not at least remove support for using 64bit modules on 64bit systems :) (or otherwise make it harder to be supported later on)

Thanks,
Vesa Jääskeläinen

Re: Warnings in kern/dl.c

[Pavel Roskin]

On Fri, 2008-07-18 at 17:14 +0300, chaac@nic.fi wrote:

> I think that using grub-emu might be a good idea for testing. I think
> there should be grub-emu architecture that could be compiled and in
> example there would be grub-emu specific disk layer, input layer and
> so on. So we could use grub-emu almost as same way we can do in real
> runtime environment. I know Marco will be using grub-emu like this on
> his GSoC project and I think it is a good thing too. But it needs some
> work of course.

Yes, that would be a different grub-emu.  Maybe it will be compiled with
the target flags.  Maybe it will need to be disabled if cross-compiling.

> We could do some special testing framework using this method. Eg. we
> could drive user input from special user input module that can be
> controlled from outside.

I'd rather use qemu for testing, as it emulated the whole system.
Anyway, we'll see.

> So please, do not at least remove support for using 64bit modules on
> 64bit systems :) (or otherwise make it harder to be supported later
> on)

OK, let's keep it.  I just don't want to force loading foreign
architecture modules into the executable memory.

-- 
Regards,
Pavel Roskin

Re: Warnings in kern/dl.c

[Bean]

On Fri, Jul 18, 2008 at 11:20 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Fri, 2008-07-18 at 17:14 +0300, chaac@nic.fi wrote:
>
>> I think that using grub-emu might be a good idea for testing. I think
>> there should be grub-emu architecture that could be compiled and in
>> example there would be grub-emu specific disk layer, input layer and
>> so on. So we could use grub-emu almost as same way we can do in real
>> runtime environment. I know Marco will be using grub-emu like this on
>> his GSoC project and I think it is a good thing too. But it needs some
>> work of course.
>
> Yes, that would be a different grub-emu.  Maybe it will be compiled with
> the target flags.  Maybe it will need to be disabled if cross-compiling.
>
>> We could do some special testing framework using this method. Eg. we
>> could drive user input from special user input module that can be
>> controlled from outside.
>
> I'd rather use qemu for testing, as it emulated the whole system.
> Anyway, we'll see.
>
>> So please, do not at least remove support for using 64bit modules on
>> 64bit systems :) (or otherwise make it harder to be supported later
>> on)
>
> OK, let's keep it.  I just don't want to force loading foreign
> architecture modules into the executable memory.

Hi,

Actually, GRUB_CPU_SIZEOF_VOID_P is the right one to use. It would
only cause problem in grub-mkimage, but kern/dl.c is not used by
grub-mkimage.

-- 
Bean

Re: Warnings in kern/dl.c

[Pavel Roskin]

On Fri, 2008-07-18 at 23:35 +0800, Bean wrote:

> Actually, GRUB_CPU_SIZEOF_VOID_P is the right one to use. It would
> only cause problem in grub-mkimage, but kern/dl.c is not used by
> grub-mkimage.

Committed.

-- 
Regards,
Pavel Roskin