> > I don't really follow the logic here.  buffer is allocated to be size of
> > block+4 and it is expected that gcov_write_words is not executed on size
> > greater than 4.  Since gcov_write_string now seems to be expected to handle
> > strings of bigger size, I think you acually need to make write_string to 
> > write
> > in chunks when you reach block boundary?
> 
> gcov_write_words is used to reserve words*4 bytes in buffer for data
> write later. The the old logic is wrong -- if 'words' is large enough,
> it will lead to out of bound access.

The old logic assumes that writes are not bigger than 4 and it is documented
somewhere in gcov-io.h if I remember right (it is not my code).
I however still think your change won't solve it in general, since you might
want to write more than block size, so you need write_string update.
> >
> > What gets into libgcov is very problematic busyness for embedded targets, 
> > where you really want
> > libgcov to be small.  Why do you need to store strings now?
> 
> It is needed to store the assembler name for functions. It allows
> lookup of the profile data using assembler name as key instead of
> using function ids. For gcc, the total size of gcda files is about 59k
> bytes without storing the names, and about 65k with names -- about 10%
> increase. For eon, the size changes from 27k to 35k bytes.
> 
> I can split the patches into two parts -- one with cfg checksum and
> one with the name string.

Well, lets make it incremental change then at time we will have use for it once
we agree it makes sense.  So you want to do symbol name resolving at GCOV
runtime?
I duno about embedded targets, 10% increase is not too bad, but I don't know.
libgcov has quite inflexible coding style just to be small and fit there, so
I don't know what are the space constraints that are considered acceptable
in that world.
> > We also need to bump gcov version, right?
> 
> Yes -- but the version is currently derived from gcc version number
> and phase number --- this is wrong -- different version of gcc may
> have compatible coverage data format.  Any suggestions to change this?
> --- probably just hard code the GCOV_VERSION string?

Hmm, I am pretty sure we used to have minor/major numbering on the file format.
Perhaps it went away with Nathan's changes.  For -fprofile-use the compatibility
across versions is pointless, at least now, but for gcov utility it makes sense.
So I would go for explicit versioning again.

Honza

Reply via email to