[Lldb-commits] [lldb] [IRInterpreter] Return zero address for missing weak function (PR #93548)
https://github.com/nikic closed https://github.com/llvm/llvm-project/pull/93548 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [IRInterpreter] Return zero address for missing weak function (PR #93548)
jimingham wrote: It's been a long time since I've looked at this code, but the change to return a 0 value rather than an error seems consistent at least. Was there no way to test this? https://github.com/llvm/llvm-project/pull/93548 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [IRInterpreter] Return zero address for missing weak function (PR #93548)
https://github.com/Michael137 approved this pull request. Seems like a reasonable thing to do (as this comment in [LoadAddressResolver ::Resolve](https://github.com/llvm/llvm-project/blob/fd8b2d2046508c027ccf0fffb50d665c8355997a/lldb/source/Expression/IRExecutionUnit.cpp#L758-L761) implies). Might want to hold off from merging in case @jimingham has any input https://github.com/llvm/llvm-project/pull/93548 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [IRInterpreter] Return zero address for missing weak function (PR #93548)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Nikita Popov (nikic)
Changes
If a weak function is missing, still return it's address (zero) rather than
failing interpretation. Otherwise we have a mismatch between Interpret() and
CanInterpret() resulting in failures that would not occur with JIT execution.
Alternatively, we could try to look for weak symbols in CanInterpret() and
generally reject them there.
This is the root cause for the issue exposed by
https://github.com/llvm/llvm-project/pull/92885. Previously, the case affected
by that always fell back to JIT because an icmp constant expression was used,
which is not supported by the interpreter. Now a normal icmp instruction is
used, which is supported. However, we fail to interpret due to incorrect
handling of weak function addresses.
---
Full diff: https://github.com/llvm/llvm-project/pull/93548.diff
1 Files Affected:
- (modified) lldb/source/Expression/IRInterpreter.cpp (+1-1)
``diff
diff --git a/lldb/source/Expression/IRInterpreter.cpp
b/lldb/source/Expression/IRInterpreter.cpp
index df02922708663..5b670067b5c43 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -264,7 +264,7 @@ class InterpreterStackFrame {
lldb_private::ConstString name(constant_func->getName());
bool missing_weak = false;
lldb::addr_t addr = m_execution_unit.FindSymbol(name, missing_weak);
-if (addr == LLDB_INVALID_ADDRESS || missing_weak)
+if (addr == LLDB_INVALID_ADDRESS)
return false;
value = APInt(m_target_data.getPointerSizeInBits(), addr);
return true;
``
https://github.com/llvm/llvm-project/pull/93548
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [IRInterpreter] Return zero address for missing weak function (PR #93548)
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/93548
If a weak function is missing, still return it's address (zero) rather than
failing interpretation. Otherwise we have a mismatch between Interpret() and
CanInterpret() resulting in failures that would not occur with JIT execution.
Alternatively, we could try to look for weak symbols in CanInterpret() and
generally reject them there.
This is the root cause for the issue exposed by
https://github.com/llvm/llvm-project/pull/92885. Previously, the case affected
by that always fell back to JIT because an icmp constant expression was used,
which is not supported by the interpreter. Now a normal icmp instruction is
used, which is supported. However, we fail to interpret due to incorrect
handling of weak function addresses.
>From 0e74f5e94e99c97a8948a521b270f2e1413cc53b Mon Sep 17 00:00:00 2001
From: Nikita Popov
Date: Tue, 28 May 2024 15:34:55 +0200
Subject: [PATCH] [IRInterpreter] Return zero address for missing weak function
If a weak function is missing, still return it's address (zero)
rather than failing interpretation. Otherwise we have a mismatch
between Interpret() and CanInterpret() resulting in failures that
would not occur with JIT execution.
Alternatively, we could try to look for weak symbols in
CanInterpret() and generally reject them there.
This is the root cause for the issue exposed by
https://github.com/llvm/llvm-project/pull/92885. Previously,
the case affected by that always fell back to JIT because an
icmp constant expression was used, which is not supported by the
interpreter. Now a normal icmp instruction is used, which is
supported. However, we fail to interpret due to incorrect
handling of weak function addresses.
---
lldb/source/Expression/IRInterpreter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Expression/IRInterpreter.cpp
b/lldb/source/Expression/IRInterpreter.cpp
index df02922708663..5b670067b5c43 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -264,7 +264,7 @@ class InterpreterStackFrame {
lldb_private::ConstString name(constant_func->getName());
bool missing_weak = false;
lldb::addr_t addr = m_execution_unit.FindSymbol(name, missing_weak);
-if (addr == LLDB_INVALID_ADDRESS || missing_weak)
+if (addr == LLDB_INVALID_ADDRESS)
return false;
value = APInt(m_target_data.getPointerSizeInBits(), addr);
return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
