Having side effects in the argument to a function call just feels like a bad
idea in general even if it seems obviously correct here e.g.
++last_symbol_id,
Can’t we just do:
uint64_t last_symbol_id =
num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() + 1:
1;
Thank you
> On Apr 21, 2021, at 2:25 AM, Pavel Labath via lldb-commits
> <lldb-commits@lists.llvm.org> wrote:
>
>
> Author: Pavel Labath
> Date: 2021-04-21T11:24:43+02:00
> New Revision: cd64273f5ed39ec697ff1e20a1fe25ebd3502629
>
> URL:
> https://github.com/llvm/llvm-project/commit/cd64273f5ed39ec697ff1e20a1fe25ebd3502629
> DIFF:
> https://github.com/llvm/llvm-project/commit/cd64273f5ed39ec697ff1e20a1fe25ebd3502629.diff
>
> LOG: [lldb/ELF] Fix IDs of synthetic eh_frame symbols
>
> The code used the total number of symbols to create a symbol ID for the
> synthetic symbols. This is not correct because the IDs of real symbols
> can be higher than their total number, as we do not add all symbols (and
> in particular, we never add symbol zero, which is not a real symbol).
>
> This meant we could have symbols with duplicate IDs, which caused
> problems if some relocations were referring to the duplicated IDs. This
> was the cause of the failure of the test D97786.
>
> This patch fixes the code to use the ID of the highest (last) symbol
> instead.
>
> Added:
> lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
>
> Modified:
> lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> index f30ed427f8535..6e94ab97992a1 100644
> --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> @@ -2901,8 +2901,11 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab
> *symbol_table,
> // recalculate the index first.
> std::vector<Symbol> new_symbols;
>
> - eh_frame->ForEachFDEEntries([this, symbol_table, section_list,
> &new_symbols](
> - lldb::addr_t file_addr, uint32_t size, dw_offset_t) {
> + size_t num_symbols = symbol_table->GetNumSymbols();
> + uint64_t last_symbol_id =
> + num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() :
> 0;
> + eh_frame->ForEachFDEEntries([&](lldb::addr_t file_addr, uint32_t size,
> + dw_offset_t) {
> Symbol *symbol = symbol_table->FindSymbolAtFileAddress(file_addr);
> if (symbol) {
> if (!symbol->GetByteSizeIsValid()) {
> @@ -2915,21 +2918,20 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab
> *symbol_table,
> if (section_sp) {
> addr_t offset = file_addr - section_sp->GetFileAddress();
> const char *symbol_name = GetNextSyntheticSymbolName().GetCString();
> - uint64_t symbol_id = symbol_table->GetNumSymbols();
> Symbol eh_symbol(
> - symbol_id, // Symbol table index.
> - symbol_name, // Symbol name.
> - eSymbolTypeCode, // Type of this symbol.
> - true, // Is this globally visible?
> - false, // Is this symbol debug info?
> - false, // Is this symbol a trampoline?
> - true, // Is this symbol artificial?
> - section_sp, // Section in which this symbol is defined or
> null.
> - offset, // Offset in section or symbol value.
> - 0, // Size: Don't specify the size as an FDE can
> - false, // Size is valid: cover multiple symbols.
> - false, // Contains linker annotations?
> - 0); // Symbol flags.
> + ++last_symbol_id, // Symbol table index.
> + symbol_name, // Symbol name.
> + eSymbolTypeCode, // Type of this symbol.
> + true, // Is this globally visible?
> + false, // Is this symbol debug info?
> + false, // Is this symbol a trampoline?
> + true, // Is this symbol artificial?
> + section_sp, // Section in which this symbol is defined or null.
> + offset, // Offset in section or symbol value.
> + 0, // Size: Don't specify the size as an FDE
> can
> + false, // Size is valid: cover multiple symbols.
> + false, // Contains linker annotations?
> + 0); // Symbol flags.
> new_symbols.push_back(eh_symbol);
> }
> }
>
> diff --git a/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> new file mode 100644
> index 0000000000000..6178a45de1b59
> --- /dev/null
> +++ b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> @@ -0,0 +1,32 @@
> +# RUN: yaml2obj %s >%t
> +# RUN: %lldb %t -o "image dump symtab" -b | FileCheck %s
> +
> +# CHECK: Index UserID DSX Type File Address/Value Load Address
> Size Flags Name
> +# CHECK: [ 0] 1 SourceFile 0x0000000000000000
> 0x0000000000000000 0x00000004 -
> +# CHECK: [ 1] 2 SX Code 0x0000000000201180
> 0x0000000000000010 0x00000000 ___lldb_unnamed_symbol1$${{.*}}
> +# CHECK: [ 2] 3 SX Code 0x0000000000201190
> 0x0000000000000006 0x00000000 ___lldb_unnamed_symbol2$${{.*}}
> +
> +--- !ELF
> +FileHeader:
> + Class: ELFCLASS64
> + Data: ELFDATA2LSB
> + Type: ET_EXEC
> + Machine: EM_X86_64
> +Sections:
> + - Name: .eh_frame
> + Type: SHT_PROGBITS
> + Flags: [ SHF_ALLOC ]
> + Address: 0x200120
> + AddressAlign: 0x8
> + Content:
> 1400000000000000017A5200017810011B0C0708900100001C0000001C000000401000000600000000410E108602430D06410C07080000001C0000003C000000301000000600000000410E108602430D06410C070800000000000000
> + - Name: .text
> + Type: SHT_PROGBITS
> + Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
> + Address: 0x201180
> + AddressAlign: 0x10
> + Content: 554889E55DC3662E0F1F840000000000554889E55DC3
> +Symbols:
> + - Name: '-'
> + Type: STT_FILE
> + Index: SHN_ABS
> +...
>
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
