<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 5, 2016 at 4:03 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>Apelete Seketeli via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
> apelete updated this revision to Diff 56361.<br>
> apelete added a comment.<br>
><br>
> [scan-build] fix warning emitted on LLVM Bitcode code base<br>
><br>
> Changes since last revision:<br>
><br>
> - initialize FnEntry8BitAbbrev, FnEntry7BitAbbrev and<br>
> FnEntry6BitAbbrev variables to 0 and assert they are non-zero prior<br>
> being read.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D19975" rel="noreferrer">http://reviews.llvm.org/D19975</a><br>
><br>
> Files:<br>
>   lib/Bitcode/Writer/BitcodeWriter.cpp<br>
><br>
> Index: lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> ===================================================================<br>
> --- lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> +++ lib/Bitcode/Writer/BitcodeWriter.cpp<br>
> @@ -2507,9 +2507,9 @@<br>
><br>
>    // For the module-level VST, add abbrev Ids for the VST_CODE_FNENTRY<br>
>    // records, which are not used in the per-function VSTs.<br>
> -  unsigned FnEntry8BitAbbrev;<br>
> -  unsigned FnEntry7BitAbbrev;<br>
> -  unsigned FnEntry6BitAbbrev;<br>
> +  unsigned FnEntry8BitAbbrev = 0;<br>
> +  unsigned FnEntry7BitAbbrev = 0;<br>
> +  unsigned FnEntry6BitAbbrev = 0;<br>
>    if (IsModuleLevel && hasVSTOffsetPlaceholder()) {<br>
>      // 8-bit fixed-width VST_CODE_FNENTRY function strings.<br>
>      BitCodeAbbrev *Abbv = new BitCodeAbbrev();<br>
> @@ -2582,11 +2582,15 @@<br>
>        NameVals.push_back(BitcodeIndex / 32);<br>
><br>
>        Code = bitc::VST_CODE_FNENTRY;<br>
> +      assert(FnEntry8BitAbbrev && "abbrev was not computed");<br>
>        AbbrevToUse = FnEntry8BitAbbrev;<br>
> -      if (Bits == SE_Char6)<br>
> +      if (Bits == SE_Char6) {<br>
> +        assert(FnEntry6BitAbbrev && "abbrev was not computed");<br>
>          AbbrevToUse = FnEntry6BitAbbrev;<br>
> -      else if (Bits == SE_Fixed7)<br>
> +      } else if (Bits == SE_Fixed7) {<br>
> +        assert(FnEntry7BitAbbrev && "abbrev was not computed");<br>
>          AbbrevToUse = FnEntry7BitAbbrev;<br>
> +      }<br>
<br>
</div></div>This isn't a very maintainable approach. If other uses of these abbrevs<br>
pop up in different paths they would also need the asserts, and no<br>
compiler diagnostics or memory error tools would help to find them.<br></blockquote><div><br></div><div>I tend to agree with Justin (and David Blaikie, who pointed this out in a separate thread), that fixing things this way can actually hide real errors from the other sanitizers. I'd like to see some resolution on that other thread ("Before we go cleaning up LLVM+Clang of all Static Analyzer Warnings...") before we decide to fix these this way.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Case in point, there are two more uses of 6_ABBREV and 7_ABBREV a couple<br>
of lines below.<br></blockquote><div><br></div><div>I actually don't think this is an issue - those abbrev ids are computed unconditionally elsewhere. It is only the FnEntry*BitAbbrevs that are conditionally setup here (for the module level VST only). </div><div><br></div><div>Teresa</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
All in all, I think that fixing the sanitizer warning in this way is<br>
actually making this code more prone to hard to detect bugs.<br>
<div><div><br>
>      } else {<br>
>        Code = bitc::VST_CODE_ENTRY;<br>
>        if (Bits == SE_Char6)<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><span style="font-family:times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:408-460-2413" value="+14084602413">408-460-2413</a></td></tr></tbody></table></span></div>
</div></div>