[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

2023-07-03 Thread Alexey Lapshin via Phabricator via lldb-commits
avl accepted this revision. avl added a comment. This revision is now accepted and ready to land. this LGTM, assuming D154329 is landed. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153740/new/ https:

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

2023-07-03 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. this LGTM. thanks! Please, wait for approve from Jonas. I think someone from lldb needs to check whether new error reporting is OK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154329/new/ https://reviews.llvm.org/D154329 __

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

2023-07-03 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added inline comments. Comment at: lldb/tools/lldb-server/lldb-platform.cpp:112 return Status("Failed to atomically write file %s", file_spec.GetPath().c_str()); return status; probably, it would be better to add error text here? `

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

2023-06-26 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a reviewer: jkorous. avl added a subscriber: jkorous. avl added a comment. added @jkorous who originally added llvm::writeFileAtomically. > Let me know what you think about it -- I considered keeping the > llvm::writeFileAtomically and migrating its underlying implementation to > llvm

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. In D138176#4002154 , @JDevlieghere wrote: > I'll add the assert you suggested to this patch. Do you want to commit the > test yourself or can I include it in my commit (I don't want to take credit > for your hard work, but also don

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. Since this patch extends number of cases when uncloned dies are created, it is worth to add following check: void CompileUnit::fixupForwardReferences() { for (const auto &Ref : ForwardDIEReferences) { DIE *RefDie; const CompileUnit *RefUnit; PatchLoca

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. Looks like I have a test case for this problem: F25661135: odr-two-units-in-single-file11.test compile units cross reference each other in this test case: CU1: 0x10 type 1 ref to 0x40 CU2: 0x40 type 1 ref to 0x10 CH

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-15 Thread Alexey Lapshin via Phabricator via lldb-commits
avl accepted this revision. avl added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138176/new/ https://reviews.llvm.org/D138176 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-15 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. In D138176#3995675 , @JDevlieghere wrote: > Last week I debugged this further. Here's what I believe is going on. > Consider the following simplified example with two CUs and a type `Foo`. > > 0x01: DW_TAG_compile_unit > > 0x

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-11-20 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. @JDevlieghere Is this assertion happened in single thread mode only, or multi-thread mode only, or both? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138176/new/ https://reviews.llvm.org/D138176 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-11-18 Thread Alexey Lapshin via Phabricator via lldb-commits
avl requested changes to this revision. avl added a comment. This revision now requires changes to proceed. I think we need to create a test case. I tried(still trying) to create one, but it seems this assertions should always be satisfied : "assert(Ref > InputDIE.getOffset());". That might be a

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-11-17 Thread Alexey Lapshin via Phabricator via lldb-commits
avl accepted this revision. avl added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138176/new/ https://reviews.llvm.org/D138176 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-11-17 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added inline comments. Comment at: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h:82 +/// Is this a reference to a DIE that hasn't been cloned yet? +bool Reference : 1; }; Probably name it UnclonedReference ? or RefShouldBePatched?(or somet

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2019-04-03 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment. @rocallahan Would you mind to share the performance results of that patch, please ? Similar to above table for rusoto test, but with timings ... Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D5474