Re: grub_machine_set_prefix, --enable-mm-debug

Re: grub_machine_set_prefix, --enable-mm-debug

[Hollis Blanchard]

On Apr 25, 2006, at 3:08 PM, Yoshinori K. Okuji wrote:
>
> 	A new machine-specific function "grub_machine_set_prefix" is
> 	defined. This is called after loading modules, so that a prefix
> 	initialization can use modules.

I think you broke grub-emu with this change, because grub-emu uses 
kern/main.c, but nothing provides grub_machine_set_prefix() there.

> 	Also, this change adds an
> 	intensive debugging feature for the memory manager via the
> 	configure option "--enable-mm-debug".

Is there a reason not to use the runtime debugging infrastructure we 
have? Most of that is "intensive" as well...

-Hollis

Re: grub_machine_set_prefix, --enable-mm-debug

[Yoshinori K. Okuji]

On Wednesday 26 April 2006 01:22, Hollis Blanchard wrote:
> Is there a reason not to use the runtime debugging infrastructure we
> have? Most of that is "intensive" as well...

What do you mean by "the runtime debugging infrastructure"? If you mean 
grub_dprintf, it is not enough, since you cannot embed file names and line 
numbers where the functions are called.

Okuji

Re: grub_machine_set_prefix, --enable-mm-debug

[Marco Gerards]

"Yoshinori K. Okuji" <okuji@gnu.org> writes:

> On Wednesday 26 April 2006 01:22, Hollis Blanchard wrote:
>> Is there a reason not to use the runtime debugging infrastructure we
>> have? Most of that is "intensive" as well...
>
> What do you mean by "the runtime debugging infrastructure"? If you mean 
> grub_dprintf, it is not enough, since you cannot embed file names and line 
> numbers where the functions are called.

IIRC this is being done automatically.  Or do I misunderstand
something?

--
Marco

Re: grub_machine_set_prefix, --enable-mm-debug

[Yoshinori K. Okuji]

On Wednesday 26 April 2006 10:57, Marco Gerards wrote:
> "Yoshinori K. Okuji" <okuji@gnu.org> writes:
> > On Wednesday 26 April 2006 01:22, Hollis Blanchard wrote:
> >> Is there a reason not to use the runtime debugging infrastructure we
> >> have? Most of that is "intensive" as well...
> >
> > What do you mean by "the runtime debugging infrastructure"? If you mean
> > grub_dprintf, it is not enough, since you cannot embed file names and
> > line numbers where the functions are called.
>
> IIRC this is being done automatically.  Or do I misunderstand
> something?

Yes. My point is that I want to embed such information into where 
malloc/realloc/memalign/free is called, but not grub_dprintf is called. If 
you use grub_dprintf, you must embed grub_dprintf into so many places 
manually. My way does not require such rewriting.

Okuji

Re: grub_machine_set_prefix, --enable-mm-debug

[Marco Gerards]

"Yoshinori K. Okuji" <okuji@enbug.org> writes:

> On Wednesday 26 April 2006 10:57, Marco Gerards wrote:
>> "Yoshinori K. Okuji" <okuji@gnu.org> writes:
>> > On Wednesday 26 April 2006 01:22, Hollis Blanchard wrote:
>> >> Is there a reason not to use the runtime debugging infrastructure we
>> >> have? Most of that is "intensive" as well...
>> >
>> > What do you mean by "the runtime debugging infrastructure"? If you mean
>> > grub_dprintf, it is not enough, since you cannot embed file names and
>> > line numbers where the functions are called.
>>
>> IIRC this is being done automatically.  Or do I misunderstand
>> something?
>
> Yes. My point is that I want to embed such information into where 
> malloc/realloc/memalign/free is called, but not grub_dprintf is called. If 
> you use grub_dprintf, you must embed grub_dprintf into so many places 
> manually. My way does not require such rewriting.

How about using a macro as a wrapper.  So something like:

#define grub_malloc(x)		\
  {				\
     grub_dprintf (...);	\
     grub_malloc_int (...)	\
  }

In that case you can use the existing debugging framework we are all
used to.

--
Marco

Re: grub_machine_set_prefix, --enable-mm-debug

[Yoshinori K. Okuji]

On Wednesday 26 April 2006 12:00, Marco Gerards wrote:
> How about using a macro as a wrapper.  So something like:
>
> #define grub_malloc(x)		\
>   {				\
>      grub_dprintf (...);	\
>      grub_malloc_int (...)	\
>   }
>
> In that case you can use the existing debugging framework we are all
> used to.

Clearly, I don't want to use grub_dprintf for two reasons:

- I need to print messages twice in some functions. If I use dprintf, it 
prefixes a filename and a line number every time. This is too annoying.

- The use of grub_dprintf requires the modification of an environment 
variable. So this affects the memory manager and the user environment. When 
the memory manager is buggy, I don't want such a side effect.

Frankly speaking, I tried grub_dprintf myself, and abandoned it, due to these 
problems. My problem was actually that I forgot to initialize a local 
variable before freeing it. When using grub_dprintf, by setting a variable, 
the stack was modified, then the fatal error disappeared as a side effect. 
This was extremely confusing. Compared with this, setting a global variable 
does not affect the memory status very much. So I will never go back to 
grub_dprintf for this purpose.

The idea of code sharing is usually very good, but not always good.

Okuji

Re: --enable-mm-debug breakage

[Hollis Blanchard]

On Apr 25, 2006, at 3:08 PM, Yoshinori K. Okuji wrote:

> 	Also, this change adds an
> 	intensive debugging feature for the memory manager via the
> 	configure option "--enable-mm-debug".

I fixed the breakage from renaming "abort".

However, the build is now broken when not using --enable-mm-debug:
	grubof_symlist.c: In function 'grub_register_exported_symbols':
	grubof_symlist.c:134: error: 'grub_debug_malloc' undeclared (first use 
in this function)

Given gensymlist.sh, I can see two ways to solve this problem:
1) Provide empty stubs for those debug functions.
2) Use the regular debugging infrastructure, as I mentioned before. 
That is what is was designed for, so it seems to be the obvious 
solution. grub_mm_debug(), which has no callers, is the only function 
that won't fit.

I will resist the urge to do this until you're able to comment.

-Hollis

Re: --enable-mm-debug breakage

[Hollis Blanchard]

On Apr 25, 2006, at 3:08 PM, Yoshinori K. Okuji wrote:

> 	Also, this change adds an
> 	intensive debugging feature for the memory manager via the
> 	configure option "--enable-mm-debug".

... and finally, the debug functions seem to introduce runtime breakage 
(out of memory errors). I have no idea how, and I've wasted too much 
time trying to figure that out. It's as easy as trying to load a 
vmlinux: out of memory. When I hack out the EXPORT stuff so I can build 
without --enable-mm-debug, everything runs fine.

-Hollis

Re: --enable-mm-debug breakage

[Yoshinori K. Okuji]

On Wednesday 26 April 2006 02:10, Hollis Blanchard wrote:
> On Apr 25, 2006, at 3:08 PM, Yoshinori K. Okuji wrote:
> > 	Also, this change adds an
> > 	intensive debugging feature for the memory manager via the
> > 	configure option "--enable-mm-debug".
>
> I fixed the breakage from renaming "abort".

Thank you.

> However, the build is now broken when not using --enable-mm-debug:
> 	grubof_symlist.c: In function 'grub_register_exported_symbols':
> 	grubof_symlist.c:134: error: 'grub_debug_malloc' undeclared (first use
> in this function)
>
> Given gensymlist.sh, I can see two ways to solve this problem:
> 1) Provide empty stubs for those debug functions.
> 2) Use the regular debugging infrastructure, as I mentioned before.
> That is what is was designed for, so it seems to be the obvious
> solution. grub_mm_debug(), which has no callers, is the only function
> that won't fit.
>

No, this problem happens only because grubof_symlist.c is not regenerated when 
you rerun configure. That is, this is a dependency problem in the Makefile.

Okuji

Re: --enable-mm-debug breakage

[Yoshinori K. Okuji]

On Wednesday 26 April 2006 10:47, Yoshinori K. Okuji wrote:
> No, this problem happens only because grubof_symlist.c is not regenerated
> when you rerun configure. That is, this is a dependency problem in the
> Makefile.

Nope. I'm wrong. The problem is that the cpp conditionals are not evaluated 
when getting exported symbols. I will fix this.

Okuji