[PATCH] D19975: [scan-build] fix warning emitted on LLVM Bitcode code base

[Justin Bogner via llvm-commits]

Apelete Seketeli via llvm-commits <llvm-commits at lists.llvm.org> writes:
> apelete updated this revision to Diff 56361.
> apelete added a comment.
>
> [scan-build] fix warning emitted on LLVM Bitcode code base
>
> Changes since last revision:
>
> - initialize FnEntry8BitAbbrev, FnEntry7BitAbbrev and
> FnEntry6BitAbbrev variables to 0 and assert they are non-zero prior
> being read.
>
>
> http://reviews.llvm.org/D19975
>
> Files:
>   lib/Bitcode/Writer/BitcodeWriter.cpp
>
> Index: lib/Bitcode/Writer/BitcodeWriter.cpp
> ===================================================================
> --- lib/Bitcode/Writer/BitcodeWriter.cpp
> +++ lib/Bitcode/Writer/BitcodeWriter.cpp
> @@ -2507,9 +2507,9 @@
>  
>    // For the module-level VST, add abbrev Ids for the VST_CODE_FNENTRY
>    // records, which are not used in the per-function VSTs.
> -  unsigned FnEntry8BitAbbrev;
> -  unsigned FnEntry7BitAbbrev;
> -  unsigned FnEntry6BitAbbrev;
> +  unsigned FnEntry8BitAbbrev = 0;
> +  unsigned FnEntry7BitAbbrev = 0;
> +  unsigned FnEntry6BitAbbrev = 0;
>    if (IsModuleLevel && hasVSTOffsetPlaceholder()) {
>      // 8-bit fixed-width VST_CODE_FNENTRY function strings.
>      BitCodeAbbrev *Abbv = new BitCodeAbbrev();
> @@ -2582,11 +2582,15 @@
>        NameVals.push_back(BitcodeIndex / 32);
>  
>        Code = bitc::VST_CODE_FNENTRY;
> +      assert(FnEntry8BitAbbrev && "abbrev was not computed");
>        AbbrevToUse = FnEntry8BitAbbrev;
> -      if (Bits == SE_Char6)
> +      if (Bits == SE_Char6) {
> +        assert(FnEntry6BitAbbrev && "abbrev was not computed");
>          AbbrevToUse = FnEntry6BitAbbrev;
> -      else if (Bits == SE_Fixed7)
> +      } else if (Bits == SE_Fixed7) {
> +        assert(FnEntry7BitAbbrev && "abbrev was not computed");
>          AbbrevToUse = FnEntry7BitAbbrev;
> +      }

This isn't a very maintainable approach. If other uses of these abbrevs
pop up in different paths they would also need the asserts, and no
compiler diagnostics or memory error tools would help to find them.

Case in point, there are two more uses of 6_ABBREV and 7_ABBREV a couple
of lines below.

All in all, I think that fixing the sanitizer warning in this way is
actually making this code more prone to hard to detect bugs.

>      } else {
>        Code = bitc::VST_CODE_ENTRY;
>        if (Bits == SE_Char6)