[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

2023-05-12 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.
Herald added a subscriber: luke.

Hi! I wondering if someone knows what is the status of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

2023-05-17 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D128612#4345912 , @asb wrote:

> In D128612#4337037 , @djtodoro 
> wrote:
>
>> Hi! I am wondering if someone knows what is the status of this.
>
> I've not seen any further progress. I think it needs at a minimum a PR 
> against the psABI doc for big endian that we can review against.

OK. Can you please share with me the GCC PR, so I can take a look how it should 
be done :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

2023-05-17 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D128612#4349291 , @asb wrote:

> In D128612#4349259 , @djtodoro 
> wrote:
>
>> In D128612#4345912 , @asb wrote:
>>
>>> In D128612#4337037 , @djtodoro 
>>> wrote:
>>>
 Hi! I am wondering if someone knows what is the status of this.
>>>
>>> I've not seen any further progress. I think it needs at a minimum a PR 
>>> against the psABI doc for big endian that we can review against.
>>
>> OK. Can you please share with me the GCC PR, so I can take a look how it 
>> should be done :)
>
> Based on comments here it sounds like GCC merged some level of support 
> without submitting corresponding changes to the psABI. I don't have those 
> patches to hand, but you can probably find them on the GCC patches list. I 
> think we've had enough unexpected compatibility issues by now that it's not 
> really justifiable to merge something that's meant to be a standard ABI 
> without having the corresponding psABI patch (even if it appears simple). I 
> don't think anyone has posted such a psABI patch but there is/was a tracking 
> issue for it on the psABI repo.

Yeah, if the https://github.com/riscv-non-isa/riscv-elf-psabi-doc is the repo, 
I was not able to find the PR for GCC. And I think the support within GCC is 
somewhat finished.

> @kito-cheng is the bigendian RISC-V work in GCC still active at all? I'm 
> wondering if there's a connection you could help make for Djordje.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f6ff07f8a39: [DebugInfo] Enable the debug entry values 
feature by default (authored by djtodoro).
Herald added subscribers: lldb-commits, cfe-commits, jrtc27.
Herald added projects: clang, LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D73534?vs=243759&id=244101#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
  llvm/lib/CodeGen/TargetOptionsImpl.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/ARM/smml.ll
  llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
  llvm/test/CodeGen/X86/call-site-info-output.ll
  llvm/test/DebugInfo/AArch64/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
  llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
  llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
  llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
  llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
  llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
  llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
  llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
  llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
  llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
  llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
  llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
  llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
  llvm/test/DebugInfo/X86/dbg-value-range.ll
  llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
  llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
  llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
  llvm/test/DebugInfo/X86/loclists-dwp.ll
  llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll
  llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
  llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
  llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
  llvm/test/tools/llvm-locstats/locstats.ll

Index: llvm/test/tools/llvm-locstats/locstats.ll
===
--- llvm/test/tools/llvm-locstats/locstats.ll
+++ llvm/test/tools/llvm-locstats/locstats.ll
@@ -9,9 +9,9 @@
 ; LOCSTATS: [10%,20%) 0 0%
 ; LOCSTATS: [20%,30%) 1 11%
 ; LOCSTATS: [30%,40%) 0 0%
-; LOCSTATS: [40%,50%) 1 11%
-; LOCSTATS: [50%,60%) 1 11%
-; LOCSTATS: [60%,70%) 1 11%
+; LOCSTATS: [40%,50%) 0 0%
+; LOCSTATS: [50%,60%) 0 0%
+; LOCSTATS: [60%,70%) 3 33%
 ; LOCSTATS: [70%,80%) 0 0%
 ; LOCST

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Reverted due to 
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/21373/steps/build-stage3-compiler/logs/stdio.

I will reland this as soon as I fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-12 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro marked an inline comment as done.
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870
 if (MI->isCandidateForCallSiteEntry() &&
-DAG->getTarget().Options.EnableDebugEntryValues)
+DAG->getTarget().Options.SupportsDebugEntryValues)
   MF.addCallArgsForwardingRegs(MI, DAG->getSDCallSiteInfo(Node));

dstenb wrote:
> I'm sorry for commenting on this so late, but when now adapting this patch 
> for our downstream target, for which we will not enable call site info by 
> default yet, I noticed that the call site information wasn't added. I think 
> that this should be `ShouldEmitDebugEntryValues()`.
I thought to restrict the production of the call site info here and only to 
allow a hand-made testing, but I think you are right, we should use the 
`ShouldEmitDebugEntryValues()`. I'll update the patch, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-18 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Thanks for reporting that, are you sure this was the cause of the failure? I'll 
revert this while investigate, but it does not seem to me this is related to 
this patch.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-18 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro updated this revision to Diff 245123.
djtodoro added a comment.

-Addressing the latest comments


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

https://reviews.llvm.org/D73534

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
  llvm/lib/CodeGen/TargetOptionsImpl.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/ARM/smml.ll
  llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
  llvm/test/CodeGen/X86/call-site-info-output.ll
  llvm/test/CodeGen/X86/tail-dup-repeat.ll
  llvm/test/DebugInfo/AArch64/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
  llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
  llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
  llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
  llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
  llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
  llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
  llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
  llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
  llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
  llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
  llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
  llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
  llvm/test/DebugInfo/X86/dbg-value-range.ll
  llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
  llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
  llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
  llvm/test/DebugInfo/X86/loclists-dwp.ll
  llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll
  llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
  llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
  llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
  llvm/test/tools/llvm-locstats/locstats.ll

Index: llvm/test/tools/llvm-locstats/locstats.ll
===
--- llvm/test/tools/llvm-locstats/locstats.ll
+++ llvm/test/tools/llvm-locstats/locstats.ll
@@ -9,9 +9,9 @@
 ; LOCSTATS: [10%,20%) 0 0%
 ; LOCSTATS: [20%,30%) 1 11%
 ; LOCSTATS: [30%,40%) 0 0%
-; LOCSTATS: [40%,50%) 1 11%
-; LOCSTATS: [50%,60%) 1 11%
-; LOCSTATS: [60%,70%) 1 11%
+; LOCSTATS: [40%,50%) 0 0%
+; LOCSTATS: [50%,60%) 0 0%
+; LOCSTATS: [60%,70%) 3 33%
 ; LOCSTATS: [70%,80%) 0 0%
 ; LOCSTATS: [80%,90%) 2 22%
 ; LOCSTATS: [90%,100%) 1 11%
Index: llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
===
--- llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extension

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-18 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

I’ve already reverted the patch, but I’ll reland it again tomorrow. Thanks.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-19 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro updated this revision to Diff 245345.
djtodoro added a comment.

- Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've 
reduced the test 
`llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir`)


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

https://reviews.llvm.org/D73534

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
  llvm/lib/CodeGen/TargetOptionsImpl.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/AArch64/arm64-anyregcc.ll
  llvm/test/CodeGen/AArch64/arm64-patchpoint.ll
  llvm/test/CodeGen/AArch64/arm64-tls-dynamics.ll
  llvm/test/CodeGen/ARM/smml.ll
  llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error1.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error2.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error3.mir
  llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
  llvm/test/CodeGen/X86/call-site-info-output.ll
  llvm/test/CodeGen/X86/hoist-invariant-load.ll
  llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
  llvm/test/CodeGen/X86/statepoint-allocas.ll
  llvm/test/CodeGen/X86/tail-dup-repeat.ll
  llvm/test/CodeGen/X86/xray-custom-log.ll
  llvm/test/CodeGen/X86/xray-typed-event-log.ll
  llvm/test/DebugInfo/AArch64/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/call-site-info-output.ll
  llvm/test/DebugInfo/ARM/entry-value-multi-byte-expr.ll
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpret-movzxi.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
  llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir
  llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir
  llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
  llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
  llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
  llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
  llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
  llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg-multiple-defs.mir
  llvm/test/DebugInfo/MIR/X86/dbg-call-site-spilled-arg.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
  llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
  llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
  llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/multiple-param-dbg-value-entry.mir
  llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
  llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
  llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll
  llvm/test/DebugInfo/X86/dbg-value-range.ll
  llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
  llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
  llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
  llvm/test/DebugInfo/X86/loclists-dwp.ll
  llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll
  llvm/test/tools/llvm-dwarfdump/X86/locstats.ll
  llvm/test/tools/llvm-dwarfdump/X86/stats-dbg-callsite-info.ll
  llvm/test/tools/llvm-dwarfdump/X86/valid-call-site-GNU-extensions.ll
  llvm/test/tools/llvm-locstats/locstats.ll

Index: llvm/test/tools/llvm-locs

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-19 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1882118 , @dstenb wrote:

> In D73534#1881353 , @nickdesaulniers 
> wrote:
>
> > As a heads up, Linaro's ToolChain Working Group's Linux kernel CI lit up on 
> > this change. I see it's been reverted (thank you). Please cc me on the 
> > updated patch and I can help test it against the Linux kernel.
> >
> > Edit: Link: 
> > https://ci.linaro.org/job/tcwg_kernel-bisect-llvm-master-arm-mainline-allyesconfig/25/artifact/artifacts/build-a82d3e8a6e67473c94a5ce6345372748e9b61718/03-build_linux/console.log
>
>
> Its hard to tell from the backtrace, but looking at the code, I think this 
> might be a bug that sneaked in when I did D70431 
> . Sorry if that is the case!
>
> `ARMBaseInstrInfo::isAddImmediate()` does a `getReg()` without any `isReg()` 
> guard:
>
>   Optional ARMBaseInstrInfo::isAddImmediate(const MachineInstr 
> &MI,  
>   
>   
> 
> Register Reg) const { 
>   
>   
>   
>  
>   [...]
> // TODO: Handle cases where Reg is a super- or sub-register of the
>   
>   
>   
>  
> // destination register.
> if (Reg != MI.getOperand(0).getReg())
>   return None;
>


@dstenb No problem, we have already addressed the issue. Thanks to @vsk and 
@nickdesaulniers! I'll update the patch.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1883022 , @nickdesaulniers 
wrote:

> In D73534#1882136 , @djtodoro wrote:
>
> > - Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've 
> > reduced the test 
> > `llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir`)
>
>
> I'd like to help test this, but `arc patch D73534` is now failing with merge 
> conflicts. Can you please rebase this on master?


I’ve already pushed this. Please rebase on the latest commits.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

@ostannard Thanks for reporting that! Please share the case.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Nice! Thanks!


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-02-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Reverted again with rG2f215cf36adc 
. The 
investigation is needed.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-11 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1915048 , @mstorsjo wrote:

> This broke compiling for mingw, repro.c:
>
>   a(short);
>   b() { a(1); }
>
>
> `clang -target x86_64-w64-mingw32 -c repro.c -g -O2`, which gives `Assertion 
> '!MI.isMoveImmediate() && "Unexpected MoveImm instruction"' failed.`


Thanks for reporting this! Since this is the case of the `X86::MOV16ri` the 
D75326  will solve this issue.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-11 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1916291 , @djtodoro wrote:

> In D73534#1915048 , @mstorsjo wrote:
>
> > This broke compiling for mingw, repro.c:
> >
> >   a(short);
> >   b() { a(1); }
> >
> >
> > `clang -target x86_64-w64-mingw32 -c repro.c -g -O2`, which gives 
> > `Assertion '!MI.isMoveImmediate() && "Unexpected MoveImm instruction"' 
> > failed.`
>
>
> Thanks for reporting this! Since this is the case of the `X86::MOV16ri` the 
> D75326  will solve this issue.


The alternative is D75974 .


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-12 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1916309 , @djtodoro wrote:

> In D73534#1916291 , @djtodoro wrote:
>
> > In D73534#1915048 , @mstorsjo 
> > wrote:
> >
> > > This broke compiling for mingw, repro.c:
> > >
> > >   a(short);
> > >   b() { a(1); }
> > >
> > >
> > > `clang -target x86_64-w64-mingw32 -c repro.c -g -O2`, which gives 
> > > `Assertion '!MI.isMoveImmediate() && "Unexpected MoveImm instruction"' 
> > > failed.`
> >
> >
> > Thanks for reporting this! Since this is the case of the `X86::MOV16ri` the 
> > D75326  will solve this issue.
>
>
> The alternative is D75974 .


The D75974  is commited.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-16 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Why reverting this one? Is it a different assertion?

The D75036  was the problem with the previous 
issue reported.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-19 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Since we landed the fix for the issue related to the D75036 
, I'll reland this again.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-19 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Re-enabled with the d9b962100942 
.

If we face a failure again, since this enables the whole feature, I recommend 
reverting smaller pieces of the feature that was causing the problem, rather 
than reverting this patch.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1933985 , @djtodoro wrote:

> Oh sorry, I thought it all has been fixed, since all the tests pass.
>
> We should revert the D75036  until we fix 
> all the issues.


@dstenb do you agree?


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Oh sorry, I thought it all has been fixed, since all the tests pass.

We should revert the D75036  until we fix all 
the issues.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D73534#1934105 , @vsk wrote:

> In D73534#1933988 , @djtodoro wrote:
>
> > In D73534#1933985 , @djtodoro 
> > wrote:
> >
> > > Oh sorry, I thought it all has been fixed, since all the tests pass.
> > >
> > > We should revert the D75036  until we 
> > > fix all the issues.
> >
> >
> > @dstenb do you agree?
>
>
> This was an oversight on my part, the fix from D76164 
>  was not complete. I need a few more minutes 
> to write a test, but I should be able to fix this very soon. If we can 
> fix-forward, that would be easier.


@vsk That would be great. Thanks a lot!


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

2020-04-01 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D77108#1952818 , @aprantl wrote:

> For the future, a clean solution would be extending the macros in Dwarf.def 
> to list the stack effects in the definitions of the DW_OP_*, for example
>
>   // opcode, name, version, vendor, in, out
>   HANDLE_DW_OP(0x12, dup, 2, DWARF, 1, 2)
>
>
> and then we could write a static verifier that ensures that the stack effects 
> of an entire expression is sound. (And we could check this in LLVM, already, 
> too).


`DW_OP_entry_value` may be requiring a special handling as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77108



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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2020-12-30 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro created this revision.
djtodoro added reviewers: labath, JDevlieghere.
djtodoro added a project: LLDB.
Herald added a subscriber: pengfei.
djtodoro requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch tries to improve memory-read from core files (in order to improve 
disassembly functionality).
I am using RHEL 7.7 (linux kernel 3.10) and for a lot of cases, I was not able 
to disassemble some functions from backtrace when debugging crashes from core 
files. It outputs some dummy code as following:

  (lldb) disassemble
  example`fn:
  0x55deb51866b0 <+0>:   addb   %al, (%rax)
  0x55deb51866b2 <+2>:   addb   %al, (%rax)
  0x55deb51866b4 <+4>:   addb   %al, (%rax)
  0x55deb51866b6 <+6>:   addb   %al, (%rax)
  

The cause of the problem was the fact we are returning all the zeros from 
`ProcessElfCore::ReadMemory()` that is being called within 
`Disassembler::ParseInstructions()` and it disassembles some dummy opcodes from 
the buffer returned. If we are about to fill the buffer with all zeros, I guess 
it is safe to just return zero, and to proceed with reading from file itself 
(using the `ReadMemoryFromFileCache()`) (an alternative is to force/prefer 
`ReadMemoryFromFileCache()` by settimg `prefer_file_cache` to true before 
calling `ReadMemory()`).

Before this patch (simple example that has been attached into the test):

  $ lldb 
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.out
 -c 
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.core
  (lldb) f 4
  frame #4: 0x7f1d2f862545 libc.so.6`__libc_start_main + 245
  libc.so.6`__libc_start_main:
  ->  0x7f1d2f862545 <+245>: addb   %al, (%rax)
  0x7f1d2f862547 <+247>: addb   %al, (%rax)
  0x7f1d2f862549 <+249>: addb   %al, (%rax)
  0x7f1d2f86254b <+251>: addb   %al, (%rax)

After this patch applied:

  (lldb) f 4
  frame #4: 0x7f1d2f862545 libc.so.6`__libc_start_main + 245
  libc.so.6`__libc_start_main:
  ->  0x7f1d2f862545 <+245>: movl   %eax, %edi
  0x7f1d2f862547 <+247>: callq  0x39d10   ; exit
  0x7f1d2f86254c <+252>: xorl   %edx, %edx
  0x7f1d2f86254e <+254>: jmp0x22489   ; <+57>

GDB was able to disassemble all the functions from core file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.core
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.out


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = 
self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = 
disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+#   ...
+# Sometimes it just prints some dummy code as:
+#   addb %al, (%rax)
+#   addb %al, (%rax)
+#   ...
+pushrbpinstr = disassembly.splitlines()[1]
+self.assertNotIn("addb %al, (%rax)", pushrbpinstr)
+
+self.dbg.DeleteTarget(disasmtarget)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -384,6 +384,11 @@
   if (zero_fill_size)
 memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
 
+  // No data found in the core file, so the buffer contains
+  // all zeros.
+  if (zero_fill_size == size)
+return 0;
+
   return bytes_copied +

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-01-18 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

(@labath Sorry for late response, I've been away from keyboard for some time.)

> Have you by any chance learned why are we zero-filling this buffer in the 
> first place? Seems like an odd thing to do... Maybe we should just stop 
> zero-filling completely?

Hmm... not sure why. It was there since the original commit that introduced the 
file (c037383aff81a61ed956858353ec003e970fb2ce). I don't see the point 
actually, and when I removed the `zero_fill_size` and zero filling with 
`memset()`, there was no regression in the lldb test suite. I guess we can 
remove it completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93939

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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-01-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro updated this revision to Diff 317873.
djtodoro edited the summary of this revision.

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

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.core
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.out


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = 
self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = 
disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+#   ...
+# Sometimes it just prints some dummy code as:
+#   addb %al, (%rax)
+#   addb %al, (%rax)
+#   ...
+framesetup = disassembly.splitlines()[1]
+self.assertNotIn("addb", framesetup)
+
+self.dbg.DeleteTarget(disasmtarget)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core 
file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -369,22 +368,15 @@
 
   // Figure out how many bytes we need to zero-fill if we are reading more
   // bytes than available in the on-disk segment
-  if (bytes_to_read > bytes_left) {
-zero_fill_size = bytes_to_read - bytes_left;
+  if (bytes_to_read > bytes_left)
 bytes_to_read = bytes_left;
-  }
 
   // If there is data available on the core file read it
   if (bytes_to_read)
 bytes_copied =
 core_objfile->CopyData(offset + file_start, bytes_to_read, buf);
 
-  assert(zero_fill_size <= size);
-  // Pad remaining bytes
-  if (zero_fill_size)
-memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
-
-  return bytes_copied + zero_fill_size;
+  return bytes_copied;
 }
 
 void ProcessElfCore::Clear() {


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-01-22 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro updated this revision to Diff 318464.
djtodoro added a comment.

nfc


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

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.core
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.out


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = 
self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = 
disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+#   ...
+# Sometimes it just prints some dummy code as:
+#   addb %al, (%rax)
+#   addb %al, (%rax)
+#   ...
+framesetup = disassembly.splitlines()[1]
+self.assertNotIn("addb", framesetup)
+
+self.dbg.DeleteTarget(disasmtarget)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core 
file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -367,24 +366,15 @@
   if (file_end > file_start + offset)
 bytes_left = file_end - (file_start + offset);
 
-  // Figure out how many bytes we need to zero-fill if we are reading more
-  // bytes than available in the on-disk segment
-  if (bytes_to_read > bytes_left) {
-zero_fill_size = bytes_to_read - bytes_left;
+  if (bytes_to_read > bytes_left)
 bytes_to_read = bytes_left;
-  }
 
   // If there is data available on the core file read it
   if (bytes_to_read)
 bytes_copied =
 core_objfile->CopyData(offset + file_start, bytes_to_read, buf);
 
-  assert(zero_fill_size <= size);
-  // Pad remaining bytes
-  if (zero_fill_size)
-memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
-
-  return bytes_copied + zero_fill_size;
+  return bytes_copied;
 }
 
 void ProcessElfCore::Clear() {


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-01-27 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

> Regarding the test, would it be possible to reuse one of the existing core 
> files? (The reason we have so few core tests is because we used to not allow 
> checked in core files at all -- now we kinda do, but it's still not ideal.) 
> I'm guessing you don't even need to disassemble a function to reproduce this 
> -- it should be sufficient to run `disassemble --start-address X 
> --end-address Y`, where the region `X--Y` crosses a core segment boundary..

We can disassemble a region that crosses core segment boundary, e.g.:
(let's use 
//lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core//)

  (lldb) disassemble --start 0x400161 --end 0x40100c
0x400161: addb   %al, (%rax)
0x400163: addb   %al, (%rax)
0x400165: addb   %al, (%rax)
0x400167: addb   %dl, (%rax,%rax)
0x40016a: addb   %al, (%rax)
0x40016c: addb   %al, (%rax)
0x40016e: addb   %al, (%rax)
0x400170: addl   %edi, 0x52(%rdx)
0x400173: addb   %al, (%rcx)
0x400175: js 0x400187
0x400177: addl   %ebx, (%rbx)
0x400179: orb$0x7, %al
0x40017b: orb%dl, 0x1c01(%rax)
0x400181: addb   %al, (%rax)
0x400183: addb   %bl, (%rax,%rax)
0x400186: addb   %al, (%rax)
0x400188: testb  %bh, %bh

and this triggers the code for zero-bytes-filling. After applying this patch, 
the output of the command is exactly the same.
I am not sure what should we check in the test if we use this existing 
core-file, any idea?


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

https://reviews.llvm.org/D93939

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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-02-01 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

> Another option would be to ditch disassembling, and check this via memory 
> reads, as that is what you are actually fixing

+1, nice! thanks!


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

https://reviews.llvm.org/D93939

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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-02-01 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro updated this revision to Diff 320483.
djtodoro added a comment.

- Test update


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

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,24 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_read_memory(self):
+"""Test that we are able to read as many bytes as available"""
+target = self.dbg.CreateTarget("linux-x86_64.out")
+process = target.LoadCore("linux-x86_64.core")
+self.assertTrue(process, PROCESS_IS_VALID)
+
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400ff0, 20, error)
+
+# read only 16 bytes without zero bytes filling
+self.assertEqual(len(bytesread), 16)
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core 
file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -367,24 +366,15 @@
   if (file_end > file_start + offset)
 bytes_left = file_end - (file_start + offset);
 
-  // Figure out how many bytes we need to zero-fill if we are reading more
-  // bytes than available in the on-disk segment
-  if (bytes_to_read > bytes_left) {
-zero_fill_size = bytes_to_read - bytes_left;
+  if (bytes_to_read > bytes_left)
 bytes_to_read = bytes_left;
-  }
 
   // If there is data available on the core file read it
   if (bytes_to_read)
 bytes_copied =
 core_objfile->CopyData(offset + file_start, bytes_to_read, buf);
 
-  assert(zero_fill_size <= size);
-  // Pad remaining bytes
-  if (zero_fill_size)
-memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
-
-  return bytes_copied + zero_fill_size;
+  return bytes_copied;
 }
 
 void ProcessElfCore::Clear() {


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,24 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_read_memory(self):
+"""Test that we are able to read as many bytes as available"""
+target = self.dbg.CreateTarget("linux-x86_64.out")
+process = target.LoadCore("linux-x86_64.core")
+self.assertTrue(process, PROCESS_IS_VALID)
+
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400ff0, 20, error)
+
+# read only 16 bytes without zero bytes filling
+self.assertEqual(len(bytesread), 16)
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -367,24 +366,15 @@
   if (file_end > file_start + offset)
 bytes_left = file_end - (file_start + offset);
 
-  // Figure out how many bytes we need to zero-fill if we are reading more
-  // bytes than available in the on-disk segment
-  if (bytes_

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-02-04 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

@labath Thanks for the review! I'll land this tomorrow.


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

https://reviews.llvm.org/D93939

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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

2021-02-08 Thread Djordje Todorovic via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9abd8c1a4c38: [elf-core] Improve reading memory from core 
file (authored by djtodoro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,24 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_read_memory(self):
+"""Test that we are able to read as many bytes as available"""
+target = self.dbg.CreateTarget("linux-x86_64.out")
+process = target.LoadCore("linux-x86_64.core")
+self.assertTrue(process, PROCESS_IS_VALID)
+
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400ff0, 20, error)
+
+# read only 16 bytes without zero bytes filling
+self.assertEqual(len(bytesread), 16)
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core 
file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -367,24 +366,15 @@
   if (file_end > file_start + offset)
 bytes_left = file_end - (file_start + offset);
 
-  // Figure out how many bytes we need to zero-fill if we are reading more
-  // bytes than available in the on-disk segment
-  if (bytes_to_read > bytes_left) {
-zero_fill_size = bytes_to_read - bytes_left;
+  if (bytes_to_read > bytes_left)
 bytes_to_read = bytes_left;
-  }
 
   // If there is data available on the core file read it
   if (bytes_to_read)
 bytes_copied =
 core_objfile->CopyData(offset + file_start, bytes_to_read, buf);
 
-  assert(zero_fill_size <= size);
-  // Pad remaining bytes
-  if (zero_fill_size)
-memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
-
-  return bytes_copied + zero_fill_size;
+  return bytes_copied;
 }
 
 void ProcessElfCore::Clear() {


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,24 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_read_memory(self):
+"""Test that we are able to read as many bytes as available"""
+target = self.dbg.CreateTarget("linux-x86_64.out")
+process = target.LoadCore("linux-x86_64.core")
+self.assertTrue(process, PROCESS_IS_VALID)
+
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400ff0, 20, error)
+
+# read only 16 bytes without zero bytes filling
+self.assertEqual(len(bytesread), 16)
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -367,24 +366,15 @@
   if (file_end > file_start + offset)
 bytes_left = file_end - (file_start + offset);
 
-  // Fi

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Thanks a lot for this!

> Nevertheless, I am still interested in making assembly-based tests for this 
> (and similar features) because it enables testing scenarios that we could not 
> get (reliably or at all) a compiler to produce.

I also think this would be more stable if we can make assembler-based tests 
(but we'll need to address all archs from {x86_64, arm, aarch64}).
I am just wondering, what are the obstacles for writing the assembler-based 
tests? Is it LLDB testing infrastructure or writing tests itself?




Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:21-22
+  ++global;
   //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
   // FUNC1-DESC: name = "sink", type = "int &", location = 
DW_OP_entry_value(DW_OP_reg5 RDI)
 }

labath wrote:
> vsk wrote:
> > labath wrote:
> > > If we remove this check, the test will be completely architecture- and 
> > > abi-independent. I don't think this check is particularly useful (we use 
> > > llvm to print the dwarf expression, and there are better ways to test the 
> > > image lookup command). Maybe we could just keep it to ensure that we 
> > > really are evaluating entry values, but change the check the just search 
> > > for the DW_OP_entry_value keyword (and then run the test on all 
> > > architectures)?
> > We should stop matching %rdi, as the purpose of the check is just to 
> > determine whether we really are testing entry value evaluation. However, 
> > llvm doesn't support entry value production for all platforms, so we would 
> > need to restrict the test to {x86_64, arm, aarch64} (still a clear 
> > improvement over the current situation).
> Sounds good. (I'm not sure we even have functioning bots for non-x86, non-arm 
> platforms).
>We should stop matching %rdi, as the purpose of the check is just to determine 
>whether we really are testing entry value evaluation. However, llvm doesn't 
>support entry value production for all platforms, so we would need to restrict 
>the test to {x86_64, arm, aarch64} (still a clear improvement over the current 
>situation).
+1
We can teach the `TestBasicEntryValuesX86_64.py` to keep arm and aarch64 as 
well.
Furthermore, we can add a function into decorators something like 
`skipUnlessEntryValuesSupportedArch` checking if the arch is in {x86_64, arm, 
aarch64}.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:61-62
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR-FAIL", 
expect_cmd_failure=True)
-  // FUNC2-EXPR-FAIL: couldn't get the value of variable x: variable not 
available
-

vsk wrote:
> labath wrote:
> > I'm not sure why this was previously expected to fail -- I don't see a 
> > reason why the compiler couldn't emit an entry value for `x` now, or before 
> > this patch. And in the new setup, the expression actually succeeds.
> IIRC this is left over from when entry value propagation in LiveDebugValues 
> was being reworked.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added inline comments.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+

I haven't refreshed the page before submitting my previous comment.

WDYT about adding a decorators-function (e.g. 
`skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

In D79491#2024771 , @labath wrote:

> In D79491#2024647 , @djtodoro wrote:
>
> > Thanks a lot for this!
> >
> > > Nevertheless, I am still interested in making assembly-based tests for 
> > > this (and similar features) because it enables testing scenarios that we 
> > > could not get (reliably or at all) a compiler to produce.
> >
> > I also think this would be more stable if we can make assembler-based tests 
> > (but we'll need to address all archs from {x86_64, arm, aarch64}).
> >  I am just wondering, what are the obstacles for writing the 
> > assembler-based tests? Is it LLDB testing infrastructure or writing tests 
> > itself?
>
>
> A bit of both, maybe. Writing a test which works on a single target (os+arch) 
> was relatively easy (for me, because I've done a lot of this stuff lately, 
> maybe not so easy for others), but the difficulties started when I wanted to 
> make that test run on other oses (which have different asm dialects). I was 
> ok with leaving the test x86-specific, since most developers have an x86 
> machine and there is nothing arch-specific about this functionality. However, 
> I did not want to restrict the test to any single OS, and since this test 
> requires a running program, the asm needed to match what the host expects.
>
> I did manage to ifdef around the asm platform quirks, but I still haven't 
> managed to get the darwin linker to recognize my hand-written dwarf. I am 
> sure this can be fixed (I think it's  because I reduced the input too much), 
> and I do want to try it out, but I am not sure the result will be something 
> we can recommend as a general practice.


I see.. Enabling it only for specific OSes  could be a temp solution, but we 
can discuss about that.

> A different way to address this would be to remove the requirement for a 
> running process. The test doesn't really require that, it just needs a 
> relatively complex static snapshot of the process. A core file is just that, 
> but we currently don't have any good way of creating such core files. If we 
> had that, we could run this test over a core file. That would still be 
> platform specific, but then it wouldn't matter (so much) because it wouldn't 
> be tied to the host platform.

Good idea! A corefile could be solution, and I believe LLDB parses corefiles 
from other architectures very well (as multiarch GDB does).




Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+

labath wrote:
> djtodoro wrote:
> > I haven't refreshed the page before submitting my previous comment.
> > 
> > WDYT about adding a decorators-function (e.g. 
> > `skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as 
> > well?
> I think that's a good idea, and that the check can be folded into the 
> existing `skipUnlessHasCallSiteInfo` decorator.
Please note that entry values and call site info are not the same thing. Entry 
values implementation ended up describing target-specific instructions (in 
terms of DWARF operations) that loads values into registers that transfers the 
parameters, and that is used for the entry values printing. Eventually, we will 
have all targets supporting the entry values, but the call site Info is 
something different. The call site info is controlled by the 
`DIFlagAllCallsDescribed` flag, and it is generated by the language front-end, 
according to DWARF standard. Therefore, it makes sense to me to add new 
independent function (`skipUnlessEntryValuesSupportedArch`), but the whole 
feature depends on both `skipUnlessEntryValuesSupportedArch` and 
`skipUnlessHasCallSiteInfo`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro accepted this revision.
djtodoro added a comment.

> It'd be nice to have a skipUnlessEntryValuesSupportedArch decorator, but I 
> don't think that has to be folded into this change.

OK. I agree.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:149
+EntryValue = 1 << 0,
+IndirectEntryValue = 1 << 1,
+CallSiteParamValue = 1 << 2

vsk wrote:
> aprantl wrote:
> > Would it make more sense call this `Indirect` (w/o EntryValue) and treat it 
> > as a separate bit, so you can still test for EntryValue independently?
> Yes, that does seem better. It's more extensible this way, and anyway, most 
> of the logic for treating direct/indirect entry values should be the same.
mot -> not



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

vsk wrote:
> djtodoro wrote:
> > I know that the final effect is the same, but should this be 
> > `DW_OP_entry_value 2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 
> > DW_OP_regN`?
> I'm not sure, the DW_OP_regN description is simply the one that has already 
> been wired up. Is there a reason to prefer 'DW_OP_bregN 0'?
I was thinking about a more complex entry values where the offset is other than 
`0`, where by using this way of representation we'll have more complex 
expression for the same thing.
If we have expressions as:

1) `DW_OP_entry_value 3 DW_OP_bregN M DW_OP_deref DW_OP_stack_value`

2) `DW_OP_entry_value 6 DW_OP_entry_value 1 DW_OP_regN DW_OP_plus_uconst M 
DW_OP_deref DW_OP_stack_value`

The two expressions have the same effect. They push the value memory location 
pointed to by value of  register `N` upon entering current subprogram plus `M` 
had upon entering of the current subprogram.

This may not be worth of considering now, but we can put a TODO marker in 
`DwarfExpression`.

(Sorry If I am asking to much) One quick question:

Should the `DW_OP_entry_value(DW_OP_reg0 W0))` be an entry value expression 
with `dw_op_deref` + `dw_op_stack_val`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Great! Thanks!

I think we should update the `LangRef.rst` (entry_values section) as well.

In addition, can we add a test case checking MIR output after `LiveDebugValues`?




Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

I know that the final effect is the same, but should this be `DW_OP_entry_value 
2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 DW_OP_regN`?



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:71
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)

Nit: !25-36! could be attached to !24



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:79
+---
+name:bar
+alignment:   4

I believe we do need all the attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-26 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro accepted this revision.
djtodoro added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

vsk wrote:
> djtodoro wrote:
> > vsk wrote:
> > > djtodoro wrote:
> > > > I know that the final effect is the same, but should this be 
> > > > `DW_OP_entry_value 2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 
> > > > DW_OP_regN`?
> > > I'm not sure, the DW_OP_regN description is simply the one that has 
> > > already been wired up. Is there a reason to prefer 'DW_OP_bregN 0'?
> > I was thinking about a more complex entry values where the offset is other 
> > than `0`, where by using this way of representation we'll have more complex 
> > expression for the same thing.
> > If we have expressions as:
> > 
> > 1) `DW_OP_entry_value 3 DW_OP_bregN M DW_OP_deref DW_OP_stack_value`
> > 
> > 2) `DW_OP_entry_value 6 DW_OP_entry_value 1 DW_OP_regN DW_OP_plus_uconst M 
> > DW_OP_deref DW_OP_stack_value`
> > 
> > The two expressions have the same effect. They push the value memory 
> > location pointed to by value of  register `N` upon entering current 
> > subprogram plus `M` had upon entering of the current subprogram.
> > 
> > This may not be worth of considering now, but we can put a TODO marker in 
> > `DwarfExpression`.
> > 
> > (Sorry If I am asking to much) One quick question:
> > 
> > Should the `DW_OP_entry_value(DW_OP_reg0 W0))` be an entry value expression 
> > with `dw_op_deref` + `dw_op_stack_val`?
> > 
> These are good questions.
> 
> If the indirect parameter offset is not 0, then it seems perfectly fine to 
> apply it outside of the nested entry value expression. However, we don't 
> support emitting entry values in this case. I've added a test to illustrate 
> that.
> 
> As for DW_OP_stack_value, I'm not sure it's the right thing to use for 
> locations. It breaks LLDB, fwiw: LLDB doesn't have the type of the object 
> available as it's evaluating a DWARF expression, so when it sees the 
> DW_OP_deref, it can't figure out the size and fails to reconstruct the object.
>If the indirect parameter offset is not 0, then it seems perfectly fine to 
>apply it outside of the nested entry value expression. However, we don't 
>support emitting entry values in this case. I've added a test to illustrate 
>that.
Thanks!


>As for DW_OP_stack_value, I'm not sure it's the right thing to use for 
>locations. It breaks LLDB, fwiw: LLDB doesn't have the type of the object 
>available as it's evaluating a DWARF expression, so when it sees the 
>DW_OP_deref, it can't figure out the size and fails to reconstruct the object.

Ah, sure.. The DW_OP_stack_value should be used for describing actual values 
rather than locations, so thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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

[Lldb-commits] [PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

2019-11-20 Thread Djordje Todorovic via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGce1f95a6e077: Reland "[clang] Remove the 
DIFlagArgumentNotModified debug info flag" (authored by djtodoro).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68206

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/debug-info-param-modification.c
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py

Index: lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
@@ -6,7 +6,8 @@
 supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
 
 lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipUnlessPlatform(supported_platforms),
+[decorators.skipIf(bugnumber="llvm.org/pr44059"),
+ decorators.skipUnlessPlatform(supported_platforms),
  decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),
  decorators.skipUnlessArch('x86_64'),
  decorators.skipUnlessHasCallSiteInfo,
Index: clang/test/CodeGen/debug-info-param-modification.c
===
--- clang/test/CodeGen/debug-info-param-modification.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
-//
-// For the os_log_helper:
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)
-//
-// RUN: %clang -g -O2 -Xclang -disable-llvm-passes -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
-// CHECK-NOT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
-//
-// For the os_log_helper:
-// CHECK: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)
-
-int fn2 (int a, int b) {
-  ++a;
-  return b;
-}
-
-void test_builtin_os_log(void *buf, int i, const char *data) {
-  __builtin_os_log_format(buf, "%d %{public}s %{private}.16P", i, data, data);
-}
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -146,10 +146,6 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
-  /// Cache function definitions relevant to use for parameters mutation
-  /// analysis.
-  llvm::DenseMap SPDefCache;
-  llvm::DenseMap ParamCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -18,7 +18,6 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3686,15 +3685,6 @@
   if (HasDecl && isa(D))
 DeclCache[D->getCanonicalDecl()].reset(SP);
 
-  // We use the SPDefCache only in the case when the debug entry values option
-  // is set, in order to speed up parameters modification analysis.
-  //
-  // FIXME: Use AbstractCallee here to support ObjCMethodDecl.
-  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)
-if (auto

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

2019-12-03 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Thanks for the reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209



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

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

2019-12-03 Thread Djordje Todorovic via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4cfceb910692: [LiveDebugValues] Introduce entry values of 
unmodified params (authored by djtodoro).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
  llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
  llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
  llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir

Index: llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
@@ -0,0 +1,184 @@
+# RUN: llc -debug-entry-values -run-pass=livedebugvalues -march=x86-64 -o - %s | FileCheck %s
+#
+#extern void fn1 (int, int, int);
+#__attribute__((noinline))
+#int
+#fn2 (int a, int b, int c) {
+#  int q = 2 + a;
+#  fn1 (5, 6, q);
+#  if (b < 17) {
+#b = b + 7;
+# fn1 (5, b, q);
+#  } else {
+#b = b + 1;
+#fn1 (1, b, q);
+#  }
+#  return b;
+#}
+# CHECK: ![[ARG_C:.*]] = !DILocalVariable(name: "c"
+# CHECK: bb.0.entry:
+# CHECK: DBG_VALUE $edx, $noreg, ![[ARG_C]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+# CHECK: bb.1.if.then:
+# CHECK: DBG_VALUE $edx, $noreg, ![[ARG_C]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+# CHECK: bb.2.if.else:
+# CHECK: DBG_VALUE $edx, $noreg, ![[ARG_C]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+# CHECK: bb.3.if.end:
+# CHECK: DBG_VALUE $edx, $noreg, ![[ARG_C]], !DIExpression(DW_OP_LLVM_entry_value, 1)
+#
+--- |
+  ; ModuleID = 'test.c'
+  source_filename = "test.c"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+
+  ; Function Attrs: noinline nounwind uwtable
+  define dso_local i32 @fn2(i32 %a, i32 %b, i32 %c) local_unnamed_addr !dbg !12 {
+  entry:
+call void @llvm.dbg.value(metadata i32 %a, metadata !16, metadata !DIExpression()), !dbg !20
+call void @llvm.dbg.value(metadata i32 %b, metadata !17, metadata !DIExpression()), !dbg !20
+call void @llvm.dbg.value(metadata i32 %c, metadata !18, metadata !DIExpression()), !dbg !20
+%add = add nsw i32 %a, 2, !dbg !21
+call void @llvm.dbg.value(metadata i32 %add, metadata !19, metadata !DIExpression()), !dbg !20
+tail call void @fn1(i32 5, i32 6, i32 %add), !dbg !22
+%cmp = icmp slt i32 %b, 17, !dbg !23
+br i1 %cmp, label %if.then, label %if.else, !dbg !25
+
+  if.then:  ; preds = %entry
+%add1 = add nsw i32 %b, 7, !dbg !26
+call void @llvm.dbg.value(metadata i32 %add1, metadata !17, metadata !DIExpression()), !dbg !20
+tail call void @fn1(i32 5, i32 %add1, i32 %add), !dbg !28
+br label %if.end, !dbg !29
+
+  if.else:  ; preds = %entry
+%add2 = add nuw nsw i32 %b, 1, !dbg !30
+call void @llvm.dbg.value(metadata i32 %add2, metadata !17, metadata !DIExpression()), !dbg !20
+tail call void @fn1(i32 1, i32 %add2, i32 %add), !dbg !32
+br label %if.end
+
+  if.end:   ; preds = %if.else, %if.then
+%b.addr.0 = phi i32 [ %add1, %if.then ], [ %add2, %if.else ], !dbg !33
+call void @llvm.dbg.value(metadata i32 %b.addr.0, metadata !17, metadata !DIExpression()), !dbg !20
+ret i32 %b.addr.0, !dbg !34
+  }
+
+  declare !dbg !4 dso_local void @fn1(i32, i32, i32) local_unnamed_addr
+
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10}
+  !llvm.ident = !{!11}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+  !1 = !DIFile(filename: "test.c", directory: "/")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "fn1", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{null, !7, !7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 2, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{!"clang version 10.0.0"}
+  !

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

2019-12-03 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added a comment.

Reverted while investigating. I am not sure what happened, since the test 
passed on my machine. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209



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

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

2019-12-04 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro marked an inline comment as done.
djtodoro added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:159
   // FUNC11-BT: func11_tailcalled{{.*}}
   // FUNC11-BT-NEXT: func12{{.*}} [artificial]
   use(x);

vsk wrote:
> The failure was:
> ```
> main.cpp:159:21: error: FUNC11-BT-NEXT: expected string not found in input
>  // FUNC11-BT-NEXT: func12{{.*}} [artificial]
> ^
> :3:2: note: scanning from here
>  frame #1: 0x0001079eae69 a.out`func12(sink=0x7ffee8215cb4, x=123) at 
> main.cpp:179:3 [opt]
> ```
> 
> The added `DESTROY_RBX` asm might confuse TailRecursionElimination into 
> believing that the callee accesses the caller's stack. Could you double-check 
> that a tail call is actually emitted in `func12` (something like `jmp 
> *%rax`)? If it //is//, this is a pre-existing lldb bug, so the func12 test 
> should be disabled.
@vsk Thanks for the comment!

The problem here is the fresh change in the code production by using the `-O1` 
level of optimization. More precisely, at very high level, after the D65410 we 
do not have a tail call where we expected.
I am proposing using the `-O2` level of the optimizations, since we are testing 
printing of the entry values in the test case, rather than tail call frames 
with particular level of optimization.
WDYT? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209



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

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

2020-01-13 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:984-991
   // from one function to another.
   if (DD->getDwarfVersion() == 4 && DD->tuneForGDB()) {
 assert(PCAddr && "Missing PC information for a call");
 addLabelAddress(CallSiteDIE, dwarf::DW_AT_low_pc, PCAddr);
   } else if (!IsTail || DD->tuneForGDB()) {
-assert(PCOffset && "Missing return PC information for a call");
-addAddressExpr(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCOffset);
+assert(PCAddr && "Missing return PC information for a call");
+addLabelAddress(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCAddr);

vsk wrote:
> dblaikie wrote:
> > Side question: How'd this end up like this? Why all these GDB tuning 
> > checks? Seems like it'd add another layer of complexity/variety that'll 
> > make it harder for us to all be evaluating the same things. 
> + @djtodoro, I'm not sure why AT_call_return_pc would be needed at a tail 
> call site as the debugger must ignore it. As for emitting DW_AT_low_pc under 
> gdb tuning, I think this might be an artifact from the original GNU 
> implementation.
>I'm not sure why AT_call_return_pc would be needed at a tail call site as the 
>debugger must ignore it.  As for emitting DW_AT_low_pc under gdb tuning, I 
>think this might be an artifact from the original GNU implementation.

Yes, that is the GNU implementation's heritage (I cannot remember why GCC 
generated the low_pc info in the case of the tail calls), but GNU GDB needs the 
low_pc (as an address) in order to handle the call_site and 
call_site_parameters debug info for non-tail calls. To avoiding the pc address 
info in the case of tail calls makes sense to me, since debuggers should avoid 
that info.


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

https://reviews.llvm.org/D72489



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

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

2020-01-13 Thread Djordje Todorovic via Phabricator via lldb-commits 
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:990
+assert(PCAddr && "Missing return PC information for a call");
+addLabelAddress(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCAddr);
   }

Why don't we use the `getDwarf5OrGNUAttr()` for the `return_pc/low_pc`, since 
we use the address in both cases now?


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

https://reviews.llvm.org/D72489



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