fix: drcov2lcov read all dumps per log file so post-popen coverage not dropped - #7974
fix: drcov2lcov read all dumps per log file so post-popen coverage not dropped#7974pavelbazika wants to merge 8 commits into
Conversation
fcaf78a to
aad4850
Compare
derekbruening
left a comment
derekbruening
left a comment
There was a problem hiding this comment.
Thank you for the contribution. The main missing piece is adding a test.
| * the process image is replaced), and again when the process or thread | ||
| * exits. Normally those two dumps land in different files, but | ||
| * posix_spawn() and popen() use a vfork-style clone that shares the | ||
| * parent's address space. The "execve" dump from the child therefore gets |
There was a problem hiding this comment.
If the only trigger was vfork, I'd say DR itself should provide a new-logfile mechanism and then this code would only ever see one dump. But since multiple dumps in the same file also happens with a failed execve, it probably is worth handling multiple dumps here.
| return false; | ||
| } | ||
| ptr = move_to_next_line(ptr); | ||
| if ((size_t)(ptr - map) + (size_t)num_bbs * sizeof(bb_entry_t) > map_size) { |
There was a problem hiding this comment.
Prefer C++-style casts in C++ code.
| /* read_bb_list() frees tables. */ | ||
| if (read_bb_list(ptr, tables, num_mods, num_bbs)) | ||
| any_bb = true; | ||
| ptr += (size_t)num_bbs * sizeof(bb_entry_t); |
There was a problem hiding this comment.
Prefer C++-style casts in C++ code.
| ptr = map; | ||
| map_end = map + (size_t)file_size; | ||
| while (ptr < map_end) { | ||
| const char *hdr = read_file_header(ptr); |
There was a problem hiding this comment.
I find it confusing to use this new var hdr here: simpler and clearer to use ptr as in all the other calls advancing ptr.
| @@ -969,40 +969,85 @@ static bool | |||
| read_drcov_file(const char *input) | |||
There was a problem hiding this comment.
Please add a regression test. It can be the exact code you have in the PR description.
2 participants
When taking coverage of following program popen.c
running
the report ends on popen line, although the program run to it's end
There's a related issue for this: #1390 (comment)
Please review the fix carefully, it was generated by AI and I don't know dynamorio. However, it works for me, with the fixed drcov2lcov I get complete coverage for this sample and also for the real project where the issue originaly occured.