[Valgrind-developers] Merging stage2.c and vg_main.c [was Re: tool-specific CLOs] (fwd)

[Nicholas N. <nj...@ca...>]

[apologies if you receive this twice;  if so, please ignore the first
version as it had mistakes]


On Sat, 10 Jan 2004, Jeremy Fitzhardinge wrote:

> So, I think the init ordering should be:
>
>      1. stage1
>              1. pad the client address space
>              2. do_exec stage2
>      2. stage2:main()
>              1. (do something to cope with .init sections of any
>                 libraries we're using)
>              2. initialize Segment list to reflect all current mappings
>                 (VG_(init_memory)
>              3. init Valgrind heap, make mmap() call VG_(mmap), deal
>                 with brk/sbrk.
>              4. init early error reporting (ie default output sink)
>              5. set up any other padding
>              6. collate arguments from all the possible sources,
>                 generating a unified vg_argv[] array
>      3. stage2:VG_(main)()
>              1. parse vg_argv
>              2. print errors/usage/early failures
>              3. dlopen() the tool
>              4. init everything else (may need to be after
>                 do_exec(client) if it depends on the shape/placement of
>                 the client address space)
>              5. remove all padding/clear all mappings out of client
>                 address space
>              6. do_exec client/set up client stack
>              7. start VCPU
>
> As you say, the break between main() and VG_(main) is pretty arbitrary.
> Perhaps we should just drop stage2.c, move everything into vg_main.c,
> rename VG_(main)() to main(), get rid of KickStartParams (another
> historical name).

I have a patch that does this -- removes stage2.c, moving it
into vg_main.c.  This allowed me to remove quite a lot of cruft, because
the two were pretty closely intertwined.  It also includes my .valgrindrc
stuff, and a fix for bug 71126, as previously mentioned.
It's at www.cl.cam.ac.uk/~njn25/startup-merge.patch (too big to mail).

There are a couple of shortcomings still: no support for quoting in the
.valgrindrc files, which Tom wants, and tool-specific CLOs can't have
colons in them, which Josef wants.

It seems to work ok.  I tried my best to just refactor things, and not
change any actual behaviour.

I've attached a file that lists all the tasks now done in main(),
and their dependencies.  It doesn't look very much like your list above,
Jeremy;  mostly I kept the existing order of things.  Valgrind has
certainly become a tricky sucker to start up.  I think these dependencies
should eventually go into comments in main();  there are already some
dependencies listed for the code that was VG_(main)(), and they're very
useful.

There is certainly potential for the order of startup events to change,
and for neatening up, and fixing the mentioned shortcomings.  However, if
nobody objects, I'd like to put this patch in sooner rather than later
since any intermediate commits could cause some pretty heinous clashes.

N