[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. So it looks like there are bugs in the "llvm-dwarfdump --verify". The blurb I posted above clearly has the function's address range contained in it, sorry for the false alarm. I have been quite a few problems with DWARF with LTO and other post production linkers. The

[Lldb-commits] [PATCH] D72698: [lldb] Add method decls to a CXXRecordDecl only after all their properties are defined

kwk added a comment. > The only code that is currently after addDecl is changing whether the special > members are > defaulted/trivial. I'm not sure if this actually fixes anything but it's > more correct than what we > did before. But at least you return immediately after calling `addDecl`.

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

kwk added a comment. I've marked some mistakes that were not addressed yet but are marked as "Done". Comment at: lldb/docs/resources/test.rst:86 + +API tests are located under ``lldb/test/API``. Thy are run with the +``dotest.py``. Tests are written in Python and test binaries

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. And also it is worth noting that just because new compilers might generate this information correctly, it doesn't mean LLDB won't be used to try and load older debug info from compilers that don't. LTO can also greatly affect the reliability of this information. And

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. And this one binary had 115 examples of this: $ llvm-dwarfdump --verify libIGL.so | grep "DIE address ranges are not contained in its parent" | wc -l 115 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78489/new/

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. So if llvm is relying on this, we should change llvm. Symbolication will fail using LLVM's DWARF parser if it is using this information for anything real. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78489/new/

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg added a comment. Sorry I might have not understood this patch's actual implementation. I thought we were switching to trusting DW_AT_ranges, which seems we are already doing. Let me read the patch code a bit more carefully, not just quickly reading the description... Repository:

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I see plenty of examples daily, from clang version 7, where this happens. The first binary I tried to verify after seeing this patch, I found this error. Dump is below. This

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

JDevlieghere added a comment. Lit's `to_string` will just return the string when it's a `str` instance, which in Python can still contain UTF-8 characters: >>> foo = "" >>> isinstance(foo, str) True >>> foo.encode('utf-8') Traceback (most recent call last): File "", line 1, in

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

JDevlieghere updated this revision to Diff 258857. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76955/new/ https://reviews.llvm.org/D76955 Files: lldb/test/API/lldbtest.py Index: lldb/test/API/lldbtest.py === ---

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib added a comment. In D78396#1993286 , @davide wrote: > This looks fine to me but please give Pavel another chance to look before > committing. Will wait for @labath approval :) ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. This looks fine to me but please give Pavel another chance to look before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

JDevlieghere added a comment. Below are the results for the situation you suggested: release (no-asserts) lldb, debug clang (no debug names). Results were collected in a Ubuntu 18.04 docker image running on CentOS. $ for i in {1..10}; do time ./build-ra/bin/lldb ./build-debug/bin/clang -o

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

lawrence_danna updated this revision to Diff 258831. lawrence_danna added a comment. fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78462/new/ https://reviews.llvm.org/D78462 Files: lldb/bindings/python/python-typemaps.swig

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

This revision was automatically updated to reflect the committed changes. Closed by commit rGe687aa828263: [lldb/Reproducers] Fix passive replay for (char*, size_t) functions. (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib updated this revision to Diff 258827. mib added a comment. Fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 Files: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib updated this revision to Diff 258826. mib marked 14 inline comments as done. mib added a comment. Address Davide's & Shafik's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 Files:

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

vsk updated this revision to Diff 258825. vsk added a comment. Address review feedback: - Make 'escape_style' a regular parameter, use raw string literals Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77843/new/ https://reviews.llvm.org/D77843

[Lldb-commits] [PATCH] D76906: [lldb] Fixing the bug that the "log timer" has no tab completion

teemperor added a comment. In D76906#1972413 , @gedatsu217 wrote: > [39/575] Linking CXX shared library lib/libc++abi.1.0.dylib > FAILED: lib/libc++abi.1.0.dylib > : && /Library/Developer/CommandLineTools/usr/bin/c++ -fPIC >

[Lldb-commits] [lldb] e687aa8 - [lldb/Reproducers] Fix passive replay for (char*, size_t) functions.

Author: Jonas Devlieghere Date: 2020-04-20T13:26:11-07:00 New Revision: e687aa82826385454a3d3a192a46b7a9d4eddae1 URL: https://github.com/llvm/llvm-project/commit/e687aa82826385454a3d3a192a46b7a9d4eddae1 DIFF:

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

jingham added a comment. I can see the cases where this would help, but the way you are doing it could lead to some odd behavior that might be confusing. Suppose I have two shared libraries, libBar.dylib and libNotBar.dylib in my project. I debug the process, find a bug in FileFromNotBar.c.

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:49 + size_t size = 0; + size += sizeof(m_ht->base); + size += sizeof(m_ht->bits); Shouldn't we check that `m_ht` is actually managing an object before attempting to

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

lawrence_danna added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:3153-3159 + long long py_return = unwrapOrSetPythonException( + As(implementor.CallMethod(callee_name))); // if it fails, print the error but

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

lawrence_danna updated this revision to Diff 258810. lawrence_danna marked 4 inline comments as done. lawrence_danna added a comment. review fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78462/new/ https://reviews.llvm.org/D78462 Files:

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

shafik added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:8 + +CFBasicHash::CFBasicHash() +: m_ptr_size(UINT32_MAX), m_byte_order(eByteOrderInvalid), Use in class member initializers please then you can use `=default` for

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

dblaikie added a comment. If I had to guess what this might've related to is the fact that LLVM puts a DW_AT_low_pc on the CU even if the CU uses discontiguous ranges - and in that case the low_pc has the constant value 0. So that all address values are resolved "relative" to that zero, making

[Lldb-commits] [lldb] e128d53 - [lldb/Test] Don't friend std::make_unique

Author: Jonas Devlieghere Date: 2020-04-20T11:48:52-07:00 New Revision: e128d5389547571abe04045e14c679517d01d1f6 URL: https://github.com/llvm/llvm-project/commit/e128d5389547571abe04045e14c679517d01d1f6 DIFF:

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. This is almost ready. After we're done with this round of cosmetics, I'll take another look a the algorithm and sign off. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19 + return true; +else if (m_ptr_size == 8 && m_ht_64)

[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

aprantl added a comment. > I'm not sure I understand why are we not adding the property to the lookup > ptr? I would assume we would call addDecl to it which should make it visible > and I don't see any ObjCPropertyDecl (?) check in that code? Thanks! This sounds like exactly the kind of

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

This revision was automatically updated to reflect the committed changes. Closed by commit rG950a8aa165eb: [lldb/Reproducers] Support new replay mode: passive replay (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

JDevlieghere added a comment. In D77759#1992252 , @labath wrote: > In D77759#1988419 , @labath wrote: > > > Hmm... well, I like how this (active) is unified at the template > > level. It still feels like there's

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

aprantl added a comment. The one "compiler" (DWARF generator) missing from this list is dsymutil, but I would expect it to produce reliable information, too. Given your thorough analysis it sounds like we should do this. @clayborg probably remembers why LLDB didn't trust the info best.

[Lldb-commits] [lldb] 950a8aa - [lldb/Reproducers] Support new replay mode: passive replay

Author: Jonas Devlieghere Date: 2020-04-20T09:41:40-07:00 New Revision: 950a8aa165ebe7c7b8b8b83b2c6c60af5fad89d7 URL: https://github.com/llvm/llvm-project/commit/950a8aa165ebe7c7b8b8b83b2c6c60af5fad89d7 DIFF:

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

teemperor added a comment. Beside Gabors comment I think I'm happy with this. Thanks! Comment at: clang/unittests/AST/ASTImporterTest.cpp:5939 + +/// An ExternalASTSource that keeps track of the tags is completed. +struct SourceWithCompletedTagList : clang::ExternalASTSource {

[Lldb-commits] [lldb] 4cfb71a - [lldb/Scripts] Add verbose and failure only mode to replay script.

Author: Jonas Devlieghere Date: 2020-04-20T09:03:48-07:00 New Revision: 4cfb71adba06037438e37dac84dbd9c3ac2b4319 URL: https://github.com/llvm/llvm-project/commit/4cfb71adba06037438e37dac84dbd9c3ac2b4319 DIFF:

[Lldb-commits] [PATCH] D77970: 2/2: [nfc] [lldb] DWARF callbacks: DIERef -> DWARFDIE

labath added a comment. Looks pretty good, I just think the std::function solution is too smart for its own good. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h:82 + /// The object will remain valid during the whole call statement: + /// Function(name,

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

This revision was automatically updated to reflect the committed changes. Closed by commit rG865996ddf626: [lldb] Remove m_last_file_sp from SourceManager (authored by emrekultursay, committed by labath). Changed prior to commit: https://reviews.llvm.org/D76806?vs=256416=258738#toc

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

This revision was automatically updated to reflect the committed changes. Closed by commit rGacae69d08c88: [lldb] Add new LLDB setting: use-source-cache (authored by emrekultursay, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

This revision was automatically updated to reflect the committed changes. Closed by commit rG6afa5c407c13: [lldb] Prefer executable files from sysroot over files from local filesystem (authored by yuri, committed by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f820fa4feda: [lldb] Fix SourceManager::SourceFileCache insertion (authored by emrekultursay, committed by labath). Changed prior to commit: https://reviews.llvm.org/D76805?vs=252881=258737#toc

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D76806#1973035 , @emrekultursay wrote: > Thanks for noticing the test breakages Pavel. I fixed the bug, and verified > that the tests you

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D77759#1988419 , @labath wrote: > Hmm... well, I like how this (active) is unified at the template > level. It still feels like there's too much

[Lldb-commits] [lldb] acae69d - [lldb] Add new LLDB setting: use-source-cache

Author: Emre Kultursay Date: 2020-04-20T16:24:25+02:00 New Revision: acae69d08c88b701e25318f8d4100f5542c44407 URL: https://github.com/llvm/llvm-project/commit/acae69d08c88b701e25318f8d4100f5542c44407 DIFF:

[Lldb-commits] [lldb] 1f820fa - [lldb] Fix SourceManager::SourceFileCache insertion

Author: Emre Kultursay Date: 2020-04-20T16:25:54+02:00 New Revision: 1f820fa4feda34d25e62762392c50049e5282330 URL: https://github.com/llvm/llvm-project/commit/1f820fa4feda34d25e62762392c50049e5282330 DIFF:

[Lldb-commits] [lldb] 865996d - [lldb] Remove m_last_file_sp from SourceManager

Author: Emre Kultursay Date: 2020-04-20T16:27:19+02:00 New Revision: 865996ddf626a4d6e2a9c401b1fffc731a1946aa URL: https://github.com/llvm/llvm-project/commit/865996ddf626a4d6e2a9c401b1fffc731a1946aa DIFF:

[Lldb-commits] [lldb] 9cd9f3f - [lldb] Fix gcc warnings in TypeCategory.cpp

Author: Pavel Labath Date: 2020-04-20T16:12:51+02:00 New Revision: 9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a URL: https://github.com/llvm/llvm-project/commit/9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a DIFF: https://github.com/llvm/llvm-project/commit/9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a.diff

[Lldb-commits] [lldb] 6afa5c4 - [lldb] Prefer executable files from sysroot over files from local filesystem

Author: Yuri Per Date: 2020-04-20T16:12:51+02:00 New Revision: 6afa5c407c1330efcadb75b8f85993660bf275a7 URL: https://github.com/llvm/llvm-project/commit/6afa5c407c1330efcadb75b8f85993660bf275a7 DIFF: https://github.com/llvm/llvm-project/commit/6afa5c407c1330efcadb75b8f85993660bf275a7.diff

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

labath created this revision. labath added reviewers: JDevlieghere, aprantl, clayborg. Herald added a project: LLDB. The code in DWARFCompileUnit::BuildAddressRangeTable tries hard to avoid relying on DW_AT_low/high_pc for compile unit range information, and this logic is a big cause of llvm/lldb

[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

teemperor added a comment. I'm not sure I understand why are we not adding the property to the lookup ptr? I would assume we would call addDecl to it which should make it visible and I don't see any ObjCPropertyDecl (?) check in that code? Comment at:

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib updated this revision to Diff 258710. mib marked 2 inline comments as done. mib added a comment. Address Pavel's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 Files:

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib marked 5 inline comments as done. mib added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71 + +size = sizeof(__CFBasicHash::RuntimeBase); +size += sizeof(__CFBasicHash::Bits); + +DataBufferHeap buffer(size, 0); +

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

n1tram1 updated this revision to Diff 258705. n1tram1 added a comment. Fix the latest patch. (I hope I got it right this time, sorry it's my first time doing this) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78421/new/

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

martong added a comment. Thanks for the test Shafik, that is pretty self explanatory! Comment at: clang/lib/AST/ASTImporter.cpp:8161 if (auto *ToRecord = dyn_cast(ToDC)) { auto *FromRecord = cast(FromDC); if (ToRecord->isCompleteDefinition()) {

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

labath added a comment. In D75750#1988669 , @kwk wrote: > In D75750#1988330 , @labath wrote: > > > The situation around finding symbols is messy enough already > > > The way in which I integrated debuginfod for now

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

labath added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:55 +return obj.takeError(); + return obj.get().AsUnsignedLongLong(); +} `obj->AsUnsignedLongLong()` ? Comment at:

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

labath added a comment. This looks like a bad upload. Each time you update the patch it should include the full list of changes, not just your recent additions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78421/new/

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

labath added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:102-107 +m_ht_64 = new __CFBasicHash(); +size_t copied = data_extractor.CopyData(0, size, m_ht_64); + +if (copied != size) { + delete m_ht_64; + m_ht_64 = nullptr;

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

labath added a comment. The test stuff looks good to me. Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:109 + EXPECT_TRUE(matches({"\0", 1}, QUOTE(""))); + EXPECT_TRUE(matches("\a", QUOTE("\\a"))); + EXPECT_TRUE(matches("\b", QUOTE("\\u{8}")));

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:206-207 template T *GetObjectForIndex(unsigned idx) { -assert(idx != 0 && "Cannot