[llvm-commits] [llvm] r162277 - /llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h

[David Blaikie]

On Tue, Aug 21, 2012 at 6:47 AM, Duncan Sands <baldrick at free.fr> wrote:
> Author: baldrick
> Date: Tue Aug 21 08:47:25 2012
> New Revision: 162277
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162277&view=rev
> Log:
> PVS-Studio noticed that EmitVBR64 would perform undefined behaviour if the
> number of bits was bigger than 32.  I checked every use of this function
> that I could find and it looks like the maximum number of bits is 32, so I've
> added an assertion checking this property, and a type cast to (hopefully) stop
> PVS-Studio from warning about this in the future.
>
> Modified:
>     llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h
>
> Modified: llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h?rev=162277&r1=162276&r2=162277&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h (original)
> +++ llvm/trunk/include/llvm/Bitcode/BitstreamWriter.h Tue Aug 21 08:47:25 2012
> @@ -155,6 +155,7 @@
>    }
>
>    void EmitVBR(uint32_t Val, unsigned NumBits) {
> +    assert(NumBits <= 32 && "Too many bits to emit!");
>      uint32_t Threshold = 1U << (NumBits-1);

I don't think this one will warn anyway, since it's assigned to
uint32_t, but the assert's probably not a bad idea anyway.

>
>      // Emit the bits with VBR encoding, NumBits-1 bits at a time.
> @@ -167,10 +168,11 @@
>    }
>
>    void EmitVBR64(uint64_t Val, unsigned NumBits) {
> +    assert(NumBits <= 32 && "Too many bits to emit!");
>      if ((uint32_t)Val == Val)
>        return EmitVBR((uint32_t)Val, NumBits);
>
> -    uint64_t Threshold = 1U << (NumBits-1);
> +    uint64_t Threshold = uint64_t(1U << (NumBits-1));

Rather than adding the cast here (I've no idea if that silences this
static analysis tool) why not just change the type to uint32_t? That
appropriately represents its range and it'll be promoted to uint64_t
for the comparison below anyway, as far as I can see.

>
>      // Emit the bits with VBR encoding, NumBits-1 bits at a time.
>      while (Val >= Threshold) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits