[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

JDevlieghere updated this revision to Diff 275915. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 Files: lldb/include/lldb/Utility/DataExtractor.h lldb/source/Utility/DataExtractor.cpp lldb/unittests/Utility/DataExtractorTest.cpp Index: ll

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. sizeof(double) instead of sizeof(float) for the double tests. Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:358 +TEST(DataExtractorTest, GetDouble) {

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

JDevlieghere updated this revision to Diff 275890. JDevlieghere added a comment. I was stil checking the Windows float thing but skipping them based on the sizeof() seems more robust. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 Files: lld

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just need some sanity checks to make sure the sizeof(float) == 4 and sizeof(double) == 8 for the tests to make sure this doesn't fail on systems where that isn't true. ===

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

JDevlieghere updated this revision to Diff 275886. JDevlieghere added a comment. Add unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 Files: lldb/include/lldb/Utility/DataExtractor.h lldb/source/Utility/DataExtractor.cpp lldb/uni

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

omjavaid updated this revision to Diff 275869. omjavaid added a comment. This new revision incorporates all suggestions on previous version. @labath What do you think? I have skipped merging switch statements and have picked all other suggestions you highlighted. If this is LGTM then i can rebas

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

omjavaid marked 10 inline comments as done. omjavaid added inline comments. Comment at: lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp:167 assert(target_arch.GetTriple().getOS() == llvm::Triple::FreeBSD); switch (target_arch.GetMachine()) { case llvm::Triple

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik added a comment. In D83008#2131776 , @teemperor wrote: > Thanks for tracking this down, this is a really nasty bug... > > The fix itself is obviously fine, but I think I'm out of the loop regarding > the testing strategy. We talked about adding a C

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik updated this revision to Diff 275865. shafik added a comment. Adding a second test that is not arm64 specific. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 Files: clang/lib/AST/RecordLayoutBuilder.cpp lldb/test/Shell/Expr/Inputs/la

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

clayborg added a comment. Here are the next values for 32 bit and 64 bit floats: float a = 2.25; // 0x4010 double b = 2.25; // 0x4002 You might disable this on windows as I believe they have 10 byte floats for doubles? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8325

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

clayborg added a comment. FYI: values like 4.0 or 2.25 work well for float values in tests as they encode perfectly on all systems. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 ___ lldb-commits m

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

clayborg added a comment. Just need some DataExtractorTest.cpp additions to test the unaligned loads for float and doubles? Comment at: lldb/source/Utility/DataExtractor.cpp:626 float DataExtractor::GetFloat(offset_t *offset_ptr) const { - typedef float float_type; - float_

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Utility/DataExtractor.h:1000-1004 + uint8_t *dst_data = reinterpret_cast(&val); + const uint8_t *src_data = reinterpret_cast(src); + for (size_t i = 0;

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

JDevlieghere updated this revision to Diff 275862. JDevlieghere marked 9 inline comments as done. JDevlieghere added a comment. Thanks for the clarifications Greg! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 Files: lldb/include/lldb/Utilit

[Lldb-commits] [PATCH] D83072: [lldb-vscode] Add Compile Unit List to Modules View

clayborg added a comment. When removing out commented code, you don't need a patch for that. I think you might have committed two things to your local branch and when you ran "arc diff" it only submitted the second commit. When doing "arc diff" make sure to use: git commit --amend ... Repo

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Utility/DataExtractor.h:24-28 +#ifndef __builtin_is_aligned +#define __builtin_is_aligned(POINTER, BYTE_COUNT)

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

clayborg added a comment. There is no requirement that data be aligned when stored in a buffer that DataExtractor points to, so this patch doesn't seem correct. Since GetData() calls can return unaligned pointers, the usual way to make this work is to to make a local variable and do a memcpy in

[Lldb-commits] [PATCH] D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types

teemperor accepted this revision. teemperor added a comment. LGTM too, thanks! Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1339 + TypeSystemClang::CompleteTagDeclarationDefinition(type); + const auto *td = TypeSystemClang::GetQualType(type.GetOpaq

[Lldb-commits] [PATCH] D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1339 + TypeSystemClang::CompleteTagDeclarationDefinition(type); + const auto *td = TypeSystemC

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

shafik added inline comments. Comment at: lldb/include/lldb/Utility/DataExtractor.h:24 +#ifndef __builtin_is_aligned +#define __builtin_is_aligned(POINTER, BYTE_COUNT) \ shafik wrote: > What platforms are we not expecting to have `

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

shafik added inline comments. Comment at: lldb/include/lldb/Utility/DataExtractor.h:24 +#ifndef __builtin_is_aligned +#define __builtin_is_aligned(POINTER, BYTE_COUNT) \ What platforms are we not expecting to have `__builtin_is_ali

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

JDevlieghere added a comment. Pavel, would you mind taking a look at the unaligned load in `TestLinuxCore.py`? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83256/new/ https://reviews.llvm.org/D83256 ___ lldb-commi

[Lldb-commits] [PATCH] D83256: [lldb] Assert on unaligned load in DataExtractor

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg. Somehow UBSan would only report the unaligned load in `TestLinuxCore.py` when running the tests with reproducers. This patch adds an assert for the `GetDouble` and `GetFloat` method, which triggers regardless of

[Lldb-commits] [lldb] 60c07fd - Use CMAKE_OSX_SYSROOT instead of the environment variable SYSROOT

Author: Adrian Prantl Date: 2020-07-06T13:17:31-07:00 New Revision: 60c07fd016a3a5f2050828f92257e1e5d33a485b URL: https://github.com/llvm/llvm-project/commit/60c07fd016a3a5f2050828f92257e1e5d33a485b DIFF: https://github.com/llvm/llvm-project/commit/60c07fd016a3a5f2050828f92257e1e5d33a485b.diff

[Lldb-commits] [PATCH] D83072: [lldb-vscode] Add Compile Unit List to Modules View

aprantl added a comment. The patch and the description don't seem to match. Deleting the commented-out code is obviously fine, but I'm somewhat confused by this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83072/new/ https://reviews.llvm.org/D83

[Lldb-commits] [PATCH] D83180: Set generic error in SBError SetErrorToGenericError

JDevlieghere added a comment. I suspect this was a copy/paste mistake. Could you add a small test for this? Other than that this LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83180/new/ https://reviews.llvm.org/D83180 _

[Lldb-commits] [PATCH] D83221: [lldb] Always round down in NSDate's formatter to match NSDate's builtin format

This revision was automatically updated to reflect the committed changes. Closed by commit rG5814255e1a7d: [lldb] Always round down in NSDate's formatter to match NSDate's builtin format (authored by teemperor). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository:

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

teemperor added a comment. In D83008#2131796 , @tschuett wrote: > You could try: > > clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp -emit-llvm > -o /dev/null > > > You could commit the record layouts without your fix to show the differe

[Lldb-commits] [PATCH] D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types

labath created this revision. labath added reviewers: teemperor, aprantl. Herald added a reviewer: shafik. Herald added a project: LLDB. Unify the code for requiring a complete type and move it into a single place. The only functional change is that the "cannot start a definition of an incomplete

[Lldb-commits] [lldb] 5814255 - [lldb] Always round down in NSDate's formatter to match NSDate's builtin format

Author: Raphael Isemann Date: 2020-07-06T16:59:37+02:00 New Revision: 5814255e1a7d2e90580d6df457ddd13b1cd156cb URL: https://github.com/llvm/llvm-project/commit/5814255e1a7d2e90580d6df457ddd13b1cd156cb DIFF: https://github.com/llvm/llvm-project/commit/5814255e1a7d2e90580d6df457ddd13b1cd156cb.dif

[Lldb-commits] [lldb] 5daa39a - [lldb/Utility] Merge Scalar::Get(Value)TypeAsCString

Author: Pavel Labath Date: 2020-07-06T10:34:12+02:00 New Revision: 5daa39aa4c355e899c2ceb371ba3c8347200a687 URL: https://github.com/llvm/llvm-project/commit/5daa39aa4c355e899c2ceb371ba3c8347200a687 DIFF: https://github.com/llvm/llvm-project/commit/5daa39aa4c355e899c2ceb371ba3c8347200a687.diff

[Lldb-commits] [lldb] b65d4b2 - [lldb/DWARF] Look for complete array element definitions in other modules

Author: Pavel Labath Date: 2020-07-06T10:09:13+02:00 New Revision: b65d4b23f6dd4da4277acbf2bb912becce4f8a57 URL: https://github.com/llvm/llvm-project/commit/b65d4b23f6dd4da4277acbf2bb912becce4f8a57 DIFF: https://github.com/llvm/llvm-project/commit/b65d4b23f6dd4da4277acbf2bb912becce4f8a57.diff