Re: RFC: --enable-checking=valgrind.

[Hans-Peter Nilsson <hp at bitrange dot com>]

On Mon, 11 Nov 2002, Zack Weinberg wrote:
> On Mon, Nov 11, 2002 at 04:55:17AM -0500, Hans-Peter Nilsson wrote:
> > ==5784== Conditional jump or move depends on uninitialised value(s)
> > ==5784==    at 0x81EE503: get_attr_prefix_0f (insn-attrtab.c:23298)

> Hmm.  This is probabably just one bug, in genattrtab, but genattrtab
> is such a mess I don't know where to begin looking.

Yes, most are this very bug or similar-looking get_attr* ones
elsewhere in insn-attrtab.c.  I didn't look close at it.

> What's on line 23298 of insn-attrtab.c?

Well, I didn't look. :-)  I want to enable the use of valgrind
*first*, *then* it's easy to catch errors and fix twisty
annotation needs.  (Actually, I saw nothing obvious -- some
macro invocations on items that looked valid, a line 391 chars
long.)

> > I don't know what causes that "Line number overflow detected"
> > message, but perhaps old binutils.
>
> The stabs debug format has a limit of 65535 lines in a single
> translation unit

Right; it dawned on me that this is the stage1 compiler running,
compiled with the host egcs-1.1.2 compiler, so it has stabs in
its *executable*; stage2 will be without.  (I'm quickstrapping
it now after my autokiller killed it for breaking the 200M
ulimit household rules.  It does slow things down running in a
simulator...)

> > 	* cppfiles.c (read_include_file) [ENABLE_VALGRIND]: When doing the
> > 	mmap+1 trick, annotate the byte after the mmap:ed area as readable.
>
> I think you also need to clear the annotation again when the map is
> deallocated, or that one byte might continue to be considered
> readable.  Probably want to hang onto the region ID, too.

Yeah, sure.  I think you're focusing too much on details at this
time.  This is primarily a conceptual patch, as mentioned, and
the point was that this kind of annotation is possible and
simple.  It's a "hey, I think we should have
--enable-checking=valgrind" with the patch as a proof of
concept.  (BTW, do you want the region ID for something in
particular?  Or do you just think it'd be handy?)

So what do you (the general you) think?
Is it worth having?  Is this a wrong move?
What'd be the showstoppers?

brgds, H-P