[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work
DavidSpickett accepted this revision. DavidSpickett added a comment. This LGTM. Comment at: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h:58 + uint32_t reg_num) override; + lldb::addr_t ReadPC(bool *success); + bool WritePC(lldb::addr_t pc); seehearfeel wrote: > lh03061238 wrote: > > DavidSpickett wrote: > > > I think the older targets use this form but for riscv they went with > > > `llvm::Optional ReadPC();` which I prefer over pointer plus > > > addr_t. > > > I think the older targets use this form but for riscv they went with > > > `llvm::Optional ReadPC();` which I prefer over pointer plus > > > addr_t. > > > > EmulateInstructionLoongArch is relatively simple Compared with riscv. If > > use llvm::Optional ReadPC(), There will be more type > > conversions here. I prefer to keep that definition for now, Considering the > > complexity of the code at this stage. > > > > Thank you. > @DavidSpickett Are you OK with the current code? > I prefer to keep that definition for now, Considering the complexity of the > code at this stage. Totally fine, this is a lot of duplicating existing code to begin with so I appreciate why you'd keep the differences minimal. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139158/new/ https://reviews.llvm.org/D139158 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3a9e07b - [LLDB][LoongArch] Make software single stepping work
Author: Weining Lu
Date: 2022-12-08T19:06:07+08:00
New Revision: 3a9e07b1e7f4718a0e117f3a732f1679c4bf2e30
URL:
https://github.com/llvm/llvm-project/commit/3a9e07b1e7f4718a0e117f3a732f1679c4bf2e30
DIFF:
https://github.com/llvm/llvm-project/commit/3a9e07b1e7f4718a0e117f3a732f1679c4bf2e30.diff
LOG: [LLDB][LoongArch] Make software single stepping work
Hardware single stepping is not currently supported by the linux kernel.
In order to support single step debugging, add EmulateInstructionLoongArch
to implement the software Single Stepping. This patch only support the
simplest single step execution of non-jump instructions.
Reviewed By: SixWeining, DavidSpickett
Differential Revision: https://reviews.llvm.org/D139158
Added:
lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
Modified:
lldb/source/Plugins/Instruction/CMakeLists.txt
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/SystemInitializerLLGS.cpp
Removed:
diff --git a/lldb/source/Plugins/Instruction/CMakeLists.txt
b/lldb/source/Plugins/Instruction/CMakeLists.txt
index 631c0b307cac..46d610f261e0 100644
--- a/lldb/source/Plugins/Instruction/CMakeLists.txt
+++ b/lldb/source/Plugins/Instruction/CMakeLists.txt
@@ -1,5 +1,6 @@
add_subdirectory(ARM)
add_subdirectory(ARM64)
+add_subdirectory(LoongArch)
add_subdirectory(MIPS)
add_subdirectory(MIPS64)
add_subdirectory(PPC64)
diff --git a/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
b/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
new file mode 100644
index ..59802ee8fa9a
--- /dev/null
+++ b/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_library(lldbPluginInstructionLoongArch PLUGIN
+ EmulateInstructionLoongArch.cpp
+
+ LINK_LIBS
+lldbCore
+lldbInterpreter
+lldbPluginProcessUtility
+lldbSymbol
+ LINK_COMPONENTS
+Support
+ )
diff --git
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
new file mode 100644
index ..14ac993d9ac0
--- /dev/null
+++ b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -0,0 +1,181 @@
+//===---EmulateInstructionLoongArch.cpp===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include
+
+#include "EmulateInstructionLoongArch.h"
+#include "Plugins/Process/Utility/InstructionUtils.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h"
+#include "Plugins/Process/Utility/lldb-loongarch-register-enums.h"
+#include "lldb/Core/Address.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Interpreter/OptionValueArray.h"
+#include "lldb/Interpreter/OptionValueDictionary.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Stream.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/MathExtras.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionLoongArch, InstructionLoongArch)
+
+namespace lldb_private {
+
+EmulateInstructionLoongArch::Opcode *
+EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
+ // TODO: Add the mask of jump instruction.
+ static EmulateInstructionLoongArch::Opcode g_opcodes[] = {
+ {0x, 0x, &EmulateInstructionLoongArch::EmulateNonJMP,
+ "NonJMP"}};
+ static const size_t num_loongarch_opcodes = std::size(g_opcodes);
+
+ for (size_t i = 0; i < num_loongarch_opcodes; ++i)
+if ((g_opcodes[i].mask & inst) == g_opcodes[i].value)
+ return &g_opcodes[i];
+ return nullptr;
+}
+
+bool EmulateInstructionLoongArch::EvaluateInstruction(uint32_t options) {
+ uint32_t inst_size = m_opcode.GetByteSize();
+ uint32_t inst = m_opcode.GetOpcode32();
+ bool increase_pc = options & eEmulateInstructionOptionAutoAdvancePC;
+ bool success = false;
+
+ Opcode *opcode_data = GetOpcodeForInstruction(inst);
+ if (!opcode_data)
+return false;
+
+ lldb::addr_t old_pc = 0;
+ if (increase_pc) {
+old_pc = ReadPC(&success);
+if (!success)
+ return false;
+ }
+
+ // Call the Emulate... function.
+ if (!(this->*opcode_data->callback)(inst))
+return false;
+
+ if (increase_pc) {
+lld
[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a9e07b1e7f4: [LLDB][LoongArch] Make software single
stepping work (authored by SixWeining).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139158/new/
https://reviews.llvm.org/D139158
Files:
lldb/source/Plugins/Instruction/CMakeLists.txt
lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/SystemInitializerLLGS.cpp
Index: lldb/tools/lldb-server/SystemInitializerLLGS.cpp
===
--- lldb/tools/lldb-server/SystemInitializerLLGS.cpp
+++ lldb/tools/lldb-server/SystemInitializerLLGS.cpp
@@ -29,6 +29,11 @@
#include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
#endif
+#if defined(__loongarch__)
+#define LLDB_TARGET_LoongArch
+#include "Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h"
+#endif
+
#if defined(__mips64__) || defined(mips64) || defined(__mips64) || \
defined(__MIPS64__) || defined(_M_MIPS64)
#define LLDB_TARGET_MIPS64
@@ -57,6 +62,9 @@
#if defined(LLDB_TARGET_ARM) || defined(LLDB_TARGET_ARM64)
EmulateInstructionARM::Initialize();
#endif
+#if defined(LLDB_TARGET_LoongArch)
+ EmulateInstructionLoongArch::Initialize();
+#endif
#if defined(LLDB_TARGET_MIPS) || defined(LLDB_TARGET_MIPS64)
EmulateInstructionMIPS::Initialize();
#endif
@@ -76,6 +84,9 @@
#if defined(LLDB_TARGET_ARM) || defined(LLDB_TARGET_ARM64)
EmulateInstructionARM::Terminate();
#endif
+#if defined(LLDB_TARGET_LoongArch)
+ EmulateInstructionLoongArch::Terminate();
+#endif
#if defined(LLDB_TARGET_MIPS) || defined(LLDB_TARGET_MIPS64)
EmulateInstructionMIPS::Terminate();
#endif
Index: lldb/tools/lldb-server/CMakeLists.txt
===
--- lldb/tools/lldb-server/CMakeLists.txt
+++ lldb/tools/lldb-server/CMakeLists.txt
@@ -51,6 +51,7 @@
lldbVersion
${LLDB_PLUGINS}
lldbPluginInstructionARM
+ lldbPluginInstructionLoongArch
lldbPluginInstructionMIPS
lldbPluginInstructionMIPS64
lldbPluginInstructionRISCV
Index: lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
===
--- lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
+++ lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
@@ -168,7 +168,7 @@
size_hint = 4;
}
} else if (arch.IsMIPS() || arch.GetTriple().isPPC64() ||
- arch.GetTriple().isRISCV())
+ arch.GetTriple().isRISCV() || arch.GetTriple().isLoongArch())
size_hint = 4;
error = process.SetBreakpoint(next_pc, size_hint, /*hardware=*/false);
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -883,7 +883,7 @@
bool NativeProcessLinux::SupportHardwareSingleStepping() const {
if (m_arch.IsMIPS() || m_arch.GetMachine() == llvm::Triple::arm ||
- m_arch.GetTriple().isRISCV())
+ m_arch.GetTriple().isRISCV() || m_arch.GetTriple().isLoongArch())
return false;
return true;
}
Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
===
--- /dev/null
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
@@ -0,0 +1,76 @@
+//===---EmulateInstructionLoongArch.h--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_INSTRUCTION_LOONGARCH_EMULATEINSTRUCTIONLOONGARCH_H
+#define LLDB_SOURCE_PLUGINS_INSTRUCTION_LOONGARCH_EMULATEINSTRUCTIONLOONGARCH_H
+
+#include "lldb/Core/EmulateInstruction.h"
+#include "lldb/Interpreter/OptionValue.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Status.h"
+
+namespace lldb_private {
+
+class EmulateInstructionLoongArch : public EmulateInstruction {
+public:
+ static llvm::StringRef GetPluginNameStatic() { return "LoongArch"; }
+
+ static llvm::StringRef GetPluginDescriptionStatic() {
+return "Emulate instructions for the LoongArch architecture.";
+ }
+
+ static bool SupportsThisInstructionType(I
[Lldb-commits] [lldb] 2bfef8d - Revert "[LLDB][LoongArch] Make software single stepping work"
Author: Weining Lu
Date: 2022-12-08T19:56:58+08:00
New Revision: 2bfef8d5370fbe9d500441b2e1e3fe4a4d68deec
URL:
https://github.com/llvm/llvm-project/commit/2bfef8d5370fbe9d500441b2e1e3fe4a4d68deec
DIFF:
https://github.com/llvm/llvm-project/commit/2bfef8d5370fbe9d500441b2e1e3fe4a4d68deec.diff
LOG: Revert "[LLDB][LoongArch] Make software single stepping work"
This reverts commit 3a9e07b1e7f4718a0e117f3a732f1679c4bf2e30.
Reason to revert: author name is wrong.
Added:
Modified:
lldb/source/Plugins/Instruction/CMakeLists.txt
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/SystemInitializerLLGS.cpp
Removed:
lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
diff --git a/lldb/source/Plugins/Instruction/CMakeLists.txt
b/lldb/source/Plugins/Instruction/CMakeLists.txt
index 46d610f261e0d..631c0b307cac4 100644
--- a/lldb/source/Plugins/Instruction/CMakeLists.txt
+++ b/lldb/source/Plugins/Instruction/CMakeLists.txt
@@ -1,6 +1,5 @@
add_subdirectory(ARM)
add_subdirectory(ARM64)
-add_subdirectory(LoongArch)
add_subdirectory(MIPS)
add_subdirectory(MIPS64)
add_subdirectory(PPC64)
diff --git a/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
b/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
deleted file mode 100644
index 59802ee8fa9ad..0
--- a/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-add_lldb_library(lldbPluginInstructionLoongArch PLUGIN
- EmulateInstructionLoongArch.cpp
-
- LINK_LIBS
-lldbCore
-lldbInterpreter
-lldbPluginProcessUtility
-lldbSymbol
- LINK_COMPONENTS
-Support
- )
diff --git
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
deleted file mode 100644
index 14ac993d9ac0f..0
--- a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
+++ /dev/null
@@ -1,181 +0,0 @@
-//===---EmulateInstructionLoongArch.cpp===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include
-
-#include "EmulateInstructionLoongArch.h"
-#include "Plugins/Process/Utility/InstructionUtils.h"
-#include "Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h"
-#include "Plugins/Process/Utility/lldb-loongarch-register-enums.h"
-#include "lldb/Core/Address.h"
-#include "lldb/Core/PluginManager.h"
-#include "lldb/Interpreter/OptionValueArray.h"
-#include "lldb/Interpreter/OptionValueDictionary.h"
-#include "lldb/Symbol/UnwindPlan.h"
-#include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/Utility/RegisterValue.h"
-#include "lldb/Utility/Stream.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/Support/MathExtras.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionLoongArch, InstructionLoongArch)
-
-namespace lldb_private {
-
-EmulateInstructionLoongArch::Opcode *
-EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
- // TODO: Add the mask of jump instruction.
- static EmulateInstructionLoongArch::Opcode g_opcodes[] = {
- {0x, 0x, &EmulateInstructionLoongArch::EmulateNonJMP,
- "NonJMP"}};
- static const size_t num_loongarch_opcodes = std::size(g_opcodes);
-
- for (size_t i = 0; i < num_loongarch_opcodes; ++i)
-if ((g_opcodes[i].mask & inst) == g_opcodes[i].value)
- return &g_opcodes[i];
- return nullptr;
-}
-
-bool EmulateInstructionLoongArch::EvaluateInstruction(uint32_t options) {
- uint32_t inst_size = m_opcode.GetByteSize();
- uint32_t inst = m_opcode.GetOpcode32();
- bool increase_pc = options & eEmulateInstructionOptionAutoAdvancePC;
- bool success = false;
-
- Opcode *opcode_data = GetOpcodeForInstruction(inst);
- if (!opcode_data)
-return false;
-
- lldb::addr_t old_pc = 0;
- if (increase_pc) {
-old_pc = ReadPC(&success);
-if (!success)
- return false;
- }
-
- // Call the Emulate... function.
- if (!(this->*opcode_data->callback)(inst))
-return false;
-
- if (increase_pc) {
-lldb::addr_t new_pc = ReadPC(&success);
-if (!success)
- return false;
-
-if (new_pc == old_pc && !WritePC(old_pc + inst_size))
- return false;
- }
- return true;
-}
-
-bool EmulateInstructionLoongArch::ReadInstruction() {
- bool
[Lldb-commits] [lldb] f0f3395 - [LLDB][LoongArch] Make software single stepping work
Author: Hui Li
Date: 2022-12-08T19:59:39+08:00
New Revision: f0f33957d03444eefc8264cbfe7f5cfe7bee6f3d
URL:
https://github.com/llvm/llvm-project/commit/f0f33957d03444eefc8264cbfe7f5cfe7bee6f3d
DIFF:
https://github.com/llvm/llvm-project/commit/f0f33957d03444eefc8264cbfe7f5cfe7bee6f3d.diff
LOG: [LLDB][LoongArch] Make software single stepping work
Hardware single stepping is not currently supported by the linux kernel.
In order to support single step debugging, add EmulateInstructionLoongArch
to implement the software Single Stepping. This patch only support the
simplest single step execution of non-jump instructions.
Reviewed By: SixWeining, DavidSpickett
Differential Revision: https://reviews.llvm.org/D139158
Added:
lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
Modified:
lldb/source/Plugins/Instruction/CMakeLists.txt
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/SystemInitializerLLGS.cpp
Removed:
diff --git a/lldb/source/Plugins/Instruction/CMakeLists.txt
b/lldb/source/Plugins/Instruction/CMakeLists.txt
index 631c0b307cac4..46d610f261e0d 100644
--- a/lldb/source/Plugins/Instruction/CMakeLists.txt
+++ b/lldb/source/Plugins/Instruction/CMakeLists.txt
@@ -1,5 +1,6 @@
add_subdirectory(ARM)
add_subdirectory(ARM64)
+add_subdirectory(LoongArch)
add_subdirectory(MIPS)
add_subdirectory(MIPS64)
add_subdirectory(PPC64)
diff --git a/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
b/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
new file mode 100644
index 0..59802ee8fa9ad
--- /dev/null
+++ b/lldb/source/Plugins/Instruction/LoongArch/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_library(lldbPluginInstructionLoongArch PLUGIN
+ EmulateInstructionLoongArch.cpp
+
+ LINK_LIBS
+lldbCore
+lldbInterpreter
+lldbPluginProcessUtility
+lldbSymbol
+ LINK_COMPONENTS
+Support
+ )
diff --git
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
new file mode 100644
index 0..14ac993d9ac0f
--- /dev/null
+++ b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -0,0 +1,181 @@
+//===---EmulateInstructionLoongArch.cpp===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include
+
+#include "EmulateInstructionLoongArch.h"
+#include "Plugins/Process/Utility/InstructionUtils.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h"
+#include "Plugins/Process/Utility/lldb-loongarch-register-enums.h"
+#include "lldb/Core/Address.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Interpreter/OptionValueArray.h"
+#include "lldb/Interpreter/OptionValueDictionary.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Stream.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/MathExtras.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionLoongArch, InstructionLoongArch)
+
+namespace lldb_private {
+
+EmulateInstructionLoongArch::Opcode *
+EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
+ // TODO: Add the mask of jump instruction.
+ static EmulateInstructionLoongArch::Opcode g_opcodes[] = {
+ {0x, 0x, &EmulateInstructionLoongArch::EmulateNonJMP,
+ "NonJMP"}};
+ static const size_t num_loongarch_opcodes = std::size(g_opcodes);
+
+ for (size_t i = 0; i < num_loongarch_opcodes; ++i)
+if ((g_opcodes[i].mask & inst) == g_opcodes[i].value)
+ return &g_opcodes[i];
+ return nullptr;
+}
+
+bool EmulateInstructionLoongArch::EvaluateInstruction(uint32_t options) {
+ uint32_t inst_size = m_opcode.GetByteSize();
+ uint32_t inst = m_opcode.GetOpcode32();
+ bool increase_pc = options & eEmulateInstructionOptionAutoAdvancePC;
+ bool success = false;
+
+ Opcode *opcode_data = GetOpcodeForInstruction(inst);
+ if (!opcode_data)
+return false;
+
+ lldb::addr_t old_pc = 0;
+ if (increase_pc) {
+old_pc = ReadPC(&success);
+if (!success)
+ return false;
+ }
+
+ // Call the Emulate... function.
+ if (!(this->*opcode_data->callback)(inst))
+return false;
+
+ if (increase_pc) {
+l
[Lldb-commits] [PATCH] D139643: [lldb][Test] TestRerunAndExpr.py: explicitly delete a.out before rebuilding it
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
**Summary**
Older versions of `make` would occasionally fail to realize
that a pre-requisite for the `a.out` target has changed. This
resulted in roughly 1 out of 10 test runs to fail. Instead of
relying on `make` to resolve this dependency simply remove the
file before rebuilding; this will give make no option but to
remake `a.out`.
**Testing**
- Confirmed that the test passes on the host for 100 runs where without the
patch it would fail after ~10
**Details**
Adding `-d` to lldbtest's `make` invocation and running the
test without this patch sometimes yielded:
Removing child 0x60308ff0 PID 19915 from chain.
Successfully remade target file `rebuild.o'.
Finished prerequisites of target file `a.out'.
Prerequisite `rebuild.o' is newer than target `a.out'.
No need to remake target `a.out'.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139643
Files:
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Force make to rebuild the executable by deleting it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Force make to rebuild the executable by deleting it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139643: [lldb][Test] TestRerunAndExpr.py: explicitly delete a.out before rebuilding it
Michael137 updated this revision to Diff 481324.
Michael137 added a comment.
Herald added a subscriber: JDevlieghere.
- Reword comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139643/new/
https://reviews.llvm.org/D139643
Files:
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Delete the executable to force make to rebuild it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Delete the executable to force make to rebuild it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ad3870d - [lldb][Test] TestRerunAndExpr.py: explicitly delete a.out before rebuilding it
Author: Michael Buch
Date: 2022-12-08T17:17:05Z
New Revision: ad3870d6552305d2d6bd6aa2faca6f0644052d9a
URL:
https://github.com/llvm/llvm-project/commit/ad3870d6552305d2d6bd6aa2faca6f0644052d9a
DIFF:
https://github.com/llvm/llvm-project/commit/ad3870d6552305d2d6bd6aa2faca6f0644052d9a.diff
LOG: [lldb][Test] TestRerunAndExpr.py: explicitly delete a.out before
rebuilding it
**Summary**
Older versions of `make` would occasionally fail to realize
that a pre-requisite for the `a.out` target has changed. This
resulted in roughly 1 out of 10 test runs to fail. Instead of
relying on `make` to resolve this dependency simply remove the
file before rebuilding; this will give make no option but to
remake `a.out`.
**Testing**
* Confirmed that the test passes on the host for 100 runs where
without the patch it would fail after ~10
**Details**
Adding `-d` to lldbtest's `make` invocation and running the
test without this patch sometimes yielded:
```
Removing child 0x60308ff0 PID 19915 from chain.
Successfully remade target file `rebuild.o'.
Finished prerequisites of target file `a.out'.
Prerequisite `rebuild.o' is newer than target `a.out'.
No need to remake target `a.out'.
```
Differential Revision: https://reviews.llvm.org/D139643
Added:
Modified:
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
Removed:
diff --git a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
b/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
index 192e13151a954..09ccf1f1a5b4c 100644
--- a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ b/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@ def test(self):
ValueCheck(name='m_val', value='42')
])
+# Delete the executable to force make to rebuild it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139643: [lldb][Test] TestRerunAndExpr.py: explicitly delete a.out before rebuilding it
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGad3870d65523: [lldb][Test] TestRerunAndExpr.py: explicitly
delete a.out before rebuilding it (authored by Michael137).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139643/new/
https://reviews.llvm.org/D139643
Files:
lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Delete the executable to force make to rebuild it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -41,6 +41,8 @@
ValueCheck(name='m_val', value='42')
])
+# Delete the executable to force make to rebuild it.
+remove_file(exe)
self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
# Rerun program within the same target
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c0ac25f - [lldb] Fix simple template names interaction with debug info declarations
Author: Arthur Eubanks
Date: 2022-12-08T09:40:43-08:00
New Revision: c0ac25f1acc4dae97f420cd6bdf1c00c632563f1
URL:
https://github.com/llvm/llvm-project/commit/c0ac25f1acc4dae97f420cd6bdf1c00c632563f1
DIFF:
https://github.com/llvm/llvm-project/commit/c0ac25f1acc4dae97f420cd6bdf1c00c632563f1.diff
LOG: [lldb] Fix simple template names interaction with debug info declarations
Without checking template parameters, we would sometimes lookup the
wrong type definition for a type declaration because different
instantiations of the same template class had the same debug info name.
The added GetForwardDeclarationDIETemplateParams() shouldn't need a
cache because we'll cache the results of the declaration -> definition
lookup anyway. (DWARFASTParserClang::ParseStructureLikeDIE()
is_forward_declaration branch)
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D138834
Added:
lldb/test/API/lang/cpp/unique-types3/Makefile
lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
lldb/test/API/lang/cpp/unique-types3/a.cpp
lldb/test/API/lang/cpp/unique-types3/a.h
lldb/test/API/lang/cpp/unique-types3/main.cpp
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index a6dba10c3057a..412616acecafc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -55,6 +55,9 @@ class DWARFASTParser {
virtual void EnsureAllDIEsInDeclContextHaveBeenParsed(
lldb_private::CompilerDeclContext decl_context) = 0;
+ virtual lldb_private::ConstString
+ GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
+
static llvm::Optional
ParseChildArrayInfo(const DWARFDIE &parent_die,
const lldb_private::ExecutionContext *exe_ctx = nullptr);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 905df91ecaeb7..d88ff00296633 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -740,6 +740,39 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext
&sc,
return type_sp;
}
+ConstString
+DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
+ if (llvm::StringRef(die.GetName()).contains("<"))
+return ConstString();
+
+ clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die,
nullptr);
+ TypeSystemClang::TemplateParameterInfos template_param_infos;
+ if (ParseTemplateParameterInfos(die, template_param_infos) &&
+ (!template_param_infos.args.empty() ||
+ template_param_infos.packed_args)) {
+// Most of the parameters here don't matter, but we make sure the base name
+// is empty so when we print the name we only get the template parameters.
+clang::ClassTemplateDecl *class_template_decl =
+m_ast.ParseClassTemplateDecl(decl_ctx, GetOwningClangModule(die),
+ eAccessPublic, "", clang::TTK_Struct,
+ template_param_infos);
+if (!class_template_decl)
+ return ConstString();
+
+clang::ClassTemplateSpecializationDecl *class_specialization_decl =
+m_ast.CreateClassTemplateSpecializationDecl(
+decl_ctx, GetOwningClangModule(die), class_template_decl,
+clang::TTK_Struct, template_param_infos);
+if (!class_specialization_decl)
+ return ConstString();
+CompilerType clang_type =
+m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
+ConstString name = clang_type.GetTypeName(/*BaseOnly*/ true);
+return name;
+ }
+ return ConstString();
+}
+
TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
@@ -1500,41 +1533,6 @@ TypeSP
DWARFASTParserClang::UpdateSymbolContextScopeForType(
return type_sp;
}
-std::string
-DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {
- if (llvm::StringRef(die.GetName()).contains("<"))
-return std::string();
- TypeSystemClang::TemplateParameterInfos template_param_infos;
- if (!ParseTemplateParameterInfos(die, template_param_infos))
-return std::string();
- std::string all_template_names;
- llvm::SmallVector args =
- template_param_infos.args;
- if (template_param_infos.hasParameterPack())
-args.append(template_param_infos.packed_args->args);
- if (args.empt
[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0ac25f1acc4: [lldb] Fix simple template names interaction
with debug info declarations (authored by aeubanks).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138834/new/
https://reviews.llvm.org/D138834
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/test/API/lang/cpp/unique-types3/Makefile
lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
lldb/test/API/lang/cpp/unique-types3/a.cpp
lldb/test/API/lang/cpp/unique-types3/a.h
lldb/test/API/lang/cpp/unique-types3/main.cpp
Index: lldb/test/API/lang/cpp/unique-types3/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+S a1;
+S a2;
+S a3;
+
+void f(S &);
+
+int main() { f(a2); }
Index: lldb/test/API/lang/cpp/unique-types3/a.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.h
@@ -0,0 +1,3 @@
+template struct S {
+ T t;
+};
Index: lldb/test/API/lang/cpp/unique-types3/a.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.cpp
@@ -0,0 +1,5 @@
+#include "a.h"
+
+void f(S &a) {
+ (void)a; // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
@@ -0,0 +1,27 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase3(TestBase):
+def do_test(self, debug_flags):
+"""Test that we display the correct template instantiation."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("a.cpp"))
+self.expect_expr("a", result_type="S")
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+# Can't directly set CFLAGS_EXTRAS here because the Makefile can't
+# override an environment variable.
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/unique-types3/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp a.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2978,6 +2978,15 @@
}
}
+// See comments below about -gsimple-template-names for why we attempt to
+// compute missing template parameter names.
+ConstString template_params;
+if (type_system) {
+ DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
+ if (dwarf_ast)
+template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+}
+
m_index->GetTypes(GetDWARFDeclContext(die), [&](DWARFDIE type_die) {
// Make sure type_die's language matches the type system we are
// looking for. We don't want to find a "Foo" type from Java if we
@@ -3049,6 +3058,27 @@
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
return true;
+ // With -gsimple-template-names, the DIE name may not contain the template
+ // parameters. If the declaration has template parameters but doesn't
+ // contain '<', check that the child template parameters match.
+ if (template_params) {
+llvm::StringRef test_base_name =
+GetTypeForDIE(type_die)->GetBaseName().GetStringRef();
+auto i = test_base_name.find('<');
+
+// Full name from clang AST doesn't contain '<' so this type_die isn't
+// a template parameter, but we're expecting template parameters, so
+// bail.
+if (i == llvm::StringRef::npos)
+ return true;
+
+llvm::StringRef test_template_params =
+test_base_name.slice(i, test_base_name.size());
+
[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params
aeubanks created this revision.
Herald added a reviewer: shafik.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This factors out the check from various callers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139649
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -747,9 +747,7 @@
clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die,
nullptr);
TypeSystemClang::TemplateParameterInfos template_param_infos;
- if (ParseTemplateParameterInfos(die, template_param_infos) &&
- (!template_param_infos.args.empty() ||
- template_param_infos.packed_args)) {
+ if (ParseTemplateParameterInfos(die, template_param_infos)) {
// Most of the parameters here don't matter, but we make sure the base name
// is empty so when we print the name we only get the template parameters.
clang::ClassTemplateDecl *class_template_decl =
@@ -1247,20 +1245,23 @@
attrs.is_inline);
std::free(name_buf);
-if (has_template_params) {
+{
TypeSystemClang::TemplateParameterInfos template_param_infos;
- ParseTemplateParameterInfos(die, template_param_infos);
- template_function_decl = m_ast.CreateFunctionDeclaration(
- ignore_containing_context ? m_ast.GetTranslationUnitDecl()
-: containing_decl_ctx,
- GetOwningClangModule(die), attrs.name.GetStringRef(), clang_type,
- attrs.storage, attrs.is_inline);
- clang::FunctionTemplateDecl *func_template_decl =
- m_ast.CreateFunctionTemplateDecl(
- containing_decl_ctx, GetOwningClangModule(die),
- template_function_decl, template_param_infos);
- m_ast.CreateFunctionTemplateSpecializationInfo(
- template_function_decl, func_template_decl,
template_param_infos);
+ if (has_template_params &&
+ ParseTemplateParameterInfos(die, template_param_infos)) {
+template_function_decl = m_ast.CreateFunctionDeclaration(
+ignore_containing_context ? m_ast.GetTranslationUnitDecl()
+ : containing_decl_ctx,
+GetOwningClangModule(die), attrs.name.GetStringRef(),
+clang_type, attrs.storage, attrs.is_inline);
+clang::FunctionTemplateDecl *func_template_decl =
+m_ast.CreateFunctionTemplateDecl(
+containing_decl_ctx, GetOwningClangModule(die),
+template_function_decl, template_param_infos);
+m_ast.CreateFunctionTemplateSpecializationInfo(
+template_function_decl, func_template_decl,
+template_param_infos);
+ }
}
lldbassert(function_decl);
@@ -1787,9 +1788,7 @@
metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
TypeSystemClang::TemplateParameterInfos template_param_infos;
-if (ParseTemplateParameterInfos(die, template_param_infos) &&
-(!template_param_infos.args.empty() ||
- template_param_infos.packed_args)) {
+if (ParseTemplateParameterInfos(die, template_param_infos)) {
clang::ClassTemplateDecl *class_template_decl =
m_ast.ParseClassTemplateDecl(
decl_ctx, GetOwningClangModule(die), attrs.accessibility,
@@ -2123,7 +2122,10 @@
break;
}
}
- return template_param_infos.args.size() == template_param_infos.names.size();
+ return template_param_infos.args.size() ==
+ template_param_infos.names.size() &&
+ (!template_param_infos.args.empty() ||
+ template_param_infos.packed_args);
}
bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -747,9 +747,7 @@
clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die, nullptr);
TypeSystemClang::TemplateParameterInfos template_param_infos;
- if (ParseTemplateParameterInfos(die, template_param_infos) &&
- (!template_param_infos.args.empty() ||
- template_param_infos.packed_args)) {
+ if (ParseTemplateParameterInfos(die, template_param_infos)) {
// Most of the parameters here don't matter, but we make sure the base name
// is empty so when we print the name we only get the template parameters.
clang::ClassTempl
[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR
compnerd added inline comments.
Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
- llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+ llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
if (!llvm::sys::fs::exists(LibPath)) {
serge-sans-paille wrote:
> Ericson2314 wrote:
> > mgorny wrote:
> > > Well, one thing I immediately notice is that technically
> > > `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations
> > > you are assuming it will always be relative. Not saying that's a blocker
> > > but I think you should add checks for that into `CMakeLists.txt`.
> > Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` with
> > an absolute path.
> >
> > OK if this isn't supported initially but we should ovoid regressing where
> > possible.
> Turns out LLVM_LIBDIR_SUFFIX is used in several situations: (a) for the
> library install dir or (b) for the location where build artifact as stored in
> CMAKE_BINARY_DIR. (a) could accept a path but not (b), unless we derive it
> from (a) but I can see a lot of complication there for the testing step.
> Would that be ok to choke on CMAKE_INSTALL_LIBDIR being a path and not a
> directory component?
>
I think that is absolutely reasonable. There is the
`CMAKE_INSTALL_FULL_LIBDIR` which should be the relatively absolute path (it is
relative to `DESTDIR`). The `CMAKE_INSTALL_LIBDIR` should be the relative
component which is added to `CMAKE_INSTALL_PREFIX`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137337/new/
https://reviews.llvm.org/D137337
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139674: Fix ValueObject::GetAddressOf for subclasses using ValueObjectConstResultImpl backends
jingham created this revision.
jingham added reviewers: Michael137, JDevlieghere, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The original code always set the m_live_address of children of the ValueObjects
that
use ValueObjectConstResultImpl backends to the parent m_live_address +
child_byte_offset.
That is correct for structure types, but wrong for pointer types, since
m_live_address
for a pointer type is the address of the storage for the pointer, not of the
pointee.
Also added a test which was failing before this patch.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139674
Files:
lldb/source/Core/ValueObjectConstResultImpl.cpp
lldb/test/API/lang/c/parray_vrs_char_array/Makefile
lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
lldb/test/API/lang/c/parray_vrs_char_array/main.c
Index: lldb/test/API/lang/c/parray_vrs_char_array/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/main.c
@@ -0,0 +1,15 @@
+struct MyStruct {
+ int before;
+ char var[5];
+ int after;
+};
+
+int
+main()
+{
+ struct MyStruct struct_arr[3] = {{112, "abcd", 221},
+ {313, "efgh", 414},
+ {515, "ijkl", 616}};
+ struct MyStruct *struct_ptr = struct_arr;
+ return struct_ptr->before; // Set a breakpoint here
+}
Index: lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
===
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
@@ -0,0 +1,37 @@
+"""
+Test that parray of a struct with an embedded char array works.
+This was failing because the "live address" of the child elements
+was calculated incorrectly - as a offset from the pointer live
+address. It only happened for char[] children because they used
+GetAddressOf which relies on the live address.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestParrayVrsCharArrayChild(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_parray_struct_with_char_array_child(self):
+"""This is the basic test for does parray get the char values right."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.do_array_test()
+
+def do_array_test(self):
+(target, process, thread, bkpt) =
lldbutil.run_to_source_breakpoint(self,
+ "Set a breakpoint here",
self.main_source_file)
+
+frame = thread.GetFrameAtIndex(0)
+
+self.expect("expr -Z 3 -- struct_ptr",
+substrs = ['before = 112, var = "abcd", after = 221',
+ 'before = 313, var = "efgh", after = 414',
+ 'before = 515, var = "ijkl", after = 616'])
+
+
Index: lldb/test/API/lang/c/parray_vrs_char_array/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Core/ValueObjectConstResultImpl.cpp
===
--- lldb/source/Core/ValueObjectConstResultImpl.cpp
+++ lldb/source/Core/ValueObjectConstResultImpl.cpp
@@ -89,13 +89,20 @@
if (!child_name_str.empty())
child_name.SetCString(child_name_str.c_str());
+lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+// Transfer the live address (with offset) to the child. But if
+// the parent is a pointer, the live address is where that pointer
+// value lives in memory, so the children live addresses aren't
+// offsets from that value, they are just other load addresses that
+// are recorded in the Value of the child ValueObjects.
+if (m_live_address != LLDB_INVALID_ADDRESS) {
+ if (!compiler_type.IsPointerType())
+child_live_addr = m_live_address + child_byte_offset;
+}
valobj = new ValueObjectConstResultChild(
*m_impl_backend, child_compiler_type, child_name, child_byte_size,
child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
-child_is_base_class, child_is_deref_of_parent,
-m_live_address == LLDB_INVALID_ADDRESS
-? m_live_address
-: m_live_address + child_byte_offset,
+child_is_base_class, child_is_deref_of_parent, child_live_addr,
language_flags);
}
Index: lldb/test/API/lang/c/parray_vrs_char_array/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/main.c
@@ -0,0 +1,15 @@
+struct MyStr
[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params
dblaikie added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1251
+ if (has_template_params &&
+ ParseTemplateParameterInfos(die, template_param_infos)) {
+template_function_decl = m_ast.CreateFunctionDeclaration(
This part changes behavior, yeah? (previously the code was only conditional on
`has_template_params` and is now conditional on both that and there actually
being template parameter DIEs) Was that intended? If so, is there any testing
that could be done to demonstrate the change in behavior?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139649/new/
https://reviews.llvm.org/D139649
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params
aeubanks added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1251
+ if (has_template_params &&
+ ParseTemplateParameterInfos(die, template_param_infos)) {
+template_function_decl = m_ast.CreateFunctionDeclaration(
dblaikie wrote:
> This part changes behavior, yeah? (previously the code was only conditional
> on `has_template_params` and is now conditional on both that and there
> actually being template parameter DIEs) Was that intended? If so, is there
> any testing that could be done to demonstrate the change in behavior?
presumably any visible change would only happen with broken debug info
`has_template_params` is true if there are any template parameter child DIEs,
but now we do more checking that those DIEs are actually reasonable
I can revert this back if you want, but this seems better and I don't think I'm
going to find time to add a test for broken debug info which may or may not
actually hit this code path
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139649/new/
https://reviews.llvm.org/D139649
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance
jingham added a comment. You need documentation for what the name & scripted metadata do here somewhere. In SBPlatform.i so that it goes into the SB API docs is one good place. Also maybe in the Platform.h where it gets passed to Create or something. Other than that, LGTM... Comment at: lldb/source/API/SBPlatform.cpp:305 + + if (!dict.IsValid() || !dict.m_impl_up) +return; Do you need to check `script_name != nullptr` here as well as checking the dict? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139249/new/ https://reviews.llvm.org/D139249 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.
clayborg added a comment. @aprantl can you check this latest patch and accept again if you like what you see? All tests passed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137900/new/ https://reviews.llvm.org/D137900 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This makes sense. I think you should at least log if you can't read the memory
dyld told you was where the image info should be. Seems like this is the sort
of thing that shouldn't happen, but if it does we probably want to say
something. Other than that minor grip, LGTM.
Comment at:
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:314
+ process->ReadPointerFromMemory(dyld_image_info, error);
+ if (error.Success()) {
image_load_addresses.push_back(addr);
Is it expected that this memory read fail? It seems weird to just not handle
that case at all? Maybe at least log something here, otherwise it looks like
we just get nothing w/o knowing why.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139453/new/
https://reviews.llvm.org/D139453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 23f145d - [NFC] Fix leak in command options configuration.
Author: Jordan Rupprecht
Date: 2022-12-08T16:37:43-08:00
New Revision: 23f145daa50c3f51a7fb8c8d68c55e5f4a8027c2
URL:
https://github.com/llvm/llvm-project/commit/23f145daa50c3f51a7fb8c8d68c55e5f4a8027c2
DIFF:
https://github.com/llvm/llvm-project/commit/23f145daa50c3f51a7fb8c8d68c55e5f4a8027c2.diff
LOG: [NFC] Fix leak in command options configuration.
`m_options.Append(new OptionPermissions())` leaks because the pointer passed in
is not owned. Use a class member to ensure lifetime, which is the common
pattern used for this API.
Found by the LLDB command interpreter fuzzer. The fuzz input is running `ap $`
twice.
Added:
Modified:
lldb/source/Commands/CommandObjectPlatform.cpp
Removed:
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 98c6a3b2dd301..d72dd06c31f9e 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -456,12 +456,13 @@ class CommandObjectPlatformMkDir : public
CommandObjectParsed {
Options *GetOptions() override {
if (!m_options.DidFinalize()) {
- m_options.Append(new OptionPermissions());
+ m_options.Append(&m_option_permissions);
m_options.Finalize();
}
return &m_options;
}
+ OptionPermissions m_option_permissions;
OptionGroupOptions m_options;
};
@@ -519,12 +520,13 @@ class CommandObjectPlatformFOpen : public
CommandObjectParsed {
Options *GetOptions() override {
if (!m_options.DidFinalize()) {
- m_options.Append(new OptionPermissions());
+ m_options.Append(&m_option_permissions);
m_options.Finalize();
}
return &m_options;
}
+ OptionPermissions m_option_permissions;
OptionGroupOptions m_options;
};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.
aprantl accepted this revision.
aprantl added inline comments.
Comment at: lldb/include/lldb/Symbol/Type.h:101
+ /// Users will need to call one of the SetMatchContext() functions prior to
+ /// doing name lookups.
+ TypeQuery() = delete;
this comment is out of date and should probably just be deleted.
Comment at: lldb/include/lldb/Symbol/Type.h:270
+ bool GetFindOne() const { return (m_options & e_find_one) != 0; }
+ void SetFindOne(bool b) {
+if (b)
Can we just delete this accessor?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137900/new/
https://reviews.llvm.org/D137900
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 28b869d - [NFC] Fix leak handling breakpoint names.
Author: Jordan Rupprecht
Date: 2022-12-08T17:14:38-08:00
New Revision: 28b869d8724207bd7fd8b80f57f6c02abe4bc607
URL:
https://github.com/llvm/llvm-project/commit/28b869d8724207bd7fd8b80f57f6c02abe4bc607
DIFF:
https://github.com/llvm/llvm-project/commit/28b869d8724207bd7fd8b80f57f6c02abe4bc607.diff
LOG: [NFC] Fix leak handling breakpoint names.
The breakpoint list is a list of raw pointers. When breakpoints are removed,
the memory is not deleted. Switch to unique pointers. I did some minor cleanup
while making this change.
Found by the LLDB command interpreter fuzzer. The input is `br m G`.
Added:
Modified:
lldb/include/lldb/Target/Target.h
lldb/source/Target/Target.cpp
Removed:
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 9e016a970cfa6..09fbf45191d4c 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -757,8 +757,7 @@ class Target : public std::enable_shared_from_this,
const BreakpointName::Permissions &permissions);
void ApplyNameToBreakpoints(BreakpointName &bp_name);
- // This takes ownership of the name obj passed in.
- void AddBreakpointName(BreakpointName *bp_name);
+ void AddBreakpointName(std::unique_ptr bp_name);
void GetBreakpointNames(std::vector &names);
@@ -1512,7 +1511,8 @@ class Target : public
std::enable_shared_from_this,
SectionLoadHistory m_section_load_history;
BreakpointList m_breakpoint_list;
BreakpointList m_internal_breakpoint_list;
- using BreakpointNameList = std::map;
+ using BreakpointNameList =
+ std::map>;
BreakpointNameList m_breakpoint_names;
lldb::BreakpointSP m_last_created_breakpoint;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index c4275db2c8a73..11a882e2e62b1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -141,10 +141,8 @@ void Target::PrimeFromDummyTarget(Target &target) {
AddBreakpoint(std::move(new_bp), false);
}
- for (auto bp_name_entry : target.m_breakpoint_names) {
-
-BreakpointName *new_bp_name = new BreakpointName(*bp_name_entry.second);
-AddBreakpointName(new_bp_name);
+ for (const auto &bp_name_entry : target.m_breakpoint_names) {
+AddBreakpointName(std::make_unique(*bp_name_entry.second));
}
m_frame_recognizer_manager_up =
std::make_unique(
@@ -712,8 +710,9 @@ void Target::AddNameToBreakpoint(BreakpointSP &bp_sp, const
char *name,
bp_sp->AddName(name);
}
-void Target::AddBreakpointName(BreakpointName *bp_name) {
- m_breakpoint_names.insert(std::make_pair(bp_name->GetName(), bp_name));
+void Target::AddBreakpointName(std::unique_ptr bp_name) {
+ m_breakpoint_names.insert(
+ std::make_pair(bp_name->GetName(), std::move(bp_name)));
}
BreakpointName *Target::FindBreakpointName(ConstString name, bool can_create,
@@ -723,19 +722,20 @@ BreakpointName *Target::FindBreakpointName(ConstString
name, bool can_create,
return nullptr;
BreakpointNameList::iterator iter = m_breakpoint_names.find(name);
- if (iter == m_breakpoint_names.end()) {
-if (!can_create) {
- error.SetErrorStringWithFormat("Breakpoint name \"%s\" doesn't exist and
"
- "can_create is false.",
- name.AsCString());
- return nullptr;
-}
+ if (iter != m_breakpoint_names.end()) {
+return iter->second.get();
+ }
-iter = m_breakpoint_names
- .insert(std::make_pair(name, new BreakpointName(name)))
- .first;
+ if (!can_create) {
+error.SetErrorStringWithFormat("Breakpoint name \"%s\" doesn't exist and "
+ "can_create is false.",
+ name.AsCString());
+return nullptr;
}
- return (iter->second);
+
+ return m_breakpoint_names
+ .insert(std::make_pair(name, std::make_unique(name)))
+ .first->second.get();
}
void Target::DeleteBreakpointName(ConstString name) {
@@ -778,8 +778,8 @@ void Target::ApplyNameToBreakpoints(BreakpointName
&bp_name) {
void Target::GetBreakpointNames(std::vector &names) {
names.clear();
- for (auto bp_name : m_breakpoint_names) {
-names.push_back(bp_name.first.AsCString());
+ for (const auto& bp_name_entry : m_breakpoint_names) {
+names.push_back(bp_name_entry.first.AsCString());
}
llvm::sort(names);
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D139684: Switch the "command script add" interactive editor to use the exe_ctx interface
jingham created this revision.
jingham added reviewers: mib, JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
We're suggesting people use the form of the command that takes an exe_ctx - it
is both more convenient and more correct - since you should not be using
GetSelected{Target, Process, etc.} in commands.
Since we write the function template, and this is purely additive, anything you
used to enter in this editor mode will still work, you just won't use the new
exe_ctx argument. So this shouldn't affect any workflows.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139684
Files:
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/test/Shell/Commands/command-script-add.test
Index: lldb/test/Shell/Commands/command-script-add.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-script-add.test
@@ -0,0 +1,9 @@
+# Test that command script add with no arguments prompts for
+# and generates the modern (exe_ctx) version of the command.
+
+# RUN: %lldb < %s | FileCheck %s
+command script add doit
+print(exe_ctx.target)
+DONE
+doit
+# CHECK: No value
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1369,7 +1369,7 @@
std::string auto_generated_function_name(GenerateUniqueName(
"lldb_autogen_python_cmd_alias_func", num_created_functions));
- sstr.Printf("def %s (debugger, args, result, internal_dict):",
+ sstr.Printf("def %s (debugger, args, exe_ctx, result, internal_dict):",
auto_generated_function_name.c_str());
if (!GenerateFunction(sstr.GetData(), user_input).Success())
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -201,7 +201,7 @@
static const char *g_python_command_instructions =
"Enter your Python command(s). Type 'DONE' to end.\n"
"You must define a Python function with this signature:\n"
-"def my_command_impl(debugger, args, result, internal_dict):\n";
+"def my_command_impl(debugger, args, exe_ctx, result, internal_dict):\n";
class CommandObjectCommandsAlias : public CommandObjectRaw {
protected:
Index: lldb/test/Shell/Commands/command-script-add.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-script-add.test
@@ -0,0 +1,9 @@
+# Test that command script add with no arguments prompts for
+# and generates the modern (exe_ctx) version of the command.
+
+# RUN: %lldb < %s | FileCheck %s
+command script add doit
+print(exe_ctx.target)
+DONE
+doit
+# CHECK: No value
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1369,7 +1369,7 @@
std::string auto_generated_function_name(GenerateUniqueName(
"lldb_autogen_python_cmd_alias_func", num_created_functions));
- sstr.Printf("def %s (debugger, args, result, internal_dict):",
+ sstr.Printf("def %s (debugger, args, exe_ctx, result, internal_dict):",
auto_generated_function_name.c_str());
if (!GenerateFunction(sstr.GetData(), user_input).Success())
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -201,7 +201,7 @@
static const char *g_python_command_instructions =
"Enter your Python command(s). Type 'DONE' to end.\n"
"You must define a Python function with this signature:\n"
-"def my_command_impl(debugger, args, result, internal_dict):\n";
+"def my_command_impl(debugger, args, exe_ctx, result, internal_dict):\n";
class CommandObjectCommandsAlias : public CommandObjectRaw {
protected:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f9d8090 - Improve error handling for invalid breakpoint `-t` and `-x` options.
Author: Jordan Rupprecht
Date: 2022-12-08T17:53:54-08:00
New Revision: f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030
URL:
https://github.com/llvm/llvm-project/commit/f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030
DIFF:
https://github.com/llvm/llvm-project/commit/f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030.diff
LOG: Improve error handling for invalid breakpoint `-t` and `-x` options.
Breakpoint option `-t` checks that `option_arg` is empty by checking
`option_arg[0] == '\0'`. This is unnecessary: the next two checks for comparing
against "current" and calling `getAsInteger` already gracefully handle an empty
StringRef. If the `option_arg` string is empty, this crashes (or triggers an
assertion failure with asserts enabled). Also, this sets the thread id to
`LLDB_INVALID_THREAD_ID` if the thread id is invalid -- it should just not set
the thread id.
Likewise of `-x` which checks `option_arg[0] == '\n'` unnecessarily.
I believe both of these bugs are unreachable via normal LLDB usage, and are
only accessible via the fuzzer -- most likely some other CLI parsing is
trimming whitespace and rejecting empty inputs. Still, it doesn't hurt to
simplify this bit.
Added:
Modified:
lldb/source/Commands/CommandObjectBreakpoint.cpp
Removed:
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 5d2141d767cb..62e46c59e169 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -110,24 +110,24 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
} break;
case 't': {
lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID;
- if (option_arg[0] != '\0') {
-if (option_arg == "current") {
- if (!execution_context) {
-error.SetErrorStringWithFormat("No context to determine current "
- "thread");
+ if (option_arg == "current") {
+if (!execution_context) {
+ error.SetErrorStringWithFormat("No context to determine current "
+ "thread");
+} else {
+ ThreadSP ctx_thread_sp = execution_context->GetThreadSP();
+ if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) {
+error.SetErrorStringWithFormat("No currently selected thread");
} else {
-ThreadSP ctx_thread_sp = execution_context->GetThreadSP();
-if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) {
- error.SetErrorStringWithFormat("No currently selected thread");
-} else {
- thread_id = ctx_thread_sp->GetID();
-}
+thread_id = ctx_thread_sp->GetID();
}
-} else if (option_arg.getAsInteger(0, thread_id))
- error.SetErrorStringWithFormat("invalid thread id string '%s'",
- option_arg.str().c_str());
+}
+ } else if (option_arg.getAsInteger(0, thread_id)) {
+error.SetErrorStringWithFormat("invalid thread id string '%s'",
+ option_arg.str().c_str());
}
- m_bp_opts.SetThreadID(thread_id);
+ if (thread_id != LLDB_INVALID_THREAD_ID)
+m_bp_opts.SetThreadID(thread_id);
} break;
case 'T':
m_bp_opts.GetThreadSpec()->SetName(option_arg.str().c_str());
@@ -137,12 +137,12 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
break;
case 'x': {
uint32_t thread_index = UINT32_MAX;
- if (option_arg[0] != '\n') {
-if (option_arg.getAsInteger(0, thread_index))
- error.SetErrorStringWithFormat("invalid thread index string '%s'",
- option_arg.str().c_str());
+ if (option_arg.getAsInteger(0, thread_index)) {
+error.SetErrorStringWithFormat("invalid thread index string '%s'",
+ option_arg.str().c_str());
+ } else {
+m_bp_opts.GetThreadSpec()->SetIndex(thread_index);
}
- m_bp_opts.GetThreadSpec()->SetIndex(thread_index);
} break;
default:
llvm_unreachable("Unimplemented option");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
