Sorry for breaking the windows tests. I don't have a windows box to investigate it but based on your description the regression is caused by my CL while I believe the bug is in ObjectFilePECOFF (is it the one used on Windows?).
The problem is that you have symbols with m_byte_size == 0 while calling Symtab::FindSymbolContainingFileAddress what is invalid in almost every case. Most likely it is caused by an issue in ObjectFilePECOFF::GetSymtab where you don't set the size of the symbols. You either have to specify the size of each symbol with calling Symbol::SetByteSize if this information is stored in the the object file (done in case of elf) or call Symtab::CalculateSymbolSizes (done in case of mach-o) after you parsed all symbols (what will set the size of each symbol with 0 size to last until the next symbol). Can you try out is doing one of these fixes your issue? Thanks, Tamas On Wed, Jan 20, 2016 at 11:19 PM Adrian McCarthy <amcca...@google.com> wrote: > In `Symtab::FindSymbolContainingFileAddress`, the address range for the > symbols we're finding always have `m_byte_size == 0`, so the extra check > `symbol->ContainsFileAddress(file_addr)` always fails, causing us to return > nullptr instead of the symbol. > > I don't know if that's the right behavior, but that is the change that's > causing thread `step-over` to fail on Windows, which takes out about a > dozen tests. > > > > On Wed, Jan 20, 2016 at 2:33 PM, Zachary Turner via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> Hi Tamas, >> >> I think this build breaks Windows. This is actually the breakage that we >> were discussing in the thread about thread step-over. >> >> 1 revision before your patch: >> D:\src\llvmbuild\ninja>C:\Python35\python_d.exe >> D:/src/llvm/tools/lldb/test/dotest.py -q --arch=i686 --executable >> D:/src/llvmbuild/ninja/bin/lldb.exe -s >> D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS >> --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p >> TestSignedTypes.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test >> --no-multiprocess >> LLDB library dir: D:/src/llvmbuild/ninja/bin >> LLDB import library dir: D:/src/llvmbuild/ninja/bin\..\lib >> lldb version 3.9.0 clang revision 257802 llvm revision 257804 >> The 'lldb-mi' executable cannot be located. The lldb-mi tests can not be >> run as a result. >> >> Session logs for test failures/errors/unexpected successes will go into >> directory 'D:/src/llvmbuild/ninja/lldb-test-traces' >> Command invoked: D:/src/llvm/tools/lldb/test/dotest.py -q --arch=i686 >> --executable D:/src/llvmbuild/ninja/bin/lldb.exe -s >> D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS >> --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p >> TestSignedTypes.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test >> --no-multiprocess >> PASS: LLDB (d:\src\llvmbuild\ninja_release\bin\clang.exe-i686) :: >> test_dwarf (TestSignedTypes.UnsignedTypesTestCase) >> ---------------------------------------------------------------------- >> Ran 1 test in 1.820s >> >> RESULT: PASSED (1 passes, 0 failures, 0 errors, 0 skipped, 0 expected >> failures, 0 unexpected successes) >> >> With your patch: >> D:\src\llvmbuild\ninja>C:\Python35\python_d.exe >> D:/src/llvm/tools/lldb/test/dotest.py -q --arch=i686 --executable >> D:/src/llvmbuild/ninja/bin/lldb.exe -s >> D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS >> --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p >> TestSignedTypes.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test >> --no-multiprocess >> LLDB library dir: D:/src/llvmbuild/ninja/bin >> LLDB import library dir: D:/src/llvmbuild/ninja/bin\..\lib >> lldb version 3.9.0 clang revision 257802 llvm revision 257804 >> The 'lldb-mi' executable cannot be located. The lldb-mi tests can not be >> run as a result. >> >> Session logs for test failures/errors/unexpected successes will go into >> directory 'D:/src/llvmbuild/ninja/lldb-test-traces' >> Command invoked: D:/src/llvm/tools/lldb/test/dotest.py -q --arch=i686 >> --executable D:/src/llvmbuild/ninja/bin/lldb.exe -s >> D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS >> --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p >> TestSignedTypes.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test >> --no-multiprocess >> FAIL: LLDB (d:\src\llvmbuild\ninja_release\bin\clang.exe-i686) :: >> test_dwarf (TestSignedTypes.UnsignedTypesTestCase) >> ====================================================================== >> FAIL: test_dwarf (TestSignedTypes.UnsignedTypesTestCase) >> Test that variables with signed types display correctly. >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File >> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\lldbtest.py", line >> 2249, in dwarf_test_method >> return attrvalue(self) >> File >> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\lang\cpp\signed_types\TestSignedTypes.py", >> line 60, in test >> "(long long) the_signed_long_long = 99"]) >> File >> "D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test\lldbtest.py", line >> 2701, in expect >> msg if msg else EXP_MSG(str, exe)) >> AssertionError: False is not True : Variable(s) displayed correctly >> Config=i686-d:\src\llvmbuild\ninja_release\bin\clang.exe >> ---------------------------------------------------------------------- >> Ran 1 test in 1.641s >> >> RESULT: FAILED (0 passes, 1 failures, 0 errors, 0 skipped, 0 expected >> failures, 0 unexpected successes) >> >> >> We're going to look into whether the test was *actually* working before >> or if it was a false positive (i.e. it actually should have failed), but is >> there anything you can do on your end to figure out if there's other >> problems with this patch? Do you guys have windows machines? >> >> On Tue, Jan 19, 2016 at 2:28 AM Tamas Berghammer via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> >>> Author: tberghammer >>> Date: Tue Jan 19 04:24:51 2016 >>> New Revision: 258113 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=258113&view=rev >>> Log: >>> Unconditionally accept symbol sizes from elf >>> >>> The ELF symbol table always contain the size of the symbols so we >>> don't have to try to guess them based on the address of the next >>> symbol (it is needed for mach-o). >>> >>> The change fixes an issue when a symbol is removed after a 0 size >>> symbol (e.g. because the second one is not public) what previously >>> caused the symbol lookup algorithm to end up with showing the 0 size >>> symbol even for the later addresses (what are not part of any symbol). >>> That symbol lookup error can confuse the user and also confuses the >>> current stack unwinder. >>> >>> Re-commit this CL after fixing the issue with gcc-4.9.2 on i386 Linux. >>> >>> Differential revision: http://reviews.llvm.org/D16186 >>> >>> Modified: >>> lldb/trunk/include/lldb/Symbol/Symbol.h >>> lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp >>> lldb/trunk/source/Symbol/Symbol.cpp >>> lldb/trunk/source/Symbol/Symtab.cpp >>> >>> Modified: lldb/trunk/include/lldb/Symbol/Symbol.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Symbol.h?rev=258113&r1=258112&r2=258113&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/include/lldb/Symbol/Symbol.h (original) >>> +++ lldb/trunk/include/lldb/Symbol/Symbol.h Tue Jan 19 04:24:51 2016 >>> @@ -383,6 +383,9 @@ public: >>> bool prefer_file_cache, >>> Stream &strm); >>> >>> + bool >>> + ContainsFileAddress (lldb::addr_t file_addr) const; >>> + >>> protected: >>> // This is the internal guts of ResolveReExportedSymbol, it assumes >>> reexport_name is not null, and that module_spec >>> // is valid. We track the modules we've already seen to make sure >>> we don't get caught in a cycle. >>> >>> Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=258113&r1=258112&r2=258113&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original) >>> +++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Tue Jan >>> 19 04:24:51 2016 >>> @@ -2283,6 +2283,12 @@ ObjectFileELF::ParseSymbols (Symtab *sym >>> mangled.SetDemangledName( ConstString((demangled_name + >>> suffix).str()) ); >>> } >>> >>> + // In ELF all symbol should have a valid size but it is not >>> true for some code symbols >>> + // coming from hand written assembly. As none of the code >>> symbol should have 0 size we try >>> + // to calculate the size for these symbols in the symtab with >>> saying that their original >>> + // size is not valid. >>> + bool symbol_size_valid = symbol.st_size != 0 || symbol_type != >>> eSymbolTypeCode; >>> + >>> Symbol dc_symbol( >>> i + start_id, // ID is the original symbol table >>> index. >>> mangled, >>> @@ -2295,7 +2301,7 @@ ObjectFileELF::ParseSymbols (Symtab *sym >>> symbol_section_sp, // Section in which this symbol is >>> defined or null. >>> symbol_value, // Offset in section or symbol >>> value. >>> symbol.st_size), // Size in bytes of this symbol. >>> - symbol.st_size != 0, // Size is valid if it is not 0 >>> + symbol_size_valid, // Symbol size is valid >>> has_suffix, // Contains linker annotations? >>> flags); // Symbol flags. >>> symtab->AddSymbol(dc_symbol); >>> @@ -2304,7 +2310,9 @@ ObjectFileELF::ParseSymbols (Symtab *sym >>> } >>> >>> unsigned >>> -ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t >>> start_id, lldb_private::Section *symtab) >>> +ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, >>> + user_id_t start_id, >>> + lldb_private::Section *symtab) >>> { >>> if (symtab->GetObjectFile() != this) >>> { >>> >>> Modified: lldb/trunk/source/Symbol/Symbol.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Symbol.cpp?rev=258113&r1=258112&r2=258113&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Symbol/Symbol.cpp (original) >>> +++ lldb/trunk/source/Symbol/Symbol.cpp Tue Jan 19 04:24:51 2016 >>> @@ -737,3 +737,10 @@ Symbol::GetDisassembly (const ExecutionC >>> } >>> return false; >>> } >>> + >>> +bool >>> +Symbol::ContainsFileAddress (lldb::addr_t file_addr) const >>> +{ >>> + return m_addr_range.ContainsFileAddress(file_addr); >>> +} >>> + >>> >>> Modified: lldb/trunk/source/Symbol/Symtab.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Symtab.cpp?rev=258113&r1=258112&r2=258113&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Symbol/Symtab.cpp (original) >>> +++ lldb/trunk/source/Symbol/Symtab.cpp Tue Jan 19 04:24:51 2016 >>> @@ -971,9 +971,11 @@ Symtab::InitAddressIndexes() >>> if (end_section_file_addr > symbol_file_addr) >>> { >>> Symbol &symbol = m_symbols[entry.data]; >>> - >>> - symbol.SetByteSize(end_section_file_addr - >>> symbol_file_addr); >>> - symbol.SetSizeIsSynthesized(true); >>> + if (!symbol.GetByteSizeIsValid()) >>> + { >>> + symbol.SetByteSize(end_section_file_addr - >>> symbol_file_addr); >>> + symbol.SetSizeIsSynthesized(true); >>> + } >>> } >>> } >>> } >>> @@ -1039,18 +1041,15 @@ Symtab::FindSymbolContainingFileAddress >>> return info.match_symbol; >>> } >>> >>> - const size_t symbol_byte_size = >>> info.match_symbol->GetByteSize(); >>> - >>> - if (symbol_byte_size == 0) >>> + if (!info.match_symbol->GetByteSizeIsValid()) >>> { >>> - // We weren't able to find the size of the symbol so lets >>> just go >>> - // with that match we found in our search... >>> + // The matched symbol dosn't have a valid byte size so lets >>> just go with that match... >>> return info.match_symbol; >>> } >>> >>> // We were able to figure out a symbol size so lets make sure >>> our >>> // offset puts "file_addr" in the symbol's address range. >>> - if (info.match_offset < symbol_byte_size) >>> + if (info.match_offset < info.match_symbol->GetByteSize()) >>> return info.match_symbol; >>> } >>> return nullptr; >>> @@ -1066,7 +1065,11 @@ Symtab::FindSymbolContainingFileAddress >>> >>> const FileRangeToIndexMap::Entry *entry = >>> m_file_addr_to_index.FindEntryThatContains(file_addr); >>> if (entry) >>> - return SymbolAtIndex(entry->data); >>> + { >>> + Symbol* symbol = SymbolAtIndex(entry->data); >>> + if (symbol->ContainsFileAddress(file_addr)) >>> + return symbol; >>> + } >>> return nullptr; >>> } >>> >>> >>> >>> _______________________________________________ >>> lldb-commits mailing list >>> lldb-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>> >> >> _______________________________________________ >> lldb-commits mailing list >> lldb-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> >> >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
