SimonKagstrom / kcov

Code coverage tool for compiled programs, Python and Bash which uses debugging information to collect and report data without special compilation options
http://simonkagstrom.github.io/kcov/
GNU General Public License v2.0
709 stars 109 forks source link

Work with Go binary on Mac #408

Closed myzhan closed 10 months ago

myzhan commented 10 months ago

See: https://github.com/SimonKagstrom/kcov/issues/405

myzhan commented 10 months ago

I think you can pick a short option for "is-go-binary".

codecov[bot] commented 10 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (4581902) 65.57% compared to head (1194bda) 65.60%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #408 +/- ## ========================================== + Coverage 65.57% 65.60% +0.02% ========================================== Files 58 58 Lines 4503 4503 Branches 4160 4160 ========================================== + Hits 2953 2954 +1 + Misses 1550 1549 -1 ``` | [Files](https://app.codecov.io/gh/SimonKagstrom/kcov/pull/408?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/configuration.cc](https://app.codecov.io/gh/SimonKagstrom/kcov/pull/408?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbmZpZ3VyYXRpb24uY2M=) | `54.86% <100.00%> (+0.12%)` | :arrow_up: | | [src/parser-manager.cc](https://app.codecov.io/gh/SimonKagstrom/kcov/pull/408?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3BhcnNlci1tYW5hZ2VyLmNj) | `95.45% <100.00%> (+4.15%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SimonKagstrom commented 10 months ago

is-go-binary is clear enough I think!

SimonKagstrom commented 10 months ago

I added a comment about error messages, which use a helper in kcov.

myzhan commented 10 months ago

is-go-binary is clear enough I think!

Because the whole file content need to be read into memory, so I think we can use a commandline option to set "is-go-binary" to true directly, then we don't need to read the whole file.

SimonKagstrom commented 10 months ago

Yes. You can always do --configure=is-go-binary=1 to just set the key, but otherwise a command line option can also be added, at least for OSX.

myzhan commented 10 months ago

Oops, don't know why some tests failed.

SimonKagstrom commented 10 months ago

Oops, don't know why some tests failed.

The FreeBSD problem is some infrastructure issue, but the OSX tests should run. Do non-go programs work with your patch?

myzhan commented 10 months ago

OK, I fixed a segfault when running non-go programs.

But I found a new issue. Tried to generate dSYM by clang with -g, but the dsymutil modified it even the file existed, and then kcov failed to read dwarf info. After commented out the dysmutil part, everything worked.

How to reproduce.


$ clang -v
Apple clang version 13.1.6(clang-1316.0.21.2.5)
$ dsymutil -v
Apple LLVM version 13.1.6(clang-1316.0.21.2.5)
$ clang -g hello.c -o hello
$ ll hello.dSYM/Contents/Resource/DWARF/hello
8.6k size
$ kcov ./report ./hello
warning: (x86_64) /my/tmp/folder/on/the/mac/hello-3838c7.o unable to open object file: No such file or directory
warning: no debug symbols in executable (-arch x86_64)
hello
$ ll hello.dSYM/Contents/Resource/DWARF/hello
8.2k size
SimonKagstrom commented 10 months ago

I believe clang puts DWARF info directly into the executable if it's compiled in a single step, i.e., without linking. This is a case I've ignored, since it basically only affects toy programs. I guess it still works fine when running with linked binaries?

myzhan commented 10 months ago

I believe clang puts DWARF info directly into the executable if it's compiled in a single step, i.e., without linking. This is a case I've ignored, since it basically only affects toy programs. I guess it still works fine when running with linked binaries?

Clang puts DWARF info into the dSYM file, I can check with objdump.

$ objdump --macho -x ./hello
./hello:

Sections:
Idx Name          Size     VMA              Type
  0 __text        0000000f 0000000100003fa0 TEXT
  1 __unwind_info 00000048 0000000100003fb0 DATA
$ objdump --macho -x hello.dSYM/Contents/Resources/DWARF/hello
...
Section
  sectname __debug_info
   segname __DWARF
      addr 0x000000010000506e
      size 0x0000000000000053
    offset 8302
     align 2^0 (1)
    reloff 0
    nreloc 0
      type S_REGULAR
attributes (none)
 reserved1 0
 reserved2 0
...

And dsymutil generates a file without DWARF info and overwrites the file that clang generates.

image

I can use the --save-temps option to tell clang not to delete the object file, so dsymutil will find it and report no warnings.

SimonKagstrom commented 10 months ago

OK, I see! So what really happens is that clang generates the dSYM info directly then? I guess this is where it would be good to be able to check that the dSYM and binary matches, and ignore dsymutil in this case. I'm on Linux right now, so can't check this locally.

Anyway, this is unrelated to your change, so merging that!

myzhan commented 10 months ago

OK, I see! So what really happens is that clang generates the dSYM info directly then? I guess this is where it would be good to be able to check that the dSYM and binary matches, and ignore dsymutil in this case. I'm on Linux right now, so can't check this locally.

Anyway, this is unrelated to your change, so merging that!

Yes, clang generates the dSYM info directly.