Re: [Lldb-commits] [PATCH] D24251: LLDB: API for Permission of object file's sections
abhishek.aggarwal closed this revision. abhishek.aggarwal added a comment. Already merged it in LLDb repo. So closing this revision. https://reviews.llvm.org/D24251 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
labath added subscribers: zturner, lldb-commits, clayborg. labath added a comment. I am cc'ing a bunch of people to make sure we don't do this behind anyone's back. I don't know whether you discussed this with anyone on lldb side (it's certainly new to me), but I think it deserves a wider audience, as this is the first case (I can recall) of moving code from lldb back to llvm. That said, I think it's a great idea. However, you should sync up with @zturner, as he has imminent plans to do a StringExtractor refactor (https://reviews.llvm.org/D24013) within lldb and you're making a copy of it here. Some other high-level questions (not directly related to this change, but I'd like to know where you're going with this): - despite the renaming, the code still has a distinct lldb feel to it. I presume this will be followed by a bunch of CL's to bring it more in line with the rest of llvm (?) - you are copying StringExtractor to llvm as well. Will you switch lldb to use that copy as well, or is the plan to bypass it completely and use llvm parsing primitives for the job? - were you able to get lldb test suite running? If not, I can help you get that working. - mostly out of curiosity: what is the motivation for this? I guess you have a need for a JSON parser in llvm ? cheers. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281025 - Fix new gdb-remote client unit test for windows
Author: labath
Date: Fri Sep 9 04:49:54 2016
New Revision: 281025
URL: http://llvm.org/viewvc/llvm-project?rev=281025&view=rev
Log:
Fix new gdb-remote client unit test for windows
The behaviour of FileSpec differed between host OS versions. Hardcode the path
syntax to posix, as we don't care about that in this test.
Modified:
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Modified:
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp?rev=281025&r1=281024&r2=281025&view=diff
==
---
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
(original)
+++
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Fri Sep 9 04:49:54 2016
@@ -196,8 +196,9 @@ TEST_F(GDBRemoteCommunicationClientTest,
llvm::Triple triple("i386-pc-linux");
- FileSpec file_specs[] = {FileSpec("/foo/bar.so", false),
- FileSpec("/foo/baz.so", false)};
+ FileSpec file_specs[] = {
+ FileSpec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix),
+ FileSpec("/foo/baz.so", false, FileSpec::ePathSyntaxPosix)};
std::future>> async_result =
std::async(std::launch::async,
[&] { return client.GetModulesInfo(file_specs, triple); });
@@ -226,7 +227,7 @@ TEST_F(GDBRemoteCommunicationClientTest,
return;
llvm::Triple triple("i386-pc-linux");
- FileSpec file_spec("/foo/bar.so", false);
+ FileSpec file_spec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix);
const char *invalid_responses[] = {
"OK", "E47", "[]",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281026 - [LLDB][MIPS] Fix TestLldbGdbServer.py failure
Author: nitesh.jain Date: Fri Sep 9 04:50:33 2016 New Revision: 281026 URL: http://llvm.org/viewvc/llvm-project?rev=281026&view=rev Log: [LLDB][MIPS] Fix TestLldbGdbServer.py failure Subscribers: jaydeep, bhushan, slthakur, lldb-commits Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py?rev=281026&r1=281025&r2=281026&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py Fri Sep 9 04:50:33 2016 @@ -724,7 +724,9 @@ class GdbRemoteTestCaseBase(TestBase): "dwarf", "generic", "container-regs", -"invalidate-regs" +"invalidate-regs", +"dynamic_size_dwarf_expr_bytes", +"dynamic_size_dwarf_len" ] def assert_valid_reg_info(self, reg_info): ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281029 - NFC: Reformat ABISysV_i386 register context into something readable
Author: labath
Date: Fri Sep 9 05:12:57 2016
New Revision: 281029
URL: http://llvm.org/viewvc/llvm-project?rev=281029&view=rev
Log:
NFC: Reformat ABISysV_i386 register context into something readable
add clang-format guards so it does not reformat it again.
Modified:
lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=281029&r1=281028&r2=281029&view=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Fri Sep 9
05:12:57 2016
@@ -113,674 +113,66 @@ enum dwarf_regnums {
};
static RegisterInfo g_register_infos[] = {
-// NAME ALT SZ OFF ENCODING FORMAT
-// EH_FRAME DWARF GENERIC
-// PROCESS PLUGIN LLDB NATIVEVALUE REGSINVALIDATE
-// REGS
-// ===== == === =
-// = =
-//
-// =======
-{"eax",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_eax, dwarf_eax, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"ebx",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_ebx, dwarf_ebx, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"ecx",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_ecx, dwarf_ecx, LLDB_REGNUM_GENERIC_ARG4, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"edx",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_edx, dwarf_edx, LLDB_REGNUM_GENERIC_ARG3, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"esi",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_esi, dwarf_esi, LLDB_REGNUM_GENERIC_ARG2, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"edi",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_edi, dwarf_edi, LLDB_REGNUM_GENERIC_ARG1, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"ebp",
- "fp",
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_ebp, dwarf_ebp, LLDB_REGNUM_GENERIC_FP, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"esp",
- "sp",
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_esp, dwarf_esp, LLDB_REGNUM_GENERIC_SP, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"eip",
- "pc",
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {dwarf_eip, dwarf_eip, LLDB_REGNUM_GENERIC_PC, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"eflags",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_REGNUM_GENERIC_FLAGS,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"cs",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"ss",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"ds",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"es",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
- nullptr,
- nullptr,
- nullptr,
- 0},
-{"fs",
- nullptr,
- 4,
- 0,
- eEncodingUint,
- eFormatHex,
- {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVA
Re: [Lldb-commits] [PATCH] D24382: Fixing a build breakage caused from a change in LLVM rL281019
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281030: Fixing a build breakage caused from a change in LLVM
rL281019 (authored by dvlahovski).
Changed prior to commit:
https://reviews.llvm.org/D24382?vs=70805&id=70806#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24382
Files:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Index:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
===
---
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -255,9 +255,7 @@
// if this argument is passed by val
if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal)) {
// strip away the byval attribute
-call_inst->removeAttribute(
-i,
-llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+call_inst->removeAttribute(i, llvm::Attribute::ByVal);
changed = true;
}
}
Index: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
===
--- lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -255,9 +255,7 @@
// if this argument is passed by val
if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal)) {
// strip away the byval attribute
-call_inst->removeAttribute(
-i,
-llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+call_inst->removeAttribute(i, llvm::Attribute::ByVal);
changed = true;
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24382: Fixing a build breakage caused from a change in LLVM rL281019
dvlahovski created this revision.
dvlahovski added a reviewer: ldrumm.
dvlahovski added a subscriber: lldb-commits.
LLVM guys did some clean-up of the Attribute getters/setters
and because of that the build was failing.
https://reviews.llvm.org/D24382
Files:
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Index:
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
===
---
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -255,9 +255,7 @@
// if this argument is passed by val
if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal)) {
// strip away the byval attribute
-call_inst->removeAttribute(
-i,
-llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+call_inst->removeAttribute(i, llvm::Attribute::ByVal);
changed = true;
}
}
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -255,9 +255,7 @@
// if this argument is passed by val
if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal)) {
// strip away the byval attribute
-call_inst->removeAttribute(
-i,
-llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+call_inst->removeAttribute(i, llvm::Attribute::ByVal);
changed = true;
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281030 - Fixing a build breakage caused from a change in LLVM rL281019
Author: dvlahovski
Date: Fri Sep 9 05:14:11 2016
New Revision: 281030
URL: http://llvm.org/viewvc/llvm-project?rev=281030&view=rev
Log:
Fixing a build breakage caused from a change in LLVM rL281019
Summary:
LLVM guys did some clean-up of the Attribute getters/setters
and because of that the build was failing.
Reviewers: ldrumm
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D24382
Modified:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Modified:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp?rev=281030&r1=281029&r2=281030&view=diff
==
---
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
(original)
+++
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
Fri Sep 9 05:14:11 2016
@@ -255,9 +255,7 @@ bool fixupRSAllocationStructByValCalls(l
// if this argument is passed by val
if (call_attribs.hasAttribute(i, llvm::Attribute::ByVal)) {
// strip away the byval attribute
-call_inst->removeAttribute(
-i,
-llvm::Attribute::get(module.getContext(), llvm::Attribute::ByVal));
+call_inst->removeAttribute(i, llvm::Attribute::ByVal);
changed = true;
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281031 - [LLDB][MIPS] Fix TestEhFrameUnwind.py for MIPS
Author: nitesh.jain
Date: Fri Sep 9 05:20:08 2016
New Revision: 281031
URL: http://llvm.org/viewvc/llvm-project?rev=281031&view=rev
Log:
[LLDB][MIPS] Fix TestEhFrameUnwind.py for MIPS
Reviewers: clayborg, labath
Subscribers: jaydeep, bhushan, slthakur, lldb-commits
Differential Revision: https://reviews.llvm.org/D24122
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c?rev=281031&r1=281030&r2=281031&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
Fri Sep 9 05:20:08 2016
@@ -1,4 +1,6 @@
void func() {
+
+#ifndef __mips__
__asm__ (
"pushq $0x10;"
".cfi_def_cfa_offset 16;"
@@ -10,11 +12,35 @@ void func() {
"movq $0x48, %rax;"
"popq %rax;"
);
-
+#elif __mips64
+ __asm__ (
+"daddiu $sp,$sp,-16;"
+".cfi_def_cfa_offset 16;"
+"sd $ra,8($sp);"
+".cfi_offset 31, -8;"
+"daddiu $ra,$zero,0;"
+"ld $ra,8($sp);"
+"daddiu $sp, $sp,16;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#else
+ // For MIPS32
+ __asm__ (
+"addiu $sp,$sp,-8;"
+".cfi_def_cfa_offset 8;"
+"sw $ra,4($sp);"
+".cfi_offset 31, -4;"
+"addiu $ra,$zero,0;"
+"lw $ra,4($sp);"
+"addiu $sp,$sp,8;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#endif
}
-
int main(int argc, char const *argv[])
{
func();
-}
\ No newline at end of file
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24122: [LLDB][MIPS] Fix TestEhFrameUnwind.py for MIPS
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281031: [LLDB][MIPS] Fix TestEhFrameUnwind.py for MIPS
(authored by nitesh.jain).
Changed prior to commit:
https://reviews.llvm.org/D24122?vs=69974&id=70807#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24122
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
@@ -1,4 +1,6 @@
void func() {
+
+#ifndef __mips__
__asm__ (
"pushq $0x10;"
".cfi_def_cfa_offset 16;"
@@ -10,11 +12,35 @@
"movq $0x48, %rax;"
"popq %rax;"
);
-
+#elif __mips64
+ __asm__ (
+"daddiu $sp,$sp,-16;"
+".cfi_def_cfa_offset 16;"
+"sd $ra,8($sp);"
+".cfi_offset 31, -8;"
+"daddiu $ra,$zero,0;"
+"ld $ra,8($sp);"
+"daddiu $sp, $sp,16;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#else
+ // For MIPS32
+ __asm__ (
+"addiu $sp,$sp,-8;"
+".cfi_def_cfa_offset 8;"
+"sw $ra,4($sp);"
+".cfi_offset 31, -4;"
+"addiu $ra,$zero,0;"
+"lw $ra,4($sp);"
+"addiu $sp,$sp,8;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#endif
}
-
int main(int argc, char const *argv[])
{
func();
-}
\ No newline at end of file
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/ehframe/main.c
@@ -1,4 +1,6 @@
void func() {
+
+#ifndef __mips__
__asm__ (
"pushq $0x10;"
".cfi_def_cfa_offset 16;"
@@ -10,11 +12,35 @@
"movq $0x48, %rax;"
"popq %rax;"
);
-
+#elif __mips64
+ __asm__ (
+"daddiu $sp,$sp,-16;"
+".cfi_def_cfa_offset 16;"
+"sd $ra,8($sp);"
+".cfi_offset 31, -8;"
+"daddiu $ra,$zero,0;"
+"ld $ra,8($sp);"
+"daddiu $sp, $sp,16;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#else
+ // For MIPS32
+ __asm__ (
+"addiu $sp,$sp,-8;"
+".cfi_def_cfa_offset 8;"
+"sw $ra,4($sp);"
+".cfi_offset 31, -4;"
+"addiu $ra,$zero,0;"
+"lw $ra,4($sp);"
+"addiu $sp,$sp,8;"
+".cfi_restore 31;"
+".cfi_def_cfa_offset 0;"
+ );
+#endif
}
-
int main(int argc, char const *argv[])
{
func();
-}
\ No newline at end of file
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281032 - [LLDB][MIPS] Fix Emulation for JALR64 Instruction
Author: nitesh.jain
Date: Fri Sep 9 05:46:25 2016
New Revision: 281032
URL: http://llvm.org/viewvc/llvm-project?rev=281032&view=rev
Log:
[LLDB][MIPS] Fix Emulation for JALR64 Instruction
Subscribers: jaydeep, bhushan, slthakur, sdardis, lldb-commits
Modified:
lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
Modified:
lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp?rev=281032&r1=281031&r2=281032&view=diff
==
--- lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
(original)
+++ lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
Fri Sep 9 05:46:25 2016
@@ -869,6 +869,7 @@ EmulateInstructionMIPS64::GetOpcodeForIn
{"JAL", &EmulateInstructionMIPS64::Emulate_JAL, "JAL target"},
{"JALX", &EmulateInstructionMIPS64::Emulate_JAL, "JALX target"},
{"JALR", &EmulateInstructionMIPS64::Emulate_JALR, "JALR target"},
+ {"JALR64", &EmulateInstructionMIPS64::Emulate_JALR, "JALR target"},
{"JALR_HB", &EmulateInstructionMIPS64::Emulate_JALR, "JALR.HB target"},
{"JIALC", &EmulateInstructionMIPS64::Emulate_JIALC, "JIALC rt,offset"},
{"JIC", &EmulateInstructionMIPS64::Emulate_JIC, "JIC rt,offset"},
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
dvlahovski created this revision.
dvlahovski added reviewers: labath, zturner.
dvlahovski added a subscriber: lldb-commits.
Herald added a subscriber: beanz.
Added parsing of the MiscInfo data stream.
The main member of it that we care about is the process_id
On Linux generated Minidump (from breakpad) we don't have
the MiscInfo, we have the /proc/$pid/status from where we can get the
pid.
Also parsing the module list - the list of all of the loaded
modules/shared libraries.
Finally - parsing the exception stream.
I have unit tests for all of that.
Also added some tests using a Minidump generated from Windows tools (not
from breakpad)
https://reviews.llvm.org/D24385
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/MinidumpTypes.h
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/fizzbuzz_no_heap.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -44,10 +44,8 @@
llvm::SmallString<128> filename = inputs_folder;
llvm::sys::path::append(filename, minidump_filename);
FileSpec minidump_file(filename.c_str(), false);
-lldb::DataBufferSP data_sp(
-minidump_file.MemoryMapFileContents(0, load_size));
-llvm::Optional optional_parser =
-MinidumpParser::Create(data_sp);
+lldb::DataBufferSP data_sp(minidump_file.MemoryMapFileContents(0, load_size));
+llvm::Optional optional_parser = MinidumpParser::Create(data_sp);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
ASSERT_GT(parser->GetByteSize(), 0UL);
@@ -80,12 +78,65 @@
TEST_F(MinidumpParserTest, GetArchitecture) {
SetUpData("linux-x86_64.dmp");
ASSERT_EQ(llvm::Triple::ArchType::x86_64,
-parser->GetArchitecture().GetTriple().getArch());
+parser->GetArchitecture().GetMachine());
}
TEST_F(MinidumpParserTest, GetMiscInfo) {
SetUpData("linux-x86_64.dmp");
const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
ASSERT_EQ(nullptr, misc_info);
- // linux breakpad generated minidump files don't have misc info stream
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatus) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional proc_status = parser->GetLinuxProcStatus();
+ ASSERT_TRUE(proc_status.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetPid) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional pid = parser->GetPid();
+ ASSERT_TRUE(pid.hasValue());
+ ASSERT_EQ(16001, pid.getValue());
+}
+
+TEST_F(MinidumpParserTest, GetModuleList) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional> modules = parser->GetModuleList();
+ ASSERT_TRUE(modules.hasValue());
+ ASSERT_EQ(8UL, modules->size());
+ //TODO check for specific modules here
+}
+
+TEST_F(MinidumpParserTest, GetExceptionStream) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional> data = parser->GetStream(MinidumpStreamType::Exception);
+ ASSERT_TRUE(data.hasValue());
+}
+
+
+// Windows Minidump tests
+// fizzbuzz_no_heap.dmp is copied from the WinMiniDump tests
+TEST_F(MinidumpParserTest, GetArchitectureWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ ASSERT_EQ(llvm::Triple::ArchType::x86, parser->GetArchitecture().GetMachine());
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatusWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ llvm::Optional proc_status = parser->GetLinuxProcStatus();
+ ASSERT_FALSE(proc_status.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetMiscInfoWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
+ ASSERT_TRUE(misc_info != nullptr);
+}
+
+TEST_F(MinidumpParserTest, GetPidWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ llvm::Optional pid = parser->GetPid();
+ ASSERT_TRUE(pid.hasValue());
+ ASSERT_EQ(4440, pid.getValue());
}
Index: unittests/Process/minidump/CMakeLists.txt
===
--- unittests/Process/minidump/CMakeLists.txt
+++ unittests/Process/minidump/CMakeLists.txt
@@ -3,6 +3,7 @@
)
set(test_inputs
- linux-x86_64.dmp)
+ linux-x86_64.dmp
+ fizzbuzz_no_heap.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Index: source/Plugins/Process/minidump/MinidumpTypes.h
===
--- source/Plugins/Process/minidump/MinidumpTypes.h
+++ source/Plugins/Process/minidump/MinidumpTypes.h
@@ -18,6 +18,9 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ConvertUTF.h"
#include
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
dvlahovski updated this revision to Diff 70816.
dvlahovski added a comment.
Forgot to run clang-format
https://reviews.llvm.org/D24385
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/MinidumpTypes.h
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/fizzbuzz_no_heap.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -80,12 +80,67 @@
TEST_F(MinidumpParserTest, GetArchitecture) {
SetUpData("linux-x86_64.dmp");
ASSERT_EQ(llvm::Triple::ArchType::x86_64,
-parser->GetArchitecture().GetTriple().getArch());
+parser->GetArchitecture().GetMachine());
}
TEST_F(MinidumpParserTest, GetMiscInfo) {
SetUpData("linux-x86_64.dmp");
const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
ASSERT_EQ(nullptr, misc_info);
- // linux breakpad generated minidump files don't have misc info stream
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatus) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional proc_status = parser->GetLinuxProcStatus();
+ ASSERT_TRUE(proc_status.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetPid) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional pid = parser->GetPid();
+ ASSERT_TRUE(pid.hasValue());
+ ASSERT_EQ(16001, pid.getValue());
+}
+
+TEST_F(MinidumpParserTest, GetModuleList) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional> modules =
+ parser->GetModuleList();
+ ASSERT_TRUE(modules.hasValue());
+ ASSERT_EQ(8UL, modules->size());
+ // TODO check for specific modules here
+}
+
+TEST_F(MinidumpParserTest, GetExceptionStream) {
+ SetUpData("linux-x86_64.dmp");
+ llvm::Optional> data =
+ parser->GetStream(MinidumpStreamType::Exception);
+ ASSERT_TRUE(data.hasValue());
+}
+
+// Windows Minidump tests
+// fizzbuzz_no_heap.dmp is copied from the WinMiniDump tests
+TEST_F(MinidumpParserTest, GetArchitectureWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ ASSERT_EQ(llvm::Triple::ArchType::x86,
+parser->GetArchitecture().GetMachine());
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatusWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ llvm::Optional proc_status = parser->GetLinuxProcStatus();
+ ASSERT_FALSE(proc_status.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetMiscInfoWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
+ ASSERT_TRUE(misc_info != nullptr);
+}
+
+TEST_F(MinidumpParserTest, GetPidWindows) {
+ SetUpData("fizzbuzz_no_heap.dmp");
+ llvm::Optional pid = parser->GetPid();
+ ASSERT_TRUE(pid.hasValue());
+ ASSERT_EQ(4440, pid.getValue());
}
Index: unittests/Process/minidump/CMakeLists.txt
===
--- unittests/Process/minidump/CMakeLists.txt
+++ unittests/Process/minidump/CMakeLists.txt
@@ -3,6 +3,7 @@
)
set(test_inputs
- linux-x86_64.dmp)
+ linux-x86_64.dmp
+ fizzbuzz_no_heap.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Index: source/Plugins/Process/minidump/MinidumpTypes.h
===
--- source/Plugins/Process/minidump/MinidumpTypes.h
+++ source/Plugins/Process/minidump/MinidumpTypes.h
@@ -18,6 +18,9 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Endian.h"
// C includes
@@ -148,6 +151,12 @@
LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ IDIVT)
};
+enum class MinidumpMiscInfoFlags : uint32_t {
+ ProcessID = (1 << 0),
+ ProcessTimes = (1 << 1),
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ ProcessTimes)
+};
+
template
Error consumeObject(llvm::ArrayRef &Buffer, const T *&Object) {
Error error;
@@ -161,6 +170,8 @@
return error;
}
+llvm::StringRef consumeString(llvm::ArrayRef &Buffer);
+
// Reference:
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680378(v=vs.85).aspx
struct MinidumpHeader {
@@ -206,6 +217,13 @@
static_assert(sizeof(MinidumpDirectory) == 12,
"sizeof MinidumpDirectory is not correct!");
+struct MinidumpString {
+ std::string buffer;
+
+ static llvm::Optional
+ Parse(llvm::ArrayRef &data);
+};
+
// Reference:
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680517(v=vs.85).aspx
struct MinidumpThread {
@@ -272,23 +290,102 @@
static_assert(sizeof(MinidumpSystemInfo) == 56,
"sizeof MinidumpSystemInfo is not correct!");
-// TODO check flags to see what's valid
// TODO misc2, misc3 ?
// Re
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
dvlahovski added a comment. Also added parsing code for Minidump strings - the string in the file are UTF-16 encoded. I used the code from the WinMiniDump plugin and it can extract a UTF-16 string and convert it to a UTF-8 one. https://reviews.llvm.org/D24385 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
zturner added reviewers: dblaikie, chandlerc. zturner added a comment. Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid the JSON parser that currently exists in LLDB won't do. As Pavel mentioned, the code has a definite LLDB feel to it, but more importantly it depends on StringExtractor which I think probably doesn't belong in LLVM. If you want to go down this route, I would start by deleting the StringExtractor changes from this CL. 90% of the stuff StringExtractor is used for, like hex string to number, string to integer, etc are covered by existing LLVM classes or methods. I'm a bit surprised there's not already a json parser in LLVM. Maybe there's a reason for it. I think this CL needs a few more reviewers on the LLVM side, so I'm adding a few people. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
It just occurred to me that clang-tidy parses json, and I believe also rolls its own json parser. Assuming nobody objects to having a json parser in llvm, i think the clang-tidy one would be the one to use as a starting point (assuming it's general enough), then we could delete lldb's On Fri, Sep 9, 2016 at 7:05 AM Zachary Turner wrote: > zturner added reviewers: dblaikie, chandlerc. > zturner added a comment. > > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid > the JSON parser that currently exists in LLDB won't do. As Pavel > mentioned, the code has a definite LLDB feel to it, but more importantly it > depends on StringExtractor which I think probably doesn't belong in LLVM. > If you want to go down this route, I would start by deleting the > StringExtractor changes from this CL. 90% of the stuff StringExtractor is > used for, like hex string to number, string to integer, etc are covered by > existing LLVM classes or methods. > > I'm a bit surprised there's not already a json parser in LLVM. Maybe > there's a reason for it. I think this CL needs a few more reviewers on the > LLVM side, so I'm adding a few people. > > > https://reviews.llvm.org/D24369 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
labath added a comment.
A collection of small remarks, mostly to do with style. Mainly, I'd like to
make the tests more strict about what they accept as reasonable values.
A higher level question: Given that we are already in the `minidump` namespace,
do we want all (most) of these symbols to be prefixed with `Minidump` ?
Thoughts?
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:65
@@ -68,1 +64,3 @@
+: m_data_sp(data_buf_sp), m_header(header),
+ m_directory_map(std::move(directory_map)) {}
this will still invoke the copy-constructor of the map. For that to work, your
constructor needs to take a rvalue reference to the map.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:154
@@ -142,1 +153,3 @@
+ triple.setOS(llvm::Triple::OSType::Linux);
+ arch_spec.SetTriple(triple);
How hard would it be to actually detect the os properly here?
Since you've already started processing windows minidumps, it would be great if
we could have a GetArchitecture() test for both OSs.
Update: So I see you already have that test, but it does not check the OS part
of the triple. If it's not too hard for some reason, let's set that as a part
of this CL.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef &Buffer) {
+ return llvm::StringRef(reinterpret_cast(Buffer.data()),
This is not consistent with the consumeObject function, which also drops the
consumed bytes from the buffer.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:127
@@ +126,3 @@
+
+llvm::Optional
+MinidumpMiscInfo::GetPid(const MinidumpMiscInfo &misc_info) {
`pid_t` is a host-specific type. Use `lldb::pid_t`.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:148
@@ +147,3 @@
+ llvm::SmallVector lines;
+ proc_status.proc_status.split(lines, '\n', 42);
+ for (auto line : lines) {
This can end up adding 43 elements to the vector. It will still work, but
you'll be doubling your memory usage for no reason. If you really want it, make
the vector larger, but as this is not performance critical, maybe you could
just use `SmallVector` ?
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:223
@@ +222,3 @@
+
+ static llvm::Optional
+ Parse(llvm::ArrayRef &data);
MinidumpString is just a wrapper around the std::string. Why not return the
string directly? (Also the "const" there is unnecessary).
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:299
@@ -282,1 +298,3 @@
+ llvm::support::ulittle32_t
+ flags1; // represent what info in the struct is valid
llvm::support::ulittle32_t process_id;
this is oddly formatted now.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:307
@@ -288,1 +306,3 @@
+
+ static llvm::Optional GetPid(const MinidumpMiscInfo &misc_info);
};
It looks like this should be a normal method instead of a static one.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:318
@@ +317,3 @@
+
+ static llvm::Optional GetPid(const LinuxProcStatus &misc_info);
+};
It looks like this should be a normal method instead of a static one.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:95
@@ +94,3 @@
+ llvm::Optional proc_status = parser->GetLinuxProcStatus();
+ ASSERT_TRUE(proc_status.hasValue());
+}
Also check whether the returned ProcStatus object has the contents you expect?
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:111
@@ +110,3 @@
+ ASSERT_EQ(8UL, modules->size());
+ // TODO check for specific modules here
+}
Fix the todo.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:118
@@ +117,3 @@
+ parser->GetStream(MinidumpStreamType::Exception);
+ ASSERT_TRUE(data.hasValue());
+}
Check the contents of the returned object.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:138
@@ +137,3 @@
+ const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
+ ASSERT_TRUE(misc_info != nullptr);
+}
Check MiscInfo contents.
https://reviews.llvm.org/D24385
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
zturner added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:88-89
@@ +87,4 @@
+MinidumpParser::GetMinidumpString(uint32_t rva) {
+ llvm::ArrayRef arr_ref(m_data_sp->GetBytes() + rva,
+ m_data_sp->GetByteSize() - rva);
+ return MinidumpString::Parse(arr_ref);
This is fine, but I prefer to avoid the math wherever possible since it makes
code slightly more difficult to read. I would write this as:
```
auto arr_ref = m_data_sp->GetData().drop_front(rva);
```
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:171
@@ +170,3 @@
+llvm::Optional MinidumpParser::GetLinuxProcStatus() {
+ llvm::Optional> data =
+ GetStream(MinidumpStreamType::LinuxProcStatus);
Same as before, an `Optional` seems a bit redundant. I would just
return an empty `ArrayRef` to represent it not being there. I dont' think
there's a valid case for a strema to actually be present, but be 0 bytes in
length.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:202
@@ +201,3 @@
+
+ return MinidumpModule::ParseModuleList(data.getValue());
+}
It will do this parsing every time you ask for the module list. How about
storing a `std::vector` in the `MinidumpParser` class
and then you can return an `ArrayRef` to it. So it's
lazy evaluation, only done once, with the side benefit of allowing you to
return an `ArrayRef` instead of a more complicated `Optional>`
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:47
@@ -45,1 +46,3 @@
+ llvm::Optional GetMinidumpString(uint32_t rva);
+
There's a lot of `Optional`'s here. Is it really possible for any subset of
these to be optional? Or is it more like if you find one, you will find them
all?
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:61
@@ +60,3 @@
+
+ llvm::Optional> GetModuleList();
+
I would drop the `Optional` here and return `ArrayRef`.
Returning an entire vector by value is wasteful on the stack. Using an
`ArrayRef` makes it very lightweight, while still providing value-like
semantics in that you won't be able to modify the underlyign container. Also,
if its size is 0, you can treat that the same as if the `Optional` did not have
a value.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef &Buffer) {
+ return llvm::StringRef(reinterpret_cast(Buffer.data()),
labath wrote:
> This is not consistent with the consumeObject function, which also drops the
> consumed bytes from the buffer.
Is this logic correct? A buffer may be arbitrarily large and have more data in
it besides the string. Perhaps you need to search forward for a null
terminator, then only return that portion of the string, then drop that many
bytes (plus the null terminator) from the input buffer?
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:223
@@ +222,3 @@
+
+ static llvm::Optional
+ Parse(llvm::ArrayRef &data);
labath wrote:
> MinidumpString is just a wrapper around the std::string. Why not return the
> string directly? (Also the "const" there is unnecessary).
Agree, the `MinidumpString` class seems unnecessary to me. Just make
`parseMinidumpString` be a file static global function and return an
`std::string`.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:299
@@ -282,1 +298,3 @@
+ llvm::support::ulittle32_t
+ flags1; // represent what info in the struct is valid
llvm::support::ulittle32_t process_id;
labath wrote:
> this is oddly formatted now.
Probably because the comment passed 80 columns. I would put the comment on a
new line above the field to fix this.
https://reviews.llvm.org/D24385
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281058 - xfail DarwinLog "filter message by regex" tests
Author: tfiala Date: Fri Sep 9 12:07:15 2016 New Revision: 281058 URL: http://llvm.org/viewvc/llvm-project?rev=281058&view=rev Log: xfail DarwinLog "filter message by regex" tests These tests are not working reliably. I'm marking them xfail until I resolve the issue. Tracked by: llvm.org/pr30299 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/filter/regex/message/TestDarwinLogFilterRegexMessage.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/filter/regex/message/TestDarwinLogFilterRegexMessage.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/filter/regex/message/TestDarwinLogFilterRegexMessage.py?rev=281058&r1=281057&r2=281058&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/filter/regex/message/TestDarwinLogFilterRegexMessage.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/filter/regex/message/TestDarwinLogFilterRegexMessage.py Fri Sep 9 12:07:15 2016 @@ -24,6 +24,8 @@ class TestDarwinLogFilterRegexMessage(da mydir = lldbtest.TestBase.compute_mydir(__file__) @decorators.skipUnlessDarwin +@decorators.expectedFailureAll(oslist=["macosx"], + bugnumber="llvm.org/pr30299") def test_filter_accept_message_full_match(self): """Test that fall-through reject, accept regex whole message works.""" log_entries = self.do_test( @@ -49,6 +51,8 @@ class TestDarwinLogFilterRegexMessage(da "First os_log call should have been skipped.") @decorators.skipUnlessDarwin +@decorators.expectedFailureAll(oslist=["macosx"], + bugnumber="llvm.org/pr30299") def test_filter_accept_message_partial_match(self): """Test that fall-through reject, accept regex message via partial match works.""" @@ -66,6 +70,8 @@ class TestDarwinLogFilterRegexMessage(da "First os_log call should have been skipped.") @decorators.skipUnlessDarwin +@decorators.expectedFailureAll(oslist=["macosx"], + bugnumber="llvm.org/pr30299") def test_filter_reject_message_full_match(self): """Test that fall-through accept, reject regex message works.""" log_entries = self.do_test( @@ -82,6 +88,8 @@ class TestDarwinLogFilterRegexMessage(da "First os_log call should have been skipped.") @decorators.skipUnlessDarwin +@decorators.expectedFailureAll(oslist=["macosx"], + bugnumber="llvm.org/pr30299") def test_filter_reject_message_partial_match(self): """Test that fall-through accept, reject regex message by partial match works.""" @@ -99,6 +107,8 @@ class TestDarwinLogFilterRegexMessage(da "First os_log call should have been skipped.") @decorators.skipUnlessDarwin +@decorators.expectedFailureAll(oslist=["macosx"], + bugnumber="llvm.org/pr30299") def test_filter_accept_message_second_rule(self): """Test that fall-through reject, accept regex message on second rule works.""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul
tfiala added a subscriber: tfiala. tfiala accepted this revision. tfiala added a reviewer: tfiala. tfiala added a comment. Accepting and then closing. https://reviews.llvm.org/D22286 ___ 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
tfiala added inline comments. Comment at: cmake/LLDBDependencies.cmake:168 @@ -167,3 +167,3 @@ # On FreeBSD/NetBSD backtrace() is provided by libexecinfo, not libc. -if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD") +if ((CMAKE_SYSTEM_NAME MATCHES "FreeBSD" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD") AND NOT CMAKE_SYSTEM_NAME STREQUAL "kFreeBSD") list(APPEND LLDB_SYSTEM_LIBS execinfo) krytarowski wrote: > brucem wrote: > > This really should turn into a check so that we don't need this to be > > extended for every single OS that gets added. > There is now support for CMake >= 3.0, which offers a builtin check for it. I agree with previous comments - if we can convert this to a CMake built-in check, that would be ideal. https://reviews.llvm.org/D23977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
tfiala added a comment. Getting back to this change. I'm going to rebase for code reformatting, remove the signature change on the JSON parsing, adjust my call sites for it, refactor the structured data parsing in the gdb-remote reception to a separate function, and then put this back up for review. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
zturner added a comment. In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > In https://reviews.llvm.org/D24369#538213, @zturner wrote: > > > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid > > the JSON parser that currently exists in LLDB won't do. As Pavel > > mentioned, the code has a definite LLDB feel to it, but more importantly it > > depends on StringExtractor which I think probably doesn't belong in LLVM. > > If you want to go down this route, I would start by deleting the > > StringExtractor changes from this CL. 90% of the stuff StringExtractor is > > used for, like hex string to number, string to integer, etc are covered by > > existing LLVM classes or methods. > > > My rationale was that reusing parser from a child project is better than > building anything new from scratch. Agree that we always want to reuse code wherever possible, but LLVM has stricter requirements on code style and class design. The JSON stuff itself can probably be a good starting point since clang-tidy doesn't have one (see my comment later), but I don't think `StringExtractor` belongs in LLVM. Looking at the code, JSON really only uses 1 or 2 methods from `StringExtractor anyway, so I don't think it's a big thing to overcome. > > > > I'm a bit surprised there's not already a json parser in LLVM. Maybe > > there's a reason for it. I think this CL needs a few more reviewers on the > > LLVM side, so I'm adding a few people. > > > I can't actually find json parser within clang-tidy. Only references that I > see lead eventually to YAML parsing: I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at it, it seems to be pretty specialized and doesn't support arbitrary JSON. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
clayborg added a comment. I like the idea of moving this down into LLVM. We should fully LLVM-ize the code on the first pass though and take the LLDB style out of it. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky added a comment. In https://reviews.llvm.org/D24369#538213, @zturner wrote: > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid the > JSON parser that currently exists in LLDB won't do. As Pavel mentioned, the > code has a definite LLDB feel to it, but more importantly it depends on > StringExtractor which I think probably doesn't belong in LLVM. If you want > to go down this route, I would start by deleting the StringExtractor changes > from this CL. 90% of the stuff StringExtractor is used for, like hex string > to number, string to integer, etc are covered by existing LLVM classes or > methods. My rationale was that reusing parser from a child project is better than building anything new from scratch. > I'm a bit surprised there's not already a json parser in LLVM. Maybe there's > a reason for it. I think this CL needs a few more reviewers on the LLVM > side, so I'm adding a few people. I can't actually find json parser within clang-tidy. Only references that I see lead eventually to YAML parsing: tool/ClangTidyMain.cpp 102:JSON array of objects: 138:Specifies a configuration in YAML/JSON format: ClangTidyOptions.cpp 36:// Map std::pair to a JSON array of size 2. ClangTidyOptions.h 245:/// \brief Parses LineFilter from JSON and stores it to the \p Options. 249:/// \brief Parses configuration from JSON and returns \c ClangTidyOptions or an https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D5867: Minimal API support for non-8-bit byte targets
Sorry Guys, I got pulled off lldb work some time ago. lldb didn't seem particularly easy to twist around to support the targets we maintained then. If there is anything I did that gets in anyone's way please just remove/ignore it. thanks Matthew Gardiner From: Ed Maste Sent: 08 September 2016 14:14 To: Gardiner, Matthew; clayb...@gmail.com; jing...@apple.com; ema...@freebsd.org Cc: lldb-commits@lists.llvm.org Subject: Re: [PATCH] D5867: Minimal API support for non-8-bit byte targets emaste added a comment. This change will no longer apply. Is it still desired? If so, can you please rebase the patch. https://reviews.llvm.org/D5867 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky updated this revision to Diff 70877.
aizatsky added a comment.
- getting rid of StringExtractor
- renaming methods to lowerCase.
https://reviews.llvm.org/D24369
Files:
.clang-tidy
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CMakeLists.txt
unittests/Support/JSONTest.cpp
Index: unittests/Support/JSONTest.cpp
===
--- /dev/null
+++ unittests/Support/JSONTest.cpp
@@ -0,0 +1,46 @@
+//===- llvm/unittest/Support/JSONTest.cpp - JSON.cpp tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "llvm/Support/JSON.h"
+
+using namespace llvm;
+
+static ::testing::AssertionResult MatchRoundtrip(std::string Text) {
+ JSONParser Parser(Text.c_str());
+ auto Obj = Parser.parseJSONValue();
+
+ if (!Obj) {
+return Text == "null"
+ ? ::testing::AssertionSuccess()
+ : ::testing::AssertionFailure() << "can't parse input: " << Text;
+ }
+
+ std::string S;
+ raw_string_ostream Out(S);
+ Obj->write(Out);
+
+ std::string Actual = Out.str();
+ if (Actual != Text) {
+return ::testing::AssertionFailure() << "expected: " << Text
+ << " actual: " << Actual;
+ }
+ return ::testing::AssertionSuccess();
+}
+
+TEST(JSON, Roundtrip) {
+ EXPECT_TRUE(MatchRoundtrip("0"));
+ EXPECT_TRUE(MatchRoundtrip("3.145150e+00"));
+ EXPECT_TRUE(MatchRoundtrip("{}"));
+ EXPECT_TRUE(MatchRoundtrip("{\"a\":1,\"b\":2}"));
+ EXPECT_TRUE(MatchRoundtrip("[]"));
+ EXPECT_TRUE(MatchRoundtrip("[0]"));
+ EXPECT_TRUE(MatchRoundtrip("[1,\"two\",3]"));
+}
Index: unittests/Support/CMakeLists.txt
===
--- unittests/Support/CMakeLists.txt
+++ unittests/Support/CMakeLists.txt
@@ -19,6 +19,7 @@
ErrorTest.cpp
ErrorOrTest.cpp
FileOutputBufferTest.cpp
+ JSONTest.cpp
LEB128Test.cpp
LineIteratorTest.cpp
LockFileManagerTest.cpp
Index: lib/Support/JSON.cpp
===
--- /dev/null
+++ lib/Support/JSON.cpp
@@ -0,0 +1,595 @@
+//===- JSON.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "llvm/Support/JSON.h"
+
+#include
+#include
+
+#include "llvm/Support/ErrorHandling.h"
+
+using namespace llvm;
+
+std::string JSONString::jsonStringQuoteMetachars(const std::string &S) {
+ if (S.find('"') == std::string::npos)
+return S;
+
+ std::string Output;
+ const size_t Size = S.size();
+ const char *Chars = S.c_str();
+ for (size_t I = 0; I < Size; I++) {
+unsigned char Ch = *(Chars + I);
+if (Ch == '"') {
+ Output.push_back('\\');
+}
+Output.push_back(Ch);
+ }
+ return Output;
+}
+
+JSONString::JSONString() : JSONValue(JSONValue::Kind::String), Data() {}
+
+JSONString::JSONString(const char *S)
+: JSONValue(JSONValue::Kind::String), Data(S ? S : "") {}
+
+JSONString::JSONString(const std::string &S)
+: JSONValue(JSONValue::Kind::String), Data(S) {}
+
+void JSONString::write(raw_ostream &OS) const {
+ OS << "\"" << jsonStringQuoteMetachars(Data) << "\"";
+}
+
+uint64_t JSONNumber::getAsUnsigned() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return Data.Unsigned;
+ case DataType::Signed:
+return (uint64_t)Data.Signed;
+ case DataType::Double:
+return (uint64_t)Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+int64_t JSONNumber::getAsSigned() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return (int64_t)Data.Unsigned;
+ case DataType::Signed:
+return Data.Signed;
+ case DataType::Double:
+return (int64_t)Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+double JSONNumber::getAsDouble() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return (double)Data.Unsigned;
+ case DataType::Signed:
+return (double)Data.Signed;
+ case DataType::Double:
+return Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+void JSONNumber::write(raw_ostream &OS) const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+OS << Data.Unsigned;
+break;
+ case DataType::Signed:
+OS << Data.Signed;
+break;
+ case DataType::Double:
+OS << Data.Double;
+break;
+ }
+}
+
+JSONTrue::JSONTrue() : JSONValue(JSONValue::Kind::True) {}
+
+void JSONTrue::write(raw_ostream &OS) const { OS << "true"; }
+
+JSONFalse::J
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky added a comment. In https://reviews.llvm.org/D24369#538472, @zturner wrote: > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > > > In https://reviews.llvm.org/D24369#538213, @zturner wrote: > > > > > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid > > > the JSON parser that currently exists in LLDB won't do. As Pavel > > > mentioned, the code has a definite LLDB feel to it, but more importantly > > > it depends on StringExtractor which I think probably doesn't belong in > > > LLVM. If you want to go down this route, I would start by deleting the > > > StringExtractor changes from this CL. 90% of the stuff StringExtractor > > > is used for, like hex string to number, string to integer, etc are > > > covered by existing LLVM classes or methods. > > > > > > My rationale was that reusing parser from a child project is better than > > building anything new from scratch. > > > Agree that we always want to reuse code wherever possible, but LLVM has > stricter requirements on code style and class design. The JSON stuff itself > can probably be a good starting point since clang-tidy doesn't have one (see > my comment later), but I don't think `StringExtractor` belongs in LLVM. > Looking at the code, JSON really only uses 1 or 2 methods from > `StringExtractor anyway, so I don't think it's a big thing to overcome. > > > > > > > > > > I'm a bit surprised there's not already a json parser in LLVM. Maybe > > > there's a reason for it. I think this CL needs a few more reviewers on > > > the LLVM side, so I'm adding a few people. > > > > > > > > > I can't actually find json parser within clang-tidy. Only references that I > > see lead eventually to YAML parsing: > > > I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at it, > it seems to be pretty specialized and doesn't support arbitrary JSON. I got rid of StringExtractor and also renamed all methods to follow the guidelines. Let me know which other style inconsistencies you see there. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky updated this revision to Diff 70879.
aizatsky added a comment.
remove .clang-tidy
https://reviews.llvm.org/D24369
Files:
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CMakeLists.txt
unittests/Support/JSONTest.cpp
Index: unittests/Support/JSONTest.cpp
===
--- /dev/null
+++ unittests/Support/JSONTest.cpp
@@ -0,0 +1,46 @@
+//===- llvm/unittest/Support/JSONTest.cpp - JSON.cpp tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "llvm/Support/JSON.h"
+
+using namespace llvm;
+
+static ::testing::AssertionResult MatchRoundtrip(std::string Text) {
+ JSONParser Parser(Text.c_str());
+ auto Obj = Parser.parseJSONValue();
+
+ if (!Obj) {
+return Text == "null"
+ ? ::testing::AssertionSuccess()
+ : ::testing::AssertionFailure() << "can't parse input: " << Text;
+ }
+
+ std::string S;
+ raw_string_ostream Out(S);
+ Obj->write(Out);
+
+ std::string Actual = Out.str();
+ if (Actual != Text) {
+return ::testing::AssertionFailure() << "expected: " << Text
+ << " actual: " << Actual;
+ }
+ return ::testing::AssertionSuccess();
+}
+
+TEST(JSON, Roundtrip) {
+ EXPECT_TRUE(MatchRoundtrip("0"));
+ EXPECT_TRUE(MatchRoundtrip("3.145150e+00"));
+ EXPECT_TRUE(MatchRoundtrip("{}"));
+ EXPECT_TRUE(MatchRoundtrip("{\"a\":1,\"b\":2}"));
+ EXPECT_TRUE(MatchRoundtrip("[]"));
+ EXPECT_TRUE(MatchRoundtrip("[0]"));
+ EXPECT_TRUE(MatchRoundtrip("[1,\"two\",3]"));
+}
Index: unittests/Support/CMakeLists.txt
===
--- unittests/Support/CMakeLists.txt
+++ unittests/Support/CMakeLists.txt
@@ -19,6 +19,7 @@
ErrorTest.cpp
ErrorOrTest.cpp
FileOutputBufferTest.cpp
+ JSONTest.cpp
LEB128Test.cpp
LineIteratorTest.cpp
LockFileManagerTest.cpp
Index: lib/Support/JSON.cpp
===
--- /dev/null
+++ lib/Support/JSON.cpp
@@ -0,0 +1,595 @@
+//===- JSON.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "llvm/Support/JSON.h"
+
+#include
+#include
+
+#include "llvm/Support/ErrorHandling.h"
+
+using namespace llvm;
+
+std::string JSONString::jsonStringQuoteMetachars(const std::string &S) {
+ if (S.find('"') == std::string::npos)
+return S;
+
+ std::string Output;
+ const size_t Size = S.size();
+ const char *Chars = S.c_str();
+ for (size_t I = 0; I < Size; I++) {
+unsigned char Ch = *(Chars + I);
+if (Ch == '"') {
+ Output.push_back('\\');
+}
+Output.push_back(Ch);
+ }
+ return Output;
+}
+
+JSONString::JSONString() : JSONValue(JSONValue::Kind::String), Data() {}
+
+JSONString::JSONString(const char *S)
+: JSONValue(JSONValue::Kind::String), Data(S ? S : "") {}
+
+JSONString::JSONString(const std::string &S)
+: JSONValue(JSONValue::Kind::String), Data(S) {}
+
+void JSONString::write(raw_ostream &OS) const {
+ OS << "\"" << jsonStringQuoteMetachars(Data) << "\"";
+}
+
+uint64_t JSONNumber::getAsUnsigned() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return Data.Unsigned;
+ case DataType::Signed:
+return (uint64_t)Data.Signed;
+ case DataType::Double:
+return (uint64_t)Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+int64_t JSONNumber::getAsSigned() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return (int64_t)Data.Unsigned;
+ case DataType::Signed:
+return Data.Signed;
+ case DataType::Double:
+return (int64_t)Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+double JSONNumber::getAsDouble() const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+return (double)Data.Unsigned;
+ case DataType::Signed:
+return (double)Data.Signed;
+ case DataType::Double:
+return Data.Double;
+ }
+ llvm_unreachable("Unhandled data type");
+}
+
+void JSONNumber::write(raw_ostream &OS) const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+OS << Data.Unsigned;
+break;
+ case DataType::Signed:
+OS << Data.Signed;
+break;
+ case DataType::Double:
+OS << Data.Double;
+break;
+ }
+}
+
+JSONTrue::JSONTrue() : JSONValue(JSONValue::Kind::True) {}
+
+void JSONTrue::write(raw_ostream &OS) const { OS << "true"; }
+
+JSONFalse::JSONFalse() : JSONValue(JSONValue::Kind::False) {}
+
+void JSO
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala retitled this revision from "Add StructuredData unit tests; remove JSON
parsing string copy" to "Add StructuredData unit tests; move packet processing
into delegate.".
tfiala updated this revision to Diff 70909.
https://reviews.llvm.org/D23884
Files:
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -20,6 +20,7 @@
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "llvm/ADT/STLExtras.h"
@@ -34,14 +35,14 @@
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
void HandleAsyncStdout(llvm::StringRef out) { output += out; }
void HandleAsyncMisc(llvm::StringRef data) { misc_data += data; }
void HandleStopReply() { ++stop_reply_called; }
- bool HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) {
-// TODO work in a test here after I fix the gtest breakage.
-return true;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+structured_data_packets.push_back(data);
}
};
@@ -321,6 +322,37 @@
EXPECT_EQ(1u, fix.delegate.stop_reply_called);
}
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateStructuredDataReceipt) {
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_payload =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+ const std::string json_packet = "JSON-async:" + json_payload;
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
+
+ // Set up the
+ StringExtractorGDBRemote response;
+ ContinueFixture fix;
+ if (HasFailure())
+return;
+
+ // Send async structured data packet, then stop.
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket(stream.GetData()));
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+ ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(1, fix.delegate.structured_data_packets.size());
+
+ // Verify the packet contents. It should have been unescaped upon packet
+ // reception.
+ ASSERT_EQ(json_packet, fix.delegate.structured_data_packets[0]);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse) {
StringExtractorGDBRemote continue_response, response;
ContinueFixture fix;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -415,8 +415,7 @@
void HandleAsyncStdout(llvm::StringRef out) override;
void HandleAsyncMisc(llvm::StringRef data) override;
void HandleStopReply() override;
- bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) override;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
using ModuleCacheKey = std::pair;
// KeyInfo for the cached module spec DenseMap.
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4808,9 +4808,54 @@
BuildDynamicRegisterInfo(true);
}
-bool ProcessGDBRemote::HandleAsyncStructuredData(
-const StructuredData::ObjectSP &object_sp) {
- return RouteAsyncStructuredData(object_sp);
+static const std::string &GetStructuredDataPacketPrefix() {
+ static const std::string prefix("JSON-async:");
+ return prefix;
+}
+
+static StructuredData::ObjectSP
+ParseStructuredDataPacket(const std::string &packet) {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+
+ // Verify this J packet is a JSON-async: packet.
+ const std::string &expected_prefix = GetStructuredDataPacketPrefix();
+ const std::string packet_prefix = packet.substr(0, expected_prefix.length());
+ if (packet_prefix != expected_prefix) {
+if (log) {
+ log->Printf("GDBRemoteCommmunicationClientBase::%s() "
+ "received $J packet but was not a "
+ "StructuredData packet: packet starts with "
+ "%s",
+ __FU
[Lldb-commits] Buildbot numbers for the last week of 8/28/2016 - 9/03/2016
Hello everyone, Below are some buildbot numbers for the last week of 8/28/2016 - 9/03/2016. Please see the same data in attached csv files: The longest time each builder was red during the last week; "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green); Count of commits by project; Number of completed builds, failed builds and average build time for successful builds per active builder; Average waiting time for a revision to get build result per active builder (response time). Thanks Galina The longest time each builder was red during the last week: buildername| was_red +--- libcxx-libcxxabi-libunwind-x86_64-linux-debian | 130:54:02 clang-ppc64le-linux-lnt| 112:46:03 clang-x86_64-linux-selfhost-modules| 107:38:13 sanitizer-windows | 73:28:02 lldb-x86_64-ubuntu-14.04-android | 62:50:55 clang-ppc64le-linux-multistage | 54:52:07 sanitizer-x86_64-linux-fuzzer | 52:23:42 llvm-sphinx-docs | 48:48:34 sanitizer-x86_64-linux-fast| 48:03:09 sanitizer-x86_64-linux-bootstrap | 47:31:29 libcxx-libcxxabi-x86_64-linux-debian | 43:44:40 clang-with-lto-ubuntu | 33:25:09 clang-x86-win2008-selfhost | 32:00:39 llvm-clang-lld-x86_64-debian-fast | 31:02:57 clang-x64-ninja-win7 | 30:18:11 lldb-windows7-android | 25:04:54 clang-x86_64-debian-fast | 20:20:17 sanitizer-x86_64-linux | 18:25:59 lld-x86_64-darwin13| 18:15:28 lldb-x86_64-darwin-13.4| 17:37:07 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 17:26:16 lldb-x86_64-ubuntu-14.04-buildserver | 16:15:00 lldb-x86_64-ubuntu-14.04-cmake | 15:27:14 clang-3stage-ubuntu| 15:23:00 clang-x86-windows-msvc2015 | 14:51:59 clang-ppc64be-linux-lnt| 14:32:11 perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only | 12:16:18 sanitizer-ppc64be-linux| 08:14:52 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 07:25:28 clang-cmake-thumbv7-a15-full-sh| 07:23:33 clang-cmake-armv7-a15-selfhost | 07:00:29 clang-ppc64be-linux| 06:27:45 clang-cmake-aarch64-full | 06:06:27 clang-cmake-mips | 05:57:52 perf-x86_64-penryn-O3-polly| 05:56:17 perf-x86_64-penryn-O3-polly-fast | 05:48:46 sanitizer-ppc64le-linux| 05:30:50 clang-sphinx-docs | 05:10:16 clang-cmake-aarch64-42vma | 05:07:34 clang-cuda-build | 04:20:15 clang-native-arm-lnt | 04:01:20 clang-ppc64le-linux| 03:59:01 perf-x86_64-penryn-O3 | 03:45:14 clang-cmake-armv7-a15-full | 03:34:32 clang-cmake-aarch64-quick | 03:20:05 perf-x86_64-penryn-O3-polly-unprofitable | 03:09:50 clang-ppc64be-linux-multistage | 02:58:03 sanitizer-x86_64-linux-autoconf| 02:51:04 perf-x86_64-penryn-O3-polly-parallel-fast | 02:51:01 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 02:43:55 llvm-hexagon-elf | 02:38:20 clang-cmake-thumbv7-a15| 02:14:05 clang-cmake-armv7-a15 | 02:13:27 libcxx-libcxxabi-x86_64-linux-ubuntu-msan | 02:06:05 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11 | 01:46:30 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 | 01:33:22 libcxx-libcxxabi-x86_64-linux-ubuntu-unstable-abi | 01:23:22 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan | 01:15:55 clang-hexagon-elf | 01:14:18 clang-s390x-linux
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
zturner added inline comments.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4812
@@ +4811,3 @@
+static const std::string &GetStructuredDataPacketPrefix() {
+ static const std::string prefix("JSON-async:");
+ return prefix;
How about just a global `llvm::StringRef`, or even a `StringRef` at local
scope? Doesn't seem worth using a function local static for this.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4817
@@ +4816,3 @@
+static StructuredData::ObjectSP
+ParseStructuredDataPacket(const std::string &packet) {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
Change the function parameter to an `llvm::StringRef`, and then you can do the
following:
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4821-4838
@@ +4820,20 @@
+ // Verify this J packet is a JSON-async: packet.
+ const std::string &expected_prefix = GetStructuredDataPacketPrefix();
+ const std::string packet_prefix = packet.substr(0, expected_prefix.length());
+ if (packet_prefix != expected_prefix) {
+if (log) {
+ log->Printf("GDBRemoteCommmunicationClientBase::%s() "
+ "received $J packet but was not a "
+ "StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__, packet_prefix.c_str());
+}
+return StructuredData::ObjectSP();
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+
+ // Parse the content into a StructuredData instance.
+ const char *const encoded_json = packet.c_str() + expected_prefix.length();
+
This entire block becomes:
```
if (!packet.consume_front("JSON-async:")) {
// print the log statement
}
auto json_sp = StructuredData::ParseJSON(packet);
```
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4856
@@ +4855,3 @@
+void ProcessGDBRemote::HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+ auto structured_data_sp = ParseStructuredDataPacket(data);
+ if (structured_data_sp)
This is doing a string copy since you're going from a `StringRef` to a
`std::string`. Use `StringRef` all the way down.
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:38
@@ -36,2 +37,3 @@
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
This can be a vector of `StringRefs` as well, unless there's some reason you
need to throw away the memory backing the `StringRef`, which it doesn't appear
you do.
Also, if you have a rough idea of how many `StringRefs` there's going to be
ahead of time, or at least an upper bound, then an
`llvm::SmallVector` will be more efficient.
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:335
@@ +334,3 @@
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
Would be nice to see `PutEscapedBytes` updated to take a `StringRef`. Every
occurrence of passing `const char * str, int len` should be replaced with
`StringRef` as we find occurrences of it.
https://reviews.llvm.org/D23884
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala marked 3 inline comments as done.
tfiala added a comment.
I'll make a few more adjustments here based on Zachary's feedback.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4812
@@ +4811,3 @@
+static const std::string &GetStructuredDataPacketPrefix() {
+ static const std::string prefix("JSON-async:");
+ return prefix;
zturner wrote:
> How about just a global `llvm::StringRef`, or even a `StringRef` at local
> scope? Doesn't seem worth using a function local static for this.
If llvm::StringRef at global scope does not incur a global constructor, that's
fine. If it does, we are prevented from adding global constructors within our
products except on a special-case basis. This would not pass that mark.
Easy enough for me to try, though. I'll check.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4821-4838
@@ +4820,20 @@
+ // Verify this J packet is a JSON-async: packet.
+ const std::string &expected_prefix = GetStructuredDataPacketPrefix();
+ const std::string packet_prefix = packet.substr(0, expected_prefix.length());
+ if (packet_prefix != expected_prefix) {
+if (log) {
+ log->Printf("GDBRemoteCommmunicationClientBase::%s() "
+ "received $J packet but was not a "
+ "StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__, packet_prefix.c_str());
+}
+return StructuredData::ObjectSP();
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+
+ // Parse the content into a StructuredData instance.
+ const char *const encoded_json = packet.c_str() + expected_prefix.length();
+
zturner wrote:
> This entire block becomes:
>
> ```
> if (!packet.consume_front("JSON-async:")) {
>// print the log statement
> }
> auto json_sp = StructuredData::ParseJSON(packet);
> ```
Yep, that looks great. Thanks!
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:38
@@ -36,2 +37,3 @@
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
zturner wrote:
> This can be a vector of `StringRefs` as well, unless there's some reason you
> need to throw away the memory backing the `StringRef`, which it doesn't
> appear you do.
>
> Also, if you have a rough idea of how many `StringRefs` there's going to be
> ahead of time, or at least an upper bound, then an
> `llvm::SmallVector` will be more efficient.
It's not clear to me what the lifetime is of the packet message content and how
that interplays with a StringRef. I was under the impression that a StringRef
will assume the backing store hangs around. That sounds like a recipe for
potential invalid memory access? I'll check the code, though. It may be
totally fine based on how the packet content is handled.
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:335
@@ +334,3 @@
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
zturner wrote:
> Would be nice to see `PutEscapedBytes` updated to take a `StringRef`. Every
> occurrence of passing `const char * str, int len` should be replaced with
> `StringRef` as we find occurrences of it.
Yep. I'd prefer to not make that part of this change, though.
https://reviews.llvm.org/D23884
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala added a comment. In https://reviews.llvm.org/D23884#539142, @tfiala wrote: > I'll make a few more adjustments here based on Zachary's feedback. Heh, ahem, that was meant to be "global *destructor*" above, not global constructor. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
zturner added inline comments.
Comment at: include/llvm/Support/JSON.h:27
@@ +26,3 @@
+public:
+ typedef std::shared_ptr SP;
+ enum class Kind { String, Number, True, False, Null, Object, Array };
I scanned the entire patch and I'm not sure I understand the need for the
covariant typedefs. The only type that ever seems to be used is `JSONValue`,
so it seems to me like the other types are unnecessary.
Also, I'm not sure `shared_ptr` is necessary. Wouldn't `unique_ptr` work just
fine? Then you could vend references to values instead of pointers.
Comment at: include/llvm/Support/JSON.h:47
@@ +46,3 @@
+ JSONString(const char *S);
+ JSONString(const std::string &S);
+
I would make this a `StringRef`. Currently if someone calls this with a
`StringRef`, it will make a copy. If you change the constructor to take a
`StringRef`, then it won't take a copy regardless of whether it's called with a
`StringRef` or a `std::string`.
Comment at: include/llvm/Support/JSON.h:56
@@ +55,3 @@
+
+ std::string getData() const { return Data; }
+
I would return a `StringRef` here. Otherwise you're guaranteed to make a copy,
if you return a `StringRef` it will only make a copy when you assign it to a
`std::string`.
Comment at: include/llvm/Support/JSON.h:63
@@ +62,3 @@
+private:
+ static std::string jsonStringQuoteMetachars(const std::string &);
+
This could be a file static in the .cpp file, and also it could take a
`StringRef` instead of a `const std::string &`.
Comment at: include/llvm/Support/JSON.h:202
@@ +201,3 @@
+
+ JSONValue::SP getObject(const std::string &Key);
+
Couple things here.
1. `Key` could be a `StringRef`
2. This method should probably be const.
3. You could make the underlying value a `unique_ptr` instead of a `shared_ptr`
and return a reference. You could also have a `tryGetValue` in cases where you
aren't sure if the value exists. Not sure what would be better.
Comment at: include/llvm/Support/JSON.h:207
@@ +206,3 @@
+private:
+ typedef std::map Map;
+ typedef Map::iterator Iterator;
`DenseMap` seems like a better choice here.
Comment at: include/llvm/Support/JSON.h:229-231
@@ +228,5 @@
+ typedef std::vector Vector;
+ typedef Vector::iterator Iterator;
+ typedef Vector::size_type Index;
+ typedef Vector::size_type Size;
+
I don't know if the `typedefs` here help or hurt readability. I would just
make them `size_t`, and remove the `Iterator` typedef as it doesn't seem to be
used.
Comment at: include/llvm/Support/JSON.h:238
@@ +237,3 @@
+
+ JSONValue::SP getObject(Index I);
+
How about providing an overloaded `operator[]` and providing `begin()` and
`end()` functions so that it can be used in a range based for syntax?
Comment at: include/llvm/Support/JSON.h:282-283
@@ +281,4 @@
+
+ std::string Buffer;
+ size_t Index;
+};
Instead of having a `std::string` here, which will copy the input JSON, I would
make this a `StringRef`. Then you can delete the `Index` field as well,
because the internal `StringRef` itself could always point to the beginning of
the next char by using a `Buffer = Buffer.drop_front(N)` like pattern. This
won't work if you ever have to do back-referencing across functions, but I
don't think that is needed. Also if `Buffer` is a `StringRef`, a lot of the
methods like `peekChar()` and `skipSpaces` become trivial one-liners that you
wouldn't even need to provide a separate function for.
Comment at: lib/Support/JSON.cpp:19
@@ +18,3 @@
+
+std::string JSONString::jsonStringQuoteMetachars(const std::string &S) {
+ if (S.find('"') == std::string::npos)
Input parameter should be a `StringRef`.
Comment at: lib/Support/JSON.cpp:23
@@ +22,3 @@
+
+ std::string Output;
+ const size_t Size = S.size();
Should probably do an `Output.reserve(S.size());`
Comment at: lib/Support/JSON.cpp:24-26
@@ +23,5 @@
+ std::string Output;
+ const size_t Size = S.size();
+ const char *Chars = S.c_str();
+ for (size_t I = 0; I < Size; I++) {
+unsigned char Ch = *(Chars + I);
`for (char CH : S)`
Comment at: lib/Support/JSON.cpp:53
@@ +52,3 @@
+ case DataType::Signed:
+return (uint64_t)Data.Signed;
+ case DataType::Double:
Can you use C++ style casting operators here and in other similar functions?
Comment at: lib/Support/JSON.cpp:115-116
@@ +114,4 @@
+ OS << '{';
+ auto Iter = Elements.begin(), End = Elements.end();
+ for (; Iter != End; Iter++) {
+if (First)
```
for (const auto &I : Elements) {
```
Comment at: lib/Support/JSON.cpp:187
@@ +186,3 @@
+
+JSON
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
zturner added inline comments. Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:335 @@ +334,3 @@ + StreamGDBRemote stream; + stream.PutEscapedBytes(json_packet.c_str(), json_packet.length()); + stream.Flush(); tfiala wrote: > zturner wrote: > > Would be nice to see `PutEscapedBytes` updated to take a `StringRef`. > > Every occurrence of passing `const char * str, int len` should be replaced > > with `StringRef` as we find occurrences of it. > Yep. I'd prefer to not make that part of this change, though. Yea you are right, since the packets are coming in asynchronously I don't think we can make any assumptions about how long it will live. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala updated this revision to Diff 70930.
https://reviews.llvm.org/D23884
Files:
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -20,6 +20,7 @@
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "llvm/ADT/STLExtras.h"
@@ -34,14 +35,14 @@
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
void HandleAsyncStdout(llvm::StringRef out) { output += out; }
void HandleAsyncMisc(llvm::StringRef data) { misc_data += data; }
void HandleStopReply() { ++stop_reply_called; }
- bool HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) {
-// TODO work in a test here after I fix the gtest breakage.
-return true;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+structured_data_packets.push_back(data);
}
};
@@ -321,6 +322,37 @@
EXPECT_EQ(1u, fix.delegate.stop_reply_called);
}
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateStructuredDataReceipt) {
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_payload =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+ const std::string json_packet = "JSON-async:" + json_payload;
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
+
+ // Set up the
+ StringExtractorGDBRemote response;
+ ContinueFixture fix;
+ if (HasFailure())
+return;
+
+ // Send async structured data packet, then stop.
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket(stream.GetData()));
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+ ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(1, fix.delegate.structured_data_packets.size());
+
+ // Verify the packet contents. It should have been unescaped upon packet
+ // reception.
+ ASSERT_EQ(json_packet, fix.delegate.structured_data_packets[0]);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse) {
StringExtractorGDBRemote continue_response, response;
ContinueFixture fix;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -415,8 +415,7 @@
void HandleAsyncStdout(llvm::StringRef out) override;
void HandleAsyncMisc(llvm::StringRef data) override;
void HandleStopReply() override;
- bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) override;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
using ModuleCacheKey = std::pair;
// KeyInfo for the cached module spec DenseMap.
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4808,9 +4808,49 @@
BuildDynamicRegisterInfo(true);
}
-bool ProcessGDBRemote::HandleAsyncStructuredData(
-const StructuredData::ObjectSP &object_sp) {
- return RouteAsyncStructuredData(object_sp);
+static const char *const s_async_json_packet_prefix = "JSON-async:";
+
+static StructuredData::ObjectSP
+ParseStructuredDataPacket(llvm::StringRef packet) {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+
+ if (!packet.consume_front(s_async_json_packet_prefix)) {
+if (log) {
+ log->Printf(
+ "GDBRemoteCommmunicationClientBase::%s() received $J packet "
+ "but was not a StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__,
+ packet.slice(0, strlen(s_async_json_packet_prefix)).str().c_str());
+}
+return StructuredData::ObjectSP();
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+ StructuredData::ObjectSP json_sp = StructuredData::ParseJSON(packet);
+ if (log) {
+if (json_sp) {
+ StreamString json_str;
+ json_sp->Dump(json_str);
+ json_str.Flush();
+ log->Printf("ProcessGDBRemote::%s()
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala updated this revision to Diff 70931.
tfiala added a comment.
Re-uploaded last patch with full context.
https://reviews.llvm.org/D23884
Files:
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -20,6 +20,7 @@
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "llvm/ADT/STLExtras.h"
@@ -34,14 +35,14 @@
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
void HandleAsyncStdout(llvm::StringRef out) { output += out; }
void HandleAsyncMisc(llvm::StringRef data) { misc_data += data; }
void HandleStopReply() { ++stop_reply_called; }
- bool HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) {
-// TODO work in a test here after I fix the gtest breakage.
-return true;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+structured_data_packets.push_back(data);
}
};
@@ -321,6 +322,37 @@
EXPECT_EQ(1u, fix.delegate.stop_reply_called);
}
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateStructuredDataReceipt) {
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_payload =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+ const std::string json_packet = "JSON-async:" + json_payload;
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
+
+ // Set up the
+ StringExtractorGDBRemote response;
+ ContinueFixture fix;
+ if (HasFailure())
+return;
+
+ // Send async structured data packet, then stop.
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket(stream.GetData()));
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+ ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(1, fix.delegate.structured_data_packets.size());
+
+ // Verify the packet contents. It should have been unescaped upon packet
+ // reception.
+ ASSERT_EQ(json_packet, fix.delegate.structured_data_packets[0]);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse) {
StringExtractorGDBRemote continue_response, response;
ContinueFixture fix;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -415,8 +415,7 @@
void HandleAsyncStdout(llvm::StringRef out) override;
void HandleAsyncMisc(llvm::StringRef data) override;
void HandleStopReply() override;
- bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) override;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
using ModuleCacheKey = std::pair;
// KeyInfo for the cached module spec DenseMap.
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4808,9 +4808,49 @@
BuildDynamicRegisterInfo(true);
}
-bool ProcessGDBRemote::HandleAsyncStructuredData(
-const StructuredData::ObjectSP &object_sp) {
- return RouteAsyncStructuredData(object_sp);
+static const char *const s_async_json_packet_prefix = "JSON-async:";
+
+static StructuredData::ObjectSP
+ParseStructuredDataPacket(llvm::StringRef packet) {
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+
+ if (!packet.consume_front(s_async_json_packet_prefix)) {
+if (log) {
+ log->Printf(
+ "GDBRemoteCommmunicationClientBase::%s() received $J packet "
+ "but was not a StructuredData packet: packet starts with "
+ "%s",
+ __FUNCTION__,
+ packet.slice(0, strlen(s_async_json_packet_prefix)).str().c_str());
+}
+return StructuredData::ObjectSP();
+ }
+
+ // This is an asynchronous JSON packet, destined for a
+ // StructuredDataPlugin.
+ StructuredData::ObjectSP json_sp = StructuredData::ParseJSON(packet);
+ if (log) {
+if (json_sp) {
+ StreamString json_str;
+ json_sp->Dump(json_str);
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala added inline comments. Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:38 @@ -36,2 +37,3 @@ unsigned stop_reply_called = 0; + std::vector structured_data_packets; Yeah, I did try a std::vector but that actually blew up since the backing storage did disappear. For the rest of the interface, I was able to pipe through the llvm::StringRef, though, since that's all serial up to the JSON parser, which itself then makes a copy that it requires. So I think that's all for the better. Thanks for the suggestion! https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
tfiala added a comment. I'm going to go ahead with this since I think the biggest concerns have been addressed. @labath we can adjust more later if you still have concerns. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281121 - async structured data packet handling improvements
Author: tfiala
Date: Fri Sep 9 19:06:29 2016
New Revision: 281121
URL: http://llvm.org/viewvc/llvm-project?rev=281121&view=rev
Log:
async structured data packet handling improvements
This change does the following:
* Changes the signature for the continuation delegate method that handles
async structured data from accepting an already-parsed structured data
element to taking just the packet contents.
* Moves the conversion of the JSON-async: packet contents from
GDBRemoteClientBase to the continuation delegate method.
* Adds a new unit test for verifying that the $JSON-asyc: packets get
decoded and that the decoded packets get forwarded on to the delegate
for further processing. Thanks to Pavel for making that whole section of
code easily unit testable!
* Tightens up the packet verification on reception of a $JSON-async:
packet contents. The code prior to this change is susceptible to a
segfault if a packet is carefully crafted that starts with $J but
has a total length shorter than the length of "$JSON-async:".
Reviewers: labath, clayborg, zturner
Differential Revision: https://reviews.llvm.org/D23884
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=281121&r1=281120&r2=281121&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Fri
Sep 9 19:06:29 2016
@@ -103,38 +103,9 @@ StateType GDBRemoteClientBase::SendConti
delegate.HandleAsyncMisc(
llvm::StringRef(response.GetStringRef()).substr(1));
break;
-
case 'J':
- // Asynchronous JSON packet, destined for a
- // StructuredDataPlugin.
- {
-// Parse the content into a StructuredData instance.
-auto payload_index = strlen("JSON-async:");
-StructuredData::ObjectSP json_sp = StructuredData::ParseJSON(
-response.GetStringRef().substr(payload_index));
-if (log) {
- if (json_sp)
-log->Printf("GDBRemoteCommmunicationClientBase::%s() "
-"received Async StructuredData packet: %s",
-__FUNCTION__,
-response.GetStringRef().substr(payload_index).c_str());
- else
-log->Printf("GDBRemoteCommmunicationClientBase::%s"
-"() received StructuredData packet:"
-" parse failure",
-__FUNCTION__);
-}
-
-// Pass the data to the process to route to the
-// appropriate plugin. The plugin controls what happens
-// to it from there.
-bool routed = delegate.HandleAsyncStructuredData(json_sp);
-if (log)
- log->Printf("GDBRemoteCommmunicationClientBase::%s()"
- " packet %s",
- __FUNCTION__, routed ? "handled" : "not handled");
-break;
- }
+ delegate.HandleAsyncStructuredDataPacket(response.GetStringRef());
+ break;
case 'T':
case 'S':
// Do this with the continue lock held.
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=281121&r1=281120&r2=281121&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Fri Sep
9 19:06:29 2016
@@ -25,14 +25,13 @@ public:
virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
virtual void HandleStopReply() = 0;
-//
-/// Processes async structured data.
+//
=
+/// Process asynchronously-received structured data.
///
-/// @return
-///true if the data was handled; otherwise, false.
-//
-virtual bool
-HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) = 0;
+/// @param[in] data
+/// The complete data packet, expected to start with JSON-async.
+//
=
+virtual void HandleAsyncStructuredDataPacket(llvm::StringRef data) = 0;
};
GDBRemoteClientBase(const char *comm_name, const char
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; move packet processing into delegate.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281121: async structured data packet handling improvements
(authored by tfiala).
Changed prior to commit:
https://reviews.llvm.org/D23884?vs=70931&id=70933#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23884
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -20,6 +20,7 @@
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Core/StreamGDBRemote.h"
#include "llvm/ADT/STLExtras.h"
@@ -34,14 +35,14 @@
std::string output;
std::string misc_data;
unsigned stop_reply_called = 0;
+ std::vector structured_data_packets;
void HandleAsyncStdout(llvm::StringRef out) { output += out; }
void HandleAsyncMisc(llvm::StringRef data) { misc_data += data; }
void HandleStopReply() { ++stop_reply_called; }
- bool HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) {
-// TODO work in a test here after I fix the gtest breakage.
-return true;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) {
+structured_data_packets.push_back(data);
}
};
@@ -321,6 +322,37 @@
EXPECT_EQ(1u, fix.delegate.stop_reply_called);
}
+TEST_F(GDBRemoteClientBaseTest, SendContinueDelegateStructuredDataReceipt) {
+ // Build the plain-text version of the JSON data we will have the
+ // server send.
+ const std::string json_payload =
+ "{ \"type\": \"MyFeatureType\", "
+ " \"elements\": [ \"entry1\", \"entry2\" ] }";
+ const std::string json_packet = "JSON-async:" + json_payload;
+
+ // Escape it properly for transit.
+ StreamGDBRemote stream;
+ stream.PutEscapedBytes(json_packet.c_str(), json_packet.length());
+ stream.Flush();
+
+ // Set up the
+ StringExtractorGDBRemote response;
+ ContinueFixture fix;
+ if (HasFailure())
+return;
+
+ // Send async structured data packet, then stop.
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket(stream.GetData()));
+ ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+ ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(1, fix.delegate.structured_data_packets.size());
+
+ // Verify the packet contents. It should have been unescaped upon packet
+ // reception.
+ ASSERT_EQ(json_packet, fix.delegate.structured_data_packets[0]);
+}
+
TEST_F(GDBRemoteClientBaseTest, InterruptNoResponse) {
StringExtractorGDBRemote continue_response, response;
ContinueFixture fix;
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -415,8 +415,7 @@
void HandleAsyncStdout(llvm::StringRef out) override;
void HandleAsyncMisc(llvm::StringRef data) override;
void HandleStopReply() override;
- bool
- HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) override;
+ void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
using ModuleCacheKey = std::pair;
// KeyInfo for the cached module spec DenseMap.
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -25,14 +25,13 @@
virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
virtual void HandleStopReply() = 0;
-//
-/// Processes async structured data.
+// =
+/// Process asynchronously-received structured data.
///
-/// @return
-///true if the data was handled; otherwise, false.
-//
-virtual bool
-HandleAsyncStructuredData(const StructuredData::ObjectSP &object_sp) = 0;
+/// @param[in] data
+/// The complete data packet, expected to start with JSON-async.
+// =
+virtual void HandleAsyncStructuredDataPacket(llvm::StringRef data) = 0;
};
GDBRemoteClientBase(c
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
amccarth added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef &Buffer) {
+ return llvm::StringRef(reinterpret_cast(Buffer.data()),
zturner wrote:
> labath wrote:
> > This is not consistent with the consumeObject function, which also drops
> > the consumed bytes from the buffer.
> Is this logic correct? A buffer may be arbitrarily large and have more data
> in it besides the string. Perhaps you need to search forward for a null
> terminator, then only return that portion of the string, then drop that many
> bytes (plus the null terminator) from the input buffer?
Minidump strings aren't zero-terminated. They're counted (in UTF16 code
units). So this would have to read the count and "consume" the appropriate
number of bytes.
Oh, but this isn't used for minidump strings. It looks like it's for these
Linux proc status fields.
https://reviews.llvm.org/D24385
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions
Even if it's length prefixed, the logic here basically just consumes the
entire buffer, which doesn't seem right
On Fri, Sep 9, 2016 at 5:43 PM Adrian McCarthy wrote:
> amccarth added inline comments.
>
>
> Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
> @@ +20,3 @@
> +llvm::StringRef
> +lldb_private::minidump::consumeString(llvm::ArrayRef &Buffer) {
> + return llvm::StringRef(reinterpret_cast(Buffer.data()),
>
> zturner wrote:
> > labath wrote:
> > > This is not consistent with the consumeObject function, which also
> drops the consumed bytes from the buffer.
> > Is this logic correct? A buffer may be arbitrarily large and have more
> data in it besides the string. Perhaps you need to search forward for a
> null terminator, then only return that portion of the string, then drop
> that many bytes (plus the null terminator) from the input buffer?
> Minidump strings aren't zero-terminated. They're counted (in UTF16 code
> units). So this would have to read the count and "consume" the appropriate
> number of bytes.
>
> Oh, but this isn't used for minidump strings. It looks like it's for
> these Linux proc status fields.
>
>
> https://reviews.llvm.org/D24385
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
chandlerc added a comment. In https://reviews.llvm.org/D24369#538472, @zturner wrote: > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > > > > I'm a bit surprised there's not already a json parser in LLVM. Maybe > > > there's a reason for it. I think this CL needs a few more reviewers on > > > the LLVM side, so I'm adding a few people. > > > There is already a JSON parser in LLVM. > > I can't actually find json parser within clang-tidy. Only references that I > > see lead eventually to YAML parsing: > > > I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at it, > it seems to be pretty specialized and doesn't support arbitrary JSON. It uses the YAML parser because JSON is a strict subset of YAML: https://en.wikipedia.org/wiki/JSON#YAML Please don't add a JSON parser to LLVM. I would much rather teach LLDB to use the YAML parser and improve that parser if and where useful. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
I had no idea yaml was a superset of json. Makes it an easy decision On Fri, Sep 9, 2016 at 6:48 PM Chandler Carruth wrote: > chandlerc added a comment. > > In https://reviews.llvm.org/D24369#538472, @zturner wrote: > > > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > > > > > > I'm a bit surprised there's not already a json parser in LLVM. > Maybe there's a reason for it. I think this CL needs a few more reviewers > on the LLVM side, so I'm adding a few people. > > > > > > > > There is already a JSON parser in LLVM. > > > > I can't actually find json parser within clang-tidy. Only references > that I see lead eventually to YAML parsing: > > > > > > > > > I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at > it, it seems to be pretty specialized and doesn't support arbitrary JSON. > > > It uses the YAML parser because JSON is a strict subset of YAML: > https://en.wikipedia.org/wiki/JSON#YAML > > Please don't add a JSON parser to LLVM. I would much rather teach LLDB to > use the YAML parser and improve that parser if and where useful. > > > https://reviews.llvm.org/D24369 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky updated this revision to Diff 70946.
aizatsky marked 10 inline comments as done.
aizatsky added a comment.
cleanup
https://reviews.llvm.org/D24369
Files:
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CMakeLists.txt
unittests/Support/JSONTest.cpp
Index: unittests/Support/JSONTest.cpp
===
--- /dev/null
+++ unittests/Support/JSONTest.cpp
@@ -0,0 +1,46 @@
+//===- llvm/unittest/Support/JSONTest.cpp - JSON.cpp tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "llvm/Support/JSON.h"
+
+using namespace llvm;
+
+static ::testing::AssertionResult MatchRoundtrip(std::string Text) {
+ JSONParser Parser(Text.c_str());
+ auto Obj = Parser.parseJSONValue();
+
+ if (!Obj) {
+return Text == "null"
+ ? ::testing::AssertionSuccess()
+ : ::testing::AssertionFailure() << "can't parse input: " << Text;
+ }
+
+ std::string S;
+ raw_string_ostream Out(S);
+ Obj->write(Out);
+
+ std::string Actual = Out.str();
+ if (Actual != Text) {
+return ::testing::AssertionFailure() << "expected: " << Text
+ << " actual: " << Actual;
+ }
+ return ::testing::AssertionSuccess();
+}
+
+TEST(JSON, Roundtrip) {
+ EXPECT_TRUE(MatchRoundtrip("0"));
+ EXPECT_TRUE(MatchRoundtrip("3.145150e+00"));
+ EXPECT_TRUE(MatchRoundtrip("{}"));
+ EXPECT_TRUE(MatchRoundtrip("{\"a\":1,\"b\":2}"));
+ EXPECT_TRUE(MatchRoundtrip("[]"));
+ EXPECT_TRUE(MatchRoundtrip("[0]"));
+ EXPECT_TRUE(MatchRoundtrip("[1,\"two\",3]"));
+}
Index: unittests/Support/CMakeLists.txt
===
--- unittests/Support/CMakeLists.txt
+++ unittests/Support/CMakeLists.txt
@@ -19,6 +19,7 @@
ErrorTest.cpp
ErrorOrTest.cpp
FileOutputBufferTest.cpp
+ JSONTest.cpp
LEB128Test.cpp
LineIteratorTest.cpp
LockFileManagerTest.cpp
Index: lib/Support/JSON.cpp
===
--- /dev/null
+++ lib/Support/JSON.cpp
@@ -0,0 +1,537 @@
+//===- JSON.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "llvm/Support/JSON.h"
+
+#include
+#include
+
+#include "llvm/Support/ErrorHandling.h"
+
+using namespace llvm;
+
+static std::string quoteJsonString(StringRef S) {
+ if (S.find('"') == std::string::npos)
+return S;
+
+ std::string Output;
+ Output.reserve(S.size());
+ for (char Ch : S.bytes()) {
+if (Ch == '"')
+ Output.push_back('\\');
+Output.push_back(Ch);
+ }
+ return Output;
+}
+
+JSONString::JSONString(StringRef S)
+: JSONValue(JSONValue::Kind::String), Data(S) {}
+
+void JSONString::write(raw_ostream &OS) const {
+ OS << "\"" << quoteJsonString(Data) << "\"";
+}
+
+void JSONNumber::write(raw_ostream &OS) const {
+ switch (TheDataType) {
+ case DataType::Unsigned:
+OS << Data.Unsigned;
+break;
+ case DataType::Signed:
+OS << Data.Signed;
+break;
+ case DataType::Double:
+OS << Data.Double;
+break;
+ }
+}
+
+JSONTrue::JSONTrue() : JSONValue(JSONValue::Kind::True) {}
+
+void JSONTrue::write(raw_ostream &OS) const { OS << "true"; }
+
+JSONFalse::JSONFalse() : JSONValue(JSONValue::Kind::False) {}
+
+void JSONFalse::write(raw_ostream &OS) const { OS << "false"; }
+
+JSONNull::JSONNull() : JSONValue(JSONValue::Kind::Null) {}
+
+void JSONNull::write(raw_ostream &OS) const { OS << "null"; }
+
+JSONObject::JSONObject() : JSONValue(JSONValue::Kind::Object) {}
+
+void JSONObject::write(raw_ostream &OS) const {
+ bool First = true;
+ OS << '{';
+ for (const auto &KV : Elements) {
+if (First)
+ First = false;
+else
+ OS << ',';
+OS << "\"" << quoteJsonString(KV.first) << "\"";
+OS << ':';
+KV.second->write(OS);
+ }
+ OS << '}';
+}
+
+bool JSONObject::set(StringRef Key, std::unique_ptr Value) {
+ if (Key.empty() || nullptr == Value.get())
+return false;
+ Elements[Key] = std::move(Value);
+ return true;
+}
+
+bool JSONObject::get(StringRef Key, JSONValue const **ValuePtr) const {
+ auto Iter = Elements.find(Key), End = Elements.end();
+ if (Iter == End)
+return false;
+ *ValuePtr = Iter->second.get();
+ return true;
+}
+
+JSONArray::JSONArray() : JSONValue(JSONValue::Kind::Array) {}
+
+void JSONArray::write(raw_ostream &OS) const {
+ bool First = true;
+ OS << '[';
+ for (const auto &Element : Elements) {
+if (First
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
Oh, that's great! Thanks for pointing this out. I'll just upload the cleaned up version in case lldb/ folks would like it. I'll figure out how to use our YAML library. On Fri, Sep 9, 2016 at 6:48 PM Chandler Carruth wrote: > chandlerc added a comment. > > In https://reviews.llvm.org/D24369#538472, @zturner wrote: > > > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > > > > > > I'm a bit surprised there's not already a json parser in LLVM. > Maybe there's a reason for it. I think this CL needs a few more reviewers > on the LLVM side, so I'm adding a few people. > > > > > > > > There is already a JSON parser in LLVM. > > > > I can't actually find json parser within clang-tidy. Only references > that I see lead eventually to YAML parsing: > > > > > > > > > I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at > it, it seems to be pretty specialized and doesn't support arbitrary JSON. > > > It uses the YAML parser because JSON is a strict subset of YAML: > https://en.wikipedia.org/wiki/JSON#YAML > > Please don't add a JSON parser to LLVM. I would much rather teach LLDB to > use the YAML parser and improve that parser if and where useful. > > > https://reviews.llvm.org/D24369 > > > > -- Mike Sent from phone ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
aizatsky abandoned this revision. Comment at: include/llvm/Support/JSON.h:207 @@ +206,3 @@ +private: + typedef std::map Map; + typedef Map::iterator Iterator; zturner wrote: > `DenseMap` seems like a better choice here. DenseMap doesn't seem to be defined for std::string keys. Comment at: include/llvm/Support/JSON.h:238 @@ +237,3 @@ + + JSONValue::SP getObject(Index I); + zturner wrote: > How about providing an overloaded `operator[]` and providing `begin()` and > `end()` functions so that it can be used in a range based for syntax? done for operator[]. begin() and end() will be tricky, because underlying storage has std::unique_ptr<> Comment at: include/llvm/Support/JSON.h:282-283 @@ +281,4 @@ + + std::string Buffer; + size_t Index; +}; zturner wrote: > Instead of having a `std::string` here, which will copy the input JSON, I > would make this a `StringRef`. Then you can delete the `Index` field as > well, because the internal `StringRef` itself could always point to the > beginning of the next char by using a `Buffer = Buffer.drop_front(N)` like > pattern. This won't work if you ever have to do back-referencing across > functions, but I don't think that is needed. Also if `Buffer` is a > `StringRef`, a lot of the methods like `peekChar()` and `skipSpaces` become > trivial one-liners that you wouldn't even need to provide a separate function > for. `std::string => StringRef` - done. As for removing `Index` - I'm not sure. It is currently used in error reporting and is the only feedback we give when we encounter an error. Comment at: lib/Support/JSON.cpp:53 @@ +52,3 @@ + case DataType::Signed: +return (uint64_t)Data.Signed; + case DataType::Double: zturner wrote: > Can you use C++ style casting operators here and in other similar functions? Removed all 3 them and added get method. https://reviews.llvm.org/D24369 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
On Fri, Sep 9, 2016 at 7:31 PM Mike Aizatsky wrote: > aizatsky abandoned this revision. > > > Comment at: include/llvm/Support/JSON.h:207 > @@ +206,3 @@ > +private: > + typedef std::map Map; > + typedef Map::iterator Iterator; > > zturner wrote: > > `DenseMap` seems like a better choice here. > DenseMap doesn't seem to be defined for std::string keys. > > > Comment at: include/llvm/Support/JSON.h:238 > @@ +237,3 @@ > + > + JSONValue::SP getObject(Index I); > + > > zturner wrote: > > How about providing an overloaded `operator[]` and providing `begin()` > and `end()` functions so that it can be used in a range based for syntax? > done for operator[]. > > begin() and end() will be tricky, because underlying storage has > std::unique_ptr<> > see llvm::pointee_iterator in include/llvm/ADT/iterator.h > > > Comment at: include/llvm/Support/JSON.h:282-283 > @@ +281,4 @@ > + > + std::string Buffer; > + size_t Index; > +}; > > zturner wrote: > > Instead of having a `std::string` here, which will copy the input JSON, > I would make this a `StringRef`. Then you can delete the `Index` field as > well, because the internal `StringRef` itself could always point to the > beginning of the next char by using a `Buffer = Buffer.drop_front(N)` like > pattern. This won't work if you ever have to do back-referencing across > functions, but I don't think that is needed. Also if `Buffer` is a > `StringRef`, a lot of the methods like `peekChar()` and `skipSpaces` become > trivial one-liners that you wouldn't even need to provide a separate > function for. > `std::string => StringRef` - done. > > As for removing `Index` - I'm not sure. It is currently used in error > reporting and is the only feedback we give when we encounter an error. > > > Comment at: lib/Support/JSON.cpp:53 > @@ +52,3 @@ > + case DataType::Signed: > +return (uint64_t)Data.Signed; > + case DataType::Double: > > zturner wrote: > > Can you use C++ style casting operators here and in other similar > functions? > Removed all 3 them and added get method. > > > https://reviews.llvm.org/D24369 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
I'll figure out how to port lldb's use over to yaml On Fri, Sep 9, 2016 at 7:31 PM Mike Aizatsky wrote: > Oh, that's great! Thanks for pointing this out. I'll just upload the > cleaned up version in case lldb/ folks would like it. I'll figure out how > to use our YAML library. > > On Fri, Sep 9, 2016 at 6:48 PM Chandler Carruth > wrote: > > chandlerc added a comment. > > In https://reviews.llvm.org/D24369#538472, @zturner wrote: > > > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote: > > > > > > I'm a bit surprised there's not already a json parser in LLVM. > Maybe there's a reason for it. I think this CL needs a few more reviewers > on the LLVM side, so I'm adding a few people. > > > > > > > > There is already a JSON parser in LLVM. > > > > I can't actually find json parser within clang-tidy. Only references > that I see lead eventually to YAML parsing: > > > > > > > > > I was thinking of `lib/Tooling/JSONCompilationDatabase`, but looking at > it, it seems to be pretty specialized and doesn't support arbitrary JSON. > > > It uses the YAML parser because JSON is a strict subset of YAML: > https://en.wikipedia.org/wiki/JSON#YAML > > Please don't add a JSON parser to LLVM. I would much rather teach LLDB to > use the YAML parser and improve that parser if and where useful. > > > https://reviews.llvm.org/D24369 > > > > -- > Mike > Sent from phone > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
