On Thu, Apr 21, 2011 at 1:43 PM, Jan Hubicka <hubi...@ucw.cz> wrote: >> > 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.
Right, 'words; can be large for string. -- the memcpy code following does not look right either. >> > >> > 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. Ok. > So you want to do symbol name resolving at GCOV > runtime? For now, it is just passed through as a tag for the profile data for each function. > 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. I will delay the string writing patch for now. >> > 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, Right, but also equally unnecessary to force version mismatch between compiler releases. >at least now, but for gcov utility it makes sense. Yes. > So I would go for explicit versioning again. > Will dig through the revision history .. David > Honza >
