[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] [llvm-dev] Ignoring coverage for noreturn decls
From: Harlan Haskins via cfe-dev <cfe-dev () lists ! llvm ! org>
Date: 2016-03-29 17:18:03
Message-ID: B762839F-3BB2-4A30-AB58-0E3FB0496C48 () apple ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Mar 28, 2016, at 8:31 PM, Vedant Kumar <vsk@apple.com> wrote:
>
> + cfe-dev
>
> > On Mar 28, 2016, at 1:23 PM, Harlan Haskins via llvm-dev \
> > <llvm-dev@lists.llvm.org> wrote:
> > Hi all,
> >
> > Recently I've noticed in coverage profiles that llvm_unreachable and the like are \
> > considered uncovered because there's no special behavior in instrumentation to \
> > ‘ignore' noreturn paths.
>
> FWIW, Daniel Dunbar and a few others have brought up the lack of a flexible \
> "suppress coverage" mechanism as a pain-point.
> There are two separate but related solutions:
>
> - We should automatically emit zero mappings for code regions dominated by a call \
> to a noreturn function.
> - We should offer some general way to 'turn off' coverage for arbitrary chunks of \
> code.
>
> > While I don't necessarily think it's ideal to ignore all noreturn decls, I think \
> > there's definitely room for some heuristics around ignoring things like \
> > llvm_unreachable (perhaps opt-in?).
>
> I think it's actually useful to have coverage for noreturn functions. Googletest \
> lets you write death tests for this sort of thing.
> Maybe it makes sense to emit zero regions at the _callsites_ of noreturn functions \
> instead. Wdyt?
Yep, this is actually what I meant. I definitely think you're right that we need to \
inspect the CFG surrounding a noreturn path as well, because code that's guarded by \
an llvm_unreachable shouldn't be counted either.
There doesn't seem to be a warning for statements after noreturn either:
$ cat noreturn.c
#include <stdio.h>
int main() {
__builtin_unreachable();
while (1) puts("Hello, world.");
return 0;
}
$ clang -Wall -Wpedantic noreturn.c -o noreturn
$ ./a.out
'./a.out' terminated by signal SIGBUS (Misaligned address error)
> > I'm investigating emitting a zero region for all noreturn decls whilst \
> > codegenning, as a start.
> > Anyone have any input as to a) if this is a good idea, or b) how best to \
> > implement and expose it?
>
> It might be interesting to try this and see if the coverage reports really are much \
> nicer.
> thanks
> vedant
[Attachment #5 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; \
-webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote \
type="cite" class=""><div class="">On Mar 28, 2016, at 8:31 PM, Vedant Kumar <<a \
href="mailto:vsk@apple.com" class="">vsk@apple.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><div class=""><div class="">+ cfe-dev<br \
class=""><br class=""><blockquote type="cite" class="">On Mar 28, 2016, at 1:23 PM, \
Harlan Haskins via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" \
class="">llvm-dev@lists.llvm.org</a>> wrote:<br class=""><br class="">Hi all,<br \
class=""><br class="">Recently I've noticed in coverage profiles that \
llvm_unreachable and the like are considered uncovered because there's no special \
behavior in instrumentation to ‘ignore' noreturn paths.<br \
class=""></blockquote><br class="">FWIW, Daniel Dunbar and a few others have brought \
up the lack of a flexible "suppress coverage" mechanism as a pain-point.<br \
class=""><br class="">There are two separate but related solutions:<br class=""><br \
class=""> - We should automatically emit zero mappings for code regions \
dominated by a call to a noreturn function.<br class=""><br class=""> - We \
should offer some general way to 'turn off' coverage for arbitrary chunks of code.<br \
class=""><br class=""><br class=""><blockquote type="cite" class="">While I don't \
necessarily think it's ideal to ignore all noreturn decls, I think there's definitely \
room for some heuristics around ignoring things like llvm_unreachable (perhaps \
opt-in?).<br class=""></blockquote><br class="">I think it's actually useful to have \
coverage for noreturn functions. Googletest lets you write death tests for this sort \
of thing.<br class=""><br class="">Maybe it makes sense to emit zero regions at the \
_callsites_ of noreturn functions instead. Wdyt?<br \
class=""></div></div></blockquote><div><br class=""></div><div>Yep, this is actually \
what I meant. I definitely think you're right that we need to inspect the CFG \
surrounding a noreturn path as well, because code that's guarded by an \
llvm_unreachable shouldn't be counted either.</div><div><br class=""></div><div>There \
doesn't seem to be a warning for statements after noreturn either:</div><div><font \
face="Menlo" class=""><br class=""></font></div><div><div><font face="Menlo" \
class="">$ cat noreturn.c</font></div><div><font face="Menlo" class="">#include \
<stdio.h></font></div><div class=""><font face="Menlo" class=""><br \
class=""></font></div><div><font face="Menlo" class="">int main() \
{</font></div><div><font face="Menlo" class=""> \
__builtin_unreachable();</font></div><div><font face="Menlo" class=""> \
while (1) puts("Hello, world.");</font></div><div><font face="Menlo" class=""> \
return 0;</font></div><div><font face="Menlo" class="">}</font></div><div \
class=""><font face="Menlo" class=""><br class=""></font></div><div class=""><font \
face="Menlo" class="">$ clang -Wall -Wpedantic noreturn.c -o \
noreturn</font></div><span class=""><font face="Menlo" class="">$ ./a.out<br \
class=""></font></span><span class=""><font face="Menlo" class="">'./a.out' \
terminated by signal SIGBUS (Misaligned address error)</font><br \
class=""></span><span class=""><br class=""></span></div><span class=""><br \
class=""></span><blockquote type="cite" class=""><div class=""><div \
class=""><blockquote type="cite" class="">I'm investigating emitting a zero region \
for all noreturn decls whilst codegenning, as a start.<br class=""><br \
class="">Anyone have any input as to a) if this is a good idea, or b) how best to \
implement and expose it?<br class=""></blockquote><br class="">It might be \
interesting to try this and see if the coverage reports really are much nicer.<br \
class=""><br class="">thanks<br class="">vedant</div></div></blockquote></div><br \
class=""></body></html>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-dev mailing list
cfe-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic