labath added a comment.

The fix seems reasonable to me. There's just one thing I don't get: Why was it 
necessary to change the iteration order here?
IIUC, the real business happens on line 904, where you add the `if 
(entry[i].base == entry[i+1].base) break;` check. I could be missing something, 
but it seems to me like that thing would work no matter what the iteration 
order is...



================
Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:1
+# RUN: %clang %S/Inputs/sizeless-symbol.s -c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
----------------
Are you sure this will work whatever the host triple happens to be ? (you can 
try it out by manually forcing a couple of random targets with the `-target` 
argument). I have a feeling the `.type`, `.size`, etc. directives are a feature 
of elf-targetting assemblers. If that doesn't work you can always force a 
specific triple with the `-target` command.


================
Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:4
+
+image lookup --address 1
+# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful
----------------
As you're fixing this bug by changing the way symbol size is computed, it would 
be nice to also put (and check it's output) a "image dump symtab" command here. 
That would also make it clear what are the assumptions the following "lookup" 
commands are operating under.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to