[llvm] [Coverage] getMaxBitmapSize: Scan `max(BitmapIdx)` instead of the last `Decision` (PR #78963)

NAKAMURA Takumi via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 22 04:33:42 PST 2024


https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/78963

>From b8cf2c9c0f810c7e1e60c5b82cc557d678aae096 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 22 Jan 2024 20:44:20 +0900
Subject: [PATCH 1/2] [Coverage] The testcase for #78922

---
 .../ProfileData/Coverage/CoverageMapping.cpp  |   1 +
 llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.c  |  13 +++++++++++
 llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.o  | Bin 0 -> 4648 bytes
 .../tools/llvm-cov/Inputs/mcdc-maxbs.proftext |   9 ++++++++
 llvm/test/tools/llvm-cov/mcdc-maxbs.test      |  21 ++++++++++++++++++
 5 files changed, 44 insertions(+)
 create mode 100644 llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.c
 create mode 100644 llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.o
 create mode 100644 llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.proftext
 create mode 100644 llvm/test/tools/llvm-cov/mcdc-maxbs.test

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eece6a2cc71797..37c52cabf53cdc 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -229,6 +229,7 @@ Expected<BitVector> CounterMappingContext::evaluateBitmap(
   unsigned SizeInBits = llvm::alignTo(uint64_t(1) << NC, CHAR_BIT);
   unsigned SizeInBytes = SizeInBits / CHAR_BIT;
 
+  assert(ID + SizeInBytes <= BitmapBytes.size() && "BitmapBytes overrun");
   ArrayRef<uint8_t> Bytes(&BitmapBytes[ID], SizeInBytes);
 
   // Mask each bitmap byte into the BitVector. Go in reverse so that the
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.c b/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.c
new file mode 100644
index 00000000000000..83644b29549455
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.c
@@ -0,0 +1,13 @@
+#define RANGE(a,b,c) ((a) <= (b) && (b) <= (c))
+
+int sub(int c) {
+  if (RANGE('0', c, '9')) return 1;
+  return (('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'));
+}
+
+extern void __llvm_profile_write_file(void);
+
+int main(int c, char **v)
+{
+  return (c > 1 ? sub(c) : 0);
+}
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.o b/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.o
new file mode 100644
index 0000000000000000000000000000000000000000..269902224ccae9ca01c2b858f020410d7f221ce5
GIT binary patch
literal 4648
zcmb_fVQf=X6u$4hb!)e7>jnb`OgltHlt&9}Y>r at oZB}9YFfp27Xu7T)leJyiZcGGh
zQhxx+46<lUH1UTf8iI-bK!HD&7>Pqj7L3H8Ch|v!{7{TBQAz4K at 14`$-L;tso}@eX
zeCM2d?)mP0_r1N+x2s>*G((VRVyn>Dko20mk`b_m0kIMyg9Qg#xx)G*4;RPX2L@=I
ztw*nutg|@t7dEE(XM$5!uK#MVAdbx*yL90fBYUGJ at TnHK`g3r~$_zL&wHjsUxx%*8
z3{7<&r^*%fq#MrRBsA6SGvxN?{WG~jxN-Wz)Gn8ndXM2hbA^`!jVCWox$mT3<D_gX
zW;WjcDog_t+5PzQxq^OZge+3f<b>ShPQn*xvIXb$=Ys|PzqR;vvD}{YB8xI=(VFP?
zXXN(h&)FP=Qy&m?4!1E^IC$VqI}P_NIAy10VGMG7-w#_)?mhk2CosNv<ZfqdZAWnL
z;l;nUpL+2fI0rSFLvLwmZE9<4*IcgpCe82|x?8IeZiiSd-p*#TPKQ__9?->VEr*s4
zM`w#J>O}h*tqN3+F5F_VuccZutWk?tCcM7ZT0`hKY7!5Mo+j~-Fnw(edX>OYo7f;8
z_q8{Jg`;XsTOxd#SPD}@(o(VB=USra0s$AKulLn22Mb5=_wR3~Zgvj6@%>sDzmT&}
zF88B>II6WeO^43vgvaZ^g?b#KSxnoVEE26c9OE`f25GgGs0OV!ESbC>`Dtcg_fyxd
z44e+Gy|%Mj+*en1m}34f#rwf~vCzCO3Mf9XN&@r2Tqy2&CNe3D(=&PfQFCDM^1$WG
zhN$n+?}*4~I5F&<h^Epb$%NP6W%^9t at JPnaT1?5>!Doav>CslWYn&%w8$5uoB;S{$
z5%babI%(W7e1K8c5LQdt4H|EO2C^|9r5S#U;K#KQQ{^TARpL%T#nHyISWXP#oCp75
zA<X(*w;_0?C0w(PpmbT5Tu>J~nlHffJn&r$X`F{6U|8(Vb$w)+O>;rA42!Y(F2iE6
z9By41a~v+{t6AcW_w)=zAwEmdysx<T?K<wPEmue*J?%R0v=IG-^LfUdLGRsyEzAJp
z%-{;jW1P=IyM$x)e`uaZBpm0*%Lb*xR#s<n{1Xz6 at qCu|NI1sxI)@dU>t-Yz=UGkm
zaRq;j>}dt(x~CL;6WO0D_!hFik#OXU*Lg$1nTJ~vj_c%j`~1Uw2pHnbKYt`J&iq&7
z27 at 3j^VuNbI1krdF5z<gY6-`9=3%RZ%ltpB;LQJX5{~mQ|8WIp{wEZi>mE~Z=Kp;K
zXa4!FS_U<-u55z|??Rg&)F`qaIhJkoV;tT#9PfoO3i6CU7G4+Qv*c)MHXJq1T6cYe
z2!$faiBvQa>fPSm(Ya}3XW!<o-rkP>ZQ~_ZfA_W>9ev#!Hum-I=<MnOmzj!=hRsZL
zG9%1LG9Hg6G7vsGIuQ?zrDCB-B9oTw!I8OkA{>vVg*lu`j*oF1EF&J4t=M=1d}FC(
zY%m^*hez0o*oK)%W}@bkyPsW`&V(cTg=tj}4-LVaX_yum-4}|bpq7Y7h9c|Y;mN_Y
z87WPg9v>tl62h^W&ZIJ7)Gx&?ja^D at 4TUpdruLq6J)9H1<K&5cr&r4cI!EkNOo8VL
zG5j`>B|ye0C4tWm$8LrJK63#ENFSvMXXy~(y^2GA6Y_aG1X#JZOO_sFj>mkA<L8_8
z>wsBH)V(k&$#1l7em}5>=VQB4{i9{-<M#pAWnHO$p8DqZ5Z6=d=fGhv)ypB4cG9Nu
z{lQK(ztZ|olvzJQyl&R`-og6tRXs;GQGHTHAJJO0E7gCC3h;Ts{lg={by at EOtX%yr
zDjY0UvIlDat01{tewOkB3*@&!vb7ea|L5uX at ligmAHSx!{zB*XLu!!YF(0SozFAk|
zKd%xGxhUAHES32Gm<q`J<JTkCWnGDXuFv^gllx)2692RJ$p6tY{NuMd*Ju9s1BRUF
z!V7;(;-LodnUzK5Z#y^<<9+7XRg};FyD`rBtj|+E`&sue!3>b*JcfUS0bpaw at e={c
z4{#u9?w{fBV1S_k21uVR!yj_S`OM!Xz>HmmB&aO>igJtc@!J3e&o7F){uTh#wPReK
O*8deHpu%r%wf?{KA<;Jg

literal 0
HcmV?d00001

diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.proftext b/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.proftext
new file mode 100644
index 00000000000000..f24e984f657f2b
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/Inputs/mcdc-maxbs.proftext
@@ -0,0 +1,9 @@
+main
+# Func Hash:
+99164
+# Num Counters:
+2
+# Counter Values:
+1
+0
+
diff --git a/llvm/test/tools/llvm-cov/mcdc-maxbs.test b/llvm/test/tools/llvm-cov/mcdc-maxbs.test
new file mode 100644
index 00000000000000..2d4e05949778e1
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/mcdc-maxbs.test
@@ -0,0 +1,21 @@
+# Test the assertion failure in llvm-cov with empty bitmap (#78922)
+# REQUIRES: asserts
+
+# RUN: llvm-profdata merge %S/Inputs/mcdc-maxbs.proftext -o %t.profdata
+# RUN: llvm-cov report --show-mcdc-summary %S/Inputs/mcdc-maxbs.o -instr-profile %t.profdata
+
+# Instructions for regenerating the test object:
+
+clang -O3 -fcoverage-mcdc -fprofile-instr-generate \
+    -fcoverage-mapping -fcoverage-compilation-dir=. \
+    mcdc-maxbs.c -c -o mcdc-maxbs.o
+
+# Instructions for regenerating the test vector:
+
+clang -fprofile-instr-generate mcdc-maxbs.o
+
+# Doesn't crash if argc > 1
+./a.out
+
+llvm-profdata merge --sparse -o default.profdata default.profraw
+llvm-profdata merge --text -o mcdc-maxbs.proftext default.profdata

>From be57b2b8802b45d638b585be98b0994b24a46065 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 21 Jan 2024 18:03:41 +0900
Subject: [PATCH 2/2] getMaxBitmapSize: Scan max(BitmapIdx).

Note that `<=` is used insted of `<`, because `BitmapIdx == 0` is valid
and `MaxbitmapID` is `unsigned`. `BitmapIdx` is unique in the record.
---
 llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 37c52cabf53cdc..9336c3120e6476 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -567,14 +567,14 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
                                  const CoverageMappingRecord &Record) {
   unsigned MaxBitmapID = 0;
   unsigned NumConditions = 0;
-  // The last DecisionRegion has the highest bitmap byte index used in the
-  // function, which when combined with its number of conditions, yields the
-  // full bitmap size.
+  // Scan max(BitmapIdx).
+  // Note that `<=` is used insted of `<`, because `BitmapIdx == 0` is valid
+  // and `MaxbitmapID is `unsigned`. `BitmapIdx` is unique in the record.
   for (const auto &Region : reverse(Record.MappingRegions)) {
-    if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
+    if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion &&
+        MaxBitmapID <= Region.MCDCParams.BitmapIdx) {
       MaxBitmapID = Region.MCDCParams.BitmapIdx;
       NumConditions = Region.MCDCParams.NumConditions;
-      break;
     }
   }
   unsigned SizeInBits = llvm::alignTo(uint64_t(1) << NumConditions, CHAR_BIT);



More information about the llvm-commits mailing list