labath added a comment. In D94064#2483578 <https://reviews.llvm.org/D94064#2483578>, @dblaikie wrote:
> In D94064#2481925 <https://reviews.llvm.org/D94064#2481925>, @labath wrote: > >> I don't think that simply setting func_lo_pc to zero will be sufficient to >> make this work. I would expect this to break in more complicated scenarios >> (like, even if we just have two of these functions). I think the only reason >> it works in this case is because for this particular function, it's base >> address (relative to the CU base) *is* zero. > > I certainly don't have high confidence in the change, to be sure - but I > think it's a bit more robust than that. Yes, it seems it is, though just by a bit. :) The thing which is missing is the location list on the variable DIE -- without it, it does not matter which address gets put here (so long as it does not trigger the assertion), as the value is totally unused. The necessity of the location list is kind of obvious from my description of the problem, though even I did not realize when I was writing it that this requires a more elaborate test case. Adjusting your test case to produce a location list for `i` I got this (probably not minimal) snippet (add -O1 to CFLAGS): #define optnone __attribute__((optnone)) #define noinline __attribute__((noinline)) void optnone noinline stop() {} void optnone noinline consume(int i) {} void noinline f1(int x) { int i = x; stop(); consume(i); i = 5; consume(i); } int main() { int j = 12; f1(j); stop(); } Here lldb fails to print the value of `i`, but if I rearrange the code so that the `f1` functions comes first, the value is printed correctly: #define optnone __attribute__((optnone)) #define noinline __attribute__((noinline)) void optnone noinline stop(); void optnone noinline consume(int i); void noinline f1(int x) { int i = x; stop(); consume(i); i = 5; consume(i); } void optnone noinline stop() {} void optnone noinline consume(int i) {} int main() { int j = 12; f1(j); stop(); } >> The purpose of func_lo_pc is pretty weird, but it's essentially used for >> runtime relocation of location lists within the function -- we take the >> static "base" of the function, and the runtime "base", and the difference >> between the two produces the load bias. Applying the same bias to all >> variable location lists "relocates" the variables. (The reason this is so >> convoluted is reading location lists straight from (unrelocated) .o files on >> mac. I seriously considered changing this when working on debug_rnglists, >> but it turned out it wasn't really necessary, so I let it go.) > > Yep, I figured a bunch of this was for DWARF in unrelocated MachO files - and > that they wouldn't be able to/want to use Split DWARF or this feature (which > is particularly relevant to Split DWARF). Does any of this logic apply > outside that unrelocated MachO scenario? Yes, this code is used everywhere, though it's role is less important (and one that could be implemented in an easier manner, were it not for MachO) -- it adjusts the variable address for ASLR, i.e., the variable/function being loaded at a different address than what the static debug info says. >> LLDB's representation of a function (lldb_private::Function) assumes that >> all functions will be contiguous (Function::GetAddressRange returns a single >> range). If we make it so that this range matches the first block of the >> function (maybe that's what happens now), > > Actually seems to adjust for discontiguous ranges and not just pick the start > of the first (as it appears in the DWARF - which isn't guaranteed to be > ordered in any way) range, instead finding the smallest and largest > addresses: > https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2357 Ah.. interesting. In that case, I believe things should work if we use the same algorithm to compute `func_lo_pc`. We may still run into weird issues for functions which are truly discontinuous, but it should be ok for single-range range lists, at least. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94064/new/ https://reviews.llvm.org/D94064 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
