Skip to content

Commit 8b544f3

Browse files
bulbazordbulbazord
authored
[lldb] Do not use LC_FUNCTION_STARTS data to determine symbol size as symbols are created (#155282)
Note: This is a resubmission of #106791. I had to revert this a year ago for a failing test that I could not understand. I have time now to try and get this in again. Summary: This improves the performance of ObjectFileMacho::ParseSymtab by removing eager and expensive work in favor of doing it later in a less-expensive fashion. Experiment: My goal was to understand LLDB's startup time. First, I produced a Debug build of LLDB (no dSYM) and a Release+NoAsserts build of LLDB. The Release build debugged the Debug build as it debugged a small C++ program. I found that ObjectFileMachO::ParseSymtab accounted for somewhere between 1.2 and 1.3 seconds consistently. After applying this change, I consistently measured a reduction of approximately 100ms, putting the time closer to 1.1s and 1.2s on average. Background: ObjectFileMachO::ParseSymtab will incrementally create symbols by parsing nlist entries from the symtab section of a MachO binary. As it does this, it eagerly tries to determine the size of symbols (e.g. how long a function is) using LC_FUNCTION_STARTS data (or eh_frame if LC_FUNCTION_STARTS is unavailable). Concretely, this is done by performing a binary search on the function starts array and calculating the distance to the next function or the end of the section (whichever is smaller). However, this work is unnecessary for 2 reasons: 1. If you have debug symbol entries (i.e. STABs), the size of a function is usually stored right after the function's entry. Performing this work right before parsing the next entry is unnecessary work. 2. Calculating symbol sizes for symbols of size 0 is already performed in `Symtab::InitAddressIndexes` after all the symbols are added to the Symtab. It also does this more efficiently by walking over a list of symbols sorted by address, so the work to calculate the size per symbol is constant instead of O(log n).
1 parent 511210d commit 8b544f3

File tree

1 file changed

+0
-122
lines changed

1 file changed

+0
-122
lines changed

‎lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Lines changed: 0 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -2785,7 +2785,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
27852785
const char *symbol_name_non_abi_mangled = NULL;
27862786

27872787
SectionSP symbol_section;
2788-
uint32_t symbol_byte_size = 0;
27892788
bool add_nlist = true;
27902789
bool is_debug = ((nlist.n_type & N_STAB) != 0);
27912790
bool demangled_is_synthesized = false;
@@ -3437,61 +3436,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
34373436
if (symbol_section) {
34383437
const addr_t section_file_addr =
34393438
symbol_section->GetFileAddress();
3440-
if (symbol_byte_size == 0 &&
3441-
function_starts_count > 0) {
3442-
addr_t symbol_lookup_file_addr = nlist.n_value;
3443-
// Do an exact address match for non-ARM addresses,
3444-
// else get the closest since the symbol might be a
3445-
// thumb symbol which has an address with bit zero
3446-
// set
3447-
FunctionStarts::Entry *func_start_entry =
3448-
function_starts.FindEntry(symbol_lookup_file_addr,
3449-
!is_arm);
3450-
if (is_arm && func_start_entry) {
3451-
// Verify that the function start address is the
3452-
// symbol address (ARM) or the symbol address + 1
3453-
// (thumb)
3454-
if (func_start_entry->addr !=
3455-
symbol_lookup_file_addr &&
3456-
func_start_entry->addr !=
3457-
(symbol_lookup_file_addr + 1)) {
3458-
// Not the right entry, NULL it out...
3459-
func_start_entry = NULL;
3460-
}
3461-
}
3462-
if (func_start_entry) {
3463-
func_start_entry->data = true;
3464-
3465-
addr_t symbol_file_addr = func_start_entry->addr;
3466-
uint32_t symbol_flags = 0;
3467-
if (is_arm) {
3468-
if (symbol_file_addr & 1)
3469-
symbol_flags = MACHO_NLIST_ARM_SYMBOL_IS_THUMB;
3470-
symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
3471-
}
3472-
3473-
const FunctionStarts::Entry *next_func_start_entry =
3474-
function_starts.FindNextEntry(func_start_entry);
3475-
const addr_t section_end_file_addr =
3476-
section_file_addr +
3477-
symbol_section->GetByteSize();
3478-
if (next_func_start_entry) {
3479-
addr_t next_symbol_file_addr =
3480-
next_func_start_entry->addr;
3481-
// Be sure the clear the Thumb address bit when
3482-
// we calculate the size from the current and
3483-
// next address
3484-
if (is_arm)
3485-
next_symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
3486-
symbol_byte_size = std::min<lldb::addr_t>(
3487-
next_symbol_file_addr - symbol_file_addr,
3488-
section_end_file_addr - symbol_file_addr);
3489-
} else {
3490-
symbol_byte_size =
3491-
section_end_file_addr - symbol_file_addr;
3492-
}
3493-
}
3494-
}
34953439
symbol_value -= section_file_addr;
34963440
}
34973441

@@ -3620,9 +3564,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
36203564
}
36213565
sym[sym_idx].SetFlags(nlist.n_type << 16 | nlist.n_desc);
36223566

3623-
if (symbol_byte_size > 0)
3624-
sym[sym_idx].SetByteSize(symbol_byte_size);
3625-
36263567
if (demangled_is_synthesized)
36273568
sym[sym_idx].SetDemangledNameIsSynthesized(true);
36283569
++sym_idx;
@@ -3711,7 +3652,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
37113652

37123653
SymbolType type = eSymbolTypeInvalid;
37133654
SectionSP symbol_section;
3714-
lldb::addr_t symbol_byte_size = 0;
37153655
bool add_nlist = true;
37163656
bool is_gsym = false;
37173657
bool demangled_is_synthesized = false;
@@ -4297,47 +4237,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
42974237

42984238
if (symbol_section) {
42994239
const addr_t section_file_addr = symbol_section->GetFileAddress();
4300-
if (symbol_byte_size == 0 && function_starts_count > 0) {
4301-
addr_t symbol_lookup_file_addr = nlist.n_value;
4302-
// Do an exact address match for non-ARM addresses, else get the
4303-
// closest since the symbol might be a thumb symbol which has an
4304-
// address with bit zero set.
4305-
FunctionStarts::Entry *func_start_entry =
4306-
function_starts.FindEntry(symbol_lookup_file_addr, !is_arm);
4307-
if (is_arm && func_start_entry) {
4308-
// Verify that the function start address is the symbol address
4309-
// (ARM) or the symbol address + 1 (thumb).
4310-
if (func_start_entry->addr != symbol_lookup_file_addr &&
4311-
func_start_entry->addr != (symbol_lookup_file_addr + 1)) {
4312-
// Not the right entry, NULL it out...
4313-
func_start_entry = nullptr;
4314-
}
4315-
}
4316-
if (func_start_entry) {
4317-
func_start_entry->data = true;
4318-
4319-
addr_t symbol_file_addr = func_start_entry->addr;
4320-
if (is_arm)
4321-
symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
4322-
4323-
const FunctionStarts::Entry *next_func_start_entry =
4324-
function_starts.FindNextEntry(func_start_entry);
4325-
const addr_t section_end_file_addr =
4326-
section_file_addr + symbol_section->GetByteSize();
4327-
if (next_func_start_entry) {
4328-
addr_t next_symbol_file_addr = next_func_start_entry->addr;
4329-
// Be sure the clear the Thumb address bit when we calculate the
4330-
// size from the current and next address
4331-
if (is_arm)
4332-
next_symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
4333-
symbol_byte_size = std::min<lldb::addr_t>(
4334-
next_symbol_file_addr - symbol_file_addr,
4335-
section_end_file_addr - symbol_file_addr);
4336-
} else {
4337-
symbol_byte_size = section_end_file_addr - symbol_file_addr;
4338-
}
4339-
}
4340-
}
43414240
symbol_value -= section_file_addr;
43424241
}
43434242

@@ -4444,9 +4343,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
44444343
if (nlist.n_desc & N_WEAK_REF)
44454344
sym[sym_idx].SetIsWeak(true);
44464345

4447-
if (symbol_byte_size > 0)
4448-
sym[sym_idx].SetByteSize(symbol_byte_size);
4449-
44504346
if (demangled_is_synthesized)
44514347
sym[sym_idx].SetDemangledNameIsSynthesized(true);
44524348

@@ -4565,23 +4461,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
45654461
Address symbol_addr;
45664462
if (module_sp->ResolveFileAddress(symbol_file_addr, symbol_addr)) {
45674463
SectionSP symbol_section(symbol_addr.GetSection());
4568-
uint32_t symbol_byte_size = 0;
45694464
if (symbol_section) {
4570-
const addr_t section_file_addr = symbol_section->GetFileAddress();
4571-
const FunctionStarts::Entry *next_func_start_entry =
4572-
function_starts.FindNextEntry(func_start_entry);
4573-
const addr_t section_end_file_addr =
4574-
section_file_addr + symbol_section->GetByteSize();
4575-
if (next_func_start_entry) {
4576-
addr_t next_symbol_file_addr = next_func_start_entry->addr;
4577-
if (is_arm)
4578-
next_symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
4579-
symbol_byte_size = std::min<lldb::addr_t>(
4580-
next_symbol_file_addr - symbol_file_addr,
4581-
section_end_file_addr - symbol_file_addr);
4582-
} else {
4583-
symbol_byte_size = section_end_file_addr - symbol_file_addr;
4584-
}
45854465
sym[sym_idx].SetID(synthetic_sym_id++);
45864466
// Don't set the name for any synthetic symbols, the Symbol
45874467
// object will generate one if needed when the name is accessed
@@ -4593,8 +4473,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
45934473
add_symbol_addr(symbol_addr.GetFileAddress());
45944474
if (symbol_flags)
45954475
sym[sym_idx].SetFlags(symbol_flags);
4596-
if (symbol_byte_size)
4597-
sym[sym_idx].SetByteSize(symbol_byte_size);
45984476
++sym_idx;
45994477
}
46004478
}

0 commit comments

Comments
 (0)