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

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

On Wed, 21 Jan 2004, Jeremy Fitzhardinge wrote:

> I think _VALGRIND_CLO should exclusively take precedence over all other
> sources of command-line options (so if it is present, then 5 is
> skipped).  That's because it is constructed out of vg_argv, which in
> turn is constructed out of all the sources when V first starts.

Done.

> > 8. set VG_(libdir) from VALGRINDLIB [after ??]      ((move to after 3?))
>
> Earlier is better.

Done.

> > 17. unpad shadow_end..0xffffffff [after 12]
>
> This should probably be shadow_end..valgrind_mmap_end; this will leave
> the padding covering Valgrind's heap/stack, which will prevent unwanted
> mappings from ending up there.  This could be problematic (since the
> stack won't be able to grow while the padding is there), so maybe the
> answer is to make sure the Segment list has already blocked out
> Valgrind's bss/heap/stack area for mapping, and just use VG_(mmap) for
> address space allocation - see below at init_memory).

I've left it unchanged for the moment.

> m_state_static is a holdover from the old code, and isn't really
> necessary anymore.  It's just used to bootstrap the initial thread
> state, but we could do it more directly.

Partially removed;  still needed for VG_(switch_to_real_cpu) as discussed
elsewhere in this thread.

> > 36. init_memory [after 23? (so we can parse proc/self/maps);  complex, because
> >     it does several things]
>
> Maybe it should be split up.  The most important thing it does is
> initialize the Segment list.  This point represents the boundary between
> "mmap must be used, VG_(mmap) must not be used" and "VG_(mmap) must be
> used, mmap must not be used".  Since this is a subtle distinction, and
> hard to police, I think it should happen as early as possible, and just
> make everyone use VG_(mmap).
>
> This is a bit tricky, since ume.c/do_exec uses mmap(), but maybe we can
> change it to use VG_(mmap), and give stage1 a stub definition of
> VG_(mmap).  If it uses VG_(mmap), it may also be able to encode more
> information about the executable in the SF_* flags.

I've inlined VG_(init_memory), but not moved the bits about, because I'm
uncertain about the dependencies of all the mapping stuff.

----

I've updated the patch, it's at:

  www.cl.cam.ac.uk/~njn25/startup-merge.patch2

I've made quite a few more simplifications, and merged together various
steps that were closely related;  it's looking relatively clean, now.  I
would like someone else to confirm it works on their machine before I
commit it.

Jeremy, would you mind going through and looking at all the places I've
marked with XXX?  I had various question about things I didn't understand,
and marked things that are broken, and where improvements could be made
but I wasn't sure how best to do it.  You could annotate my patch, or I
could commit and then let you make more changes, whatever you prefer.

N