Re: [Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian
nitesh.jain updated this revision to Diff 72618.
nitesh.jain added a comment.
Herald added a subscriber: ki.stfu.
Updated patch as per suggestion.
https://reviews.llvm.org/D24603
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
source/Plugins/Process/Utility/RegisterInfos_mips.h
source/Plugins/Process/Utility/RegisterInfos_mips64.h
source/Plugins/Process/Utility/lldb-mips-linux-register-enums.h
Index: source/Plugins/Process/Utility/lldb-mips-linux-register-enums.h
===
--- source/Plugins/Process/Utility/lldb-mips-linux-register-enums.h
+++ source/Plugins/Process/Utility/lldb-mips-linux-register-enums.h
@@ -282,6 +282,84 @@
k_num_fpr_registers_mips64 +
k_num_msa_registers_mips64
};
+
+// Register no. for RegisterKind = eRegisterKindProcessPlugin
+// The ptrace request PTRACE_PEEKUSER/PTRACE_POKEUSER used this number
+enum {
+ ptrace_zero_mips,
+ ptrace_r1_mips,
+ ptrace_r2_mips,
+ ptrace_r3_mips,
+ ptrace_r4_mips,
+ ptrace_r5_mips,
+ ptrace_r6_mips,
+ ptrace_r7_mips,
+ ptrace_r8_mips,
+ ptrace_r9_mips,
+ ptrace_r10_mips,
+ ptrace_r11_mips,
+ ptrace_r12_mips,
+ ptrace_r13_mips,
+ ptrace_r14_mips,
+ ptrace_r15_mips,
+ ptrace_r16_mips,
+ ptrace_r17_mips,
+ ptrace_r18_mips,
+ ptrace_r19_mips,
+ ptrace_r20_mips,
+ ptrace_r21_mips,
+ ptrace_r22_mips,
+ ptrace_r23_mips,
+ ptrace_r24_mips,
+ ptrace_r25_mips,
+ ptrace_r26_mips,
+ ptrace_r27_mips,
+ ptrace_gp_mips,
+ ptrace_sp_mips,
+ ptrace_r30_mips,
+ ptrace_ra_mips,
+ ptrace_f0_mips,
+ ptrace_f1_mips,
+ ptrace_f2_mips,
+ ptrace_f3_mips,
+ ptrace_f4_mips,
+ ptrace_f5_mips,
+ ptrace_f6_mips,
+ ptrace_f7_mips,
+ ptrace_f8_mips,
+ ptrace_f9_mips,
+ ptrace_f10_mips,
+ ptrace_f11_mips,
+ ptrace_f12_mips,
+ ptrace_f13_mips,
+ ptrace_f14_mips,
+ ptrace_f15_mips,
+ ptrace_f16_mips,
+ ptrace_f17_mips,
+ ptrace_f18_mips,
+ ptrace_f19_mips,
+ ptrace_f20_mips,
+ ptrace_f21_mips,
+ ptrace_f22_mips,
+ ptrace_f23_mips,
+ ptrace_f24_mips,
+ ptrace_f25_mips,
+ ptrace_f26_mips,
+ ptrace_f27_mips,
+ ptrace_f28_mips,
+ ptrace_f29_mips,
+ ptrace_f30_mips,
+ ptrace_f31_mips,
+ ptrace_pc_mips,
+ ptrace_cause_mips,
+ ptrace_badvaddr_mips,
+ ptrace_mulhi_mips,
+ ptrace_mullo_mips,
+ ptrace_fcsr_mips,
+ ptrace_fir_mips,
+ ptrace_sr_mips,
+ ptrace_config5_mips
+};
}
#endif // #ifndef lldb_mips_linux_register_enums_h
Index: source/Plugins/Process/Utility/RegisterInfos_mips64.h
===
--- source/Plugins/Process/Utility/RegisterInfos_mips64.h
+++ source/Plugins/Process/Utility/RegisterInfos_mips64.h
@@ -42,11 +42,11 @@
// Note that the size and offset will be updated by platform-specific classes.
#ifdef LINUX_MIPS64
-#define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4) \
+#define DEFINE_GPR(reg, alt, kind1, kind2, kind3) \
{\
#reg, alt, sizeof(((GPR_linux_mips *) 0)->reg),\
GPR_OFFSET(reg), eEncodingUint, eFormatHex, \
- {kind1, kind2, kind3, kind4, \
+ {kind1, kind2, kind3, ptrace_##reg##_mips,\
gpr_##reg##_mips64 },\
NULL, NULL, NULL, 0 \
}
@@ -61,11 +61,11 @@
}
#endif
-#define DEFINE_GPR_INFO(reg, alt, kind1, kind2, kind3, kind4) \
+#define DEFINE_GPR_INFO(reg, alt, kind1, kind2, kind3) \
{\
#reg, alt, sizeof(((GPR_linux_mips *) 0)->reg) / 2,\
GPR_OFFSET(reg), eEncodingUint, eFormatHex, \
- {kind1, kind2, kind3, kind4, \
+ {kind1, kind2, kind3, ptrace_##reg##_mips,\
gpr_##reg##_mips64 },\
NULL, NULL, NULL, 0 \
}
@@ -75,21 +75,21 @@
llvm::dwarf::DW_OP_lit26, llvm::dwarf::DW_OP_shl, llvm::dwarf::DW_OP_and,
llvm::dwarf::DW_OP_lit26, llvm::dwarf::DW_OP_shr};
-#define DEFINE_FPR(reg, alt, kind1, kind2, kind3, kind4) \
+#define DEFINE_FPR(reg, alt, kind1, kind2, kind3) \
{\
#reg, alt, sizeof(((FPR_linux_mips *) 0)->reg),\
FPR_OFFSET(reg), e
Re: [Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian
nitesh.jain added a comment. In https://reviews.llvm.org/D24603#548902, @clayborg wrote: > So it seems like this can be fixed by doing a few things: > 1 - just set the RegisterInfo.offset to the actual offset in the buffer and > don't use the offset as the ptrace key used to grab the register > 2 - modify the RegisterInfo.kinds[eRegisterKindProcessPlugin] to be the > offset to you to use for ptrace > > There should be no need what so ever to do manual bit twiddling with src and > dst. The floating point register size can be dynamically changes . Hence following cases are possible when we read registers from ptrace in FPR_linux_mips buffer (In case of MIPS, ptrace always return value in 64 bit container irrespective of Arch). 1. Floating point register size is 32 (SR.FR = 0). a) In case of big endian system, the FPR_linux_mips.f0 will contains two floating point registers . The $f0 register will be store in upper 32 bit and $f1 register in lower 32 bit of FPR_linux_mips.f0. The FPR_linux_mips.f1 will contain don't care value and similarly for all other registers. Thus each even buffer will contain valid value and represent two registers. b) In case of liitle endian , the $f0 will be store in lower 32 bit and $f1 in upper 32bit of FPR_linux_mips.f0. The FPR_linux_mips.f1 will contain don't care value and similarly for all other registers. 2. Floating point register size is 64 (SR.FR = 1). In these case all buffer will have valid value. Hence we need bit twiddling with src and dst (in case of SR.FR = 0) so that each buffer will represent value for single register. https://reviews.llvm.org/D24603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D24610#553331, @omjavaid wrote: > This is a new version of what seems to me fully implementing functionality we > intend to have here. > > On a second thought nuking ClearHardwareWatchpoint function seems to be the > wrong approach here. I spent some time taking different approaches and it > turns out that if we do not ClearHardwareWatchpoint when back-end asks us to > remove it then we wont be able to step over watchpoints. On ARM targets we > have to first clear and then reinstall watchpoints to step over the > watchpoint instruction. > > On the other hand if we call > NativeRegisterContextLinux_arm::ClearHardwareWatchpoint then that watchpoint > stands removed if call is just to delete watch on one of the bytes. And if we > follow up with creating a new watchpoint on a different word the slot being > used may appear vaccant which is actually inconsistent behavior. > > So I have a new approach that does clear watchpoint registers if > NativeRegisterContextLinux_arm::ClearHardwareWatchpoint is called but we > still track reference counts by re-introducing refcount that I removed in my > last patch. This will mean that a follow up create may fail just because > there are still references to disabled watchpoint and watchpoint slots are > still not vaccant. I have made changes to the test to reflect this behaviour. > > Please comment if you have any reservation about this approach. Hmm I am indeed starting to have serious reservations about this. The more I think about this, the more it starts to look like a big hack. So, now ClearHardwareWatchpoint still maintains a refcount on the number of users of the watchpoint slot, but it disables the slot everytime the slot usage count is decremented ? (as opposed to when the refcount reaches zero). And this is supposed to be the reason that step-over watchpoint (a pretty critical piece of functionality) works ? And the reason why we are still able to do the watchpoints is that before a continue (but not before a single-step, because that would break the previous item) we nuke the watchpoint registers (and their reference counts) and start over? I am not convinced that having watchpoint slot sharing is important enough to balance the amount of technical debt this introduces. https://reviews.llvm.org/D24610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
labath added a subscriber: labath. Comment at: include/lldb/Interpreter/Args.h:150 @@ -147,19 +149,3 @@ /// /// @return /// An array of NULL terminate C string argument pointers that I think doxygen will complain about `@return` in a function returning void. BTW, with the NRVO, move semantics and everything, is there any reason to keep using the return-through-reference-argument pattern? It would be a lot more natural if this indeed _was_ a return value. https://reviews.llvm.org/D24952 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
amccarth added a subscriber: amccarth.
Comment at: include/lldb/Interpreter/Args.h:154
@@ -167,3 +153,3 @@
//--
- const char **GetConstArgumentVector() const;
+ void GetArgumentVector(std::vector &args) const;
Perhaps this should actually return the vector instead of void, and rely on the
return value optimization or, at worst, vector move. It would be easier to use
and it would let you initialize variables as you declare them, (which means you
could use it in constructor intializers and you could declare the vector as
const). It also would eliminate the ambiguity of what happens if the `args`
already contains some values (e.g., the caller might expect it to append rather
than replace).
Comment at: source/API/SBCommandInterpreter.cpp:119
@@ +118,3 @@
+std::vector argv;
+command.GetArgumentVector(argv);
+bool ret = m_backend->DoExecute(debugger_sb, &argv[0], sb_return);
Here's an example where, if GetArgumentVector actually returned the result, you
could simplify the delcaration as:
const auto argv = command.GetArgumentVector();
Comment at: source/Host/common/FileSpec.cpp:251
@@ +250,3 @@
+
+ for (auto &name : name_list) {
+matches.AppendString(name);
Should `name` be `const`? As in:
for (const auto &name : name_list) { ... }
Comment at: source/Interpreter/Args.cpp:46
@@ -44,3 +45,3 @@
// strings in rhs.m_args. We need to copy the string list and update our
// own m_argv appropriately.
//--
I think your change eliminated the need for this comment. If not, it at least
needs an update.
Comment at: source/Interpreter/Args.cpp:275
@@ +274,3 @@
+ // new_argv.
+ // So make a copy and then swap it in.
+ std::vector new_args(new_argv.begin(), new_argv.end());
clang-format didn't do a great job wrapping the long lines in this comment.
Can you fix it up?
Comment at: source/Interpreter/Args.cpp:427
@@ -512,1 +426,3 @@
+ GetArgumentVector(args);
+ args.insert(args.begin(), "dummy");
while (1) {
This is an example where the ambiguity of whether GetArgumentVector appends or
not could mislead the caller into doing the wrong this. This looks plausible
(and more efficient):
std::vector args;
args.push_back("dummy");
GetArgumentVector(args);
If GetArgumentVector returned the vector, then this ambiguity wouldn't exist.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner updated this revision to Diff 72657.
zturner added a comment.
Updated
https://reviews.llvm.org/D24952
Files:
examples/plugins/commands/fooplugin.cpp
include/lldb/API/SBCommandInterpreter.h
include/lldb/Host/OptionParser.h
include/lldb/Interpreter/Args.h
source/API/SBCommandInterpreter.cpp
source/API/SBLaunchInfo.cpp
source/API/SBProcess.cpp
source/API/SBTarget.cpp
source/Commands/CommandObjectBreakpoint.cpp
source/Commands/CommandObjectLog.cpp
source/Host/common/OptionParser.cpp
source/Interpreter/Args.cpp
source/Interpreter/CommandInterpreter.cpp
source/Interpreter/CommandObject.cpp
source/Interpreter/OptionValueArgs.cpp
source/Interpreter/OptionValueArray.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
source/Target/ProcessInfo.cpp
source/Target/ProcessLaunchInfo.cpp
Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -339,8 +339,8 @@
if (m_shell) {
std::string shell_executable = m_shell.GetPath();
- const char **argv = GetArguments().GetConstArgumentVector();
- if (argv == nullptr || argv[0] == nullptr)
+ const auto argv = GetArguments().GetArgumentVector();
+ if (!argv[0])
return false;
Args shell_arguments;
std::string safe_arg;
Index: source/Target/ProcessInfo.cpp
===
--- source/Target/ProcessInfo.cpp
+++ source/Target/ProcessInfo.cpp
@@ -91,7 +91,7 @@
void ProcessInfo::SetArguments(char const **argv,
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+ m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
// Is the first argument the executable?
if (first_arg_is_executable) {
Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -421,15 +421,11 @@
}
// Send the environment and the program + arguments after we connect
- const char **envp =
- launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-
- if (envp) {
-const char *env_entry;
-for (int i = 0; (env_entry = envp[i]); ++i) {
- if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
-break;
-}
+ const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
+
+ for (auto env_entry : argv) {
+if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
+ break;
}
ArchSpec arch_spec = launch_info.GetArchitecture();
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1391,6 +1391,7 @@
if (!shell)
return 1;
+ // CLEANUP: This function could be made nicer / safer with StringRef.
std::string shell_string = shell.GetPath();
const char *shell_name = strrchr(shell_string.c_str(), '/');
if (shell_name == NULL)
@@ -1402,13 +1403,10 @@
// /bin/sh re-exec's itself as /bin/bash requiring another resume.
// But it only does this if the COMMAND_MODE environment variable
// is set to "legacy".
-const char **envp =
-launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-if (envp != NULL) {
- for (int i = 0; envp[i] != NULL; i++) {
-if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0)
- return 2;
- }
+const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
+for (auto env_entry : argv) {
+ if (strcmp(env_entry, "COMMAND_MODE=legacy") == 0)
+return 2;
}
return 1;
} else if (strcmp(shell_name, "csh") == 0 ||
Index: source/Interpreter/OptionValueArray.cpp
===
--- source/Interpreter/OptionValueArray.cpp
+++ source/Interpreter/OptionValueArray.cpp
@@ -148,7 +148,7 @@
if (argv.empty())
args.Clear();
else
-args.SetArguments(argv.size(), &argv[0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
}
Index: source/Interpreter/OptionValueArgs.cpp
===
--- source/Interpreter/OptionValueArgs.cpp
+++ source/Interpreter/OptionValueArgs.cpp
@@ -30,6 +30,6 @@
if (argv.empty())
args.Clear();
else
-args.SetArguments(argv.size(), &argv[0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
}
Index: source/Interpreter/CommandObject.cpp
===
--- source/Interpreter/CommandObject.cpp
+++ source/Inter
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
dvlahovski marked 5 inline comments as done.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
+ }
+
zturner wrote:
> dvlahovski wrote:
> > sizeof(uint16_t), sizeof(uint32_t), etc ?
> I think my comments got out of line and this is no longer at the position I
> suggested it. that said, I actually didn't notice all these constants being
> passed into `writeRegister`. I would propose changing this. Find the
> `RegisterInfo` class in `lldb-private-types.h` and add the following methods:
>
> ```
> llvm::ArrayRef data(const uint8_t *context_base) const {
> return llvm::ArrayRef(context_base + byte_offset, byte_size);
> }
>
> llvm::MutableArrayRef mutable_data(uint8_t *context_base) const {
> return llvm::MutableArrayRef(context_base + byte_offset,
> byte_size);
> }
> ```
>
> Then you can write:
>
> ```writeRegister(source_data,
> reg_info[lldb_cs_x86_64].mutable_data(result_base));```
This will make the code very clean indeed, but there is a problem.
From the `RegisterInfo` I only get the `byte_offset` and I don't use the
`byte_size` because some of the registers in the Minidump context are smaller
(e.g. cs is 16 bit). All of the registers in the Linux context are 64 bits in
size. So that's why I basically hard-coded the size in each of the funciton
calls. Now with the next revision I tried to make the code clearer.
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:38
@@ +37,3 @@
+// - uint64_t p4_home
+// - uint64_t p5_home
+// - uint64_t p6_home
So I had the actual struct for the register context in the previous revision,
but now I've removed it, because I actually don't use it. So added a comment
describing it's layout.
For the endianness part - because this code only permutes the bytes in the
layout the Linux register context expects them, I don't think that here is the
place to handle that. I was hoping that the Linux register context will handle
it, but I don't think that's the case when I look at the code in
`RegisterContextCorePOSIX_x86_64::ReadRegister`.
Am I right - do the Linux context assume little endian? If so maybe I can also
fix that.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+static void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+ ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}
amccarth wrote:
> +1 to Zach's idea.
>
> Also, consider whether `EXPECT_EQ` is more appropriate than `ASSERT_EQ` for
> these.
Probably `EXPECT_EQ` is better because if there is a mismatch in the reg
values, one can see all of the values.
I still don't have the sense when to use `EXPECT_EQ` and when `ASSERT_EQ`.
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
dvlahovski updated this revision to Diff 72659.
dvlahovski added a comment.
Addressing Zachary's and Adrian's comments
https://reviews.llvm.org/D24919
Files:
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -8,16 +8,19 @@
//===--===//
// Project includes
+#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
#include "Plugins/Process/minidump/MinidumpParser.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
// Other libraries and framework includes
#include "gtest/gtest.h"
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataExtractor.h"
#include "lldb/Host/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -50,7 +53,7 @@
MinidumpParser::Create(data_sp);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
-ASSERT_GT(parser->GetByteSize(), 0UL);
+ASSERT_GT(parser->GetData().size(), 0UL);
}
llvm::SmallString<128> inputs_folder;
@@ -167,3 +170,61 @@
ASSERT_TRUE(pid.hasValue());
ASSERT_EQ(4440UL, pid.getValue());
}
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::ArrayRef thread_list = parser->GetThreads();
+ const MinidumpThread thread = thread_list[0];
+ llvm::ArrayRef registers(parser->GetData().data() +
+thread.thread_context.rva,
+thread.thread_context.data_size);
+
+ ArchSpec arch = parser->GetArchitecture();
+ RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+ lldb::DataBufferSP buf =
+ ConvertMinidumpContextToRegIface(registers, reg_interface);
+ ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+ const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+ std::map reg_values;
+
+ // clang-format off
+ reg_values[lldb_rax_x86_64]= 0x;
+ reg_values[lldb_rbx_x86_64]= 0x;
+ reg_values[lldb_rcx_x86_64]= 0x0010;
+ reg_values[lldb_rdx_x86_64]= 0x;
+ reg_values[lldb_rdi_x86_64]= 0x7ffceb349cf0;
+ reg_values[lldb_rsi_x86_64]= 0x;
+ reg_values[lldb_rbp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_rsp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_r8_x86_64] = 0x7fe9bc1aa9c0;
+ reg_values[lldb_r9_x86_64] = 0x;
+ reg_values[lldb_r10_x86_64]= 0x7fe9bc3f16a0;
+ reg_values[lldb_r11_x86_64]= 0x0246;
+ reg_values[lldb_r12_x86_64]= 0x00401c92;
+ reg_values[lldb_r13_x86_64]= 0x7ffceb34a430;
+ reg_values[lldb_r14_x86_64]= 0x;
+ reg_values[lldb_r15_x86_64]= 0x;
+ reg_values[lldb_rip_x86_64]= 0x00401dc6;
+ reg_values[lldb_rflags_x86_64] = 0x00010206;
+ reg_values[lldb_cs_x86_64] = 0x0033;
+ reg_values[lldb_fs_x86_64] = 0x;
+ reg_values[lldb_gs_x86_64] = 0x;
+ reg_values[lldb_ss_x86_64] = 0x;
+ reg_values[lldb_ds_x86_64] = 0x;
+ reg_values[lldb_es_x86_64] = 0x;
+ // clang-format on
+
+ for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+ ++reg_index) {
+if (reg_values.find(reg_index) != reg_values.end()) {
+ EXPECT_EQ(reg_values[reg_index],
+REG_VAL(buf->GetBytes() + reg_info[reg_index].byte_offset));
+}
+ }
+}
Index: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
===
--- /dev/null
+++ source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -0,0 +1,119 @@
+//===-- RegisterContextMinidump_x86_64.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
zturner added a comment. `ASSERT_EQ` causes the test method to return immediately if the condition fails. `EXPECT_EQ` will continue running the same test body. I like to use `ASSERT_EQ`, for example, if you are checking the value of a pointer returned by a function. First you want to check if it's null, then you want to check that its value is the value you expect. So I would do ASSERT_NE(p, nullptr); EXPECT_EQ(*p, 42); Because if it's null, the rest of the statements in the test body are undefined. Another example is if a method returns some value, say number of items in a list, and then you want to call another method to process that many items. Say: int x = foo.GetItemCount(); const Item *items = foo.GetItems(); bool Success = ProcessItems(items, x); If the number of items is not what you expect, it doesn't make sense to run the next statement. So you could write: int x = foo.GetItemCount(); ASSERT_EQ(42, x); const Item *items = foo.GetItems(); EXPECT_TRUE(ProcessItems(items, x)); https://reviews.llvm.org/D24919 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13350: [lldb] Fix evaluation of qualified global variables
evgeny777 abandoned this revision. evgeny777 added a comment. This patch is longer valid after more than 9 months on review due to changes in namespace handing in ClangExpressionDeclMap. It probably should be done in a different way, but I don't have time for this now. https://reviews.llvm.org/D13350 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
dvlahovski added a comment. Thanks for the explanation and examples! :) Will keep this in mind in the future. https://reviews.llvm.org/D24919 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
amccarth added inline comments.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
`EXPECT_xxx` will check the condition and report if it's wrong. But then the
test proceeds.
`ASSERT_xxx` will check the condition, and, if it's wrong, it will report _and_
terminate the current test.
So my rule of thumb is:
1. Prefer `EXPECT_xxx` when checking that the code does what it's supposed to
do.
2. Use `ASSERT_xxx` when the rest of the test depends on this condition.
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r282496 - xfail TestExec.py on macOS
Author: tfiala Date: Tue Sep 27 10:57:12 2016 New Revision: 282496 URL: http://llvm.org/viewvc/llvm-project?rev=282496&view=rev Log: xfail TestExec.py on macOS Tracked by: rdar://28476369 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py?rev=282496&r1=282495&r2=282496&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py Tue Sep 27 10:57:12 2016 @@ -27,6 +27,7 @@ class ExecTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) @skipUnlessDarwin +@expectedFailureAll(oslist=["macosx"], bugnumber="rdar://28476369") def test(self): if self.getArchitecture() == 'x86_64': source = os.path.join(os.getcwd(), "main.cpp") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
dvlahovski added inline comments.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
amccarth wrote:
> `EXPECT_xxx` will check the condition and report if it's wrong. But then the
> test proceeds.
>
> `ASSERT_xxx` will check the condition, and, if it's wrong, it will report
> _and_ terminate the current test.
>
> So my rule of thumb is:
>
> 1. Prefer `EXPECT_xxx` when checking that the code does what it's supposed
> to do.
> 2. Use `ASSERT_xxx` when the rest of the test depends on this condition.
Thanks :)
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
zturner accepted this revision.
This revision is now accepted and ready to land.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:50
@@ +49,3 @@
+size = 8;
+break;
+ }
If they are just smaller (but never bigger), you can call `take_front(N)` on
the result. So you could say:
```
writeRegister(source_data,
reg_info[lldb_cs_x86_64].mutable_data(result_base).take_front(2));
```
Now the line is starting to get long so might look ugly if it wraps, so it's up
to you. It is somewhat safer at least since you will assert if you
accidentally specify an invalid size, and you don't have to worry about getting
the computation right.
Perhaps you could combine this with the approach you've already taken here.
Add the functions to `RegisterInfo`, then change `getDestRegister` to look like
this:
```
llvm::MutableArrayRef getDestRegister(const RegisterInfo ®,
uint8_t *context, uint32_t
lldb_reg_num) {
auto bytes = reg.mutable_data(context);
switch (lldb_reg_num) {
case lldb_cs_x86_64:
case lldb_ds_x86_64:
case lldb_es_x86_64:
case lldb_fs_x86_64:
case lldb_gs_x86_64:
case lldb_ss_x86_64:
return bytes.take_front(2);
break;
case lldb_rflags_x86_64:
return bytes.take_front(4);
break;
default:
return bytes.take_front(8);
}
```
then you could call `writeRegister` like this:
```
writeRegister(source_data, getDestRegister(reg_info, result_base,
lldb_cs_x86_64));
```
At this point though I'm not sure what's better. So don't consider this a
requirement, up to you :)
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comments.
Comment at: source/Interpreter/Args.cpp:264-265
@@ -315,3 +263,4 @@
size_t Args::GetArgumentCount() const {
- if (m_argv.empty())
+ if (m_args.empty())
return 0;
+ return m_args.size();
Remove this if, it is no longer needed. Just return m_args.size()
Comment at: source/Interpreter/Args.cpp:269-272
@@ -320,6 +268,6 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
Convert this over to return StringRef?
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
Remove this comment. The whole point off this function is to get an argument
vector that is NULL terminated. Feel free to rename the function as desired to
indicate this, but that is the whole point of this function. Feel free to
document it if needed as well.
Comment at: source/Interpreter/Args.cpp:327
@@ -381,11 +326,3 @@
char quote_char) {
- // Since we are using a std::list to hold onto the copied C string and
- // we don't have direct access to the elements, we have to iterate to
- // find the value.
- arg_sstr_collection::iterator pos, end = m_args.end();
- size_t i = idx;
- for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
---i;
-
- pos = m_args.insert(pos, arg_str);
+ m_args.insert(m_args.begin() + idx, arg_str);
Is there a conversion operator that converts StringRef to std::string
somewhere? How is this working? Iterators being detected?
Comment at: source/Interpreter/Args.cpp:373-376
@@ -460,21 +372,6 @@
-void Args::SetArguments(const char **argv) {
- // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
- // no need to clear it here.
- m_args.clear();
- m_args_quote_char.clear();
-
- if (argv) {
-// First copy each string
-for (size_t i = 0; argv[i]; ++i) {
- m_args.push_back(argv[i]);
- if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
-m_args_quote_char.push_back(argv[i][0]);
- else
-m_args_quote_char.push_back('\0');
-}
- }
-
- UpdateArgvFromArgs();
+void Args::SetArguments(const Args &other) {
+ m_args = other.m_args;
+ m_args_quote_char = other.m_args_quote_char;
}
Should we change this to an assignment operator? If so, rename "other" to "rhs".
Comment at: source/Interpreter/CommandInterpreter.cpp:2049-2050
@@ -2048,5 +2048,4 @@
cmd_args.Clear();
-cmd_args.SetArguments(new_args.GetArgumentCount(),
- new_args.GetConstArgumentVector());
+cmd_args.SetArguments(new_args);
} else {
If we do the operator = above, then:
```
cmd_args = new_args;
```
Comment at: source/Interpreter/CommandInterpreter.cpp:2059-2060
@@ -2059,5 +2058,4 @@
if (wants_raw_input) {
cmd_args.Clear();
- cmd_args.SetArguments(new_args.GetArgumentCount(),
-new_args.GetConstArgumentVector());
+ cmd_args.SetArguments(new_args);
}
If we do the operator = above, then:
```
cmd_args = new_args;
```
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
Why was this removed?
Comment at: source/Interpreter/OptionValueArgs.cpp:33
@@ -32,3 +32,3 @@
else
-args.SetArguments(argv.size(), &argv[0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
args = argv;
Comment at: source/Interpreter/OptionValueArray.cpp:151
@@ -150,3 +150,3 @@
else
-args.SetArguments(argv.size(), &argv[0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
args = argv;
Comment at: source/Target/ProcessInfo.cpp:94
@@ -93,3 +93,3 @@
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+ m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
What not have a SetArguments that takes a "const char **" and does this
internally? It is no less safe and makes the API more usable
https://r
Re: [Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian
clayborg added a comment. Please comment why you are manually bit twiddling due to the size of the register that can change. We don't want anyone else copying this kind of code. Another solution would be to update the offset of the register when you change the byte size so none of this would need to be done. We are already updating the byte size of the register in the RegisterInfo, so why not do the same for the offset. You could store the original offset in the RegisterInfo.kinds[eRegisterKindProcessPlugin] as this field is only for the process plug-in's use. https://reviews.llvm.org/D24603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r282508 - convert TestFatArchives.py over to no-debug-info test
Author: tfiala
Date: Tue Sep 27 12:17:21 2016
New Revision: 282508
URL: http://llvm.org/viewvc/llvm-project?rev=282508&view=rev
Log:
convert TestFatArchives.py over to no-debug-info test
We only use the .o-style debug info here regardless, so having
it run all three debuginfo styles was a waste.
This also strips out the custom build function and uses the
TestBase.build() method.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/fat_archives/TestFatArchives.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/fat_archives/TestFatArchives.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/fat_archives/TestFatArchives.py?rev=282508&r1=282507&r2=282508&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/fat_archives/TestFatArchives.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/fat_archives/TestFatArchives.py
Tue Sep 27 12:17:21 2016
@@ -13,23 +13,16 @@ from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
-def execute_command(command):
-# print('%% %s' % (command))
-(exit_status, output) = seven.get_command_status_output(command)
-# if output:
-# print(output)
-# print('status = %u' % (exit_status))
-return exit_status
-
-
class FatArchiveTestCase(TestBase):
mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
@skipUnlessDarwin
-def test(self):
+def test_breakpoint_resolution_dwarf(self):
if self.getArchitecture() == 'x86_64':
-execute_command("make CC='%s'" % (os.environ["CC"]))
+self.build()
self.main()
else:
self.skipTest(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:269-272
@@ -320,6 +268,6 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
clayborg wrote:
> Convert this over to return StringRef?
Yes, that is the next item on my list. But this function is used EVERYWHERE,
so that will be a very large CL and should be done orthogonally.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
clayborg wrote:
> Remove this comment. The whole point off this function is to get an argument
> vector that is NULL terminated. Feel free to rename the function as desired
> to indicate this, but that is the whole point of this function. Feel free to
> document it if needed as well.
There are two ways we use these argument vectors. One is in a call to a
function which takes an argc and an argv. In that case the null terminator is
unnecessary because you're specifying the argc explicitly. Another is in a
call to a function which you do not specify an argc, and in tht case it
requires it to be null terminated.
The point of this function isn't "return me something that's null terminated",
it's "return me something I can use like an argv". That doesn't require null
termination. And in fact, if you look at the callsites I fixed up, not a
single one depends on the null termination. Everyone just needs to know the
size. And for that you can call `result.size()`. If you add the null
terminator, then you have to write `result.size()-1`, which is unnecessarily
confusing. I believe it provides the most logical behavior as it is currently
written.
Comment at: source/Interpreter/Args.cpp:327
@@ -381,11 +326,3 @@
char quote_char) {
- // Since we are using a std::list to hold onto the copied C string and
- // we don't have direct access to the elements, we have to iterate to
- // find the value.
- arg_sstr_collection::iterator pos, end = m_args.end();
- size_t i = idx;
- for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
---i;
-
- pos = m_args.insert(pos, arg_str);
+ m_args.insert(m_args.begin() + idx, arg_str);
clayborg wrote:
> Is there a conversion operator that converts StringRef to std::string
> somewhere? How is this working? Iterators being detected?
Yes, StringRef has an `operator std::string()`.
Comment at: source/Interpreter/Args.cpp:373-376
@@ -460,21 +372,6 @@
-void Args::SetArguments(const char **argv) {
- // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
- // no need to clear it here.
- m_args.clear();
- m_args_quote_char.clear();
-
- if (argv) {
-// First copy each string
-for (size_t i = 0; argv[i]; ++i) {
- m_args.push_back(argv[i]);
- if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
-m_args_quote_char.push_back(argv[i][0]);
- else
-m_args_quote_char.push_back('\0');
-}
- }
-
- UpdateArgvFromArgs();
+void Args::SetArguments(const Args &other) {
+ m_args = other.m_args;
+ m_args_quote_char = other.m_args_quote_char;
}
clayborg wrote:
> Should we change this to an assignment operator? If so, rename "other" to
> "rhs".
I thought about it, but it seems like a minor stylistic issue. I think it
could go either way. I'm fine adding it or not.
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
clayborg wrote:
> Why was this removed?
Because it's a little weird to do it at this high of a level. This requires
internal knowledge of the workings of `Args::ParseOptions()`, which itself
calls `OptionParser::Parse()`, It's strange for a function this high up to
require such low-level knowledge of the workings of a function. So instead, I
moved this behavior into `Args::ParseOptions`. If you check that function, you
will see that it injects this argument into the beginning.
This is nice because now it never modifies the internal state of the `Args`
itself, but only the copy that gets passed to `getopt_long_only`.
Comment at: source/Target/ProcessInfo.cpp:94
@@ -93,3 +93,3 @@
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
clayborg added a comment.
Ok, so just add the boolean arg to Args::GetArgumentVector and we are good to
go.
Comment at: source/Interpreter/Args.cpp:270-272
@@ -321,5 +269,5 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
Ok, sounds good, for a later CL then.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
zturner wrote:
> clayborg wrote:
> > Remove this comment. The whole point off this function is to get an
> > argument vector that is NULL terminated. Feel free to rename the function
> > as desired to indicate this, but that is the whole point of this function.
> > Feel free to document it if needed as well.
> There are two ways we use these argument vectors. One is in a call to a
> function which takes an argc and an argv. In that case the null terminator
> is unnecessary because you're specifying the argc explicitly. Another is in
> a call to a function which you do not specify an argc, and in tht case it
> requires it to be null terminated.
>
> The point of this function isn't "return me something that's null
> terminated", it's "return me something I can use like an argv". That doesn't
> require null termination. And in fact, if you look at the callsites I fixed
> up, not a single one depends on the null termination. Everyone just needs to
> know the size. And for that you can call `result.size()`. If you add the
> null terminator, then you have to write `result.size()-1`, which is
> unnecessarily confusing. I believe it provides the most logical behavior as
> it is currently written.
Remove the comment. Even in "main(int argc, const char **argv)" the "argv" is
null terminated. So all unix variants expect the null termination. The correct
solution is to add a boolean argument:
```
std::vector Args::GetArgumentVector(bool null_terminate = true)
const {
```
Then it is clear.
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
zturner wrote:
> clayborg wrote:
> > Why was this removed?
> Because it's a little weird to do it at this high of a level. This requires
> internal knowledge of the workings of `Args::ParseOptions()`, which itself
> calls `OptionParser::Parse()`, It's strange for a function this high up to
> require such low-level knowledge of the workings of a function. So instead,
> I moved this behavior into `Args::ParseOptions`. If you check that function,
> you will see that it injects this argument into the beginning.
>
> This is nice because now it never modifies the internal state of the `Args`
> itself, but only the copy that gets passed to `getopt_long_only`.
Sounds good. I hate the fact that we have to do this. We might be able to play
with the getopt_long globals to work around this and not have to add it, but as
long as you took it into account I am fine.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
clayborg wrote:
> zturner wrote:
> > clayborg wrote:
> > > Remove this comment. The whole point off this function is to get an
> > > argument vector that is NULL terminated. Feel free to rename the function
> > > as desired to indicate this, but that is the whole point of this
> > > function. Feel free to document it if needed as well.
> > There are two ways we use these argument vectors. One is in a call to a
> > function which takes an argc and an argv. In that case the null terminator
> > is unnecessary because you're specifying the argc explicitly. Another is
> > in a call to a function which you do not specify an argc, and in tht case
> > it requires it to be null terminated.
> >
> > The point of this function isn't "return me something that's null
> > terminated", it's "return me something I can use like an argv". That
> > doesn't require null termination. And in fact, if you look at the
> > callsites I fixed up, not a single one depends on the null termination.
> > Everyone just needs to know the size. And for that you can call
> > `result.size()`. If you add the null terminator, then you have to write
> > `result.size()-1`, which is unnecessarily confusing. I believe it provides
> > the most logical behavior as it is currently written.
> Remove the comment. Even in "main(int argc, const char **argv)" the "argv" is
> null terminated. So all unix variants expect the null termination. The
> correct solution is to add a boolean argument:
>
> ```
> std::vector Args::GetArgumentVector(bool null_terminate = true)
> const {
> ```
>
> Then it is clear.
Even in main though, the null is not counted in the argc, right? So as written
it will conform to the semantics of main. `argc` is the number of non-null
pointers, but the null is still there anyway. If you add the boolean argument
and pass true, it will give you back something whose `argc` (i.e.
`result.size()`) is one too high.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
zturner wrote:
> clayborg wrote:
> > zturner wrote:
> > > clayborg wrote:
> > > > Remove this comment. The whole point off this function is to get an
> > > > argument vector that is NULL terminated. Feel free to rename the
> > > > function as desired to indicate this, but that is the whole point of
> > > > this function. Feel free to document it if needed as well.
> > > There are two ways we use these argument vectors. One is in a call to a
> > > function which takes an argc and an argv. In that case the null
> > > terminator is unnecessary because you're specifying the argc explicitly.
> > > Another is in a call to a function which you do not specify an argc, and
> > > in tht case it requires it to be null terminated.
> > >
> > > The point of this function isn't "return me something that's null
> > > terminated", it's "return me something I can use like an argv". That
> > > doesn't require null termination. And in fact, if you look at the
> > > callsites I fixed up, not a single one depends on the null termination.
> > > Everyone just needs to know the size. And for that you can call
> > > `result.size()`. If you add the null terminator, then you have to write
> > > `result.size()-1`, which is unnecessarily confusing. I believe it
> > > provides the most logical behavior as it is currently written.
> > Remove the comment. Even in "main(int argc, const char **argv)" the "argv"
> > is null terminated. So all unix variants expect the null termination. The
> > correct solution is to add a boolean argument:
> >
> > ```
> > std::vector Args::GetArgumentVector(bool null_terminate =
> > true) const {
> > ```
> >
> > Then it is clear.
> Even in main though, the null is not counted in the argc, right? So as
> written it will conform to the semantics of main. `argc` is the number of
> non-null pointers, but the null is still there anyway. If you add the
> boolean argument and pass true, it will give you back something whose `argc`
> (i.e. `result.size()`) is one too high.
BTW, `llvm::SmallString` uses this same trick to return a null terminated
string without modifying the size. Look at the implementation of
`SmallString::c_str()` in `SmallString.h`
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24960: Remove unreachable from apis in posix terminal compat windows and return 0
carlokok created this revision.
carlokok added a subscriber: LLDB.
carlokok set the repository for this revision to rL LLVM.
carlokok added a project: LLDB.
These apis are called by lldb process launching but the unreachable causes them
to fail on Windows. However, they don't have to do anything because Windows has
implied support for terminals.
Repository:
rL LLVM
https://reviews.llvm.org/D24960
Files:
/lldb/trunk/include/lldb/Host/windows/PosixApi.h
Index: /lldb/trunk/include/lldb/Host/windows/PosixApi.h
===
--- /lldb/trunk/include/lldb/Host/windows/PosixApi.h
+++ /lldb/trunk/include/lldb/Host/windows/PosixApi.h
@@ -98,19 +98,19 @@
int strncasecmp(const char *s1, const char *s2, size_t n);
// empty functions
-inline int posix_openpt(int flag) { LLVM_BUILTIN_UNREACHABLE; }
+inline int posix_openpt(int flag) { return 0; }
inline int strerror_r(int errnum, char *buf, size_t buflen) {
- LLVM_BUILTIN_UNREACHABLE;
+ return 0;
}
-inline int unlockpt(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline int grantpt(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline char *ptsname(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline pid_t fork(void) { LLVM_BUILTIN_UNREACHABLE; }
-inline pid_t setsid(void) { LLVM_BUILTIN_UNREACHABLE; }
+inline int unlockpt(int fd) { return 0; }
+inline int grantpt(int fd) { return 0; }
+inline char *ptsname(int fd) { return 0; }
+inline pid_t fork(void) { return 0; }
+inline pid_t setsid(void) { return 0; }
// vsnprintf and snprintf are provided in MSVC 2015 and higher.
#if _MSC_VER < 1900
namespace lldb_private {
Index: /lldb/trunk/include/lldb/Host/windows/PosixApi.h
===
--- /lldb/trunk/include/lldb/Host/windows/PosixApi.h
+++ /lldb/trunk/include/lldb/Host/windows/PosixApi.h
@@ -98,19 +98,19 @@
int strncasecmp(const char *s1, const char *s2, size_t n);
// empty functions
-inline int posix_openpt(int flag) { LLVM_BUILTIN_UNREACHABLE; }
+inline int posix_openpt(int flag) { return 0; }
inline int strerror_r(int errnum, char *buf, size_t buflen) {
- LLVM_BUILTIN_UNREACHABLE;
+ return 0;
}
-inline int unlockpt(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline int grantpt(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline char *ptsname(int fd) { LLVM_BUILTIN_UNREACHABLE; }
-inline pid_t fork(void) { LLVM_BUILTIN_UNREACHABLE; }
-inline pid_t setsid(void) { LLVM_BUILTIN_UNREACHABLE; }
+inline int unlockpt(int fd) { return 0; }
+inline int grantpt(int fd) { return 0; }
+inline char *ptsname(int fd) { return 0; }
+inline pid_t fork(void) { return 0; }
+inline pid_t setsid(void) { return 0; }
// vsnprintf and snprintf are provided in MSVC 2015 and higher.
#if _MSC_VER < 1900
namespace lldb_private {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
dvlahovski updated this revision to Diff 72688.
dvlahovski added a comment.
I like the combined approach more. So this is the implementation
https://reviews.llvm.org/D24919
Files:
include/lldb/lldb-private-types.h
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -8,16 +8,19 @@
//===--===//
// Project includes
+#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
#include "Plugins/Process/minidump/MinidumpParser.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
// Other libraries and framework includes
#include "gtest/gtest.h"
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataExtractor.h"
#include "lldb/Host/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -50,7 +53,7 @@
MinidumpParser::Create(data_sp);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
-ASSERT_GT(parser->GetByteSize(), 0UL);
+ASSERT_GT(parser->GetData().size(), 0UL);
}
llvm::SmallString<128> inputs_folder;
@@ -167,3 +170,61 @@
ASSERT_TRUE(pid.hasValue());
ASSERT_EQ(4440UL, pid.getValue());
}
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::ArrayRef thread_list = parser->GetThreads();
+ const MinidumpThread thread = thread_list[0];
+ llvm::ArrayRef registers(parser->GetData().data() +
+thread.thread_context.rva,
+thread.thread_context.data_size);
+
+ ArchSpec arch = parser->GetArchitecture();
+ RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+ lldb::DataBufferSP buf =
+ ConvertMinidumpContextToRegIface(registers, reg_interface);
+ ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+ const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+ std::map reg_values;
+
+ // clang-format off
+ reg_values[lldb_rax_x86_64]= 0x;
+ reg_values[lldb_rbx_x86_64]= 0x;
+ reg_values[lldb_rcx_x86_64]= 0x0010;
+ reg_values[lldb_rdx_x86_64]= 0x;
+ reg_values[lldb_rdi_x86_64]= 0x7ffceb349cf0;
+ reg_values[lldb_rsi_x86_64]= 0x;
+ reg_values[lldb_rbp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_rsp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_r8_x86_64] = 0x7fe9bc1aa9c0;
+ reg_values[lldb_r9_x86_64] = 0x;
+ reg_values[lldb_r10_x86_64]= 0x7fe9bc3f16a0;
+ reg_values[lldb_r11_x86_64]= 0x0246;
+ reg_values[lldb_r12_x86_64]= 0x00401c92;
+ reg_values[lldb_r13_x86_64]= 0x7ffceb34a430;
+ reg_values[lldb_r14_x86_64]= 0x;
+ reg_values[lldb_r15_x86_64]= 0x;
+ reg_values[lldb_rip_x86_64]= 0x00401dc6;
+ reg_values[lldb_rflags_x86_64] = 0x00010206;
+ reg_values[lldb_cs_x86_64] = 0x0033;
+ reg_values[lldb_fs_x86_64] = 0x;
+ reg_values[lldb_gs_x86_64] = 0x;
+ reg_values[lldb_ss_x86_64] = 0x;
+ reg_values[lldb_ds_x86_64] = 0x;
+ reg_values[lldb_es_x86_64] = 0x;
+ // clang-format on
+
+ for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+ ++reg_index) {
+if (reg_values.find(reg_index) != reg_values.end()) {
+ EXPECT_EQ(reg_values[reg_index],
+REG_VAL(buf->GetBytes() + reg_info[reg_index].byte_offset));
+}
+ }
+}
Index: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
===
--- /dev/null
+++ source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -0,0 +1,119 @@
+//===-- RegisterContextMinidump_x86_64.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
[Lldb-commits] [lldb] r282529 - Adding a RegisterContextMinidump_x86_64 converter
Author: dvlahovski
Date: Tue Sep 27 14:05:55 2016
New Revision: 282529
URL: http://llvm.org/viewvc/llvm-project?rev=282529&view=rev
Log:
Adding a RegisterContextMinidump_x86_64 converter
Summary:
This is a register context converter from Minidump to Linux reg context.
This knows the layout of the register context in the Minidump file
(which is the same as in Windows FYI) and as a result emits a binary data
buffer that matches the Linux register context binary layout.
This way we can reuse the existing RegisterContextLinux_x86_64 and
RegisterContextCorePOSIX_x86_64 classes.
Reviewers: labath, zturner
Subscribers: beanz, mgorny, lldb-commits, amccarth
Differential Revision: https://reviews.llvm.org/D24919
Added:
lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
Modified:
lldb/trunk/include/lldb/lldb-private-types.h
lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Modified: lldb/trunk/include/lldb/lldb-private-types.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-private-types.h?rev=282529&r1=282528&r2=282529&view=diff
==
--- lldb/trunk/include/lldb/lldb-private-types.h (original)
+++ lldb/trunk/include/lldb/lldb-private-types.h Tue Sep 27 14:05:55 2016
@@ -14,6 +14,8 @@
#include "lldb/lldb-private.h"
+#include "llvm/ADT/ArrayRef.h"
+
namespace llvm {
namespace sys {
class DynamicLibrary;
@@ -61,6 +63,15 @@ struct RegisterInfo {
// the byte size of this register.
size_t dynamic_size_dwarf_len; // The length of the DWARF expression in bytes
// in the dynamic_size_dwarf_expr_bytes
member.
+
+ llvm::ArrayRef data(const uint8_t *context_base) const {
+return llvm::ArrayRef(context_base + byte_offset, byte_size);
+ }
+
+ llvm::MutableArrayRef mutable_data(uint8_t *context_base) const {
+return llvm::MutableArrayRef(context_base + byte_offset,
+ byte_size);
+ }
};
//--
Modified: lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt?rev=282529&r1=282528&r2=282529&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt Tue Sep 27
14:05:55 2016
@@ -3,4 +3,5 @@ include_directories(../Utility)
add_lldb_library(lldbPluginProcessMinidump
MinidumpTypes.cpp
MinidumpParser.cpp
+ RegisterContextMinidump_x86_64.cpp
)
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=282529&r1=282528&r2=282529&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Tue Sep 27
14:05:55 2016
@@ -64,8 +64,9 @@ MinidumpParser::MinidumpParser(
: m_data_sp(data_buf_sp), m_header(header), m_directory_map(directory_map)
{
}
-lldb::offset_t MinidumpParser::GetByteSize() {
- return m_data_sp->GetByteSize();
+llvm::ArrayRef MinidumpParser::GetData() {
+ return llvm::ArrayRef(m_data_sp->GetBytes(),
+ m_data_sp->GetByteSize());
}
llvm::ArrayRef
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=282529&r1=282528&r2=282529&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Tue Sep 27
14:05:55 2016
@@ -39,7 +39,7 @@ public:
static llvm::Optional
Create(const lldb::DataBufferSP &data_buf_sp);
- lldb::offset_t GetByteSize();
+ llvm::ArrayRef GetData();
llvm::ArrayRef GetStream(MinidumpStreamType stream_type);
@@ -71,6 +71,6 @@ private:
llvm::DenseMap &&directory_map);
};
-} // namespace minidump
-} // namespace lldb_private
+} // end namespace minidump
+} // end namespace lldb_private
#endif // liblldb_MinidumpParser_h_
Added:
lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x
Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282529: Adding a RegisterContextMinidump_x86_64 converter
(authored by dvlahovski).
Changed prior to commit:
https://reviews.llvm.org/D24919?vs=72688&id=72689#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24919
Files:
lldb/trunk/include/lldb/lldb-private-types.h
lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
lldb/trunk/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -8,16 +8,19 @@
//===--===//
// Project includes
+#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
#include "Plugins/Process/minidump/MinidumpParser.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
// Other libraries and framework includes
#include "gtest/gtest.h"
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataExtractor.h"
#include "lldb/Host/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -50,7 +53,7 @@
MinidumpParser::Create(data_sp);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
-ASSERT_GT(parser->GetByteSize(), 0UL);
+ASSERT_GT(parser->GetData().size(), 0UL);
}
llvm::SmallString<128> inputs_folder;
@@ -167,3 +170,61 @@
ASSERT_TRUE(pid.hasValue());
ASSERT_EQ(4440UL, pid.getValue());
}
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::ArrayRef thread_list = parser->GetThreads();
+ const MinidumpThread thread = thread_list[0];
+ llvm::ArrayRef registers(parser->GetData().data() +
+thread.thread_context.rva,
+thread.thread_context.data_size);
+
+ ArchSpec arch = parser->GetArchitecture();
+ RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+ lldb::DataBufferSP buf =
+ ConvertMinidumpContextToRegIface(registers, reg_interface);
+ ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+ const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+ std::map reg_values;
+
+ // clang-format off
+ reg_values[lldb_rax_x86_64]= 0x;
+ reg_values[lldb_rbx_x86_64]= 0x;
+ reg_values[lldb_rcx_x86_64]= 0x0010;
+ reg_values[lldb_rdx_x86_64]= 0x;
+ reg_values[lldb_rdi_x86_64]= 0x7ffceb349cf0;
+ reg_values[lldb_rsi_x86_64]= 0x;
+ reg_values[lldb_rbp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_rsp_x86_64]= 0x7ffceb34a210;
+ reg_values[lldb_r8_x86_64] = 0x7fe9bc1aa9c0;
+ reg_values[lldb_r9_x86_64] = 0x;
+ reg_values[lldb_r10_x86_64]= 0x7fe9bc3f16a0;
+ reg_values[lldb_r11_x86_64]= 0x0246;
+ reg_values[lldb_r12_x86_64]= 0x00401c92;
+ reg_values[lldb_r13_x86_64]= 0x7ffceb34a430;
+ reg_values[lldb_r14_x86_64]= 0x;
+ reg_values[lldb_r15_x86_64]= 0x;
+ reg_values[lldb_rip_x86_64]= 0x00401dc6;
+ reg_values[lldb_rflags_x86_64] = 0x00010206;
+ reg_values[lldb_cs_x86_64] = 0x0033;
+ reg_values[lldb_fs_x86_64] = 0x;
+ reg_values[lldb_gs_x86_64] = 0x;
+ reg_values[lldb_ss_x86_64] = 0x;
+ reg_values[lldb_ds_x86_64] = 0x;
+ reg_values[lldb_es_x86_64] = 0x;
+ // clang-format on
+
+ for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+ ++reg_index) {
+if (reg_values.find(reg_index) != reg_values.end()) {
+ EXPECT_EQ(reg_values[reg_index],
+REG_VAL(buf->GetBytes() + reg_info[reg_index].byte_offset));
+}
+ }
+}
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpPars
[Lldb-commits] [lldb] r282537 - Update FileSpec's interface to use StringRefs.
Author: zturner
Date: Tue Sep 27 15:48:37 2016
New Revision: 282537
URL: http://llvm.org/viewvc/llvm-project?rev=282537&view=rev
Log:
Update FileSpec's interface to use StringRefs.
Differential Revision: https://reviews.llvm.org/D24936
Modified:
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/source/Host/common/FileSpec.cpp
Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=282537&r1=282536&r2=282537&view=diff
==
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Tue Sep 27 15:48:37 2016
@@ -80,16 +80,10 @@ public:
///
/// @see FileSpec::SetFile (const char *path, bool resolve)
//--
- // TODO: Convert this constructor to use a StringRef.
- explicit FileSpec(const char *path, bool resolve_path,
+ explicit FileSpec(llvm::StringRef path, bool resolve_path,
PathSyntax syntax = ePathSyntaxHostNative);
- explicit FileSpec(const char *path, bool resolve_path, ArchSpec arch);
-
- explicit FileSpec(const std::string &path, bool resolve_path,
-PathSyntax syntax = ePathSyntaxHostNative);
-
- explicit FileSpec(const std::string &path, bool resolve_path, ArchSpec arch);
+ explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch);
//--
/// Copy constructor
@@ -657,15 +651,10 @@ public:
/// If \b true, then we will try to resolve links the path using
/// the static FileSpec::Resolve.
//--
- void SetFile(const char *path, bool resolve_path,
- PathSyntax syntax = ePathSyntaxHostNative);
-
- void SetFile(const char *path, bool resolve_path, ArchSpec arch);
-
- void SetFile(const std::string &path, bool resolve_path,
+ void SetFile(llvm::StringRef path, bool resolve_path,
PathSyntax syntax = ePathSyntaxHostNative);
- void SetFile(const std::string &path, bool resolve_path, ArchSpec arch);
+ void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch);
bool IsResolved() const { return m_is_resolved; }
@@ -709,20 +698,13 @@ public:
//--
static void Resolve(llvm::SmallVectorImpl &path);
- FileSpec CopyByAppendingPathComponent(const char *new_path) const;
-
+ FileSpec CopyByAppendingPathComponent(llvm::StringRef component) const;
FileSpec CopyByRemovingLastPathComponent() const;
- void PrependPathComponent(const char *new_path);
-
- void PrependPathComponent(const std::string &new_path);
-
+ void PrependPathComponent(llvm::StringRef component);
void PrependPathComponent(const FileSpec &new_path);
- void AppendPathComponent(const char *new_path);
-
- void AppendPathComponent(const std::string &new_path);
-
+ void AppendPathComponent(llvm::StringRef component);
void AppendPathComponent(const FileSpec &new_path);
void RemoveLastPathComponent();
@@ -746,7 +728,7 @@ public:
//--
static void ResolveUsername(llvm::SmallVectorImpl &path);
- static size_t ResolvePartialUsername(const char *partial_name,
+ static size_t ResolvePartialUsername(llvm::StringRef partial_name,
StringList &matches);
enum EnumerateDirectoryResult {
@@ -763,7 +745,7 @@ public:
void *baton, FileType file_type, const FileSpec &spec);
static EnumerateDirectoryResult
- EnumerateDirectory(const char *dir_path, bool find_directories,
+ EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
bool find_files, bool find_other,
EnumerateDirectoryCallbackType callback,
void *callback_baton);
@@ -773,7 +755,7 @@ public:
DirectoryCallback;
static EnumerateDirectoryResult
- ForEachItemInDirectory(const char *dir_path,
+ ForEachItemInDirectory(llvm::StringRef dir_path,
DirectoryCallback const &callback);
protected:
Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=282537&r1=282536&r2=282537&view=diff
==
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Tue Sep 27 15:48:37 2016
@@ -37,6 +37,7 @@
#include "lldb/Host/Host.h"
#include "lldb/Utility/CleanUp.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/FileSystem.h"
@@ -58,7 +59,7 @@ const char *GetPathSeparators(FileSp
Re: [Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282537: Update FileSpec's interface to use StringRefs.
(authored by zturner).
Changed prior to commit:
https://reviews.llvm.org/D24936?vs=72555&id=72705#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24936
Files:
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/source/Host/common/FileSpec.cpp
Index: lldb/trunk/include/lldb/Host/FileSpec.h
===
--- lldb/trunk/include/lldb/Host/FileSpec.h
+++ lldb/trunk/include/lldb/Host/FileSpec.h
@@ -80,16 +80,10 @@
///
/// @see FileSpec::SetFile (const char *path, bool resolve)
//--
- // TODO: Convert this constructor to use a StringRef.
- explicit FileSpec(const char *path, bool resolve_path,
+ explicit FileSpec(llvm::StringRef path, bool resolve_path,
PathSyntax syntax = ePathSyntaxHostNative);
- explicit FileSpec(const char *path, bool resolve_path, ArchSpec arch);
-
- explicit FileSpec(const std::string &path, bool resolve_path,
-PathSyntax syntax = ePathSyntaxHostNative);
-
- explicit FileSpec(const std::string &path, bool resolve_path, ArchSpec arch);
+ explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch);
//--
/// Copy constructor
@@ -657,15 +651,10 @@
/// If \b true, then we will try to resolve links the path using
/// the static FileSpec::Resolve.
//--
- void SetFile(const char *path, bool resolve_path,
- PathSyntax syntax = ePathSyntaxHostNative);
-
- void SetFile(const char *path, bool resolve_path, ArchSpec arch);
-
- void SetFile(const std::string &path, bool resolve_path,
+ void SetFile(llvm::StringRef path, bool resolve_path,
PathSyntax syntax = ePathSyntaxHostNative);
- void SetFile(const std::string &path, bool resolve_path, ArchSpec arch);
+ void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch);
bool IsResolved() const { return m_is_resolved; }
@@ -709,20 +698,13 @@
//--
static void Resolve(llvm::SmallVectorImpl &path);
- FileSpec CopyByAppendingPathComponent(const char *new_path) const;
-
+ FileSpec CopyByAppendingPathComponent(llvm::StringRef component) const;
FileSpec CopyByRemovingLastPathComponent() const;
- void PrependPathComponent(const char *new_path);
-
- void PrependPathComponent(const std::string &new_path);
-
+ void PrependPathComponent(llvm::StringRef component);
void PrependPathComponent(const FileSpec &new_path);
- void AppendPathComponent(const char *new_path);
-
- void AppendPathComponent(const std::string &new_path);
-
+ void AppendPathComponent(llvm::StringRef component);
void AppendPathComponent(const FileSpec &new_path);
void RemoveLastPathComponent();
@@ -746,7 +728,7 @@
//--
static void ResolveUsername(llvm::SmallVectorImpl &path);
- static size_t ResolvePartialUsername(const char *partial_name,
+ static size_t ResolvePartialUsername(llvm::StringRef partial_name,
StringList &matches);
enum EnumerateDirectoryResult {
@@ -763,7 +745,7 @@
void *baton, FileType file_type, const FileSpec &spec);
static EnumerateDirectoryResult
- EnumerateDirectory(const char *dir_path, bool find_directories,
+ EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
bool find_files, bool find_other,
EnumerateDirectoryCallbackType callback,
void *callback_baton);
@@ -773,7 +755,7 @@
DirectoryCallback;
static EnumerateDirectoryResult
- ForEachItemInDirectory(const char *dir_path,
+ ForEachItemInDirectory(llvm::StringRef dir_path,
DirectoryCallback const &callback);
protected:
Index: lldb/trunk/source/Host/common/FileSpec.cpp
===
--- lldb/trunk/source/Host/common/FileSpec.cpp
+++ lldb/trunk/source/Host/common/FileSpec.cpp
@@ -37,6 +37,7 @@
#include "lldb/Host/Host.h"
#include "lldb/Utility/CleanUp.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/FileSystem.h"
@@ -58,7 +59,7 @@
return PathSyntaxIsPosix(syntax) ? "/" : "\\/";
}
-char GetPrefferedPathSeparator(FileSpec::PathSyntax syntax) {
+char GetPreferredPathSeparator(FileSpec::PathSyntax syntax) {
return GetPathSeparators(syntax)[0];
}
@@ -228,27 +229,27 @@
#endif
}
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername
[Lldb-commits] [PATCH] D24988: Improvements to testing blacklist
fjricci created this revision.
fjricci added reviewers: zturner, labath, tfiala, jingham.
fjricci added subscribers: sas, lldb-commits.
This patch is necessary because individual test cases are not required
to have unique names. Therefore, test cases must now
be specified explicitly in the form ..
Because it works by regex matching, passing just will
still disable an entire file.
This also allows for multiple exclusion files to be specified.
https://reviews.llvm.org/D24988
Files:
packages/Python/lldbsuite/test/configuration.py
packages/Python/lldbsuite/test/dotest.py
packages/Python/lldbsuite/test/dotest_args.py
packages/Python/lldbsuite/test/test_result.py
Index: packages/Python/lldbsuite/test/test_result.py
===
--- packages/Python/lldbsuite/test/test_result.py
+++ packages/Python/lldbsuite/test/test_result.py
@@ -139,8 +139,8 @@
self.getCategoriesForTest(test)):
self.hardMarkAsSkipped(test)
if self.checkExclusion(
-configuration.skip_methods,
-test._testMethodName):
+configuration.skip_tests,
+strclass(test.__class__) + '.' + test._testMethodName):
self.hardMarkAsSkipped(test)
configuration.setCrashInfoHook(
@@ -161,11 +161,8 @@
def addSuccess(self, test):
if self.checkExclusion(
-configuration.xfail_files,
-strclass(
-test.__class__)) or self.checkExclusion(
-configuration.xfail_methods,
-test._testMethodName):
+configuration.xfail_tests,
+strclass(test.__class__) + '.' + test._testMethodName):
self.addUnexpectedSuccess(test, None)
return
@@ -239,11 +236,8 @@
def addFailure(self, test, err):
if self.checkExclusion(
-configuration.xfail_files,
-strclass(
-test.__class__)) or self.checkExclusion(
-configuration.xfail_methods,
-test._testMethodName):
+configuration.xfail_tests,
+strclass(test.__class__) + '.' + test._testMethodName):
self.addExpectedFailure(test, err, None)
return
Index: packages/Python/lldbsuite/test/dotest_args.py
===
--- packages/Python/lldbsuite/test/dotest_args.py
+++ packages/Python/lldbsuite/test/dotest_args.py
@@ -96,7 +96,7 @@
'-p',
metavar='pattern',
help='Specify a regexp filename pattern for inclusion in the test suite')
-group.add_argument('--excluded', metavar='exclusion-file', help=textwrap.dedent(
+group.add_argument('--excluded', metavar='exclusion-file', action='append', help=textwrap.dedent(
'''Specify a file for tests to exclude. File should contain lists of regular expressions for test files or methods,
with each list under a matching header (xfail files, xfail methods, skip files, skip methods)'''))
group.add_argument(
Index: packages/Python/lldbsuite/test/dotest.py
===
--- packages/Python/lldbsuite/test/dotest.py
+++ packages/Python/lldbsuite/test/dotest.py
@@ -216,33 +216,24 @@
"""
excl_type = None
-case_type = None
with open(exclusion_file) as f:
for line in f:
+line = line.strip()
if not excl_type:
-[excl_type, case_type] = line.split()
+excl_type = line
continue
-line = line.strip()
if not line:
excl_type = None
-elif excl_type == 'skip' and case_type == 'files':
-if not configuration.skip_files:
-configuration.skip_files = []
-configuration.skip_files.append(line)
-elif excl_type == 'skip' and case_type == 'methods':
-if not configuration.skip_methods:
-configuration.skip_methods = []
-configuration.skip_methods.append(line)
-elif excl_type == 'xfail' and case_type == 'files':
-if not configuration.xfail_files:
-configuration.xfail_files = []
-configuration.xfail_files.append(line)
-elif excl_type == 'xfail' and case_type == 'methods':
-if not configuration.xfail_methods:
-configuration.xfail_methods = []
-configuration.xfail_methods.append(line)
+elif excl_type == 'skip':
+if not configuration.skip_tests:
+configuration.skip_tests = []
+configuration.skip_tests.append(line)
+elif excl_type == 'xfail':
+if not configuration.xfail_tests:
+
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid updated this revision to Diff 72723.
omjavaid added a comment.
Give this approach a rethink I dont see a lot of problems with this final
implementation unless it fails on other architectures.
We are already hacking our way to have these byte selection watchpoints working
in existing code. New code seems to be improving the hack in my opinion.
Let me explain what I am doing and may be you can provide your suggestion and
feedback.
Watchpoint Install => Register watchpoint into hardware watchpoint register
cache
Watchpoint Enable => Enable in cache and write registers using ptrace interface.
Ideally we should be able to install/uninstall watchpoints in cache and then
enable them all on a resume.
In case of arm If a watchpoint is hit we should be able to disable that
watchpoint.
Step over watchpoint instruction and then re-enable the watchpoint.
Our existing implementation will require a lot of changes to do that so thats
why here is what i am doing.
SetHardwareWatchpoint => Performs Install and Enable
- If a new watchpoint slot is going to be used we Install and enable.
- For new watchpoint we should be able to complete both Install and or we
report an error.
- If a duplicate slot is going to be used we Install and enable if required.
- Install means updating size if needed plus updating ref count.
- Enable means updating registers if size was updated.
ClearHardwareWatchpoint
- Disable and uinstall watchpoint means
- Decrement ref count and clear hardware watchpoint regsiters.
- Advantage of keeping ref counts is:
- If refcount is greater than zero then SetHardwareWatchpoint cannot use this
slot for a new watchpoint (new address).
- But SetHardwareWatchpoint can be use this slot to install duplicate
watchpoints (same address but different byte or word)
ClearAllHardwareWatchpoint -- Just clear the whole watchpoint cache and call
SetHardwareWatchpoint for all available watchpoints.
NativeThreadLinux:
On Watchpoint Remove -> Invalidate watchpoint cache
On Resume - > Re-validate watchpoints by creating a new cache and re-enabling
all watchpoints
So this fixes our step-over issue and also preserves watchpoint slot if it is
being used by multiple watchpoints.
Can you think of any scenarios which might fail for this approach?
https://reviews.llvm.org/D24610
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeThreadLinux.cpp
source/Plugins/Process/Linux/NativeThreadLinux.h
Index: source/Plugins/Process/Linux/NativeThreadLinux.h
===
--- source/Plugins/Process/Linux/NativeThreadLinux.h
+++ source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -108,6 +108,7 @@
using WatchpointIndexMap = std::map;
WatchpointIndexMap m_watchpoint_index_map;
cpu_set_t m_original_cpu_set; // For single-step workaround.
+ bool m_invalidate_watchpoints;
};
typedef std::shared_ptr NativeThreadLinuxSP;
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -87,7 +87,8 @@
NativeThreadLinux::NativeThreadLinux(NativeProcessLinux *process,
lldb::tid_t tid)
: NativeThreadProtocol(process, tid), m_state(StateType::eStateInvalid),
- m_stop_info(), m_reg_context_sp(), m_stop_description() {}
+ m_stop_info(), m_reg_context_sp(), m_stop_description(),
+ m_invalidate_watchpoints(false) {}
std::string NativeThreadLinux::GetName() {
NativeProcessProtocolSP process_sp = m_process_wp.lock();
@@ -185,8 +186,10 @@
return Error();
uint32_t wp_index = wp->second;
m_watchpoint_index_map.erase(wp);
- if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index))
+ if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index)) {
+m_invalidate_watchpoints = true;
return Error();
+ }
return Error("Clearing hardware watchpoint failed.");
}
@@ -198,8 +201,13 @@
m_stop_info.reason = StopReason::eStopReasonNone;
m_stop_description.clear();
- // If watchpoints have been set, but none on this thread,
- // then this is a new thread. So set all existing watchpoints.
+ // Invalidate watchpoint index map for a re-sync
+ if (m_invalidate_watchpoints) {
+m_invalidate_watchpoints = false;
+m_watchpoint_index_map.clear();
+ }
+
+ // Re-sync all available watchpoints.
if (m_watchpoint_index_map.empty()) {
NativeProcessLinux &process = GetProcess();
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_ar
Re: [Lldb-commits] [PATCH] D10216: Initial diff for FreeBSD kernel debugging support
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Looks like patch was not committed. Need to be rebased and format with Clang-format. https://reviews.llvm.org/D10216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D10167: Fix TestJoinAfterBreak test on Windows
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko closed this revision. Eugene.Zelenko added a comment. Committed in https://reviews.llvm.org/rL238787. https://reviews.llvm.org/D10167 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Project] LLDB
penryu added a watcher: penryu. PROJECT DETAIL https://reviews.llvm.org/project/profile/39/ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Project] LLDB
penryu added a member: penryu. PROJECT DETAIL https://reviews.llvm.org/project/profile/39/ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] Use explicit delete in DISALLOW_COPY_AND_ASSIGN
Explicit delete is preferable to declaring a method private when your intention is to prevent that method from ever being used. * better compiler error messages * can't be bypassed by friendship * clearer intent This was discussed here: http://lists.llvm.org/pipermail/lldb-dev/2016-September/011394.html diff --git a/include/lldb/lldb-defines.h b/include/lldb/lldb-defines.h index a8e6776..a1318f4 100644 --- a/include/lldb/lldb-defines.h +++ b/include/lldb/lldb-defines.h @@ -157,8 +157,8 @@ /// assignment operators in C++ classes. //-- #define DISALLOW_COPY_AND_ASSIGN(TypeName) \ - TypeName(const TypeName &); \ - const TypeName &operator=(const TypeName &) + TypeName(const TypeName &) = delete; \ + const TypeName &operator=(const TypeName &) = delete #endif // #if defined(__cplusplus) signature.asc Description: OpenPGP digital signature ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] Use explicit delete in DISALLOW_COPY_AND_ASSIGN
Not sure why operator= is returning a const&. It doesn't matter in practice but it's a bit strange. Anyway lgtm On Tue, Sep 27, 2016 at 7:11 PM Daniel Austin Noland via lldb-commits < lldb-commits@lists.llvm.org> wrote: > Explicit delete is preferable to declaring a method private when > your intention is to prevent that method from ever being used. > > * better compiler error messages > * can't be bypassed by friendship > * clearer intent > > This was discussed here: > http://lists.llvm.org/pipermail/lldb-dev/2016-September/011394.html > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23977: Support of lldb on Kfreebsd
sylvestre.ledru updated this revision to Diff 72758.
sylvestre.ledru added a comment.
Herald added subscribers: mgorny, beanz.
Updated by Pino!
https://reviews.llvm.org/D23977
Files:
cmake/LLDBDependencies.cmake
cmake/modules/LLDBConfig.cmake
scripts/Python/modules/CMakeLists.txt
scripts/utilsOsType.py
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -410,3 +410,5 @@
list(APPEND system_libs ${CURSES_LIBRARIES})
include_directories(${CURSES_INCLUDE_DIR})
endif ()
+
+find_package(Backtrace REQUIRED)
Index: scripts/Python/modules/CMakeLists.txt
===
--- scripts/Python/modules/CMakeLists.txt
+++ scripts/Python/modules/CMakeLists.txt
@@ -5,7 +5,7 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-macro-redefined")
endif ()
-# build the Python readline suppression module only on Linux
-if (CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT __ANDROID_NDK__)
+# build the Python readline suppression module only on Linux or GNU systems
+if ((CMAKE_SYSTEM_NAME MATCHES "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "GNU" OR
CMAKE_SYSTEM_NAME STREQUAL "kFreeBSD") AND NOT __ANDROID_NDK__)
add_subdirectory(readline)
endif()
Index: scripts/utilsOsType.py
===
--- scripts/utilsOsType.py
+++ scripts/utilsOsType.py
@@ -35,6 +35,7 @@
Linux = 3
NetBSD = 4
Windows = 5
+kFreeBSD = 6
else:
class EnumOsType(object):
values = ["Unknown",
@@ -42,7 +43,8 @@
"FreeBSD",
"Linux",
"NetBSD",
- "Windows"]
+ "Windows",
+ "kFreeBSD"]
class __metaclass__(type):
#++---
# Details: Fn acts as an enumeration.
@@ -86,5 +88,7 @@
eOSType = EnumOsType.NetBSD
elif strOS == "win32":
eOSType = EnumOsType.Windows
+elif strOS.startswith("gnukfreebsd"):
+eOSType = EnumOsType.kFreeBSD
return eOSType
Index: cmake/LLDBDependencies.cmake
===
--- cmake/LLDBDependencies.cmake
+++ cmake/LLDBDependencies.cmake
@@ -152,10 +152,7 @@
endif()
endif()
endif()
-# On FreeBSD/NetBSD backtrace() is provided by libexecinfo, not libc.
-if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- list(APPEND LLDB_SYSTEM_LIBS execinfo)
-endif()
+list(APPEND LLDB_SYSTEM_LIBS ${Backtrace_LIBRARY})
if (NOT LLDB_DISABLE_PYTHON AND NOT LLVM_BUILD_STATIC)
list(APPEND LLDB_SYSTEM_LIBS ${PYTHON_LIBRARIES})
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -410,3 +410,5 @@
list(APPEND system_libs ${CURSES_LIBRARIES})
include_directories(${CURSES_INCLUDE_DIR})
endif ()
+
+find_package(Backtrace REQUIRED)
Index: scripts/Python/modules/CMakeLists.txt
===
--- scripts/Python/modules/CMakeLists.txt
+++ scripts/Python/modules/CMakeLists.txt
@@ -5,7 +5,7 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-macro-redefined")
endif ()
-# build the Python readline suppression module only on Linux
-if (CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT __ANDROID_NDK__)
+# build the Python readline suppression module only on Linux or GNU systems
+if ((CMAKE_SYSTEM_NAME MATCHES "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "GNU" OR CMAKE_SYSTEM_NAME STREQUAL "kFreeBSD") AND NOT __ANDROID_NDK__)
add_subdirectory(readline)
endif()
Index: scripts/utilsOsType.py
===
--- scripts/utilsOsType.py
+++ scripts/utilsOsType.py
@@ -35,6 +35,7 @@
Linux = 3
NetBSD = 4
Windows = 5
+kFreeBSD = 6
else:
class EnumOsType(object):
values = ["Unknown",
@@ -42,7 +43,8 @@
"FreeBSD",
"Linux",
"NetBSD",
- "Windows"]
+ "Windows",
+ "kFreeBSD"]
class __metaclass__(type):
#++---
# Details: Fn acts as an enumeration.
@@ -86,5 +88,7 @@
eOSType = EnumOsType.NetBSD
elif strOS == "win32":
eOSType = EnumOsType.Windows
+elif strOS.startswith("gnukfreebsd"):
+eOSType = EnumOsType.kFreeBSD
return eOSType
Index: cmake/LLDBDependencies.cmake
===
--- cmake/LLDBDependencies.cmake
+++ cmake/LLDBDependencies.cmake
@@ -152,10 +152,7 @@
endif()
endif()
endif()
-# On FreeBSD/NetBSD backtrace() is pr
