[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Pavel Labath via Phabricator via lldb-commits]

labath edited reviewers, added: amccarth, labath; removed: zturner, 
llvm-commits.
labath added a subscriber: amccarth.
labath added a comment.

s/@zturner/@amccarth, as Zach probably won't have time to review this




Comment at: lit/Modules/PECOFF/export-dllfunc.yaml:11-12
+
+# timestamp and build id of debug info in the coff header varies. So does its 
UUID.
+# BASIC-CHECK-DAG: UUID: {{[0-9A-F]{7,}[0-9A-F]}}-{{.*}}
 

Since the U(G)UID needs to be stable and match the value computed from other 
sources, it would be good to have a test where we check that a file has some 
exact UUID.

Is there any way to use yaml2obj to generate such a file? For instance, if we 
drop the `lld-link` step and yamlify the resulting `dll` file instead. Would 
that work?



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:60
+  llvm::StringRef pdb_file;
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);

Can `getDebugPDBInfo` succeed and still return a null pdb_info? If not, can we 
delete the second part?

Instead I believe you should check the CVSignature field of the returned struct 
to see that it indeed contains a PDB70 record.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:866
+
+  m_uuid = GetCoffUUID(GetFileSpec());
+  return m_uuid;

ObjectFilePECOFF already has a `llvm::object::Binary` created for the 
underlying file. I think it's super-wasteful (and potentially racy, etc.) to 
create a second one just to read out it's GUID. If you make a second version of 
this function, taking a `Binary` (and have the FileSpec version delegate to 
that), then you can avoid this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56229/new/

https://reviews.llvm.org/D56229



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60667: Allow direct comparison of ConstString against StringRef

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked 2 inline comments as done.
teemperor added a comment.

In D60667#1467387 , @amccarth wrote:

> I, too, have some concern that this could have unintended side effects.  To 
> make the temporary `StringRef`s from the zero-terminated strings requires a 
> `strlen` call each time.  So we're making two passes over a string each time 
> (once to measure and once to compare).  Granted, these are mostly very short.


I think I'm not understanding something here. There is one `strlen` call to 
make the StringRef from the literal, but I don't see why we need a second one 
to compare (as StringRef and ConstString both store the string length and don't 
recompute it when comparing). Simple godbolt test case here: 
https://godbolt.org/z/9X1RRt So both the old and new version both require one 
`strlen` call from what I can see (which is anyway optimized out for comparing 
against literals).

> I'd love to see the static const variables for the short constant strings, 
> just to weigh readability hit.

I can maybe make a new revision to show this, but I hope it's obvious from 
looking at the code that I changed in this revision. Also there are the 
problems/style difficulties that hopefully don't need a full revision to see:

1. We preferable want to have variables with the same name as the string 
content, but often the strings contain reserved words or words that don't 
translate into our lower_case_code_style so that's not possible. E.g. `var == 
this_` is not really as readable as `var == "this"` (and it looks like a typo). 
Same with stuff like `if (arg_type.GetTypeName() == "bool")` vs `if 
(arg_type.GetTypeName() == bool_)` or `image_infos[i].segments[k].name == 
ConstString("__TEXT"))` vs `image_infos[i].segments[k].name == text)`.
2. We get a bunch of local variables that just bloat the code:

  if (descriptor->IsCFType()) {
ConstString type_name(valobj.GetTypeName());
if (type_name == "__CFMutableBitVector" || type_name == "__CFBitVector" ||
type_name == "CFMutableBitVectorRef" || type_name == "CFBitVectorRef") {
  
  
  if (valobj.IsPointerType())
is_type_ok = true;
}
  }

becomes this

  if (descriptor->IsCFType()) {
ConstString type_name(valobj.GetTypeName());
static const ConstString cf_mutable_bit_vector("__CFMutableBitVector"),
 cf_bit_vector("__CFBitVector"),
 cf_mutable_bit_vector_ref("CFMutableBitVectorRef"),
 cf_bit_vector_ref("CFBitVectorRef")
if (type_name == cf_mutable_bit_vector || type_name == cf_bit_vector ||
type_name == cf_mutable_bit_vector_ref || type_name == 
cf_bit_vector_ref) {
  if (valobj.IsPointerType())
is_type_ok = true;
}
  }

3. Code becomes much harder to grep. E.g. if I want to check for the location 
where we filter the `this` variable from the local variables, I can currently 
do `git grep "\"this\"" | grep ==` and directly see where we do the comparison. 
But with local variables I have to do some funky grep with context filtering. 
In general grepping for these string literals without a few lines of context 
will be pointless with ConstString variables for literals.




Comment at: lldb/include/lldb/Utility/ConstString.h:173
+// StringRef doesn't. Therefore we have to do this check manually now.
+if (!m_string ^ !rhs.data())
+  return false;

amccarth wrote:
> This is very clever way to express the constraint, but it's quite a bit of 
> cognitive load for the reader to figure out what's being tested here.  The 
> comment sort of explains, but leaves it up to the reader to see how that maps 
> to the condition.  (It also leaves me wondering whether it's useful to have 
> ConstString treat empty strings and nullptr as distinct things.)
> 
> A simpler way to communicate the condition to humans might be:
> 
> if (m_string == nullptr && rhs.data() != nullptr) return false;
> if (m_string != nullptr && rhs.data() == nullptr) return false;
> 
> The clever expression requires the reader to know or deduce:
> 
> 1.  that `m_string` and `rhs.data()` are pointers,
> 2.  that `!p` is equivalent to `p != 0` and that the `0` will be implicitly 
> converted to `nullptr`, yielding a `bool` that's `true` if `p` is a nullptr,
> 3.  that `^` is bitwise exclusive or,
> 4.  that boolean `!` and bitwise `^` have the appropriate operator 
> precedence, which is not always obvious when mixing types like this,
> 5.  that `true` and `false` are guaranteed to have values such that bitwise 
> `^` will do the expected thing to the promoted types,
> 6.  and that the resulting `int` will be implicitly compared for inequality 
> to 0.
> 
> Most of these are reasonable things to expect a C++ programmer to know, but 
> expecting them to apply all of that knowledge (correctly) to figure out what 
> the expression does seems like an unnecessarily high cog

[Lldb-commits] [PATCH] D60667: Allow direct comparison of ConstString against StringRef

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 195310.
teemperor removed a reviewer: espindola.
teemperor added a comment.
Herald added a reviewer: espindola.

- Made empty/nullptr check more readable.
- Removed some uses of the new comparison operator for cases where we don't 
have a literal to compare against (which makes it hard to estimate if this is 
really better).
- Reworded documentation so that the length of the string doesn't really matter 
getting a performance benefit, only the fact that whether ConstString we pass 
is temporary or not.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60667/new/

https://reviews.llvm.org/D60667

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/unittests/Utility/ConstStringTest.cpp

Index: lldb/unittests/Utility/ConstStringTest.cpp
===
--- lldb/unittests/Utility/ConstStringTest.cpp
+++ lldb/unittests/Utility/ConstStringTest.cpp
@@ -89,3 +89,51 @@
   EXPECT_TRUE(null.IsEmpty());
   EXPECT_TRUE(null.IsNull());
 }
+
+TEST(ConstStringTest, CompareConstString) {
+  ConstString foo("foo");
+  ConstString foo2("foo");
+  ConstString bar("bar");
+
+  EXPECT_TRUE(foo == foo2);
+  EXPECT_TRUE(foo2 == foo);
+  EXPECT_TRUE(foo == ConstString("foo"));
+
+  EXPECT_FALSE(foo == bar);
+  EXPECT_FALSE(foo2 == bar);
+  EXPECT_FALSE(foo == ConstString("bar"));
+  EXPECT_FALSE(foo == ConstString("different"));
+  EXPECT_FALSE(foo == ConstString(""));
+  EXPECT_FALSE(foo == ConstString());
+
+  ConstString empty("");
+  EXPECT_FALSE(empty == ConstString("bar"));
+  EXPECT_FALSE(empty == ConstString());
+  EXPECT_TRUE(empty == ConstString(""));
+
+  ConstString null;
+  EXPECT_FALSE(null == ConstString("bar"));
+  EXPECT_TRUE(null == ConstString());
+  EXPECT_FALSE(null == ConstString(""));
+}
+
+TEST(ConstStringTest, CompareStringRef) {
+  ConstString foo("foo");
+
+  EXPECT_TRUE(foo == "foo");
+  EXPECT_TRUE(foo != "");
+  EXPECT_FALSE(foo == static_cast(nullptr));
+  EXPECT_TRUE(foo != "bar");
+
+  ConstString empty("");
+  EXPECT_FALSE(empty == "foo");
+  EXPECT_FALSE(empty != "");
+  EXPECT_FALSE(empty == static_cast(nullptr));
+  EXPECT_TRUE(empty != "bar");
+
+  ConstString null;
+  EXPECT_FALSE(null == "foo");
+  EXPECT_TRUE(null != "");
+  EXPECT_TRUE(null == static_cast(nullptr));
+  EXPECT_TRUE(null != "bar");
+}
Index: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
===
--- lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
@@ -454,8 +454,7 @@
 ThreadSP SystemRuntimeMacOSX::GetExtendedBacktraceThread(ThreadSP real_thread,
  ConstString type) {
   ThreadSP originating_thread_sp;
-  if (BacktraceRecordingHeadersInitialized() &&
-  type == ConstString("libdispatch")) {
+  if (BacktraceRecordingHeadersInitialized() && type == "libdispatch") {
 Status error;
 
 // real_thread is either an actual, live thread (in which case we need to
@@ -554,7 +553,7 @@
 SystemRuntimeMacOSX::GetExtendedBacktraceForQueueItem(QueueItemSP queue_item_sp,
   ConstString type) {
   ThreadSP extended_thread_sp;
-  if (type != ConstString("libdispatch"))
+  if (type != "libdispatch")
 return extended_thread_sp;
 
   bool stop_id_is_valid = true;
Index: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -302,7 +302,7 @@
const FileSpec &dst_file_spec) {
   // For oat file we can try to fetch additional debug info from the device
   ConstString extension = module_sp

[Lldb-commits] [lldb] r358477 - Correctly check if a warning message lacks a trailing new line

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Tue Apr 16 00:48:11 2019
New Revision: 358477

URL: http://llvm.org/viewvc/llvm-project?rev=358477&view=rev
Log:
Correctly check if a warning message lacks a trailing new line

Summary: Fixes LLVM bug 41489.

Reviewers: clayborg

Reviewed By: clayborg

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D60653

Modified:
lldb/trunk/source/Core/Module.cpp

Modified: lldb/trunk/source/Core/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=358477&r1=358476&r2=358477&view=diff
==
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Tue Apr 16 00:48:11 2019
@@ -1120,7 +1120,7 @@ void Module::ReportError(const char *for
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
@@ -1152,7 +1152,7 @@ void Module::ReportErrorIfModifyDetected
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 strm.PutCString("The debug session should be aborted as the original "
@@ -1178,7 +1178,7 @@ void Module::ReportWarning(const char *f
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogWarning, "%s", strm.GetData());


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60653: Correctly check if a warning message lacks a trailing new line

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358477: Correctly check if a warning message lacks a 
trailing new line (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60653?vs=195022&id=195313#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60653/new/

https://reviews.llvm.org/D60653

Files:
  lldb/trunk/source/Core/Module.cpp


Index: lldb/trunk/source/Core/Module.cpp
===
--- lldb/trunk/source/Core/Module.cpp
+++ lldb/trunk/source/Core/Module.cpp
@@ -1120,7 +1120,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
@@ -1152,7 +1152,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 strm.PutCString("The debug session should be aborted as the original "
@@ -1178,7 +1178,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogWarning, "%s", strm.GetData());


Index: lldb/trunk/source/Core/Module.cpp
===
--- lldb/trunk/source/Core/Module.cpp
+++ lldb/trunk/source/Core/Module.cpp
@@ -1120,7 +1120,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
@@ -1152,7 +1152,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 strm.PutCString("The debug session should be aborted as the original "
@@ -1178,7 +1178,7 @@
 const int format_len = strlen(format);
 if (format_len > 0) {
   const char last_char = format[format_len - 1];
-  if (last_char != '\n' || last_char != '\r')
+  if (last_char != '\n' && last_char != '\r')
 strm.EOL();
 }
 Host::SystemLog(Host::eSystemLogWarning, "%s", strm.GetData());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358479 - Fix typo in ArmUnwindInfo::GetUnwindPlan

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Tue Apr 16 01:06:56 2019
New Revision: 358479

URL: http://llvm.org/viewvc/llvm-project?rev=358479&view=rev
Log:
Fix typo in ArmUnwindInfo::GetUnwindPlan

Summary:
As reported in LLVM bug 41486, the check `(byte1 & 0xf8) == 0xc0` is wrong. We 
want to check for `11010nnn`,
so the proper value we want to compare against is `0xd0` (`0xc0` would check 
for the value `11000nnn` which we
already checked for above as described in the bug report).

Reviewers: #lldb, jasonmolenda

Reviewed By: #lldb, jasonmolenda

Subscribers: jasonmolenda, javed.absar, kristof.beyls, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D60655

Modified:
lldb/trunk/source/Symbol/ArmUnwindInfo.cpp

Modified: lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ArmUnwindInfo.cpp?rev=358479&r1=358478&r2=358479&view=diff
==
--- lldb/trunk/source/Symbol/ArmUnwindInfo.cpp (original)
+++ lldb/trunk/source/Symbol/ArmUnwindInfo.cpp Tue Apr 16 01:06:56 2019
@@ -304,7 +304,7 @@ bool ArmUnwindInfo::GetUnwindPlan(Target
   // 11001yyy
   // Spare (yyy != 000, 001)
   return false;
-} else if ((byte1 & 0xf8) == 0xc0) {
+} else if ((byte1 & 0xf8) == 0xd0) {
   // 11010nnn
   // Pop VFP double-precision registers D[8]-D[8+nnn] saved (as if) by
   // FSTMFDD (see remark d)


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60655: Fix typo in ArmUnwindInfo::GetUnwindPlan

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358479: Fix typo in ArmUnwindInfo::GetUnwindPlan (authored 
by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60655?vs=195029&id=195322#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60655/new/

https://reviews.llvm.org/D60655

Files:
  lldb/trunk/source/Symbol/ArmUnwindInfo.cpp


Index: lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
===
--- lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
+++ lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
@@ -304,7 +304,7 @@
   // 11001yyy
   // Spare (yyy != 000, 001)
   return false;
-} else if ((byte1 & 0xf8) == 0xc0) {
+} else if ((byte1 & 0xf8) == 0xd0) {
   // 11010nnn
   // Pop VFP double-precision registers D[8]-D[8+nnn] saved (as if) by
   // FSTMFDD (see remark d)


Index: lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
===
--- lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
+++ lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
@@ -304,7 +304,7 @@
   // 11001yyy
   // Spare (yyy != 000, 001)
   return false;
-} else if ((byte1 & 0xf8) == 0xc0) {
+} else if ((byte1 & 0xf8) == 0xd0) {
   // 11010nnn
   // Pop VFP double-precision registers D[8]-D[8+nnn] saved (as if) by
   // FSTMFDD (see remark d)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:61
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);
+

Is there a specific reason you used this particular encoding of the UUID, or 
did you do that just because it was the easiest? I am asking because I have a 
reason to have this use a somewhat different encoding. :)

Let me elaborate: I think there are two things we can want from the UUID here: 
The first one is for it to match the UUID encoding we get from other sources 
(so that they can agree on whether they are talking about the same binary). The 
second one is for the uuid encoding to match the "native" UUID format of the 
given platform.

Right now, this implementation achieves neither. :) It fails the second 
criterion because the UUID strings comes out in different endianness than what 
the windows native tools produce (I'm using `dumpbin` as reference here.). And 
it also fails the first one, because e.g. minidump reading code parses the UUID 
differently .

Now, for windows, these two criteria are slightly at odds with one another. In 
order to fully match the dumpbin format, we would need to have some kind of a 
special field for the "age" bit. But lldb has no such concept, and there 
doesn't seem to be much need to introduce it. However, including the "age" 
field in the "uuid" seems like the right thing to do, as two files with 
different "ages" should be considered different for debug info matching 
purposes (at least, that's what my limited understanding of pdbs tells me. if 
some of this is wrong, please let me know). So, in  I made a somewhat 
arbitrary decision to attach the age field to the UUID in big endian.
That's the format that made most sense to me, though that can certainly be 
changed (the most important thing is for these things to stay in sync).

So, if you have a reason to use a different encoding, please let me know, so we 
can agree on a consistent implementation. Otherwise, could you change this to 
use the same UUID format as the minidump parser?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56229/new/

https://reviews.llvm.org/D56229



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

[Anton Kolesov via Phabricator via lldb-commits]

anton.kolesov added inline comments.



Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:420
+  std::unique_ptr pPathBuffer(new char[PATH_MAX]);
+  lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX);
 

clayborg wrote:
> There is a variant of FileSpec::GetPath() that returns a std::string:
> ```
> std::string path = lineEntry.GetFileSpec().GetPath();
> ```
> Then you can get rid of the code above:
> ```
> std::unique_ptr pPathBuffer(new char[PATH_MAX]);
> ```
LLDB MI uses SBFileSpec rather then FileSpec, and SBFileSpec doesn't have such 
a function.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59015/new/

https://reviews.llvm.org/D59015



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358499 - test/PECOFF: Remove REQUIRES: system-windows

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr 16 07:51:27 2019
New Revision: 358499

URL: http://llvm.org/viewvc/llvm-project?rev=358499&view=rev
Log:
test/PECOFF: Remove REQUIRES: system-windows

These tests run fine on non-windows platforms too. Instead I add
REQUIRES: lld, as that is what they really require.

Modified:
lldb/trunk/lit/Modules/PECOFF/dep-modules.yaml
lldb/trunk/lit/Modules/PECOFF/export-dllfunc.yaml

Modified: lldb/trunk/lit/Modules/PECOFF/dep-modules.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/PECOFF/dep-modules.yaml?rev=358499&r1=358498&r2=358499&view=diff
==
--- lldb/trunk/lit/Modules/PECOFF/dep-modules.yaml (original)
+++ lldb/trunk/lit/Modules/PECOFF/dep-modules.yaml Tue Apr 16 07:51:27 2019
@@ -1,4 +1,4 @@
-# REQUIRES: system-windows
+# REQUIRES: lld
 # RUN: yaml2obj < %p/export-dllfunc.yaml > %t.export-dllfunc.obj
 # RUN: yaml2obj < %s > %t.obj
 #

Modified: lldb/trunk/lit/Modules/PECOFF/export-dllfunc.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/PECOFF/export-dllfunc.yaml?rev=358499&r1=358498&r2=358499&view=diff
==
--- lldb/trunk/lit/Modules/PECOFF/export-dllfunc.yaml (original)
+++ lldb/trunk/lit/Modules/PECOFF/export-dllfunc.yaml Tue Apr 16 07:51:27 2019
@@ -1,4 +1,4 @@
-# REQUIRES: system-windows
+# REQUIRES: lld
 # RUN: yaml2obj < %s > %t.obj
 #
 # RUN: lld-link /machine:x64 /out:%t.dll /noentry /nodefaultlib /dll %t.obj 
/export:DllFunc


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358500 - Breakpad: Match the new UUID algorithm in minidumps

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr 16 07:51:47 2019
New Revision: 358500

URL: http://llvm.org/viewvc/llvm-project?rev=358500&view=rev
Log:
Breakpad: Match the new UUID algorithm in minidumps

D59433 and D60501 changed the way UUIDs are computed from minidump
files. This was done to synchronize the U(G)UID representation with the
native tools of given platforms, but it created a mismatch between
minidumps and breakpad files.

This updates the breakpad algorithm to match the one found in minidumps,
and also adds a couple of tests which should fail if these two ever get
out of sync. Incidentally, this means that the module id in the breakpad
files is almost identical to our notion of UUIDs, so the computation
algorithm can be somewhat simplified.

Added:
lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.syms
lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.yaml
lldb/trunk/lit/Modules/Breakpad/uuid-matching-mac.test

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/fizzbuzz.syms
Modified:
lldb/trunk/lit/Modules/Breakpad/breakpad-identification.test

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Added: lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.syms?rev=358500&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.syms (added)
+++ lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.syms Tue Apr 16 
07:51:47 2019
@@ -0,0 +1,2 @@
+MODULE mac x86_64 A0AB76409C3B3A279E521045D84FA2DC0 a.out
+FUNC f90 1b 0 main

Added: lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.yaml?rev=358500&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.yaml (added)
+++ lldb/trunk/lit/Modules/Breakpad/Inputs/uuid-matching-mac.yaml Tue Apr 16 
07:51:47 2019
@@ -0,0 +1,59 @@
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x8003
+  filetype:0x0002
+  ncmds:   14
+  sizeofcmds:  744
+  flags:   0x00200085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+maxprot: 7
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x00010F90
+size:27
+offset:  0x0F90
+align:   4
+reloff:  0x
+nreloc:  0
+flags:   0x8400
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:A0AB7640-9C3B-3A27-9E52-1045D84FA2DC
+  - cmd: LC_BUILD_VERSION
+cmdsize: 32
+platform:1
+minos:   658944
+sdk: 658944
+ntools:  1
+Tools:   
+  - tool:3
+version: 29491968
+...

Modified: lldb/trunk/lit/Modules/Breakpad/breakpad-identification.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/breakpad-identification.test?rev=358500&r1=358499&r2=358500&view=diff
==
--- lldb/trunk/lit/Modules/Breakpad/breakpad-identification.test (original)
+++ lldb/trunk/lit/Modules/Breakpad/breakpad-identification.test Tue Apr 16 
07:51:47 2019
@@ -15,7 +15,7 @@ LINUX: Strata: user
 
 MAC: Plugin name: breakpad
 MAC: Architecture: x86_64--macosx
-MAC: UUID: 680E8CD9-8920-1BAA-EACD-6A8C1F16707B
+MAC: UUID: D98C0E68-2089-AA1B-EACD-6A8C1F16707B
 MAC: Executable: false
 MAC: Stripped: false
 MAC: Type: debug info
@@ -23,7 +23,7 @@ MAC: Strata: user
 
 WINDOWS: Plugin name: breakpad
 WINDOWS: Architecture: i386--windows
-WINDOWS: UUID: 5716C9A0-B580-0949-81A1-925EA62165C0-0100
+WINDOWS: UUID: A0C91657-80B5-4909-81A1-925EA62165C0-0

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:60
+  llvm::StringRef pdb_file;
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);

labath wrote:
> Can `getDebugPDBInfo` succeed and still return a null pdb_info? If not, can 
> we delete the second part?
> 
> Instead I believe you should check the CVSignature field of the returned 
> struct to see that it indeed contains a PDB70 record.
If the exe/dll is built without any debug info, the function succeeds and still 
returns null pdb_info.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:61
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);
+

labath wrote:
> Is there a specific reason you used this particular encoding of the UUID, or 
> did you do that just because it was the easiest? I am asking because I have a 
> reason to have this use a somewhat different encoding. :)
> 
> Let me elaborate: I think there are two things we can want from the UUID 
> here: The first one is for it to match the UUID encoding we get from other 
> sources (so that they can agree on whether they are talking about the same 
> binary). The second one is for the uuid encoding to match the "native" UUID 
> format of the given platform.
> 
> Right now, this implementation achieves neither. :) It fails the second 
> criterion because the UUID strings comes out in different endianness than 
> what the windows native tools produce (I'm using `dumpbin` as reference 
> here.). And it also fails the first one, because e.g. minidump reading code 
> parses the UUID differently .
> 
> Now, for windows, these two criteria are slightly at odds with one another. 
> In order to fully match the dumpbin format, we would need to have some kind 
> of a special field for the "age" bit. But lldb has no such concept, and there 
> doesn't seem to be much need to introduce it. However, including the "age" 
> field in the "uuid" seems like the right thing to do, as two files with 
> different "ages" should be considered different for debug info matching 
> purposes (at least, that's what my limited understanding of pdbs tells me. if 
> some of this is wrong, please let me know). So, in  I made a somewhat 
> arbitrary decision to attach the age field to the UUID in big endian.
> That's the format that made most sense to me, though that can certainly be 
> changed (the most important thing is for these things to stay in sync).
> 
> So, if you have a reason to use a different encoding, please let me know, so 
> we can agree on a consistent implementation. Otherwise, could you change this 
> to use the same UUID format as the minidump parser?
You are right. The encoding of MS struct GUID and the PDB70DebugInfo::Signature 
are different.  Can UUID format and the method to yield it from minidump parser 
be available in class COFFObjectFile?




Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:866
+
+  m_uuid = GetCoffUUID(GetFileSpec());
+  return m_uuid;

labath wrote:
> ObjectFilePECOFF already has a `llvm::object::Binary` created for the 
> underlying file. I think it's super-wasteful (and potentially racy, etc.) to 
> create a second one just to read out it's GUID. If you make a second version 
> of this function, taking a `Binary` (and have the FileSpec version delegate 
> to that), then you can avoid this.
In addition, it is possible to simplify ObjectFilePECOFF 
::GetModuleSpecifications API with such Binary.  In this sense, none of the 
arguments, like data_sp, data_offset will be used. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56229/new/

https://reviews.llvm.org/D56229



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:419
+  // Get a full path to the file.
+  std::unique_ptr pPathBuffer(new char[PATH_MAX]);
+  lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX);

Confused as to why we are calling malloc and free here for pPathBuffer? Why not 
just:
```
char pPathBuffer[PATH_MAX];
```


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59015/new/

https://reviews.llvm.org/D59015



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:60
+  llvm::StringRef pdb_file;
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);

Hui wrote:
> labath wrote:
> > Can `getDebugPDBInfo` succeed and still return a null pdb_info? If not, can 
> > we delete the second part?
> > 
> > Instead I believe you should check the CVSignature field of the returned 
> > struct to see that it indeed contains a PDB70 record.
> If the exe/dll is built without any debug info, the function succeeds and 
> still returns null pdb_info.
Ah, ok. Thanks for explaining.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:61
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+return UUID::fromOptionalData(pdb_info->PDB70.Signature);
+

Hui wrote:
> labath wrote:
> > Is there a specific reason you used this particular encoding of the UUID, 
> > or did you do that just because it was the easiest? I am asking because I 
> > have a reason to have this use a somewhat different encoding. :)
> > 
> > Let me elaborate: I think there are two things we can want from the UUID 
> > here: The first one is for it to match the UUID encoding we get from other 
> > sources (so that they can agree on whether they are talking about the same 
> > binary). The second one is for the uuid encoding to match the "native" UUID 
> > format of the given platform.
> > 
> > Right now, this implementation achieves neither. :) It fails the second 
> > criterion because the UUID strings comes out in different endianness than 
> > what the windows native tools produce (I'm using `dumpbin` as reference 
> > here.). And it also fails the first one, because e.g. minidump reading code 
> > parses the UUID differently .
> > 
> > Now, for windows, these two criteria are slightly at odds with one another. 
> > In order to fully match the dumpbin format, we would need to have some kind 
> > of a special field for the "age" bit. But lldb has no such concept, and 
> > there doesn't seem to be much need to introduce it. However, including the 
> > "age" field in the "uuid" seems like the right thing to do, as two files 
> > with different "ages" should be considered different for debug info 
> > matching purposes (at least, that's what my limited understanding of pdbs 
> > tells me. if some of this is wrong, please let me know). So, in  I 
> > made a somewhat arbitrary decision to attach the age field to the UUID in 
> > big endian.
> > That's the format that made most sense to me, though that can certainly be 
> > changed (the most important thing is for these things to stay in sync).
> > 
> > So, if you have a reason to use a different encoding, please let me know, 
> > so we can agree on a consistent implementation. Otherwise, could you change 
> > this to use the same UUID format as the minidump parser?
> You are right. The encoding of MS struct GUID and the 
> PDB70DebugInfo::Signature are different.  Can UUID format and the method to 
> yield it from minidump parser be available in class COFFObjectFile?
> 
I don't think you can put that in the COFFObjectFile, as it lives in llvm, and 
the UUID class is an lldb concept. It might be possible to put some utility 
function into llvm to help with that, but it's not clear to me how exactly that 
would look (and that would need to be a separate patch with a separate review).

What would make kind of sense is to add another factory function to the `UUID` 
class in lldb (`UUID::fromCvRecord` ?), which both ObjectFilePECOFF and 
ProcessMinidump could call into. However, the problem with that is that the 
definition of the CvRecord is in llvm/Object, and it seems silly to have 
lldb/Utility depend on that just to pull the single struct. I think it would 
make sense to move this struct into llvm/BinaryFormat (which lldb/Utility 
already depends on) and then everything would be fine. If you want to try that 
out, then go ahead, but I don't think that's really necessary here. (swapping 
the bytes around should be just a couple of LOC).



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:866
+
+  m_uuid = GetCoffUUID(GetFileSpec());
+  return m_uuid;

Hui wrote:
> labath wrote:
> > ObjectFilePECOFF already has a `llvm::object::Binary` created for the 
> > underlying file. I think it's super-wasteful (and potentially racy, etc.) 
> > to create a second one just to read out it's GUID. If you make a second 
> > version of this function, taking a `Binary` (and have the FileSpec version 
> > delegate to that), then you can avoid this.
> In addition, it is possible to simplify ObjectFilePECOFF 
> ::GetModuleSpecifications API with such Binary.  In this sense, none of the 
> arguments, like data_sp, data_offset will be used. 
Yeah, I noticed that too, but I didn't want 

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: JDevlieghere, friss.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

Saves some build times, and they're not part of the usual
developer workflow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60780

Files:
  lldb/tools/CMakeLists.txt


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,16 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+if (LLDB_ENABLE_TOOL_INSTR)
+  add_subdirectory(lldb-instr)
+endif()
+
+if (LLDB_ENABLE_TOOL_VSCODE)
+  add_subdirectory(lldb-vscode)
+endif()
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,16 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+if (LLDB_ENABLE_TOOL_INSTR)
+  add_subdirectory(lldb-instr)
+endif()
+
+if (LLDB_ENABLE_TOOL_VSCODE)
+  add_subdirectory(lldb-vscode)
+endif()
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358508 - Fix symtab-macho.test broken by r358500

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr 16 09:57:41 2019
New Revision: 358508

URL: http://llvm.org/viewvc/llvm-project?rev=358508&view=rev
Log:
Fix symtab-macho.test broken by r358500

Put the correct UUID string into the breakpad file.

Modified:
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms
lldb/trunk/lit/SymbolFile/Breakpad/symtab-macho.test

Modified: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms?rev=358508&r1=358507&r2=358508&view=diff
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms (original)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms Tue Apr 16 
09:57:41 2019
@@ -1,2 +1,2 @@
-MODULE mac x86_64 601705B3B1227B7D39F9240E077D625B0 mac.out
+MODULE mac x86_64 B305176022B17D7B39F9240E077D625B0 mac.out
 PUBLIC ff0 0 _start

Modified: lldb/trunk/lit/SymbolFile/Breakpad/symtab-macho.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/symtab-macho.test?rev=358508&r1=358507&r2=358508&view=diff
==
--- lldb/trunk/lit/SymbolFile/Breakpad/symtab-macho.test (original)
+++ lldb/trunk/lit/SymbolFile/Breakpad/symtab-macho.test Tue Apr 16 09:57:41 
2019
@@ -1,5 +1,5 @@
 # RUN: yaml2obj %S/Inputs/basic-macho.yaml > %T/symtab-macho.out
-# RUN: %lldb %T/symtab-macho.out -o "target symbols add -s symtab-macho.out 
%S/Inputs/symtab-macho.syms" \
+# RUN: %lldb %T/symtab-macho.out -o "target symbols add 
%S/Inputs/symtab-macho.syms" \
 # RUN:   -s %s | FileCheck %s
 
 image dump symtab symtab-macho.out


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

We need to define these variables in the cache and make sure the test suite 
knows about them.

For lldb-instr for example, which lives in lit/tools/lldb-instr, you'd have to 
configure `lit.site.cfg.py.in` to know about wheter it's enabled or not (for 
example `config.have_lldb_instr`) and then check for this configuration 
variable in the lit.site.cfg in the lldb-instr test dir.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

OK, on it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60737: [lldb] Don't filter variable list when doing a lookup by mangled name in SymbolFileDWARF::FindGlobalVariables

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek updated this revision to Diff 195413.
kubamracek added a comment.

Adding a test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60737/new/

https://reviews.llvm.org/D60737

Files:
  packages/Python/lldbsuite/test/lang/cpp/global_variables/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
  packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2036,6 +2036,7 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
+  bool name_is_mangled = (bool)Mangled(name);
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
@@ -2085,7 +2086,8 @@
  &variables);
   while (pruned_idx < variables.GetSize()) {
 VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+if (name_is_mangled ||
+var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
   ++pruned_idx;
 else
   variables.RemoveVariableAtIndex(pruned_idx);
Index: packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
===
--- packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
@@ -0,0 +1,17 @@
+//===-- main.c --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include 
+
+namespace abc {
+	int g_file_global_int = 42;
+}
+
+int main (int argc, char const *argv[])
+{
+return abc::g_file_global_int; // Set break point at this line.   break $source:$line; continue; var -global g_a -global g_global_int
+}
Index: packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
===
--- packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
+++ packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
@@ -0,0 +1,58 @@
+"""Show global variables and check that they do indeed have global scopes."""
+
+from __future__ import print_function
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class GlobalVariablesCppTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.source = 'main.cpp'
+self.line = line_number(self.source, '// Set break point at this line.')
+self.shlib_names = ["a"]
+
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
+@expectedFailureAll(oslist=["linux"], archs=["aarch64"], bugnumber="llvm.org/pr37301")
+def test_without_process(self):
+"""Test that static initialized variables can be inspected without
+process."""
+self.build()
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, self.source, self.line, num_expected_locations=1, loc_exact=True)
+
+# Now launch the process, and do not stop at entry point.
+process = target.LaunchSimple(None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Check that we can access g_file_global_int by its name
+self.expect("target variable g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+self.expect("target variable abc::g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+self.expect("target variable xyz::g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+error=True, substrs=['can\'t find global variable'])
+
+# Check that we can access g_file_global_int by its mangled name
+addr = target.EvaluateExpression("&abc::g_file_global_int").GetValueAsUnsigned()
+self.assertTrue(addr != 0)
+mangled = lldb.SBAddress(addr, 

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

LLDB_TOOL_$TOOL_BUILD would be a better name for consistency with llvm. In 
fact, if we started using the llvm cmake macros like `add_llvm_subdirectory`, 
it would handle this automatically, including creating the cache variable.

The only difference would be that the variable would default to "On", whereas 
you seem to want default-off. (I'm a bit torn on whether this is good. On one 
hand, I understand the desire to not build this stuff, but on the other, this 
creates an inconsistency with llvm, which builds everything by default, and 
consistency is a good thing.)

Also, I'd like to nominate lldb-mi as an additional optional feature. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60737: [lldb] Don't filter variable list when doing a lookup by mangled name in SymbolFileDWARF::FindGlobalVariables

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

One picky comment about the test, otherwise this looks good to me.  Pavel had 
his hands in this code most recently, however, so we should wait on his opinion.




Comment at: 
packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py:31-41
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, self.source, self.line, num_expected_locations=1, 
loc_exact=True)
+

Can you use lldbutil.run_to_source_breakpoint to do this?  Like:

lldbutil.run_to_source_breakpoint(self, "//Set break point at this line", 
self.source)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60737/new/

https://reviews.llvm.org/D60737



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D60780#1468927 , @labath wrote:

> LLDB_TOOL_$TOOL_BUILD would be a better name for consistency with llvm. In 
> fact, if we started using the llvm cmake macros like `add_llvm_subdirectory`, 
> it would handle this automatically, including creating the cache variable.
>
> The only difference would be that the variable would default to "On", whereas 
> you seem to want default-off. (I'm a bit torn on whether this is good. On one 
> hand, I understand the desire to not build this stuff, but on the other, this 
> creates an inconsistency with llvm, which builds everything by default, and 
> consistency is a good thing.)
>
> Also, I'd like to nominate lldb-mi as an additional optional feature. :)


+1 on everything Pavel said. I'd also prefer to keep this on by default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60737: [lldb] Don't filter variable list when doing a lookup by mangled name in SymbolFileDWARF::FindGlobalVariables

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek updated this revision to Diff 195422.
kubamracek added a comment.

Updating test to use run_to_source_breakpoint


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60737/new/

https://reviews.llvm.org/D60737

Files:
  packages/Python/lldbsuite/test/lang/cpp/global_variables/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
  packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2036,6 +2036,7 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
+  bool name_is_mangled = (bool)Mangled(name);
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
@@ -2085,7 +2086,8 @@
  &variables);
   while (pruned_idx < variables.GetSize()) {
 VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+if (name_is_mangled ||
+var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
   ++pruned_idx;
 else
   variables.RemoveVariableAtIndex(pruned_idx);
Index: packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
===
--- packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/global_variables/main.cpp
@@ -0,0 +1,17 @@
+//===-- main.c --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include 
+
+namespace abc {
+	int g_file_global_int = 42;
+}
+
+int main (int argc, char const *argv[])
+{
+return abc::g_file_global_int; // Set break point at this line.
+}
Index: packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
===
--- packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
+++ packages/Python/lldbsuite/test/lang/cpp/global_variables/TestGlobalVariables.py
@@ -0,0 +1,42 @@
+"""Test that C++ global variables can be inspected by name and also their mangled name."""
+
+from __future__ import print_function
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class GlobalVariablesCppTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = lldb.SBFileSpec('main.cpp')
+
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
+@expectedFailureAll(oslist=["linux"], archs=["aarch64"], bugnumber="llvm.org/pr37301")
+def test(self):
+self.build()
+
+(target, _, _, _) = lldbutil.run_to_source_breakpoint(self, "// Set break point at this line.", self.source)
+
+# Check that we can access g_file_global_int by its name
+self.expect("target variable g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+self.expect("target variable abc::g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+self.expect("target variable xyz::g_file_global_int", VARIABLES_DISPLAYED_CORRECTLY,
+error=True, substrs=['can\'t find global variable'])
+
+# Check that we can access g_file_global_int by its mangled name
+addr = target.EvaluateExpression("&abc::g_file_global_int").GetValueAsUnsigned()
+self.assertTrue(addr != 0)
+mangled = lldb.SBAddress(addr, target).GetSymbol().GetMangledName()
+self.assertTrue(mangled != None)
+gv = target.FindFirstGlobalVariable(mangled)
+self.assertTrue(gv.IsValid())
+self.assertEqual(gv.GetName(), "abc::g_file_global_int")
+self.assertEqual(gv.GetValueAsUnsigned(), 42)
Index: packages/Python/lldbsuite/test/lang/cpp/global_variables/Makefile
===
--- packages/Python/lldbsuite/test/lang/cpp/global_variables/Makefile
+++ packages/Python/lldbsuite/test/lang/cpp/global_variables/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
___
lldb-commits mailing list
lldb-

[Lldb-commits] [PATCH] D60519: [Windows] Dump more information about access violation exception

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

I ran local tests with this and a couple of the tests from the lldb suite 
failed:

  lldb-Suite :: functionalities/inferior-crashing/TestInferiorCrashing.py
  lldb-Suite :: functionalities/thread/crash_during_step/TestCrashDuringStep.py

Here's the error from one of them (they are the same):

  ##[error]CUSTOMBUILD(0,0): Error : test_step_inst_with_dwarf 
(TestCrashDuringStep.CrashDuringStepTestCase)
   1>CUSTOMBUILD : error : test_step_inst_with_dwarf 
(TestCrashDuringStep.CrashDuringStepTestCase) 
[e:\_work\32\b\LLVMBuild\check-all.vcxproj]
   
  Test thread creation during step-inst handling.
   
   
--
   
   Traceback (most recent call last):
   
 File 
"E:\_work\32\s\llvm\tools\lldb\packages\Python\lldbsuite\test\lldbtest.py", 
line 1746, in test_method
   
   return attrvalue(self)
   
 File 
"E:\_work\32\s\llvm\tools\lldb\packages\Python\lldbsuite\test\decorators.py", 
line 114, in wrapper
   
   func(*args, **kwargs)
   
 File 
"E:\_work\32\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\thread\crash_during_step\TestCrashDuringStep.py",
 line 51, in test_step_inst_with
   
   while process.GetState() == lldb.eStateStopped and not 
lldbutil.is_thread_crashed(self, thread):
   
 File 
"E:\_work\32\s\llvm\tools\lldb\packages\Python\lldbsuite\test\lldbutil.py", 
line 740, in is_thread_crashed
   
   return "Exception 0xc005" in thread.GetStopDescription(100)
   
 File 
"E:\_work\32\b\LLVMBuild\Release\lib\site-packages\lldb\__init__.py", line 
11874, in GetStopDescription
   
   return _lldb.SBThread_GetStopDescription(self, dst)
   
   UnicodeDecodeError: 'utf-8' codec can't decode byte 0x97 in position 
104: invalid start byte
   
   Config=x86_64-E:\_work\32\b\LLVMBuild\Release\bin\clang.exe


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60519/new/

https://reviews.llvm.org/D60519



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 195443.
davide added a comment.

updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/lit/lit.site.cfg.py.in
  lldb/lit/tools/lldb-instr/lit.local.cfg
  lldb/tools/CMakeLists.txt


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_optional_tool_subdirectory(lldb-instr)
+add_lldb_optional_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lldb/lit/tools/lldb-instr/lit.local.cfg
===
--- /dev/null
+++ lldb/lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if config.has_lldb_instr == "OFF":
+config.unsupported = True
Index: lldb/lit/lit.site.cfg.py.in
===
--- lldb/lit/lit.site.cfg.py.in
+++ lldb/lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.has_lldb_instr = "@LLDB_TOOL_LLDB_INSTR_BUILD@"
+config.has_lldb_vscode = "@LLDB_TOOL_LLDB_VSCODE_BUILD@"
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_optional_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_optional_tool_subdirectory(lldb-instr)
+add_lldb_optional_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lldb/lit/tools/lldb-instr/lit.local.cfg
===
--- /dev/null
+++ lldb/lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if config.has_lldb_instr == "OFF":
+config.unsupported = True
Index: lldb/lit/lit.site.cfg.py.in
===
--- lldb/lit/lit.site.cfg.py.in
+++ lldb/lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.has_lldb_instr = "@LLDB_TOOL_LLDB_INSTR_BUILD@"
+config.has_lldb_vscode = "@LLDB_TOOL_LLDB_VSCODE_BUILD@"
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_optional_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/cmake/modules/AddLLDB.cmake:145
+
+macro(add_lldb_optional_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})

Let's drop optional from the name



Comment at: lldb/lit/lit.site.cfg.py.in:21
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.has_lldb_instr = "@LLDB_TOOL_LLDB_INSTR_BUILD@"
+config.has_lldb_vscode = "@LLDB_TOOL_LLDB_VSCODE_BUILD@"

You can use `llvm_canonicalize_cmake_booleans` to make it a real boolean 
similar to config.have_zlib

Also, maybe use "have" instead of "has" for consistency? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358525 - [debugserver] Relax the codesigning identity check

[Frederic Riss via lldb-commits]

Author: friss
Date: Tue Apr 16 13:54:42 2019
New Revision: 358525

URL: http://llvm.org/viewvc/llvm-project?rev=358525&view=rev
Log:
[debugserver] Relax the codesigning identity check

In an effort to help new LLDB developers, we added checks and messaging around
the selection of your codesigning identity on macOS. While helpful, it is not
actually correct. It's perfectly valid to codesign with an identity that is
not named lldb_codesign. Currently this fails the build.

This patch keeps a warning that informs developers how to setup lldb_codesign
and how to pass it to cmake, but it allows the build to proceed with a
different identity.

Modified:
lldb/trunk/tools/debugserver/source/CMakeLists.txt

Modified: lldb/trunk/tools/debugserver/source/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/CMakeLists.txt?rev=358525&r1=358524&r2=358525&view=diff
==
--- lldb/trunk/tools/debugserver/source/CMakeLists.txt (original)
+++ lldb/trunk/tools/debugserver/source/CMakeLists.txt Tue Apr 16 13:54:42 2019
@@ -93,18 +93,14 @@ elseif(NOT LLDB_NO_DEBUGSERVER)
   # Default case: on Darwin we need the right code signing ID.
   # See lldb/docs/code-signing.txt for details.
   if(CMAKE_HOST_APPLE AND NOT LLVM_CODESIGNING_IDENTITY STREQUAL 
"lldb_codesign")
-set(problem "Cannot code sign debugserver with LLVM_CODESIGNING_IDENTITY 
'${LLVM_CODESIGNING_IDENTITY}'.")
-set(advice "Pass -DLLDB_CODESIGN_IDENTITY=lldb_codesign to override the 
LLVM value for debugserver.")
-if(system_debugserver)
-  set(effect "Will fall back to system's debugserver.")
-  set(use_system_debugserver ON)
-else()
-  set(effect "debugserver will not be available.")
-endif()
-message(WARNING "${problem} ${effect} ${advice}")
-  else()
-set(build_and_sign_debugserver ON)
+message(WARNING "Codesigning debugserver with identity 
${LLVM_CODESIGNING_IDENTITY}. "
+"The usual setup uses the \"lldb_codesign\" identity 
created with "
+"scripts/macos-setup-codesign.sh. As a result your 
debugserver might "
+"not be able to attach to processes.\n"
+"Pass -DLLDB_CODESIGN_IDENTITY=lldb_codesign to use the 
development "
+"signing identity.")
   endif()
+  set(build_and_sign_debugserver ON)
 endif()
 
 # TODO: We don't use the $ generator expression here,


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 195454.
davide added a comment.

Jonas' comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/lit/CMakeLists.txt
  lldb/lit/lit.site.cfg.py.in
  lldb/lit/tools/lldb-instr/lit.local.cfg
  lldb/tools/CMakeLists.txt


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_tool_subdirectory(lldb-instr)
+add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lldb/lit/tools/lldb-instr/lit.local.cfg
===
--- /dev/null
+++ lldb/lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if not config.have_lldb_instr:
+config.unsupported = True
Index: lldb/lit/lit.site.cfg.py.in
===
--- lldb/lit/lit.site.cfg.py.in
+++ lldb/lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.have_lldb_instr = @LLDB_TOOL_LLDB_INSTR_BUILD@
+config.have_lldb_vscode = @LLDB_TOOL_LLDB_VSCODE_BUILD@
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lldb/lit/CMakeLists.txt
===
--- lldb/lit/CMakeLists.txt
+++ lldb/lit/CMakeLists.txt
@@ -36,6 +36,8 @@
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
+  LLDB_TOOL_LLDB_INSTR_BUILD
+  LLDB_TOOL_LLDB_VSCODE_BUILD
   LLVM_ENABLE_ZLIB
   LLDB_IS_64_BITS)
 
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()


Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_tool_subdirectory(lldb-instr)
+add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lldb/lit/tools/lldb-instr/lit.local.cfg
===
--- /dev/null
+++ lldb/lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if not config.have_lldb_instr:
+config.unsupported = True
Index: lldb/lit/lit.site.cfg.py.in
===
--- lldb/lit/lit.site.cfg.py.in
+++ lldb/lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.have_lldb_instr = @LLDB_TOOL_LLDB_INSTR_BUILD@
+config.have_lldb_vscode = @LLDB_TOOL_LLDB_VSCODE_BUILD@
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lldb/lit/CMakeLists.txt
===
--- lldb/lit/CMakeLists.txt
+++ lldb/lit/CMakeLists.txt
@@ -36,6 +36,8 @@
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
+  LLDB_TOOL_LLDB_INSTR_BUILD
+  LLDB_TOOL_LLDB_VSCODE_BUILD
   LLVM_ENABLE_ZLIB
   LLDB_IS_64_BITS)
 
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358528 - [tools] Make vscode and lldb-instr optional.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Apr 16 14:15:28 2019
New Revision: 358528

URL: http://llvm.org/viewvc/llvm-project?rev=358528&view=rev
Log:
[tools] Make vscode and lldb-instr optional.

Summary:
Saves some build times, and they're not part of the usual
developer workflow.

Reviewers: JDevlieghere, friss

Subscribers: mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D60780

Added:
lldb/trunk/lit/tools/lldb-instr/lit.local.cfg
Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/lit/CMakeLists.txt
lldb/trunk/lit/lit.site.cfg.py.in
lldb/trunk/tools/CMakeLists.txt

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=358528&r1=358527&r2=358528&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Tue Apr 16 14:15:28 2019
@@ -141,6 +141,11 @@ function(add_lldb_executable name)
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=358528&r1=358527&r2=358528&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Tue Apr 16 14:15:28 2019
@@ -36,6 +36,8 @@ endif()
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
+  LLDB_TOOL_LLDB_INSTR_BUILD
+  LLDB_TOOL_LLDB_VSCODE_BUILD
   LLVM_ENABLE_ZLIB
   LLDB_IS_64_BITS)
 

Modified: lldb/trunk/lit/lit.site.cfg.py.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.site.cfg.py.in?rev=358528&r1=358527&r2=358528&view=diff
==
--- lldb/trunk/lit/lit.site.cfg.py.in (original)
+++ lldb/trunk/lit/lit.site.cfg.py.in Tue Apr 16 14:15:28 2019
@@ -18,6 +18,8 @@ config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.have_lldb_instr = @LLDB_TOOL_LLDB_INSTR_BUILD@
+config.have_lldb_vscode = @LLDB_TOOL_LLDB_VSCODE_BUILD@
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is

Added: lldb/trunk/lit/tools/lldb-instr/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-instr/lit.local.cfg?rev=358528&view=auto
==
--- lldb/trunk/lit/tools/lldb-instr/lit.local.cfg (added)
+++ lldb/trunk/lit/tools/lldb-instr/lit.local.cfg Tue Apr 16 14:15:28 2019
@@ -0,0 +1,4 @@
+import sys
+  
+if not config.have_lldb_instr:
+config.unsupported = True

Modified: lldb/trunk/tools/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/CMakeLists.txt?rev=358528&r1=358527&r2=358528&view=diff
==
--- lldb/trunk/tools/CMakeLists.txt (original)
+++ lldb/trunk/tools/CMakeLists.txt Tue Apr 16 14:15:28 2019
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_tool_subdirectory(lldb-instr)
+add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB358528: [tools] Make vscode and lldb-instr optional. 
(authored by davide, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60780?vs=195454&id=195459#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60780/new/

https://reviews.llvm.org/D60780

Files:
  cmake/modules/AddLLDB.cmake
  lit/CMakeLists.txt
  lit/lit.site.cfg.py.in
  lit/tools/lldb-instr/lit.local.cfg
  tools/CMakeLists.txt


Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_tool_subdirectory(lldb-instr)
+add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lit/tools/lldb-instr/lit.local.cfg
===
--- lit/tools/lldb-instr/lit.local.cfg
+++ lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if not config.have_lldb_instr:
+config.unsupported = True
Index: lit/lit.site.cfg.py.in
===
--- lit/lit.site.cfg.py.in
+++ lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.have_lldb_instr = @LLDB_TOOL_LLDB_INSTR_BUILD@
+config.have_lldb_vscode = @LLDB_TOOL_LLDB_VSCODE_BUILD@
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -36,6 +36,8 @@
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
+  LLDB_TOOL_LLDB_INSTR_BUILD
+  LLDB_TOOL_LLDB_VSCODE_BUILD
   LLVM_ENABLE_ZLIB
   LLDB_IS_64_BITS)
 
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()


Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -1,10 +1,11 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
-add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
-add_subdirectory(lldb-vscode)
+
+add_lldb_tool_subdirectory(lldb-instr)
+add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
Index: lit/tools/lldb-instr/lit.local.cfg
===
--- lit/tools/lldb-instr/lit.local.cfg
+++ lit/tools/lldb-instr/lit.local.cfg
@@ -0,0 +1,4 @@
+import sys
+  
+if not config.have_lldb_instr:
+config.unsupported = True
Index: lit/lit.site.cfg.py.in
===
--- lit/lit.site.cfg.py.in
+++ lit/lit.site.cfg.py.in
@@ -18,6 +18,8 @@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.have_lldb_instr = @LLDB_TOOL_LLDB_INSTR_BUILD@
+config.have_lldb_vscode = @LLDB_TOOL_LLDB_VSCODE_BUILD@
 config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -36,6 +36,8 @@
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
+  LLDB_TOOL_LLDB_INSTR_BUILD
+  LLDB_TOOL_LLDB_VSCODE_BUILD
   LLVM_ENABLE_ZLIB
   LLDB_IS_64_BITS)
 
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -141,6 +141,11 @@
   endif()
 endfunction(add_lldb_executable)
 
+
+macro(add_lldb_tool_subdirectory name)
+  add_llvm_subdirectory(LLDB TOOL ${name})
+endmacro()
+
 function(add_lldb_tool name)
   add_lldb_executable(${name} GENERATE_INSTALL ${ARGN})
 endfunction()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.or

[Lldb-commits] [lldb] r358530 - [Process] Fix linux arm64 single step compilation failure

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Apr 16 14:21:28 2019
New Revision: 358530

URL: http://llvm.org/viewvc/llvm-project?rev=358530&view=rev
Log:
[Process] Fix linux arm64 single step compilation failure

This was updated in r356703 to use llvm::sys::RetryAfterSignal, which
comes from llvm/Support/Errno.h. The header wasn't added, so it fails if
you compile for arm64/aarch64.

Modified:
lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp

Modified: lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp?rev=358530&r1=358529&r2=358530&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp Tue Apr 16 
14:21:28 2019
@@ -16,6 +16,7 @@
 #include "NativeProcessLinux.h"
 
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Errno.h"
 
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/linux/Ptrace.h"


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r358533 - [tools] Only build lldb-test when needed.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Apr 16 14:56:07 2019
New Revision: 358533

URL: http://llvm.org/viewvc/llvm-project?rev=358533&view=rev
Log:
[tools] Only build lldb-test when needed.

Modified:
lldb/trunk/tools/CMakeLists.txt

Modified: lldb/trunk/tools/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/CMakeLists.txt?rev=358533&r1=358532&r2=358533&view=diff
==
--- lldb/trunk/tools/CMakeLists.txt (original)
+++ lldb/trunk/tools/CMakeLists.txt Tue Apr 16 14:56:07 2019
@@ -2,7 +2,11 @@ add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
 add_subdirectory(lldb-mi)
-add_subdirectory(lldb-test)
+
+# We want lldb-test to be built only when it's needed,
+# i.e. if a target requires it as dependency. The typical
+# example is `check-lldb`. So, we pass EXCLUDE_FROM_ALL here.
+add_subdirectory(lldb-test EXCLUDE_FROM_ALL)
 
 add_lldb_tool_subdirectory(lldb-instr)
 add_lldb_tool_subdirectory(lldb-vscode)


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

Thanks! Please let me know if it happens again and I'll try my best to debug it.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60153/new/

https://reviews.llvm.org/D60153



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Aaron Smith via Phabricator via lldb-commits]

asmith updated this revision to Diff 195504.
asmith edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56229/new/

https://reviews.llvm.org/D56229

Files:
  lit/Modules/PECOFF/export-dllfunc.yaml
  lit/Modules/PECOFF/uuid.yaml
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -286,6 +286,8 @@
   llvm::Optional m_deps_filespec;
   typedef llvm::object::OwningBinary OWNBINType;
   llvm::Optional m_owningbin;
+
+  lldb_private::UUID m_uuid;
 };
 
 #endif // liblldb_ObjectFilePECOFF_h_
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -40,6 +40,56 @@
 using namespace lldb;
 using namespace lldb_private;
 
+namespace {
+using namespace llvm;
+typedef struct CVInfoPdb70 {
+  // 16-byte GUID
+  struct _Guid {
+support::ulittle32_t Data1;
+support::ulittle16_t Data2;
+support::ulittle16_t Data3;
+uint8_t Data4[8];
+  } Guid;
+
+  support::ulittle32_t Age;
+} CVInfoPdb70;
+
+UUID GetCoffUUID(llvm::object::COFFObjectFile *coff_obj) {
+  if (!coff_obj)
+return UUID();
+
+  const llvm::codeview::DebugInfo *pdb_info = nullptr;
+  llvm::StringRef pdb_file;
+
+  // This part is similar with what has done in minidump parser.
+  if (!coff_obj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) {
+if (pdb_info->PDB70.CVSignature == llvm::OMF::Signature::PDB70) {
+  const uint8_t *sig = pdb_info->PDB70.Signature;
+  CVInfoPdb70 info;
+  info.Guid.Data1 = support::endian::read(sig);
+  sig += 4;
+  info.Guid.Data2 = support::endian::read(sig);
+  sig += 2;
+  info.Guid.Data3 = support::endian::read(sig);
+  sig += 2;
+  memcpy(info.Guid.Data4, sig, 8);
+
+  // Return 20-byte UUID if the Age is not zero
+  if (pdb_info->PDB70.Age) {
+info.Age =
+support::endian::read(&pdb_info->PDB70.Age);
+return UUID::fromOptionalData((uint8_t *)&info, sizeof(info));
+  }
+  // Otherwise return 16-byte GUID
+  return UUID::fromOptionalData((uint8_t *)&info.Guid, sizeof(info.Guid));
+}
+  }
+
+  return UUID();
+}
+
+} // namespace
+
 void ObjectFilePECOFF::Initialize() {
   PluginManager::RegisterPlugin(
   GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
@@ -114,35 +164,41 @@
 lldb::offset_t length, lldb_private::ModuleSpecList &specs) {
   const size_t initial_count = specs.GetSize();
 
-  if (ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
-DataExtractor data;
-data.SetData(data_sp, data_offset, length);
-data.SetByteOrder(eByteOrderLittle);
+  auto binary = llvm::object::createBinary(file.GetPath());
+  if (!binary)
+return initial_count;
 
-dos_header_t dos_header;
-coff_header_t coff_header;
+  if (!binary->getBinary()->isCOFF() &&
+  !binary->getBinary()->isCOFFImportFile())
+return initial_count;
 
-if (ParseDOSHeader(data, dos_header)) {
-  lldb::offset_t offset = dos_header.e_lfanew;
-  uint32_t pe_signature = data.GetU32(&offset);
-  if (pe_signature != IMAGE_NT_SIGNATURE)
-return false;
-  if (ParseCOFFHeader(data, &offset, coff_header)) {
-ArchSpec spec;
-if (coff_header.machine == MachineAmd64) {
-  spec.SetTriple("x86_64-pc-windows");
-  specs.Append(ModuleSpec(file, spec));
-} else if (coff_header.machine == MachineX86) {
-  spec.SetTriple("i386-pc-windows");
-  specs.Append(ModuleSpec(file, spec));
-  spec.SetTriple("i686-pc-windows");
-  specs.Append(ModuleSpec(file, spec));
-} else if (coff_header.machine == MachineArmNt) {
-  spec.SetTriple("arm-pc-windows");
-  specs.Append(ModuleSpec(file, spec));
-}
-  }
-}
+  auto COFFObj =
+  llvm::dyn_cast(binary->getBinary());
+  assert(COFFObj);
+
+  ModuleSpec module_spec(file);
+  ArchSpec &spec = module_spec.GetArchitecture();
+  lldb_private::UUID &uuid = module_spec.GetUUID();
+  if (!uuid.IsValid())
+uuid = GetCoffUUID(COFFObj);
+
+  switch (COFFObj->getMachine()) {
+  case MachineAmd64:
+spec.SetTriple("x86_64-pc-windows");
+specs.Append(module_spec);
+break;
+  case MachineX86:
+spec.SetTriple("i386-pc-windows");
+specs.Append(module_spec);
+spec.SetTriple("i686-pc-windows");
+specs.Append(module_spec);
+break;
+  case MachineArmNt:
+spec.SetTriple("arm-pc-windows");
+specs.Append(module_spec);
+break;
+  default:
+break;
   }
 
   return specs.GetSize() - initial_count;
@@ -828,7 +884

[Lldb-commits] [lldb] r358550 - Clear the output string passed to GetHostName()

[Aaron Smith via lldb-commits]

Author: asmith
Date: Tue Apr 16 20:13:06 2019
New Revision: 358550

URL: http://llvm.org/viewvc/llvm-project?rev=358550&view=rev
Log:
Clear the output string passed to GetHostName()

LLVM's wchar to UTF8 conversion routine expects an empty string to store the 
output.
GetHostName() on Windows is sometimes called with a non-empty string which 
triggers
an assert. The simple fix is to clear the output string before the conversion.


Modified:
lldb/trunk/source/Host/windows/HostInfoWindows.cpp
lldb/trunk/unittests/Host/HostInfoTest.cpp

Modified: lldb/trunk/source/Host/windows/HostInfoWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=358550&r1=358549&r2=358550&view=diff
==
--- lldb/trunk/source/Host/windows/HostInfoWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/HostInfoWindows.cpp Tue Apr 16 20:13:06 2019
@@ -95,6 +95,8 @@ bool HostInfoWindows::GetHostname(std::s
   if (!::GetComputerNameW(buffer, &dwSize))
 return false;
 
+  // The conversion requires an empty string.
+  s.clear();
   return llvm::convertWideToUTF8(buffer, s);
 }
 

Modified: lldb/trunk/unittests/Host/HostInfoTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/HostInfoTest.cpp?rev=358550&r1=358549&r2=358550&view=diff
==
--- lldb/trunk/unittests/Host/HostInfoTest.cpp (original)
+++ lldb/trunk/unittests/Host/HostInfoTest.cpp Tue Apr 16 20:13:06 2019
@@ -50,3 +50,9 @@ TEST_F(HostInfoTest, GetAugmentedArchSpe
   EXPECT_EQ(HostInfo::GetAugmentedArchSpec(LLDB_ARCH_DEFAULT).GetTriple(),
 HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple());
 }
+
+TEST_F(HostInfoTest, GetHostname) {
+  // Check non-empty string input works correctly.
+  std::string s("abc");
+  EXPECT_TRUE(HostInfo::GetHostname(s));
+}


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits