[Valgrind-developers] Re: mega-merge status

[Jeremy F. <je...@go...>]

On Wed, 2002-11-13 at 15:26, Julian Seward wrote:
> Hi Jeremy (also Nick :) -- see analysis, questions below, esp 41-linefix)
> 
> Ok, have merged bazillions of patches in, specifically:
> 
>   20-hg-lockgraph-report
>   21-hg-hashed-lockset
>   03a-open-nonblock
>   22-mutex-destroy-unlock
>   23-intercept-select-poll
>   24-sym-offset
>   25-hg-free-mutex
>   26-hg-client-reqs
>   29-hg-rename-hg_mutex_t
>   30-hg-clo
>   31-hg-shadow-execontext
>   32-hg-lifetime-segments
>   33-pre_mutex_lock
>   36-hg-optim
>   37-hg-private-stack
>   38-hg-lazy-lasttouch
>   39-lock-prefix
>   40-hg-tidstate
>   42-hg-show-all-locks
> 
> Thank you for what is evidently a large amount of work.

Thanks for that.  I'll sync up now.

> Analysis, questions:
> 
> - 03a-open-nonblock I merged, even though am not entirely happy about.
>   Is it sufficiently robust to not cause problems later?

I'm least happy about this one, but I can't think of a better simple
solution, and it does fix a real observed problem.  A more correct
solution (from the kernel's perspective) would be to fire off a clone
thread to do blocking syscalls in, so that we can get exactly the right
semantics.  I have no idea how to make the rest of V happy with this
though.

> - 27-nvalgrind -- is this really actually needed?  Do you have some
>   performance numbers indicating, when running natively, that the 12-insn
>   sequence really causes significant performance loss?

I doubt there's a good technical justification for it, but when trying
to convince people to use Valgrind, I've found it most useful to be able
to say "you can compile out all the Valgrind annotations for production
builds".  It's just one less argument for people to make.  And when
describing the client callback mechanism to people, they invariably ask
whether the callbacks can be compiled out within about 1 to 2
milliseconds.

> - 28-sigtrap -- didn't merge because I'm paranoid about breaking the
>   already fragile signal simulation stuff.  Am not per se opposed to it,
>   but it's too late at night to sanity check this one in detail.

It doesn't seem quite right.  It doesn't break any existing correct
behaviour, but it doesn't always print the right error message on a
crash.

> - 37-hg-private-stack: at first was concerned about not checking %EBP
>   relative references since %EBP is a general reg when -fomit-frame-pointer.
>   However I see that you have some mini-dataflow analysis to get round
>   this problem (I think?)  Can you just confirm that it all makes sense
>   even if -fomit-frame-pointer ?

Hm, looking at the code, it doesn't quite get it right - it always
assumes EBP is a stack reference.  This is hard to fix.  The dataflow
stuff looks within a basic block to work out what temps are holding
addresses derived from a stack reference (ie, ESP or EBP), so it can
work out whether to suppress the check generation.

Of course, I'm not doing any global dataflow analysis, so I can't tell
if any of the archregs are holding stack addresses, so I just assume
that ESP and EBP always do, and the rest don't.

Any ideas on how to work out if the current code has been compiled with
-fomit-frame-pointer?  I guess the safe way is to generate a runtime
check per basic block which sees if EBP is near ESP.  Not really keen on
that.

I also need to work out whether a given piece of code is compiled with
-fomit-frame-pointer to interpret the stab info on where a variable is
living.  I think.

> I think that's all for now.  How's it going with the fancy symbol reader?

Well, I've got the stab type parser chewing through all the shared
libraries I've tested it on now, so its time to actually use all that
information.  I'm hoping to get it going in the next few days, and maybe
convince someone else to write the DWARF2 parser...

	J