> On Feb 20, 2016, at 1:00 PM, Duncan P. N. Exon Smith via cfe-commits
> <cfe-commits@lists.llvm.org> wrote:
>
> Author: dexonsmith
> Date: Sat Feb 20 15:00:58 2016
> New Revision: 261448
>
> URL: http://llvm.org/viewvc/llvm-project?rev=261448&view=rev
> Log:
> Lex: Check buckets on header map construction
>
> If the number of buckets is not a power of two, immediately recognize
> the header map as corrupt, rather than waiting for the first lookup. I
> converted the later check to an assert.
>
> Modified:
> cfe/trunk/lib/Lex/HeaderMap.cpp
> cfe/trunk/unittests/Lex/HeaderMapTest.cpp
>
> Modified: cfe/trunk/lib/Lex/HeaderMap.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderMap.cpp?rev=261448&r1=261447&r2=261448&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/HeaderMap.cpp (original)
> +++ cfe/trunk/lib/Lex/HeaderMap.cpp Sat Feb 20 15:00:58 2016
> @@ -19,6 +19,7 @@
> #include "llvm/Support/DataTypes.h"
> #include "llvm/Support/MathExtras.h"
> #include "llvm/Support/MemoryBuffer.h"
> +#include "llvm/Support/SwapByteOrder.h"
> #include <cstdio>
> #include <memory>
> using namespace clang;
> @@ -82,6 +83,15 @@ bool HeaderMapImpl::checkHeader(const ll
> if (Header->Reserved != 0)
> return false;
>
> + // Check the number of buckets.
> + auto NumBuckets = NeedsByteSwap
> + ? llvm::sys::getSwappedBytes(Header->NumBuckets)
> + : Header->NumBuckets;
> +
> + // If the number of buckets is not a power of two, the headermap is
> corrupt.
Support/MathExtras.h defines isPowerOf2_32 which is IMHO more readable and also
checks for 0 (which may or may not be what we want here).
-- adrian
> + if (NumBuckets & (NumBuckets - 1))
> + return false;
> +
> // Okay, everything looks good.
> return true;
> }
> @@ -191,10 +201,8 @@ StringRef HeaderMapImpl::lookupFilename(
> const HMapHeader &Hdr = getHeader();
> unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
>
> - // If the number of buckets is not a power of two, the headermap is
> corrupt.
> - // Don't probe infinitely.
> - if (NumBuckets & (NumBuckets-1))
> - return StringRef();
> + // Don't probe infinitely. This should be checked before constructing.
> + assert(!(NumBuckets & (NumBuckets - 1)) && "Expected power of 2");
>
> // Linearly probe the hash table.
> for (unsigned Bucket = HashHMapKey(Filename);; ++Bucket) {
>
> Modified: cfe/trunk/unittests/Lex/HeaderMapTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/HeaderMapTest.cpp?rev=261448&r1=261447&r2=261448&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Lex/HeaderMapTest.cpp (original)
> +++ cfe/trunk/unittests/Lex/HeaderMapTest.cpp Sat Feb 20 15:00:58 2016
> @@ -91,4 +91,13 @@ TEST(HeaderMapTest, checkHeaderValidButE
> ASSERT_TRUE(NeedsSwap);
> }
>
> +TEST(HeaderMapTest, checkHeader3Buckets) {
> + MapFile<3, 1> File;
> + ASSERT_EQ(3 * sizeof(HMapBucket), sizeof(File.Buckets));
> +
> + File.init();
> + bool NeedsSwap;
> + ASSERT_FALSE(HeaderMapImpl::checkHeader(*File.getBuffer(), NeedsSwap));
> +}
> +
> } // end namespace
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
