[PATCH] D32647: [ELF] - Do not produce duplicate values in .gdb_index's constant pool area.

[Rafael Avila de Espindola via llvm-commits]

LGTM

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 97944.
> grimar added a comment.
>
> - Addressed review comments.
>
>
> https://reviews.llvm.org/D32647
>
> Files:
>   ELF/SyntheticSections.cpp
>   ELF/SyntheticSections.h
>   test/ELF/gdb-index-dup-types.s
>
> Index: test/ELF/gdb-index-dup-types.s
> ===================================================================
> --- test/ELF/gdb-index-dup-types.s
> +++ test/ELF/gdb-index-dup-types.s
> @@ -0,0 +1,60 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: ld.lld --gdb-index %t.o -o %t
> +# RUN: llvm-dwarfdump -debug-dump=gdb_index %t | FileCheck %s
> +
> +## Testcase is based on output produced by gcc version 5.4.1 20160904
> +## it has duplicate entries in .debug_gnu_pubtypes which seems to be
> +## compiler bug. In that case it is useless to have them in .gdb_index
> +## and we filter such entries out to reduce size of .gdb_index.
> +
> +## CHECK: Constant pool offset = {{.*}}, has 1 CU vectors:
> +## CHECK-NOT: 0(0x0): 0x90000000 0x90000000
> +
> +.section .debug_abbrev,"", at progbits
> + .byte 1                       # Abbreviation Code
> + .byte 17                      # DW_TAG_compile_unit
> + .byte 0                       # DW_CHILDREN_no
> + .byte 16                      # DW_AT_stmt_list
> + .byte 23                      # DW_FORM_sec_offset
> + .ascii "\260B"                # DW_AT_GNU_dwo_name
> + .byte 14                      # DW_FORM_strp
> + .byte 27                      # DW_AT_comp_dir
> + .byte 14                      # DW_FORM_strp
> + .ascii "\264B"                # DW_AT_GNU_pubnames
> + .byte 25                      # DW_FORM_flag_present
> + .ascii "\261B"                # DW_AT_GNU_dwo_id
> + .byte 7                       # DW_FORM_data8
> + .ascii "\263B"                # DW_AT_GNU_addr_base
> + .byte 23                      # DW_FORM_sec_offset
> + .byte 0                       # EOM(1)
> + .byte 0                       # EOM(2)
> + .byte 0                       # EOM(3)
> +
> +.section .debug_info,"", at progbits
> +.Lcu_begin0:
> + .long 32                       # Length of Unit
> + .short 4                       # DWARF version number
> + .long .debug_abbrev            # Offset Into Abbrev. Section
> + .byte 8                        # Address Size (in bytes)
> + .byte 1                        # Abbrev [1] 0xb:0x19 DW_TAG_compile_unit
> + .long 0                        # DW_AT_stmt_list
> + .long 0                        # DW_AT_GNU_dwo_name
> + .long 0                        # DW_AT_comp_dir
> + .quad 0                        # DW_AT_GNU_dwo_id
> + .long 0                        # DW_AT_GNU_addr_base
> +
> +.section .debug_gnu_pubtypes,"", at progbits
> +.long .LpubTypes_end0-.LpubTypes_begin0 # Length of Public Types Info
> +.LpubTypes_begin0:
> + .short 2                      # DWARF Version
> + .long .Lcu_begin0             # Offset of Compilation Unit Info
> + .long 36                      # Compilation Unit Length
> + .long 36                      # DIE offset
> + .byte 144                     # Kind: TYPE, STATIC
> + .asciz "int"                  # External Name
> + .long 36                      # DIE offset
> + .byte 144                     # Kind: TYPE, STATIC
> + .asciz "int"                  # External Name
> + .long 0                       # End Mark
> +.LpubTypes_end0:
> Index: ELF/SyntheticSections.h
> ===================================================================
> --- ELF/SyntheticSections.h
> +++ ELF/SyntheticSections.h
> @@ -27,6 +27,8 @@
>  #include "llvm/ADT/MapVector.h"
>  #include "llvm/MC/StringTableBuilder.h"
>  
> +#include <set>
> +
>  namespace lld {
>  namespace elf {
>  
> @@ -511,7 +513,7 @@
>    GdbHashTab SymbolTable;
>  
>    // The CU vector portion of the constant pool.
> -  std::vector<std::vector<std::pair<uint32_t, uint8_t>>> CuVectors;
> +  std::vector<std::set<uint32_t>> CuVectors;
>  
>    std::vector<AddressEntry> AddressArea;
>  
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1788,11 +1788,10 @@
>      std::tie(IsNew, Sym) = SymbolTable.add(Hash, Offset);
>      if (IsNew) {
>        Sym->CuVectorIndex = CuVectors.size();
> -      CuVectors.push_back({{CuId, Pair.second}});
> -      continue;
> +      CuVectors.push_back({});
>      }
>  
> -    CuVectors[Sym->CuVectorIndex].push_back({CuId, Pair.second});
> +    CuVectors[Sym->CuVectorIndex].insert(CuId | (Pair.second << 24));
>    }
>  }
>  
> @@ -1816,7 +1815,7 @@
>    ConstantPoolOffset =
>        SymTabOffset + SymbolTable.getCapacity() * SymTabEntrySize;
>  
> -  for (std::vector<std::pair<uint32_t, uint8_t>> &CuVec : CuVectors) {
> +  for (std::set<uint32_t> &CuVec : CuVectors) {
>      CuVectorsOffset.push_back(CuVectorsSize);
>      CuVectorsSize += OffsetTypeSize * (CuVec.size() + 1);
>    }
> @@ -1869,14 +1868,11 @@
>    }
>  
>    // Write the CU vectors into the constant pool.
> -  for (std::vector<std::pair<uint32_t, uint8_t>> &CuVec : CuVectors) {
> +  for (std::set<uint32_t> &CuVec : CuVectors) {
>      write32le(Buf, CuVec.size());
>      Buf += 4;
> -    for (std::pair<uint32_t, uint8_t> &P : CuVec) {
> -      uint32_t Index = P.first;
> -      uint8_t Flags = P.second;
> -      Index |= Flags << 24;
> -      write32le(Buf, Index);
> +    for (uint32_t Val : CuVec) {
> +      write32le(Buf, Val);
>        Buf += 4;
>      }
>    }