JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, teemperor.
JDevlieghere requested review of this revision.
As I was trying to understand what `IRExecutionUnit::FindInSymbols` was doing,
I couldn't help myself but refactor the code a little bit:
- No else after return.
- Move the lambda out of the loop.
- Replace `GetSize() == 0` with `IsEmpty()`
- Eliminate pitfall of not clearing the `SymbolContextList` by moving it into
the lexical scope of the if-clause.
And probably some other small things that bothered me.
https://reviews.llvm.org/D107206
Files:
lldb/source/Expression/IRExecutionUnit.cpp
Index: lldb/source/Expression/IRExecutionUnit.cpp
===================================================================
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -781,121 +781,112 @@
return LLDB_INVALID_ADDRESS;
}
- for (const SearchSpec &spec : specs) {
- SymbolContextList sc_list;
-
- lldb::addr_t best_internal_load_address = LLDB_INVALID_ADDRESS;
-
- std::function<bool(lldb::addr_t &, SymbolContextList &,
- const lldb_private::SymbolContext &)>
- get_external_load_address = [&best_internal_load_address, target,
- &symbol_was_missing_weak](
- lldb::addr_t &load_address, SymbolContextList &sc_list,
- const lldb_private::SymbolContext &sc) -> lldb::addr_t {
- load_address = LLDB_INVALID_ADDRESS;
-
- if (sc_list.GetSize() == 0)
- return false;
-
- // missing_weak_symbol will be true only if we found only weak undefined
- // references to this symbol.
- symbol_was_missing_weak = true;
- for (auto candidate_sc : sc_list.SymbolContexts()) {
- // Only symbols can be weak undefined:
- if (!candidate_sc.symbol)
- symbol_was_missing_weak = false;
- else if (candidate_sc.symbol->GetType() != lldb::eSymbolTypeUndefined
- || !candidate_sc.symbol->IsWeak())
- symbol_was_missing_weak = false;
-
- const bool is_external =
- (candidate_sc.function) ||
- (candidate_sc.symbol && candidate_sc.symbol->IsExternal());
- if (candidate_sc.symbol) {
- load_address = candidate_sc.symbol->ResolveCallableAddress(*target);
-
- if (load_address == LLDB_INVALID_ADDRESS) {
- if (target->GetProcessSP())
- load_address =
- candidate_sc.symbol->GetAddress().GetLoadAddress(target);
- else
- load_address = candidate_sc.symbol->GetAddress().GetFileAddress();
- }
- }
+ auto get_external_load_address =
+ [target, &symbol_was_missing_weak](
+ lldb::addr_t &load_address, lldb::addr_t &best_internal_load_address,
+ SymbolContextList &sc_list,
+ const lldb_private::SymbolContext &sc) -> lldb::addr_t {
+ load_address = LLDB_INVALID_ADDRESS;
+
+ if (sc_list.IsEmpty())
+ return false;
- if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
+ // Missing_weak_symbol will be true only if we found only weak undefined
+ // references to this symbol.
+ symbol_was_missing_weak = true;
+ for (auto candidate_sc : sc_list.SymbolContexts()) {
+ // Only symbols can be weak undefined.
+ if (!candidate_sc.symbol ||
+ candidate_sc.symbol->GetType() != lldb::eSymbolTypeUndefined ||
+ !candidate_sc.symbol->IsWeak())
+ symbol_was_missing_weak = false;
+
+ // First try the symbol.
+ if (candidate_sc.symbol) {
+ load_address = candidate_sc.symbol->ResolveCallableAddress(*target);
+
+ if (load_address == LLDB_INVALID_ADDRESS) {
if (target->GetProcessSP())
- load_address = candidate_sc.function->GetAddressRange()
- .GetBaseAddress()
- .GetLoadAddress(target);
+ load_address =
+ candidate_sc.symbol->GetAddress().GetLoadAddress(target);
else
- load_address = candidate_sc.function->GetAddressRange()
- .GetBaseAddress()
- .GetFileAddress();
+ load_address = candidate_sc.symbol->GetAddress().GetFileAddress();
}
+ }
- if (load_address != LLDB_INVALID_ADDRESS) {
- if (is_external) {
- return true;
- } else if (best_internal_load_address == LLDB_INVALID_ADDRESS) {
- best_internal_load_address = load_address;
- load_address = LLDB_INVALID_ADDRESS;
- }
- }
+ // If that didn't work, try the function.
+ if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
+ if (target->GetProcessSP())
+ load_address = candidate_sc.function->GetAddressRange()
+ .GetBaseAddress()
+ .GetLoadAddress(target);
+ else
+ load_address = candidate_sc.function->GetAddressRange()
+ .GetBaseAddress()
+ .GetFileAddress();
}
- // You test the address of a weak symbol against NULL to see if it is
- // present. So we should return 0 for a missing weak symbol.
- if (symbol_was_missing_weak) {
- load_address = 0;
- return true;
+ if (load_address != LLDB_INVALID_ADDRESS) {
+ // If we found a load address and it's external, we're done.
+ const bool is_external =
+ (candidate_sc.function) ||
+ (candidate_sc.symbol && candidate_sc.symbol->IsExternal());
+ if (is_external)
+ return true;
+
+ if (best_internal_load_address == LLDB_INVALID_ADDRESS) {
+ best_internal_load_address = load_address;
+ load_address = LLDB_INVALID_ADDRESS;
+ }
}
-
- return false;
- };
+ }
- if (sc.module_sp) {
- sc.module_sp->FindFunctions(spec.name, CompilerDeclContext(), spec.mask,
- true, // include_symbols
- false, // include_inlines
- sc_list);
+ // You test the address of a weak symbol against NULL to see if it is
+ // present. So we should return 0 for a missing weak symbol.
+ if (symbol_was_missing_weak) {
+ load_address = 0;
+ return true;
}
- lldb::addr_t load_address = LLDB_INVALID_ADDRESS;
+ return false;
+ };
- if (get_external_load_address(load_address, sc_list, sc)) {
- return load_address;
- } else {
- sc_list.Clear();
- }
+ const bool include_symbols = true;
+ const bool include_inlines = false;
+
+ for (const SearchSpec &spec : specs) {
+ lldb::addr_t best_internal_load_address = LLDB_INVALID_ADDRESS;
+ lldb::addr_t load_address = LLDB_INVALID_ADDRESS;
- if (sc_list.GetSize() == 0 && sc.target_sp) {
- sc.target_sp->GetImages().FindFunctions(spec.name, spec.mask,
- true, // include_symbols
- false, // include_inlines
- sc_list);
+ if (sc.module_sp) {
+ SymbolContextList sc_list;
+ sc.module_sp->FindFunctions(spec.name, CompilerDeclContext(), spec.mask,
+ include_symbols, include_inlines, sc_list);
+ if (get_external_load_address(load_address, best_internal_load_address,
+ sc_list, sc))
+ return load_address;
}
- if (get_external_load_address(load_address, sc_list, sc)) {
- return load_address;
- } else {
- sc_list.Clear();
+ if (sc.target_sp) {
+ SymbolContextList sc_list;
+ sc.target_sp->GetImages().FindFunctions(
+ spec.name, spec.mask, include_symbols, include_inlines, sc_list);
+ if (get_external_load_address(load_address, best_internal_load_address,
+ sc_list, sc))
+ return load_address;
}
- if (sc_list.GetSize() == 0 && sc.target_sp) {
+ if (sc.target_sp) {
+ SymbolContextList sc_list;
sc.target_sp->GetImages().FindSymbolsWithNameAndType(
spec.name, lldb::eSymbolTypeAny, sc_list);
+ if (get_external_load_address(load_address, best_internal_load_address,
+ sc_list, sc))
+ return load_address;
}
- if (get_external_load_address(load_address, sc_list, sc)) {
- return load_address;
- }
- // if there are any searches we try after this, add an sc_list.Clear() in
- // an "else" clause here
-
- if (best_internal_load_address != LLDB_INVALID_ADDRESS) {
+ if (best_internal_load_address != LLDB_INVALID_ADDRESS)
return best_internal_load_address;
- }
}
return LLDB_INVALID_ADDRESS;
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
