From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com [IPv6:2a00:1450:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id E73B43858D37 for ; Tue, 5 Apr 2022 20:07:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E73B43858D37 Received: by mail-lj1-x22e.google.com with SMTP id m12so457768ljp.8 for ; Tue, 05 Apr 2022 13:07:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=LLdeQJFu8bjm5eUBE4P3nZYC+WcLKEp3ZrBrrnbaGzc=; b=IcX8vKr7FR/vNDhb/flww7uDEyZUw0yJh5oY26pS03hlQZE3xKtEURlgdiES13aTi6 8ifwYuIYCV0EPAROlWVhcB4A/zhkTqhrELOx7qDasBYTj0r6b9SJWDsLSvBWSsAonQkh +9ikLiEN3s41AKE7YXEWVhH5TWAdO2Ko7jZeNSdRe/EhCJhhe1ewrC6vDQunxr9+3LRE pc+fJmDeqYADaNJ4CkhBm9cvOwA/Mld27w5i46HtMACpE6EVebK0GhBYVk61qGOdt3HT Da7QuBl/keuUDzOlMKIlkxTNcryyzG6hkrrukrNNNteOnqWP5p12BF6T9WB/vwrciFdH /9ow== X-Gm-Message-State: AOAM530QxtBuzKnulbVtQAZY7NufFRS/Du7eigEIKJSshmOno9f9Top4 ei12VoZlV9ptbYOnvzlbWse4Gw== X-Google-Smtp-Source: ABdhPJzReWUzTmZIhOCio8dqW2RJUU9HaSz7+ViKBDawmVNT8O6jMpOJOm4W/woQi26PCgof7UXlaA== X-Received: by 2002:a05:651c:2123:b0:24b:d55:ad89 with SMTP id a35-20020a05651c212300b0024b0d55ad89mr3086781ljq.156.1649189256924; Tue, 05 Apr 2022 13:07:36 -0700 (PDT) Received: from [192.168.100.2] ([84.214.39.69]) by smtp.gmail.com with ESMTPSA id a29-20020a056512021d00b0045dcb4ea6c0sm416195lfo.65.2022.04.05.13.07.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Apr 2022 13:07:36 -0700 (PDT) Message-ID: <135f1f97-9359-f3fa-7718-4384aa9a376f@woven-planet.global> Date: Tue, 5 Apr 2022 22:07:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: [PATCH] Add condition coverage profiling Content-Language: en-US To: Sebastian Huber , gcc-patches@gcc.gnu.org References: <1ee43d5d-9c6c-f55b-e5b1-c82a6ff5f653@embedded-brains.de> From: =?UTF-8?Q?J=c3=b8rgen_Kvalsvik?= In-Reply-To: <1ee43d5d-9c6c-f55b-e5b1-c82a6ff5f653@embedded-brains.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Apr 2022 20:07:41 -0000 On 04/04/2022 10:14, Sebastian Huber wrote: > Hello Jørgen, > > having support for MC/DC coverage in GCC would be really nice. I tried out your > latest patch on an arm cross-compiler with Newlib (inhibit_libc is defined). > Could you please add the following fix to your patch: > > diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c > index 89741f637e1..9e3e8ee5657 100644 > --- a/libgcc/libgcov-merge.c > +++ b/libgcc/libgcov-merge.c > @@ -33,6 +33,11 @@ void __gcov_merge_add (gcov_type *counters __attribute__ > ((unused)), >                         unsigned n_counters __attribute__ ((unused))) {} >  #endif > > +#ifdef L_gcov_merge_ior > +void __gcov_merge_ior (gcov_type *counters  __attribute__ ((unused)), > +                      unsigned n_counters __attribute__ ((unused))) {} > +#endif > + >  #ifdef L_gcov_merge_topn >  void __gcov_merge_topn (gcov_type *counters  __attribute__ ((unused)), >                         unsigned n_counters __attribute__ ((unused))) {} > > It seems that support for the new GCOV_TAG_CONDS is missing in gcov-tool and > gcov-dump, see "tag_table" in gcc/gcov-dump.c and libgcc/libgcov-util.c. > > On 21/03/2022 12:55, Jørgen Kvalsvik via Gcc-patches wrote: > [...] >> Like Wahlen et al this implementation uses bitsets to store conditions, >> which gcov later interprets. This is very fast, but introduces an max >> limit for the number of terms in a single boolean expression. This limit >> is the number of bits in a gcov_unsigned_type (which is typedef'd to >> uint64_t), so for most practical purposes this would be acceptable. >> limitation can be relaxed with a more sophisticated way of storing and >> updating bitsets (for example length-encoding). > > For multi-threaded applications using -fprofile-update=atomic is quite > important. Unfortunately, not all 32-bit targets support 64-bit atomic > operations in hardware. There is a target hook to select the size of gcov_type. > Maybe a dedicated 64-bit type should be used for the bitfield using two 32-bit > atomic OR if necessary. I was not very clear here - I select operations (and limits) based on the width of gcov_unsigned_type, which (judging from other snippets in tree-profile.cc) can be 32 or 64-bit depending on platform. I assume gcov_type is 32 bit on the platforms you allude to, in which case it should still work fine but fail on expressions that would work on 64-bit systems. If I am wrong here I suppose we should consider what you propose (concatenating bitfields would also be a strategy for supporting >64 conditions), but in that case I think the other counters are wrong. I would appreciate it if someone would take a closer look at the code touching the gcov type to see if I got it right, which also includes the atomic code. Should something need fixing I will be happy to do it. > >> >> In action it looks pretty similar to the branch coverage. The -g short >> opt carries no significance, but was chosen because it was an available >> option with the upper-case free too. >> >> gcov --conditions: >> >>          3:   17:void fn (int a, int b, int c, int d) { >>          3:   18:    if ((a && (b || c)) && d) >> conditions covered 5/8 >> condition  1 not covered (false) >> condition  2 not covered (true) >> condition  2 not covered (false) >>          1:   19:        x = 1; >>          -:   20:    else >>          2:   21:        x = 2; >>          3:   22:} > > I have some trouble to understand the output. Would 8/8 mean that we have 100% > MC/DC coverage? What does "not covered (false)" or "not covered (true)" mean? > Yes, 8/8 would mean that the expression is 100% covered (all conditions take on both values and have independent effect on the outcome). "not covered" is a report of missing coverage, that is "condition 1 not covered (false)" means that bit N (N = 1, b in this case) has not taken on false yet, and to achieve 100% coverage you need a test case where b = false. The wording is arbitrary, and I tried to strike a balance between density, clarity, grepability and noise. I am open to other suggestions that would improve this. Unrelated to this, in typing up some notes on this I found a few minor and one quite significant (really, the masking algorithm is broken) error in the algorithm, which I am working on correcting. I will propose the new patch with these fixes too once I have finished writing and testing it.