Re: [Valgrind-developers] Porting valgrind to FreeBSD

[Doug R. <df...@nl...>]

On Sat, 2004-01-03 at 22:30, Jeremy Fitzhardinge wrote:
> On Fri, 2004-01-02 at 03:56, Doug Rabson wrote:
> > I've updated the port to work with the post FV valgrind. You can get a
> > patch against today's CVS from
> > http://people.freebsd.org/~dfr/valgrind-20040102-dfr.diff.
> 
> That's great!  I'll have a look at your latest patch in detail later
> today.
> 
> > The only really dodgy bits in this patch are in stage1.c where I had to
> > stub out the stack alignment bits. The code couldn't code when the
>                                                        ^^^^ cope?

yes :-)

> > alignment offset wasn't exactly zero because it assumed that the new aux
> > entries would fit exactly into the gap.
> 
> Do you mean the stuff in fix_auxv?

Thats right. My FreeBSD system started valgrind with a stack that wasn't
aligned to 16 bytes and this confused fix_auxv enough to corrupt the
auxv entries given to stage2.

> 
> >  I also had problems moving the
> > brk() up past the end of stage2 so I punted on that and just overrode
> > brk() and sbrk() instead.
> 
> Is that because FreeBSD doesn't overcommit by default?  The reason for
> trying to get the brk base into the right place for stage2 is so that
> stage2 can use libraries which want to use brk (eg, libc malloc).  If
> you override brk in stage2, will that definitely override all possible
> ways the brk() syscall can be called?  Because it could get pretty
> disastrous if brk(2) accidentally got called.  Also, in FreeBSD, is sbrk
> a syscall or just a library function?

The main problem (I think) is that the default data size limit for
FreeBSD is only 0.5G. We do overcommit by default but moving the brk
this way generates valid mappings (lazy allocated zero-filled mappings
but still valid) for the entire ~3G address space and this is greater
than the limit.

Overriding brk, sbrk (and even malloc itself) is perfectly safe in
FreeBSD and it should override all ways that the syscall can be called
apart from a pathological use of syscall(3). I made sure that stage2 was
able to call malloc safely with these changes although there was a
problem with non-trivial use of malloc because stage2's copy of libc.so
ended up being loaded very close to the end of stage2 which didn't leave
much spare to expand the 'brk'.

The implementation of brk and sbrk in FreeBSD is a hybrid of library and
syscall. The current and minimum break values are handled in user code
and new break locations are given to the kernel via the syscall. Its
very simple and easy to understand if you read the code
(src/lib/libc/i386/sys/brk.S and src/lib/libc/i386/sys/sbrk.S in any
FreeBSD source tree).

> 
> Also, what does FreeBSD's RLIMIT_DATA govern?  In Linux it is
> effectively useless, because it only governs the brk syscall (and a.out
> executable BSS size).  This is useless because glibc will always fall
> back to using mmap() if brk fails, so if brk fails because of
> RLIMIT_DATA, it will still keep allocating memory.  I was thinking of
> taking advantage of this by setting RLIMIT_DATA to 0 so that I can be
> sure that brk(2) will always fail and not cause problems.

I think that RLIMIT_DATA in FreeBSD governs the virtual size of the
brk-managed region. The libc implementation will not fall back to mmap
if sbrk (or brk) fails.

> 
> > I had problems with vg_signals.c for the async signal handlers.
> > Currently FreeBSD has a bug (which will be fixed RSN) with sigaltstack
> > that means that all threads share the same stack setting. This meant
> > that async signals (intended for the proxylwp) were being delivered on
> > the signal stack instead of the proxylwp stack. I just changed the code
> > to not set SA_ONSTACK for those signals.
> 
> That sounds OK.
> 
> > Attaching GDB doesn't work very well with this port since we have no
> > equivalent of /proc/self/fd. I guess the code could be changed to
> > remember the exec filename and pass that instead of /proc/self/fd/$d.
> 
> Yes, I was planning something like that anyway, cause all the /proc
> stuff is pretty non-portable.  That said, the GDB attach stuff is broken
> anyway, because it's pretty hard to get right (it's very hard for a
> program to attach gdb to itself, and set its own state up into something
> which gdb wants to examine).  In particular, the state of %ebp gets
> trashed as part of the exec of gdb, so gdb doesn't see a sane backtrace
> at the moment.
> 
> > This also affected the implementation of VG_(resolve_filename) which I
> > worked around by using the file descriptor tracking code to lookup the
> > filename.
> 
> For just the cases where the fd was obviously created out of a name?

Thats right. I also made it unconditional for dup-like calls where the
original file descriptor might have had a name.

> 
> > I haven't tested this patch with a Linux box - my RH9 machine is several
> > miles away and switched off for the holidays. I've tried to keep things
> > decently conditional but there are almost certainly one or two nits.
> 
> I think the best thing to do is use this as a basis for factoring all
> the OS-specific code into OS-specific files.

Right - I think that process has already been started.