https://github.com/kovdan01 closed
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/jasonmolenda approved this pull request.
Looks good, please land it. Thanks for the pings and rewriting the test case!
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
kovdan01 wrote:
Thanks @jasonmolenda for your feedback and suggestion! See
f96989dd1f832284b74d07d1e457a15a0b16c199 - I've deleted the test with corefile
and added the test you've mentioned. Basically, I've just left the most simple
test from "normal" `Testx86AssemblyInspectionEngine` and
jasonmolenda wrote:
Hi sorry @kovdan01 I missed this one in the emails. You're using an lldb which
was built without the `LLVM_TARGETS_TO_BUILD` including X86, and running that
lldb on an x86 corefile, got it. I have low confidence how well lldb will work
in this situation, e.g. inferior
kovdan01 wrote:
@jasonmolenda A kind reminder regarding the PR - see answers to your previous
comments above
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
kovdan01 wrote:
@jasonmolenda Would be glad to see your feedback - see answers to your previous
comments above
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
DavidSpickett wrote:
I will be away for a while so @jasonmolenda please merge it if it looks fine to
you.
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
kovdan01 wrote:
@jasonmolenda Just in case you've missed - I've provided the use case
description leading to the issue (as you requested) above
https://github.com/llvm/llvm-project/pull/82603#issuecomment-1970019956. Would
be glad if you let me know if it gives you enough info & if this
kovdan01 wrote:
> As this plugin seems related to backtrace perhaps the test should at least
> run `bt` and check for the first line of the output.
I had similar thoughts, but I'm not sure if placing the test in
`lldb/test/Shell/Commands` as `command-backtrace-missing-x86.test` is a nice
@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86
kovdan01 wrote:
Yes, I've checked that with `X86` in `LLVM_TARGETS_TO_BUILD`, the test becomes
unsupported. So, it looks like it's currently OK
https://github.com/llvm/llvm-project/pull/82603
@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86
DavidSpickett wrote:
Actually, I'm wrong.
We get the configured targets from llvm-config and I think that adds "X86" as a
feature, target- is about what the default triple of llvm is.
@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86
DavidSpickett wrote:
This should be:
```
UNSUPPORTED: target-x86_64
```
Which I'm pretty sure is lldb's equivalent of `x86-registered-target`. Which is
whether the lldb has that target built into it, vs. whether it's running
DavidSpickett wrote:
The spirit of the test is fine but I think we can avoid adding another core
file.
As this plugin seems related to backtrace perhaps the test should at least run
`bt` and check for the first line of the output. Which means we can justify
putting the test in
kovdan01 wrote:
> Can this code be hit when using an x86 core file? Then you could write a
> shell test thatis `UNSUPPORTED: x86-registered-target` (whatever the proper
> syntax is) and assert that it does not crash.
@DavidSpickett The issue is present when loading an x86 core files via `lldb
kovdan01 wrote:
> Can this code be hit when using an x86 core file?
Thanks for suggestion! I'll try that and notify here if such approach succeeded.
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
kovdan01 wrote:
> tbh I have no problems with the patch, but I think it's fixing something that
> I think should be reconsidered altogether, I'm interested to hear more about
> what the use case looks like that led to this being a problem.
@jasonmolenda The use case is very simple, describing
jasonmolenda wrote:
tbh I have no problems with the patch, but I think it's fixing something that I
think should be reconsidered altogether, I'm interested to hear more about what
the use case looks like that led to this being a problem.
https://github.com/llvm/llvm-project/pull/82603
JDevlieghere wrote:
Also this is plugin code, it's probably more appropriate to disable the whole
plugin if the necessary backend isn't there?
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
jasonmolenda wrote:
Well, it would change source-level next and step too if we didn't have the
disassembler. When we're doing a source-level step/next, we look at the stream
of instructions and when we have a block of non-branching instructions, we put
a breakpoint after the block and
jasonmolenda wrote:
tbh most of lldb would work without the llvm target compiled in, but the
disassembler and the Intel assembly scanning unwind plan creator both need it,
and the latter is more important.
https://github.com/llvm/llvm-project/pull/82603
jasonmolenda wrote:
I'm a little curious the use case that led to this failure. We have a build of
llvm/lldb without the X86 target, and we're using that lldb to debug an
i386/x86_64 target (gdb connection or corefile)? Because we can't use
instruction emulation based unwinding, if we don't
DavidSpickett wrote:
Can this code be hit when using an x86 core file? Then you could write a shell
test thatis `UNSUPPORTED: x86-registered-target` (whatever the proper syntax
is) and assert that it does not crash.
It won't get run on the build bots, they enable all targets, but it's nice to
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Daniil Kovalev (kovdan01)
Changes
If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an x86
binary in lldb, we get a `nullptr` dereference in `LLVMDisasmInstruction(...)`.
We try to call `getDisAsm()` method on a
https://github.com/kovdan01 ready_for_review
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
kovdan01 wrote:
Please suggest ideas how this could be tested. It looks like that
UnwindAssembly unittests is the right place for it - but UnwindAssemblyx86Tests
cmake target is not built without X86 in `LLVM_TARGETS_TO_BUILD`, which is the
pre-condition of the issue this PR is fixing.
https://github.com/kovdan01 created
https://github.com/llvm/llvm-project/pull/82603
If `LLVM_TARGETS_TO_BUILD` does not contain `X86` and we try to run an x86
binary in lldb, we get a `nullptr` dereference in `LLVMDisasmInstruction(...)`.
We try to call `getDisAsm()` method on a
26 matches
Mail list logo
