[PATCH] D68069: Explicitly set entry point arch when it's thumb

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

Yea, I see what you mean. I wouldn't spend too much time on fixing that though. The ability for SymbolFiles to add symtab entries is a fairly new thing. Not all issues with it have been ironed out, and I don't think it's up to you to fix them. The same kind of conflict could have happened with any other synthetic symbol (e.g. the eh_frame stuff) being added by the object file, and you were just unlucky that I wrote that particular test to check for the entry point symbol. I think you can just tweak the test it doesn't trigger this issue (e.g. you can just delete the entry point from the yamlized elf file being used as input).



================
Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2714
+    ArchSpec arch = GetArchitecture();
+    auto entry_point_addr = GetEntryPointAddress().GetFileAddress();
+    if (entry_point_addr != LLDB_INVALID_ADDRESS) {
----------------
When playing around with the test failure, I realized that you're creating this entry point symbol even if the entry point is empty/zero. I think you should only create it if is matching a known section (and maybe also only if this object is an executable file (not a library)). In fact, probably GetEntryPointAddress should check that, and not return anything in this case.


================
Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2719
+        SectionSP section_sp =
+            GetSectionList()->FindSectionContainingFileAddress(entry_point_addr);
+        Symbol symbol(
----------------
This actually shouldn't be needed if you saved the original Address object returned from `GetEntryPointAddress()` as the section should already be present there.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68069/new/

https://reviews.llvm.org/D68069