From 88c8581d9fe449f07781ff8abedf598b9410e727 Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Tue, 16 Jun 2020 17:05:51 +0300 Subject: [PATCH] [llvm-readobj] - Do not crash when GnuHashTable->symndx is greater than the dynamic symbols count. `Elf_GnuHash_Impl` has the following method: ``` ArrayRef values(unsigned DynamicSymCount) const { return ArrayRef(buckets().end(), DynamicSymCount - symndx); } ``` When DynamicSymCount is less than symndx we return an array with the huge broken size. This patch fixes the issue and adds an assert. This assert helped to fix an issue in one of the test cases. Differential revision: https://reviews.llvm.org/D81937 --- llvm/include/llvm/Object/ELFTypes.h | 1 + llvm/test/tools/llvm-readobj/ELF/gnuhash.test | 24 +++++++++++++++------- .../tools/llvm-readobj/ELF/hash-histogram.test | 14 ++++++------- llvm/tools/llvm-readobj/ELFDumper.cpp | 13 ++++++------ 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h index ac9eb48..5e85e6c 100644 --- a/llvm/include/llvm/Object/ELFTypes.h +++ b/llvm/include/llvm/Object/ELFTypes.h @@ -539,6 +539,7 @@ struct Elf_GnuHash_Impl { } ArrayRef values(unsigned DynamicSymCount) const { + assert(DynamicSymCount >= symndx); return ArrayRef(buckets().end(), DynamicSymCount - symndx); } }; diff --git a/llvm/test/tools/llvm-readobj/ELF/gnuhash.test b/llvm/test/tools/llvm-readobj/ELF/gnuhash.test index 415c3b4f..84a2938 100644 --- a/llvm/test/tools/llvm-readobj/ELF/gnuhash.test +++ b/llvm/test/tools/llvm-readobj/ELF/gnuhash.test @@ -229,15 +229,25 @@ ProgramHeaders: ## which is one larger than the index of the last dynamic symbol. ## For empty tables however, this value is unimportant and can be ignored. -# RUN: yaml2obj --docnum=5 %s -o %t.empty -# RUN: llvm-readobj --gnu-hash-table %t.empty 2>&1 \ -# RUN: | FileCheck %s -DFILE=%t.empty --check-prefix=EMPTY --implicit-check-not="warning:" -# RUN: llvm-readelf --gnu-hash-table %t.empty 2>&1 \ -# RUN: | FileCheck %s -DFILE=%t.empty --check-prefix=EMPTY --implicit-check-not="warning:" +## Case A: set the index of the first symbol in the dynamic symbol table to +## the number of dynamic symbols. +# RUN: yaml2obj --docnum=5 -DSYMNDX=0x1 %s -o %t.empty.1 +# RUN: llvm-readobj --gnu-hash-table %t.empty.1 2>&1 \ +# RUN: | FileCheck %s -DFILE=%t.empty.1 -DSYMNDX=1 --check-prefix=EMPTY --implicit-check-not="warning:" +# RUN: llvm-readelf --gnu-hash-table %t.empty.1 2>&1 \ +# RUN: | FileCheck %s -DFILE=%t.empty.1 -DSYMNDX=1 --check-prefix=EMPTY --implicit-check-not="warning:" + +## Case B: set the index of the first symbol in the dynamic symbol table to +## an arbitrary value that is larger than the number of dynamic symbols. +# RUN: yaml2obj --docnum=5 -DSYMNDX=0x2 %s -o %t.empty.2 +# RUN: llvm-readobj --gnu-hash-table %t.empty.2 2>&1 \ +# RUN: | FileCheck %s -DFILE=%t.empty.2 -DSYMNDX=2 --check-prefix=EMPTY --implicit-check-not="warning:" +# RUN: llvm-readelf --gnu-hash-table %t.empty.2 2>&1 \ +# RUN: | FileCheck %s -DFILE=%t.empty.2 -DSYMNDX=2 --check-prefix=EMPTY --implicit-check-not="warning:" # EMPTY: GnuHashTable { # EMPTY-NEXT: Num Buckets: 1 -# EMPTY-NEXT: First Hashed Symbol Index: 1 +# EMPTY-NEXT: First Hashed Symbol Index: [[SYMNDX]] # EMPTY-NEXT: Num Mask Words: 1 # EMPTY-NEXT: Shift Count: 0 # EMPTY-NEXT: Bloom Filter: [0x0] @@ -256,7 +266,7 @@ Sections: Type: SHT_GNU_HASH Flags: [ SHF_ALLOC ] Header: - SymNdx: 0x1 + SymNdx: [[SYMNDX]] Shift2: 0x0 BloomFilter: [ 0x0 ] HashBuckets: [ 0x0 ] diff --git a/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test b/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test index a9d2455..6628891 100644 --- a/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test +++ b/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test @@ -211,8 +211,8 @@ ProgramHeaders: ## Check we dump a histogram for the .gnu.hash table even when the .hash table is skipped. ## Case A: the .hash table has no data to build histogram and it is skipped. -## (NBUCKET == 0x1 is a no-op: it does not change the number of buckets described with the "Bucket" key). -# RUN: yaml2obj --docnum=5 -DNBUCKET=0x1 %s -o %t5.o +## (NBUCKET == 0x2 is a no-op: it does not change the number of buckets described by the "Bucket" key). +# RUN: yaml2obj --docnum=5 -DNBUCKET=0x2 %s -o %t5.o # RUN: llvm-readelf --elf-hash-histogram %t5.o 2>&1 | \ # RUN: FileCheck %s --check-prefix=GNU-HASH --implicit-check-not="Histogram" @@ -222,8 +222,8 @@ ProgramHeaders: # RUN: llvm-readelf --elf-hash-histogram %t6.o 2>&1 | \ # RUN: FileCheck %s -DFILE=%t6.o --check-prefixes=WARN,GNU-HASH -# WARN: warning: '[[FILE]]': the hash table at offset 0x78 goes past the end of the file (0x350), nbucket = 4294967295, nchain = 1 -# GNU-HASH: Histogram for `.gnu.hash' bucket list length (total of 1 buckets) +# WARN: warning: '[[FILE]]': the hash table at offset 0x78 goes past the end of the file (0x358), nbucket = 4294967295, nchain = 2 +# GNU-HASH: Histogram for `.gnu.hash' bucket list length (total of 3 buckets) --- !ELF FileHeader: @@ -237,7 +237,7 @@ Sections: Flags: [ SHF_ALLOC ] Bucket: [ 0 ] NBucket: [[NBUCKET]] - Chain: [ 0 ] + Chain: [ 0, 0 ] - Name: .gnu.hash Type: SHT_GNU_HASH Flags: [ SHF_ALLOC ] @@ -254,8 +254,8 @@ Sections: - Tag: DT_HASH Value: 0x0 - Tag: DT_GNU_HASH -## sizeof(.hash) == 0x28. - Value: 0x28 +## sizeof(.hash) == 0x14. + Value: 0x14 - Tag: DT_NULL Value: 0x0 DynamicSymbols: diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 6b3c92d..29776b1 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -2731,18 +2731,19 @@ getGnuHashTableChains(const typename ELFT::SymRange DynSymTable, // A normal empty GNU hash table section produced by linker might have // symndx set to the number of dynamic symbols + 1 (for the zero symbol) // and have dummy null values in the Bloom filter and in the buckets - // vector. It happens because the value of symndx is not important for - // dynamic loaders when the GNU hash table is empty. They just skip the - // whole object during symbol lookup. In such cases, the symndx value is - // irrelevant and we should not report a warning. + // vector (or no values at all). It happens because the value of symndx is not + // important for dynamic loaders when the GNU hash table is empty. They just + // skip the whole object during symbol lookup. In such cases, the symndx value + // is irrelevant and we should not report a warning. ArrayRef Buckets = GnuHashTable->buckets(); if (!llvm::all_of(Buckets, [](typename ELFT::Word V) { return V == 0; })) return createError("the first hashed symbol index (" + Twine(GnuHashTable->symndx) + ") is larger than the number of dynamic symbols (" + Twine(NumSyms) + ")"); - - return GnuHashTable->values(NumSyms); + // There is no way to represent an array of (dynamic symbols count - symndx) + // length. + return ArrayRef(); } template -- 2.7.4