[llvm-dev] [lld] - LLD (ELF) code covered by test cases.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] [lld] - LLD (ELF) code covered by test cases.

Dean Michael Berris via llvm-dev

Hello guys,

Today I tried to find the amount of LLD(elf) code covered by our test cases. So my aim was to run the LLD tests we have (run check-lld task) and find out which code was executed/covered and which was not.

I used the approach from the next article to do that:
http://logan.tw/posts/2015/04/28/check-code-coverage-with-clang-and-lcov/#create-a-wrapper-script-for-lcov

In short, it is based on using -fprofile-arcs -ftest-coverage clang flags and then generating the HTML report using lcov and genhtml tools.

The results are actually not that bad (I expected we would have much more issues, but glad to see we seem don't :) 
Coverage screenshot:
https://drive.google.com/file/d/1rjFdMA0HOjvz4vw_nDttv_dlbMiNYiaw/view

We have few problematic places, but generally seems we cover the most of the code, except the few specific
errors and conditions. The full HTML report is available here:
https://drive.google.com/file/d/1ROo9rZ4p7-IT68jI0nnhNMOqPNpk1l0c/view?usp=sharing

Noticeable places I have to mention though are (we have some large code parts uncovered):

1) I was able to place `assert(false);` on line 386 below in ICF.cpp and all our tests pass.
(https://github.com/llvm-mirror/lld/blob/master/ELF/ICF.cpp#L386)

2) In Relocations.cpp, all code inside `if`:
https://github.com/llvm-mirror/lld/blob/master/ELF/Relocations.cpp#L1177
Is not covered by tests.

3) MicroMipsR6Thunk::writeTo(), MicroMipsR6Thunk::addSymbols(), 
MicroMipsR6Thunk::getTargetInputSection() are not covered.

That seems to be all major places I saw in report.

Now we probably can think about what we can do with that?
* Ideal scenario I can imagine is that we could fix all the places and, for example, setup the watching bot
looking for LLD code coverage for each commit.
* Minimal scenario I see is to fix the major known places mentioned at least.

What do you think guys?​


Best regards,
George | Developer | Access Softek, Inc

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [lld] - LLD (ELF) code covered by test cases.

Dean Michael Berris via llvm-dev
Thanks very much for running the experiment, that is useful information.

On 26 April 2018 at 14:11, George Rimar via llvm-dev
<[hidden email]> wrote:

>
> 2) In Relocations.cpp, all code inside `if`:
> https://github.com/llvm-mirror/lld/blob/master/ELF/Relocations.cpp#L1177
> Is not covered by tests.
>

I'll volunteer to add a test case for that one. Will hopefully have
something at least by early next week.

> Now we probably can think about what we can do with that?
> * Ideal scenario I can imagine is that we could fix all the places and, for
> example, setup the watching bot
> looking for LLD code coverage for each commit.
> * Minimal scenario I see is to fix the major known places mentioned at
> least.
>

> What do you think guys?

I agree that we should fix the areas that are missing in the report. I
think it would be a great just to have the coverage information
available to look at as it would be useful to add extra tests for
areas that we'd missed. Whether such a bot should be silent, warn or
error based on some heuristic is a trickier question.

Peter

>
>
> Best regards,
> George | Developer | Access Softek, Inc
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [lld] - LLD (ELF) code covered by test cases.

Dean Michael Berris via llvm-dev
In reply to this post by Dean Michael Berris via llvm-dev
Thanks a lot for doing this.

Having it automated an in a bot would be really nice.

Cheers,
Rafael

George Rimar <[hidden email]> writes:

> Hello guys,
>
> Today I tried to find the amount of LLD(elf) code covered by our test cases. So my aim was to run the LLD tests we have (run check-lld task) and find out which code was executed/covered and which was not.
>
> I used the approach from the next article to do that:
> http://logan.tw/posts/2015/04/28/check-code-coverage-with-clang-and-lcov/#create-a-wrapper-script-for-lcov
>
> In short, it is based on using -fprofile-arcs -ftest-coverage clang flags and then generating the HTML report using lcov and genhtml tools.
>
> The results are actually not that bad (I expected we would have much more issues, but glad to see we seem don't :)
> Coverage screenshot:
> https://drive.google.com/file/d/1rjFdMA0HOjvz4vw_nDttv_dlbMiNYiaw/view
>
> We have few problematic places, but generally seems we cover the most of the code, except the few specific
> errors and conditions. The full HTML report is available here:
> https://drive.google.com/file/d/1ROo9rZ4p7-IT68jI0nnhNMOqPNpk1l0c/view?usp=sharing
>
> Noticeable places I have to mention though are (we have some large code parts uncovered):
>
> 1) I was able to place `assert(false);` on line 386 below in ICF.cpp and all our tests pass.
> (https://github.com/llvm-mirror/lld/blob/master/ELF/ICF.cpp#L386)
>
> 2) In Relocations.cpp, all code inside `if`:
> https://github.com/llvm-mirror/lld/blob/master/ELF/Relocations.cpp#L1177
> Is not covered by tests.
>
> 3) MicroMipsR6Thunk::writeTo(), MicroMipsR6Thunk::addSymbols(),
> MicroMipsR6Thunk::getTargetInputSection() are not covered.
>
> That seems to be all major places I saw in report.
>
> Now we probably can think about what we can do with that?
> * Ideal scenario I can imagine is that we could fix all the places and, for example, setup the watching bot
> looking for LLD code coverage for each commit.
> * Minimal scenario I see is to fix the major known places mentioned at least.
>
> What do you think guys??
>
>
> Best regards,
> George | Developer | Access Softek, Inc
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [lld] - LLD (ELF) code covered by test cases.

Dean Michael Berris via llvm-dev
Got the same idea.
Now need to figure out where to host the results.

Thanks

Galina

On Thu, Apr 26, 2018 at 9:10 AM, Rafael Avila de Espindola <[hidden email]> wrote:
Thanks a lot for doing this.

Having it automated an in a bot would be really nice.

Cheers,
Rafael

George Rimar <[hidden email]> writes:

> Hello guys,
>
> Today I tried to find the amount of LLD(elf) code covered by our test cases. So my aim was to run the LLD tests we have (run check-lld task) and find out which code was executed/covered and which was not.
>
> I used the approach from the next article to do that:
> http://logan.tw/posts/2015/04/28/check-code-coverage-with-clang-and-lcov/#create-a-wrapper-script-for-lcov
>
> In short, it is based on using -fprofile-arcs -ftest-coverage clang flags and then generating the HTML report using lcov and genhtml tools.
>
> The results are actually not that bad (I expected we would have much more issues, but glad to see we seem don't :)
> Coverage screenshot:
> https://drive.google.com/file/d/1rjFdMA0HOjvz4vw_nDttv_dlbMiNYiaw/view
>
> We have few problematic places, but generally seems we cover the most of the code, except the few specific
> errors and conditions. The full HTML report is available here:
> https://drive.google.com/file/d/1ROo9rZ4p7-IT68jI0nnhNMOqPNpk1l0c/view?usp=sharing
>
> Noticeable places I have to mention though are (we have some large code parts uncovered):
>
> 1) I was able to place `assert(false);` on line 386 below in ICF.cpp and all our tests pass.
> (https://github.com/llvm-mirror/lld/blob/master/ELF/ICF.cpp#L386)
>
> 2) In Relocations.cpp, all code inside `if`:
> https://github.com/llvm-mirror/lld/blob/master/ELF/Relocations.cpp#L1177
> Is not covered by tests.
>
> 3) MicroMipsR6Thunk::writeTo(), MicroMipsR6Thunk::addSymbols(),
> MicroMipsR6Thunk::getTargetInputSection() are not covered.
>
> That seems to be all major places I saw in report.
>
> Now we probably can think about what we can do with that?
> * Ideal scenario I can imagine is that we could fix all the places and, for example, setup the watching bot
> looking for LLD code coverage for each commit.
> * Minimal scenario I see is to fix the major known places mentioned at least.
>
> What do you think guys??
>
>
> Best regards,
> George | Developer | Access Softek, Inc


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev