[Lldb-commits] [PATCH] D63540: Fix lookup of symbols with the same address range but different binding
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 19 03:43:31 PST 2019
labath added a comment.
Defining some sort of a preference based on symbol type seems like a good idea, but I don't think this is a good way to implement it. If I read this patch correctly, then this for example means that the "less global" symbols will not be reported through the Symtab::ForEachSymbolContainingFileAddress API, which seems like a bad thing.
I'm also not happy that this is supposed to be a replacement for the size setting patch, as I believe (and I think we've agreed on that while reviewing the original patch) that *not* fiddling with the sizes of those symbols is a good thing. I don't think reverting a patch that has been sitting in the tree for several months because it "breaks expression evaluation" is a good idea, when that patch has nothing to do with expression evaluation, the failure only happens on a target which has a lot of failures anyway, and the failure only happens on some machine that the original author does not have access to. (I should've said this a while ago, but I was hoping that this problem would be resolved easily..)
Anyway, I think @jankratochvil has done far more than could be expected of any committer to diagnose that problem, and I think it should be up to @omjavaid to explain out how the expression evaluation failures relate to sizeless symbols. At that point, we can re-evaluate whether our decision to *not* fiddle with size of these symbols was correct.
================
Comment at: lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s:10-11
+ .byte 0
+global:
+globalX:
+ .globl globalX
----------------
I found these names pretty confusing. I'd suggest something like `case1_local`+`case1_global`, `case2_local`+`case2_weak`, etc.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63540/new/
https://reviews.llvm.org/D63540
More information about the lldb-commits
mailing list