[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

2018-04-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thank you.

PS: I will need someone to update the XCode project after this.


https://reviews.llvm.org/D44306



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


[Lldb-commits] [lldb] r329677 - Move Args::StringTo*** functions to a new OptionArgParser class

2018-04-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Apr 10 02:03:59 2018
New Revision: 329677

URL: http://llvm.org/viewvc/llvm-project?rev=329677&view=rev
Log:
Move Args::StringTo*** functions to a new OptionArgParser class

Summary:
The idea behind this is to move the functionality which depend on other lldb
classes into a separate class. This way, the Args class can be turned
into a lightweight arc+argv wrapper and moved into the lower lldb
layers.

Reviewers: jingham, zturner

Subscribers: lldb-commits

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

Added:
lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
lldb/trunk/source/Interpreter/OptionArgParser.cpp
lldb/trunk/unittests/Interpreter/TestOptionArgParser.cpp
Modified:
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/include/lldb/Interpreter/Options.h
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/trunk/source/Commands/CommandObjectCommands.cpp
lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Commands/CommandObjectLog.cpp
lldb/trunk/source/Commands/CommandObjectMemory.cpp
lldb/trunk/source/Commands/CommandObjectProcess.cpp
lldb/trunk/source/Commands/CommandObjectSource.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Commands/CommandObjectType.cpp
lldb/trunk/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/trunk/source/Interpreter/Args.cpp
lldb/trunk/source/Interpreter/CMakeLists.txt
lldb/trunk/source/Interpreter/OptionGroupValueObjectDisplay.cpp
lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
lldb/trunk/source/Interpreter/OptionValueBoolean.cpp
lldb/trunk/source/Interpreter/OptionValueChar.cpp
lldb/trunk/source/Interpreter/OptionValueFormat.cpp
lldb/trunk/source/Interpreter/Property.cpp

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/unittests/Interpreter/CMakeLists.txt
lldb/trunk/unittests/Interpreter/TestArgs.cpp

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=329677&r1=329676&r2=329677&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Tue Apr 10 02:03:59 2018
@@ -12,7 +12,6 @@
 
 // C Includes
 // C++ Includes
-#include 
 #include 
 #include 
 #include 
@@ -22,7 +21,6 @@
 #include "llvm/ADT/StringRef.h"
 // Project includes
 #include "lldb/Utility/Environment.h"
-#include "lldb/Utility/Status.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-types.h"
 
@@ -343,30 +341,6 @@ public:
 return min <= sval64 && sval64 <= max;
   }
 
-  static lldb::addr_t StringToAddress(const ExecutionContext *exe_ctx,
-  llvm::StringRef s,
-  lldb::addr_t fail_value, Status *error);
-
-  static bool StringToBoolean(llvm::StringRef s, bool fail_value,
-  bool *success_ptr);
-
-  static char StringToChar(llvm::StringRef s, char fail_value,
-   bool *success_ptr);
-
-  static int64_t StringToOptionEnum(llvm::StringRef s,
-OptionEnumValueElement *enum_values,
-int32_t fail_value, Status &error);
-
-  static lldb::ScriptLanguage
-  StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value,
- bool *success_ptr);
-
-  // TODO: Use StringRef
-  static Status StringToFormat(const char *s, lldb::Format &format,
-   size_t *byte_size_ptr); // If non-NULL, then a
-   // byte size can precede
-   // the format character
-
   static lldb::Encoding
   StringToEncoding(llvm::StringRef s,
lldb::Encoding fail_value = lldb::eEncodingInvalid);

Added: lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionArgParser.h?rev=329677&view=auto
==
--- lldb/trunk/include/lldb/Interpreter/OptionArgParser.h (added)
+++ lldb/trun

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

2018-04-10 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329677: Move Args::StringTo*** functions to a new 
OptionArgParser class (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44306

Files:
  lldb/trunk/include/lldb/Interpreter/Args.h
  lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
  lldb/trunk/include/lldb/Interpreter/Options.h
  lldb/trunk/source/API/SBDebugger.cpp
  lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
  lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/trunk/source/Commands/CommandObjectCommands.cpp
  lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
  lldb/trunk/source/Commands/CommandObjectExpression.cpp
  lldb/trunk/source/Commands/CommandObjectLog.cpp
  lldb/trunk/source/Commands/CommandObjectMemory.cpp
  lldb/trunk/source/Commands/CommandObjectProcess.cpp
  lldb/trunk/source/Commands/CommandObjectSource.cpp
  lldb/trunk/source/Commands/CommandObjectTarget.cpp
  lldb/trunk/source/Commands/CommandObjectThread.cpp
  lldb/trunk/source/Commands/CommandObjectType.cpp
  lldb/trunk/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/trunk/source/Interpreter/Args.cpp
  lldb/trunk/source/Interpreter/CMakeLists.txt
  lldb/trunk/source/Interpreter/OptionArgParser.cpp
  lldb/trunk/source/Interpreter/OptionGroupValueObjectDisplay.cpp
  lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
  lldb/trunk/source/Interpreter/OptionValueBoolean.cpp
  lldb/trunk/source/Interpreter/OptionValueChar.cpp
  lldb/trunk/source/Interpreter/OptionValueFormat.cpp
  lldb/trunk/source/Interpreter/Property.cpp
  
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/unittests/Interpreter/CMakeLists.txt
  lldb/trunk/unittests/Interpreter/TestArgs.cpp
  lldb/trunk/unittests/Interpreter/TestOptionArgParser.cpp

Index: lldb/trunk/include/lldb/Interpreter/Options.h
===
--- lldb/trunk/include/lldb/Interpreter/Options.h
+++ lldb/trunk/include/lldb/Interpreter/Options.h
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Interpreter/Args.h"
+#include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-private.h"
 
Index: lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
===
--- lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
+++ lldb/trunk/include/lldb/Interpreter/OptionArgParser.h
@@ -0,0 +1,43 @@
+//===-- OptionArgParser.h ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_INTERPRETER_OPTIONARGPARSER_H
+#define LLDB_INTERPRETER_OPTIONARGPARSER_H
+
+#include "lldb/lldb-private-types.h"
+
+namespace lldb_private {
+
+struct OptionArgParser {
+  static lldb::addr_t ToAddress(const ExecutionContext *exe_ctx,
+llvm::StringRef s, lldb::addr_t fail_value,
+Status *error);
+
+  static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
+
+  static char ToChar(llvm::StringRef s, char fail_value, bool *success_ptr);
+
+  static int64_t ToOptionEnum(llvm::StringRef s,
+  OptionEnumValueElement *enum_values,
+  int32_t fail_value, Status &error);
+
+  static lldb::ScriptLanguage ToScriptLanguage(llvm::StringRef s,
+   lldb::ScriptLanguage fail_value,
+   bool *success_ptr);
+
+  // TODO: Use StringRef
+  static Status ToFormat(const char *s, lldb::Format &format,
+ size_t *byte_size_ptr); // If non-NULL, then a
+ // byte size can precede
+ // the format character
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_OPTIONARGPARSER_H
Index: lldb/trunk/include/lldb/Interpreter/Args.h
===
--- lldb/trunk/include/lldb/Interpreter/Args.h
+++ lldb/trunk/include/lldb/Interpreter/Args.h
@@ -12,7 +12,6 @@
 
 // C Includes
 // C++ Includes
-#include 
 #include 
 #include 
 #include 
@@ -22,7 +21,6 @@
 #

[Lldb-commits] [lldb] r329679 - Move OptionElementVector helper structs from Args to Options

2018-04-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Apr 10 02:48:05 2018
New Revision: 329679

URL: http://llvm.org/viewvc/llvm-project?rev=329679&view=rev
Log:
Move OptionElementVector helper structs from Args to Options

These are not used anywhere in the Args class. They should have been
moved as a part of r327110 (Moving Option parsing from Args to Options),
but I did not notice them then.

This does not affect the layering in any way, but in makes sense for the
structs to be defined in the near the code that uses them.

Modified:
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/include/lldb/Interpreter/CommandObject.h
lldb/trunk/include/lldb/Interpreter/Options.h

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=329679&r1=329678&r2=329679&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Tue Apr 10 02:48:05 2018
@@ -26,22 +26,6 @@
 
 namespace lldb_private {
 
-typedef std::vector> OptionArgVector;
-typedef std::shared_ptr OptionArgVectorSP;
-
-struct OptionArgElement {
-  enum { eUnrecognizedArg = -1, eBareDash = -2, eBareDoubleDash = -3 };
-
-  OptionArgElement(int defs_index, int pos, int arg_pos)
-  : opt_defs_index(defs_index), opt_pos(pos), opt_arg_pos(arg_pos) {}
-
-  int opt_defs_index;
-  int opt_pos;
-  int opt_arg_pos;
-};
-
-typedef std::vector OptionElementVector;
-
 //--
 /// @class Args Args.h "lldb/Interpreter/Args.h"
 /// @brief A command line argument class.

Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObject.h?rev=329679&r1=329678&r2=329679&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandObject.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandObject.h Tue Apr 10 02:48:05 2018
@@ -22,6 +22,7 @@
 
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Interpreter/CommandCompletions.h"
+#include "lldb/Interpreter/Options.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/lldb-private.h"

Modified: lldb/trunk/include/lldb/Interpreter/Options.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Options.h?rev=329679&r1=329678&r2=329679&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Options.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Options.h Tue Apr 10 02:48:05 2018
@@ -28,6 +28,22 @@ namespace lldb_private {
 
 struct Option;
 
+typedef std::vector> OptionArgVector;
+typedef std::shared_ptr OptionArgVectorSP;
+
+struct OptionArgElement {
+  enum { eUnrecognizedArg = -1, eBareDash = -2, eBareDoubleDash = -3 };
+
+  OptionArgElement(int defs_index, int pos, int arg_pos)
+  : opt_defs_index(defs_index), opt_pos(pos), opt_arg_pos(arg_pos) {}
+
+  int opt_defs_index;
+  int opt_pos;
+  int opt_arg_pos;
+};
+
+typedef std::vector OptionElementVector;
+
 static inline bool isprint8(int ch) {
   if (ch & 0xff00u)
 return false;


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


[Lldb-commits] [lldb] r329682 - Args: replace isprint8 usage with isprint

2018-04-10 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Apr 10 03:07:22 2018
New Revision: 329682

URL: http://llvm.org/viewvc/llvm-project?rev=329682&view=rev
Log:
Args: replace isprint8 usage with isprint

It looks like we introduced isprint8 way back in r169417 to be used on
getopt's short_options, which we sometimes set to values which are out
of range for normal chars to indicate options with no short form.

However, this is not how the function is used in the Args class, where
we explicitly process a string character by character.

This removes the last external dependency from the Args class.

Modified:
lldb/trunk/source/Interpreter/Args.cpp

Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=329682&r1=329681&r2=329682&view=diff
==
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Tue Apr 10 03:07:22 2018
@@ -13,7 +13,6 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Interpreter/Args.h"
-#include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Stream.h"
@@ -620,7 +619,7 @@ void Args::ExpandEscapedCharacters(const
   dst.clear();
   if (src) {
 for (const char *p = src; *p != '\0'; ++p) {
-  if (isprint8(*p))
+  if (isprint(*p))
 dst.append(1, *p);
   else {
 switch (*p) {


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


[Lldb-commits] [PATCH] D45480: Move Args.cpp from Interpreter to Utility

2018-04-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: zturner, jingham, davide.
Herald added a subscriber: mgorny.

The Args class is used in plenty of places besides the command
interpreter (e.g., anything requiring an argc+argv combo, such as when
launching a process), so it needs to be in a lower layer. Now that the
class has no external dependencies, it can be moved down to the Utility
module.

This removes the last (direct) dependency from the Host module to
Interpreter, so I remove the Interpreter module from Host's dependency
list.


https://reviews.llvm.org/D45480

Files:
  include/lldb/Interpreter/Args.h
  include/lldb/Interpreter/CommandAlias.h
  include/lldb/Interpreter/CommandInterpreter.h
  include/lldb/Interpreter/CommandObject.h
  include/lldb/Interpreter/Options.h
  include/lldb/Target/ProcessInfo.h
  include/lldb/Utility/Args.h
  source/API/SBDebugger.cpp
  source/API/SBPlatform.cpp
  source/API/SBProcess.cpp
  source/API/SBTarget.cpp
  source/Breakpoint/BreakpointIDList.cpp
  source/Commands/CommandCompletions.cpp
  source/Commands/CommandObjectApropos.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectFrame.cpp
  source/Commands/CommandObjectLog.cpp
  source/Commands/CommandObjectMemory.cpp
  source/Commands/CommandObjectPlatform.cpp
  source/Commands/CommandObjectProcess.cpp
  source/Commands/CommandObjectRegister.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/RegisterValue.cpp
  source/Host/CMakeLists.txt
  source/Host/macosx/HostInfoMacOSX.mm
  source/Interpreter/Args.cpp
  source/Interpreter/CMakeLists.txt
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/CommandObjectScript.cpp
  source/Interpreter/OptionValueArch.cpp
  source/Interpreter/OptionValueArgs.cpp
  source/Interpreter/OptionValueArray.cpp
  source/Interpreter/OptionValueDictionary.cpp
  source/Interpreter/OptionValueFileSpec.cpp
  source/Interpreter/OptionValueFileSpecLIst.cpp
  source/Interpreter/OptionValueLanguage.cpp
  source/Interpreter/OptionValuePathMappings.cpp
  source/Interpreter/OptionValueProperties.cpp
  source/Interpreter/OptionValueString.cpp
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptScriptGroup.cpp
  source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Utility/Args.cpp
  source/Utility/CMakeLists.txt
  tools/lldb-server/LLDBServerUtilities.cpp
  unittests/Interpreter/CMakeLists.txt
  unittests/Interpreter/TestArgs.cpp
  unittests/Utility/ArgsTest.cpp
  unittests/Utility/CMakeLists.txt
  unittests/tools/lldb-server/tests/MessageObjects.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp

Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -11,8 +11,8 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
-#include "lldb/Interpreter/Args.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
+#include "lldb/Utility/Args.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
Index: unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -8,7 +8,7 @@
 //===--===//
 
 #include "MessageObjects.h"
-#include "lldb/Interpreter/Args.h"
+#include "lldb/Utility/Args.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "llvm/ADT/StringExtras.h"
 #include "gtest/gtest.h"
Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  ArgsTest.cpp
   ArchSpecTest.cpp
   CleanUpTest.cpp
   ConstStringTest.cpp
Index: unittests/Utility/ArgsTest.cpp
===
--- unittests/Utility/ArgsTest.cpp
+++ unittests/Utility/ArgsTest.cpp
@@ -9,7 +9,7 @@
 
 #include "gtest/gtest.h"
 
-#include "lldb/Interpreter/Args.h"
+#include "lldb/Utility/Args.h"
 #include "lldb/Utility/StringList.h"
 
 #include 
@@ -120,7 +120,9 @@
 
 TEST(ArgsTest, StringListConstructor) {
   StringList list;
-  list << "foo" << "bar" << "baz";
+  list << "foo"
+   << "bar"
+   << "baz";
   Args args(list);
   

[Lldb-commits] [lldb] r329697 - s/LLVM_ON_WIN32/_WIN32/, lldb

2018-04-10 Thread Nico Weber via lldb-commits
Author: nico
Date: Tue Apr 10 06:33:45 2018
New Revision: 329697

URL: http://llvm.org/viewvc/llvm-project?rev=329697&view=rev
Log:
s/LLVM_ON_WIN32/_WIN32/, lldb

LLVM_ON_WIN32 is set exactly with MSVC and MinGW (but not Cygwin) in
HandleLLVMOptions.cmake, which is where _WIN32 defined too.  Just use the   
 
default macro instead of a reinvented one.  

See thread "Replacing LLVM_ON_WIN32 with just _WIN32" on llvm-dev and cfe-dev.  
No intended behavior change.

Modified:
lldb/trunk/include/lldb/Host/windows/PosixApi.h
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Core/PluginManager.cpp
lldb/trunk/source/Host/common/File.cpp
lldb/trunk/source/Host/common/MainLoop.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Host/common/TCPSocket.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h
lldb/trunk/source/Utility/FileSpec.cpp
lldb/trunk/source/Utility/Log.cpp
lldb/trunk/source/Utility/TildeExpressionResolver.cpp

Modified: lldb/trunk/include/lldb/Host/windows/PosixApi.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/windows/PosixApi.h?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/include/lldb/Host/windows/PosixApi.h (original)
+++ lldb/trunk/include/lldb/Host/windows/PosixApi.h Tue Apr 10 06:33:45 2018
@@ -11,7 +11,7 @@
 #define liblldb_Host_windows_PosixApi_h
 
 #include "llvm/Support/Compiler.h"
-#if !defined(LLVM_ON_WIN32)
+#if !defined(_WIN32)
 #error "windows/PosixApi.h being #included on non Windows system!"
 #endif
 

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Tue Apr 10 06:33:45 2018
@@ -48,7 +48,7 @@
 #include "lldb/Utility/StreamCallback.h"
 #include "lldb/Utility/StreamString.h"
 
-#if defined(LLVM_ON_WIN32)
+#if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 

Modified: lldb/trunk/source/Core/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Tue Apr 10 06:33:45 2018
@@ -46,7 +46,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
-#if defined(LLVM_ON_WIN32)
+#if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 

Modified: lldb/trunk/source/Core/ModuleList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ModuleList.cpp?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/source/Core/ModuleList.cpp (original)
+++ lldb/trunk/source/Core/ModuleList.cpp Tue Apr 10 06:33:45 2018
@@ -26,7 +26,7 @@
 #include "lldb/Utility/UUID.h"// for UUID, operator!=, operator==
 #include "lldb/lldb-defines.h"// for LLDB_INVALID_INDEX32
 
-#if defined(LLVM_ON_WIN32)
+#if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 

Modified: lldb/trunk/source/Core/PluginManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/PluginManager.cpp?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/source/Core/PluginManager.cpp (original)
+++ lldb/trunk/source/Core/PluginManager.cpp Tue Apr 10 06:33:45 2018
@@ -17,7 +17,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringList.h" // for StringList
 
-#if defined(LLVM_ON_WIN32)
+#if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 

Modified: lldb/trunk/source/Host/common/File.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=329697&r1=329696&r2=329697&view=diff
==
--- lldb/trunk/source/Host/common/File.cpp (original)
+++ lldb/trunk/source/Host/common/File.cpp Tue Apr 10 06:33:45 2018
@@ -100,7 +100,7 @@ int File::GetDescriptor() const {
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
   if (StreamIsValid()) {
-#if defined(LLVM_ON_WIN32)
+#if defined(_WIN32)
 return _fileno(m_stream);
 #else
 return fileno(m_stream);

Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Greg Clayton via lldb-commits


> On Apr 7, 2018, at 2:04 AM, Jan Kratochvil  wrote:
> 
> On Sat, 07 Apr 2018 00:52:26 +0200, Greg Clayton wrote:
>> Take look at how LLVM does it. I believe my changes mirror that approach.
> 
> LLVM does not support partial units so there is nothing to look at there.
> 
> 
>> DWARFUnit should be the base class for anything that needs to hand out DIEs.
> 
> That's OK for partial units.
> 
> 
>> Any specializations should be inheriting from DWARFUnit, like both
>> DWARFCompileUnit and DWARFTypeUnit.
> 
> I see now the source of misunderstanding:
> 
> DWARFCompileUnit = DW_TAG_compile_unit
> DWARFTypeUnit = DW_TAG_type_unit
> DWARFPartialUnit != DW_TAG_partial_unit
>
> BTW DW_TAG_imported_unit is importing (="caller") tag, not a unit (="callee").
> 
> DW_TAG_partial_unit gets read in (by
> DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded) as DWARFCompileUnit because
> there is no quick enough way to find the difference.  It would require reading
> the first DIE tag which means to read and decode .debug_abbrev for each unit
> being scanned.

If there is no structural difference other than the first DW_TAG, is there a 
reason you would need to know the difference? It would be fine to just add 
extra methods on DWARFCompileUnit that do partial unit kind of things if it is 
a partial unit? Let me know if I am missing something?

> 
> DWARFPartialUnit is used only as a remapping of DWARFCompileUnit to a new
> offset without any new data (there is stored only a new remapped offset whose
> value is only made up internally in LLDB) at the moment someone uses
> DW_TAG_imported_unit for it - at that moment we easily know that unit has to
> be DW_TAG_partial_unit.

It is remapped to a new offset just to keep LLDB's lldb::user_id_t stuff happy? 
A partial unit can refer to an external file on disk. The remapping is solely 
making a unique offset by adding an offset to the offset of the external file?

> 
> Therefore DWARFCompileUnit and DWARFTypeUnit both contain some their own data.
> But DWARFPartialUnit is just a remapping of DWARFCompileUnit (containing
> DW_TAG_partial_unit) to a new offset without any new data. Particularly
> m_die_array is not in DWARFPartialUnit.

In reading the DWZ it sounded like you can only have 1 external debug info file 
and that external debug info file can't itself refer to another? Is this what 
the DWARFPartialUnit would be for and this is the only remapping that needs to 
happen?

> 
> DWARFTypeUnit can be recognized easily as it is either in .debug_types
> (<=DWARF-4) or the unit header contains DW_UT_type (>=DWARF-5).
> DWARFPartialUnit (for DW_TAG_partial_unit) cannot be recognized easily first.
> Besides that one would need then some DWARFRemappedPartialUnit for what I use
> DWARFPartialUnit now.
> 
> I have implemented it according to your advice from this mail - at least
> according to how I understood it:
>   [lldb-dev] RFC for DWZ = DW_TAG_imported_unit + DWARF-5 supplementary 
> files
>   https://lists.llvm.org/pipermail/lldb-dev/2017-August/012664.html
> 
> It does not try to share anything at AST level, it only shares DWARF data (and
> thus DWARFCompileUnit). Given that DWZ finds arbitrary unique DWARF subtrees
> I find more logical to decode it at DWARF (and not at AST) level.
> 
> You wrote:
> # The drawback is this won't allow sharing /tmp/shared1 or /tmp/shared2
> # between two different top level DWARF files, but it does allow one
> # clang::ASTContext to be used between all of them.
> 
> In my implementation /tmp/shared1
> (/usr/lib/debug/.dwz/coreutils-8.27-20.fc27.x86_64) is shared between multiple
> *.so files (which use DW_TAG_imported_unit) at DWARF level, also
> clang::ASTContext is shared.
> 
> 
> # SymbolFileDWARFDebugMap makes it lldb::user_id_t contain the CU index in the
> # top 32 bits, and the DIE offset within that .o file's DWARF in the bottom 32
> # bits. You could do something similar in your case where the top 32 bits is
> # the index of the DWARF file in the "dwarf[]" array that would be maintained
> # in a new SymbolFileDWARFDWZ subclass.
> 
> DW_TAG_imported_unit+DW_TAG_partial_unit can be also used for optimization of
> a single file (without /usr/lib/debug/.dwz/* file which is used exclusively
> for DW_TAG_partial_unit entries). Additionally the tags can be also used for
> recursive inclusion. I haven't found how to use the top 32 bits for that.
> I just reserve new remapped offset space for each DW_TAG_imported_unit (in the
> bottom 32 bits).
> 
> I tried first to implement dw_offset_t caller (=unit with
> DW_TAG_imported_unit) to be tracked along any dw_offset_t DIE offset but that
> would require huge changes of the DWARF parsing code everywhere.  Also it
> cannot work well given the inclusion is recursive (so we would need
> std::vector of the callers stack).
> 
> 
> 
>> I have no idea what are in your other patches
> 
> OK, so there was a gross misunderstanding and my DWARFUnit implemented
> something very diff

[Lldb-commits] [PATCH] D45480: Move Args.cpp from Interpreter to Utility

2018-04-10 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

> This removes the last (direct) dependency from the Host module to 
> Interpreter, so I remove the Interpreter module from Host's dependency list.

Big milestone!  Kudos


https://reviews.llvm.org/D45480



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


[Lldb-commits] [lldb] r329722 - Fix the Xcode build for the addition of OptionArgsParser.

2018-04-10 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Tue Apr 10 10:20:27 2018
New Revision: 329722

URL: http://llvm.org/viewvc/llvm-project?rev=329722&view=rev
Log:
Fix the Xcode build for the addition of OptionArgsParser.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=329722&r1=329721&r2=329722&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Apr 10 10:20:27 2018
@@ -732,6 +732,8 @@
4C56543119D1EFAA002E9C44 /* ThreadPlanPython.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 4C56543019D1EFAA002E9C44 /* 
ThreadPlanPython.cpp */; };
4C56543519D2297A002E9C44 /* SBThreadPlan.h in Headers */ = {isa 
= PBXBuildFile; fileRef = 4C56543419D2297A002E9C44 /* SBThreadPlan.h */; 
settings = {ATTRIBUTES = (Public, ); }; };
4C56543719D22B32002E9C44 /* SBThreadPlan.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 4C56543619D22B32002E9C44 /* SBThreadPlan.cpp */; 
};
+   4C719395207D235400FDF430 /* OptionArgParser.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 4C719394207D235400FDF430 /* OptionArgParser.cpp 
*/; };
+   4C719399207D23E300FDF430 /* TestOptionArgParser.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 4C719398207D23E300FDF430 /* 
TestOptionArgParser.cpp */; };
4C7D48241F5099A1005314B4 /* SymbolFileDWARFDwp.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 4C7D481C1F509963005314B4 /* 
SymbolFileDWARFDwp.cpp */; };
4C7D48251F5099B2005314B4 /* SymbolFileDWARFDwoDwp.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 4C7D481F1F509964005314B4 /* 
SymbolFileDWARFDwoDwp.cpp */; };
4C877B391F30EF990068FCFB /* SBProcessInfo.h in Headers */ = 
{isa = PBXBuildFile; fileRef = 4987FB201F30EC9900E5C17D /* SBProcessInfo.h */; 
settings = {ATTRIBUTES = (Public, ); }; };
@@ -765,7 +767,6 @@
4CF52AF8142829390051E832 /* SBFileSpecList.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 4CF52AF7142829390051E832 /* SBFileSpecList.cpp 
*/; };
54067BF11DF2041B00749AA5 /* UBSanRuntime.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 54067BEC1DF2034B00749AA5 /* UBSanRuntime.cpp */; 
};
6B74D89B200696BB0074051B /* Environment.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 22DC561920064C9600A7E9E8 /* Environment.cpp */; 
};
-   6B88947A2065AE5D002E5C59 /* CommandObjectStats.cpp in CopyFiles 
*/ = {isa = PBXBuildFile; fileRef = 6B8894782065AE5C002E5C59 /* 
CommandObjectStats.cpp */; };
6D0F61431C80AAAE00A4ECEE /* JavaASTContext.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 6D0F61411C8000A4ECEE /* JavaASTContext.cpp 
*/; };
6D0F61481C80AAD600A4ECEE /* DWARFASTParserJava.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 6D0F61441C80AACF00A4ECEE /* 
DWARFASTParserJava.cpp */; };
6D0F614E1C80AB0700A4ECEE /* JavaLanguageRuntime.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 6D0F614A1C80AB0400A4ECEE /* 
JavaLanguageRuntime.cpp */; };
@@ -785,7 +786,6 @@
6D99A3631BBC2F3200979793 /* ArmUnwindInfo.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp 
*/; };
6D9AB3DD1BB2B74E003F2289 /* TypeMap.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 6D9AB3DC1BB2B74E003F2289 /* TypeMap.cpp */; };
6DEC6F391BD66D750091ABA6 /* TaskPool.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 6DEC6F381BD66D750091ABA6 /* TaskPool.cpp */; };
-   7F94D7182040A13A006EE3EA /* CleanUpTest.cpp in CopyFiles */ = 
{isa = PBXBuildFile; fileRef = 7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */; 
};
8C26C4261C3EA5F90031DF7C /* TSanRuntime.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 8C26C4241C3EA4340031DF7C /* TSanRuntime.cpp */; 
};
8C2D6A53197A1EAF006989C9 /* MemoryHistory.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 8C2D6A52197A1EAF006989C9 /* MemoryHistory.cpp 
*/; };
8C2D6A5E197A250F006989C9 /* MemoryHistoryASan.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 8C2D6A5A197A1FDC006989C9 /* 
MemoryHistoryASan.cpp */; };
@@ -2594,6 +2594,10 @@
4C56543819D22FD9002E9C44 /* SBThreadPlan.i */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.c.preprocessed; path = 
SBThreadPlan.i; sourceTree = ""; };
4C5DBBC611E3FEC60035160F /* CommandObjectCommands.cpp */ = {isa 
= PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = CommandObjectCommands.cpp; path = 
source/Commands/CommandObjectCommands.cpp; sourceTree = ""; };
4C5DBBC711E3FEC60035160F /

[Lldb-commits] [lldb] r329727 - Fix a typo in the gtest build target for Debug configuration.

2018-04-10 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Tue Apr 10 10:49:56 2018
New Revision: 329727

URL: http://llvm.org/viewvc/llvm-project?rev=329727&view=rev
Log:
Fix a typo in the gtest build target for Debug configuration.

I usually run DebugClang...

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=329727&r1=329726&r2=329727&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Apr 10 10:49:56 2018
@@ -8818,7 +8818,7 @@

"$(LLVM_SOURCE_DIR)/tools/clang/include",

"$(LLVM_BUILD_DIR)/$(LLVM_BUILD_DIR_ARCH)/tools/clang/include",
);
-   LLDB_GTESTS_CFLAGS = "-I unittests -I 
$(LLVM_SOURCE_DIR)/utils/unittest/googlemock/include -I 
$(LLVM_SOURCE_DIR)/utils/unittest/googletest/include -I 
$(LLVM_SOURCE_DIR)/include -I $(LLVM_BUILD_DIR)/x86_64/include 
-DYAML2OBJ=\"\\\"$(LLVM_BUILD_DIR)/x86_64/bin/yaml2obj\\\"\" -I include -I 
source -I 
$PYTHON_FRAMEWORK_PATH)/Versions/$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)/include/python$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)";
+   LLDB_GTESTS_CFLAGS = "-I unittests -I 
$(LLVM_SOURCE_DIR)/utils/unittest/googlemock/include -I 
$(LLVM_SOURCE_DIR)/utils/unittest/googletest/include -I 
$(LLVM_SOURCE_DIR)/include -I $(LLVM_BUILD_DIR)/x86_64/include 
-DYAML2OBJ=\"\\\"$(LLVM_BUILD_DIR)/x86_64/bin/yaml2obj\\\"\" -I include -I 
source -I 
$(PYTHON_FRAMEWORK_PATH)/Versions/$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)/include/python$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)";
LLDB_GTESTS_LDFLAGS = 
"$(LLVM_BUILD_DIR)/x86_64/lib/libgtest.a -L 
$(PYTHON_FRAMEWORK_PATH)/Versions/$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)/lib
 -l python$(PYTHON_VERSION_MAJOR).$(PYTHON_VERSION_MINOR)";
OTHER_CFLAGS = (
"-fno-rtti",

Modified: lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata?rev=329727&r1=329726&r2=329727&view=diff
==
--- lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata (original)
+++ lldb/trunk/lldb.xcworkspace/contents.xcworkspacedata Tue Apr 10 10:49:56 
2018
@@ -2,9 +2,6 @@
 

-   
-   

http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45497: Don't assume the backing thread shares a Protocol ID

2018-04-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, clayborg.

When we're dealing with virtual (memory) threads created by the OS
plugins, there's no guarantee that the real thread and the backing
thread share a protocol ID. Instead, we should iterate over the memory
threads to find the virtual thread that is backed by the current real
thread.

rdar://36485830


Repository:
  rL LLVM

https://reviews.llvm.org/D45497

Files:
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1820,11 +1820,13 @@
   if (!thread_sp->StopInfoIsUpToDate()) {
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it
-// to calcualte StopInfo.
-ThreadSP memory_thread_sp =
-m_thread_list.FindThreadByProtocolID(thread_sp->GetProtocolID());
-if (memory_thread_sp)
-  thread_sp = memory_thread_sp;
+// to calculate StopInfo.
+size_t num_threads = m_thread_list.GetSize(false);
+for (size_t i = 0; i < num_threads; ++i) {
+  ThreadSP mem_thread = m_thread_list.GetThreadAtIndex(i, false);
+  if (mem_thread && mem_thread->GetBackingThread() == thread_sp)
+thread_sp = mem_thread;
+}
 
 if (exc_type != 0) {
   const size_t exc_data_size = exc_data.size();


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1820,11 +1820,13 @@
   if (!thread_sp->StopInfoIsUpToDate()) {
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it
-// to calcualte StopInfo.
-ThreadSP memory_thread_sp =
-m_thread_list.FindThreadByProtocolID(thread_sp->GetProtocolID());
-if (memory_thread_sp)
-  thread_sp = memory_thread_sp;
+// to calculate StopInfo.
+size_t num_threads = m_thread_list.GetSize(false);
+for (size_t i = 0; i < num_threads; ++i) {
+  ThreadSP mem_thread = m_thread_list.GetThreadAtIndex(i, false);
+  if (mem_thread && mem_thread->GetBackingThread() == thread_sp)
+thread_sp = mem_thread;
+}
 
 if (exc_type != 0) {
   const size_t exc_data_size = exc_data.size();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45497: Don't assume the backing thread shares a Protocol ID

2018-04-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

(I'll update the diff once I've figured out how to test this)


Repository:
  rL LLVM

https://reviews.llvm.org/D45497



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


[Lldb-commits] [PATCH] D45497: Don't assume the backing thread shares a Protocol ID

2018-04-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1826-1827
+for (size_t i = 0; i < num_threads; ++i) {
+  ThreadSP mem_thread = m_thread_list.GetThreadAtIndex(i, false);
+  if (mem_thread && mem_thread->GetBackingThread() == thread_sp)
+thread_sp = mem_thread;

I would add a new function to ThreadList:

```
ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread);
```

Since there is a mutex inside the thread list, we don't want another thread to 
be able to mutate the thread list while doing this iteration. So this code 
would become:

```
ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp);
if (memory_thread_sp)
  thread_sp = memory_thread_sp;
```



Repository:
  rL LLVM

https://reviews.llvm.org/D45497



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


[Lldb-commits] [lldb] r329745 - Convert an absolute to a group relative reference for TestOptionArgParser.cpp.

2018-04-10 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Tue Apr 10 12:29:37 2018
New Revision: 329745

URL: http://llvm.org/viewvc/llvm-project?rev=329745&view=rev
Log:
Convert an absolute to a group relative reference for TestOptionArgParser.cpp.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=329745&r1=329744&r2=329745&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Apr 10 12:29:37 2018
@@ -2597,7 +2597,7 @@
4C719394207D235400FDF430 /* OptionArgParser.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = OptionArgParser.cpp; path = source/Interpreter/OptionArgParser.cpp; 
sourceTree = ""; };
4C719396207D237100FDF430 /* CommandOptionValidators.h */ = {isa 
= PBXFileReference; lastKnownFileType = sourcecode.c.h; name = 
CommandOptionValidators.h; path = 
include/lldb/Interpreter/CommandOptionValidators.h; sourceTree = ""; };
4C719397207D237100FDF430 /* OptionArgParser.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = OptionArgParser.h; 
path = include/lldb/Interpreter/OptionArgParser.h; sourceTree = ""; };
-   4C719398207D23E300FDF430 /* TestOptionArgParser.cpp */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = 
TestOptionArgParser.cpp; path = 
"/Volumes/ThePlayground/Users/jingham/Work/LLDB/llvm-dot-org/lldb-working/unittests/Interpreter/TestOptionArgParser.cpp";
 sourceTree = ""; };
+   4C719398207D23E300FDF430 /* TestOptionArgParser.cpp */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = 
TestOptionArgParser.cpp; sourceTree = ""; };
4C73152119B7D71700F865A4 /* Iterable.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
Iterable.h; path = include/lldb/Utility/Iterable.h; sourceTree = ""; };
4C7CF7E31295E10E00B4FBB5 /* ThreadPlanCallUserExpression.h */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; 
name = ThreadPlanCallUserExpression.h; path = 
include/lldb/Target/ThreadPlanCallUserExpression.h; sourceTree = ""; };
4C7CF7E51295E12B00B4FBB5 /* ThreadPlanCallUserExpression.cpp */ 
= {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; name = ThreadPlanCallUserExpression.cpp; path = 
source/Target/ThreadPlanCallUserExpression.cpp; sourceTree = ""; };


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


Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Greg Clayton via lldb-commits
ok, so I was able to create a DWZ example and see how this is laid out and I 
now understand the format. I will see if I can modify this patch in a way that 
will work for DWZ and for .debug_types. 


> On Apr 10, 2018, at 8:46 AM, Greg Clayton  wrote:
> 
> 
> 
>> On Apr 7, 2018, at 2:04 AM, Jan Kratochvil > > wrote:
>> 
>> On Sat, 07 Apr 2018 00:52:26 +0200, Greg Clayton wrote:
>>> Take look at how LLVM does it. I believe my changes mirror that approach.
>> 
>> LLVM does not support partial units so there is nothing to look at there.
>> 
>> 
>>> DWARFUnit should be the base class for anything that needs to hand out DIEs.
>> 
>> That's OK for partial units.
>> 
>> 
>>> Any specializations should be inheriting from DWARFUnit, like both
>>> DWARFCompileUnit and DWARFTypeUnit.
>> 
>> I see now the source of misunderstanding:
>> 
>> DWARFCompileUnit = DW_TAG_compile_unit
>> DWARFTypeUnit = DW_TAG_type_unit
>> DWARFPartialUnit != DW_TAG_partial_unit
>>
>> BTW DW_TAG_imported_unit is importing (="caller") tag, not a unit 
>> (="callee").
>> 
>> DW_TAG_partial_unit gets read in (by
>> DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded) as DWARFCompileUnit because
>> there is no quick enough way to find the difference.  It would require 
>> reading
>> the first DIE tag which means to read and decode .debug_abbrev for each unit
>> being scanned.
> 
> If there is no structural difference other than the first DW_TAG, is there a 
> reason you would need to know the difference? It would be fine to just add 
> extra methods on DWARFCompileUnit that do partial unit kind of things if it 
> is a partial unit? Let me know if I am missing something?
> 
>> 
>> DWARFPartialUnit is used only as a remapping of DWARFCompileUnit to a new
>> offset without any new data (there is stored only a new remapped offset whose
>> value is only made up internally in LLDB) at the moment someone uses
>> DW_TAG_imported_unit for it - at that moment we easily know that unit has to
>> be DW_TAG_partial_unit.
> 
> It is remapped to a new offset just to keep LLDB's lldb::user_id_t stuff 
> happy? A partial unit can refer to an external file on disk. The remapping is 
> solely making a unique offset by adding an offset to the offset of the 
> external file?
> 
>> 
>> Therefore DWARFCompileUnit and DWARFTypeUnit both contain some their own 
>> data.
>> But DWARFPartialUnit is just a remapping of DWARFCompileUnit (containing
>> DW_TAG_partial_unit) to a new offset without any new data. Particularly
>> m_die_array is not in DWARFPartialUnit.
> 
> In reading the DWZ it sounded like you can only have 1 external debug info 
> file and that external debug info file can't itself refer to another? Is this 
> what the DWARFPartialUnit would be for and this is the only remapping that 
> needs to happen?
> 
>> 
>> DWARFTypeUnit can be recognized easily as it is either in .debug_types
>> (<=DWARF-4) or the unit header contains DW_UT_type (>=DWARF-5).
>> DWARFPartialUnit (for DW_TAG_partial_unit) cannot be recognized easily first.
>> Besides that one would need then some DWARFRemappedPartialUnit for what I use
>> DWARFPartialUnit now.
>> 
>> I have implemented it according to your advice from this mail - at least
>> according to how I understood it:
>>  [lldb-dev] RFC for DWZ = DW_TAG_imported_unit + DWARF-5 supplementary 
>> files
>>  https://lists.llvm.org/pipermail/lldb-dev/2017-August/012664.html 
>> 
>> 
>> It does not try to share anything at AST level, it only shares DWARF data 
>> (and
>> thus DWARFCompileUnit). Given that DWZ finds arbitrary unique DWARF subtrees
>> I find more logical to decode it at DWARF (and not at AST) level.
>> 
>> You wrote:
>> # The drawback is this won't allow sharing /tmp/shared1 or /tmp/shared2
>> # between two different top level DWARF files, but it does allow one
>> # clang::ASTContext to be used between all of them.
>> 
>> In my implementation /tmp/shared1
>> (/usr/lib/debug/.dwz/coreutils-8.27-20.fc27.x86_64) is shared between 
>> multiple
>> *.so files (which use DW_TAG_imported_unit) at DWARF level, also
>> clang::ASTContext is shared.
>> 
>> 
>> # SymbolFileDWARFDebugMap makes it lldb::user_id_t contain the CU index in 
>> the
>> # top 32 bits, and the DIE offset within that .o file's DWARF in the bottom 
>> 32
>> # bits. You could do something similar in your case where the top 32 bits is
>> # the index of the DWARF file in the "dwarf[]" array that would be maintained
>> # in a new SymbolFileDWARFDWZ subclass.
>> 
>> DW_TAG_imported_unit+DW_TAG_partial_unit can be also used for optimization of
>> a single file (without /usr/lib/debug/.dwz/* file which is used exclusively
>> for DW_TAG_partial_unit entries). Additionally the tags can be also used for
>> recursive inclusion. I haven't found how to use the top 32 bits for that.
>> I just reserve new remapped offset space for each DW_TAG_imported_un

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

2018-04-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

In https://reviews.llvm.org/D32167#1062618, @clayborg wrote:

> we should be using the AST importer to import the type from that file into 
> the AST for the current DWARF file. We do have done this with -gmodules 
> already, so DWZ shouldn't be that different.


Is AST usable for imported declarations of `DW_TAG_variable`, 
`DW_TAG_subprogram`, `DW_TAG_enumeration_type`+`DW_TAG_enumerator`s etc.? Still 
imported `DW_TAG_partial_unit` cannot contain any code or data addresses which 
may make it easier for AST.




Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:595
+  uint64_t debug_info_size = get_debug_info_data().GetByteSize();
+  data_segment.m_data.OffsetData(debug_info_size);
+}

clayborg wrote:
> jankratochvil wrote:
> > I do not like this `DWARFDataExtractor::m_start` modification, it sort of 
> > corrupts the `DataExtractor` and various operations stop working then - 
> > such as `DWARFDataExtractor::GetByteSize()`. DWZ patch makes from current 
> > `dw_offset_t` a virtual (remapped) offset and introduces new physical file 
> > section offset which is looked up for data extraction. The file offset is 
> > represented as `DWARFFileOffset` in D40474, instead of `bool m_is_dwz;` 
> > there could be some `enum { DEBUG_INFO, DEBUG_TYPES, DWZ_DEBUG_INFO } 
> > m_where;` instead.
> This means that this diff doesn't affect all of the other DWARF code. Nothing 
> in .debug_types will refer to anything else (not DW_FORM_ref_addr, or any 
> external references). So this trick allows us to just treat .debug_info as if 
> .debug_types was appended to the end since nothing in .debug_types refers to 
> any DIE outside of its type unit. This also mirrors what will actually happen 
> with DWARF5 when all of the data is contained inside of the .debug_info 
> section. This allows each DIE to have a unique "ID". Any other change 
> requires a lot of changes to the DWARF parser and logic. So I actually like 
> this feature. We can fix the GetByteSize() if needed. Basically every object 
> in DWARf right now must be able to make a unique 64 bit unsigned integer ID 
> in some way that we can get back to that info and partially parse more. These 
> are handed out as lldb::user_id_t values for types, functions and more. Each 
> flavor of DWARF will encode what they want into here. The normal DWARF it is 
> just the absolute offset within the .debug_info. With .debug_types we just 
> add the size of the .debug_info to the ID. For DWARF in .o files on Darwin, 
> we encode the compile unit index into the top 32 bits and the DIE offset into 
> the lower, DWO does something just as DWZ will need to. DWARFFileOffset 
> doesn't mean much if there are multiple files. We have many competing type 
> uniquing/debug info size reduction strategies being employed here. I can't 
> believe we have DWO, DWZ, and debug types... But we have to make them all 
> work. We can't just use the absolute file offset because DWO used external 
> files where the file offsets could be the same in the external .o files... 
> Not sure how this works with DWZ or what the best option is. I will read up 
> on DWZ so I can propose some viable options. But each new flavor of the day 
> that gets added the DWARF parser is adding a bunch of logic and edge cases. 
> If two technologies (DWZ + DWO, DWZ + debug_types, etc) are used together, we 
> need to ensure they can.
> Any other change requires a lot of changes to the DWARF parser and logic. So 
> I actually like this feature.

I agree it is a fine quick&dirty hack. Just if my DWZ support gets accepted 
later anyway then this `.debug_types` feature could be implemented by its 
framework in a clean way (as a regular DIEs remapping which is required for DWZ 
anyway).

> If two technologies (DWZ + DWO, DWZ + debug_types, etc) are used together, we 
> need to ensure they can.

`DWZ + DWO` do not make sense to me.  I haven't tried to use DWZ for DWO but 
DWZ finds common DWARF subtrees typically across CUs so it would not find much.

`DWZ + debug_types` is explicitly supported by the DWZ tool although that is 
IMO for compatibility only, DWZ can make the common type references slightly 
smaller than debug_types. I will sure need to implement debug_types support 
into my DWZ-for-LLDB patchset later.



https://reviews.llvm.org/D32167



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


Re: [Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

2018-04-10 Thread Greg Clayton via lldb-commits


> On Apr 10, 2018, at 2:13 PM, Jan Kratochvil via Phabricator 
>  wrote:
> 
> jankratochvil added a comment.
> 
> In https://reviews.llvm.org/D32167#1062618, @clayborg wrote:
> 
>> we should be using the AST importer to import the type from that file into 
>> the AST for the current DWARF file. We do have done this with -gmodules 
>> already, so DWZ shouldn't be that different.
> 
> 
> Is AST usable for imported declarations of `DW_TAG_variable`, 
> `DW_TAG_subprogram`, `DW_TAG_enumeration_type`+`DW_TAG_enumerator`s etc.? 
> Still imported `DW_TAG_partial_unit` cannot contain any code or data 
> addresses which may make it easier for AST.
> 

Yes.

> 
> 
> 
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:595
> +  uint64_t debug_info_size = get_debug_info_data().GetByteSize();
> +  data_segment.m_data.OffsetData(debug_info_size);
> +}
> 
> clayborg wrote:
>> jankratochvil wrote:
>>> I do not like this `DWARFDataExtractor::m_start` modification, it sort of 
>>> corrupts the `DataExtractor` and various operations stop working then - 
>>> such as `DWARFDataExtractor::GetByteSize()`. DWZ patch makes from current 
>>> `dw_offset_t` a virtual (remapped) offset and introduces new physical file 
>>> section offset which is looked up for data extraction. The file offset is 
>>> represented as `DWARFFileOffset` in D40474, instead of `bool m_is_dwz;` 
>>> there could be some `enum { DEBUG_INFO, DEBUG_TYPES, DWZ_DEBUG_INFO } 
>>> m_where;` instead.
>> This means that this diff doesn't affect all of the other DWARF code. 
>> Nothing in .debug_types will refer to anything else (not DW_FORM_ref_addr, 
>> or any external references). So this trick allows us to just treat 
>> .debug_info as if .debug_types was appended to the end since nothing in 
>> .debug_types refers to any DIE outside of its type unit. This also mirrors 
>> what will actually happen with DWARF5 when all of the data is contained 
>> inside of the .debug_info section. This allows each DIE to have a unique 
>> "ID". Any other change requires a lot of changes to the DWARF parser and 
>> logic. So I actually like this feature. We can fix the GetByteSize() if 
>> needed. Basically every object in DWARf right now must be able to make a 
>> unique 64 bit unsigned integer ID in some way that we can get back to that 
>> info and partially parse more. These are handed out as lldb::user_id_t 
>> values for types, functions and more. Each flavor of DWARF will encode what 
>> they want into here. The normal DWARF it is just the absolute offset within 
>> the .debug_info. With .debug_types we just add the size of the .debug_info 
>> to the ID. For DWARF in .o files on Darwin, we encode the compile unit index 
>> into the top 32 bits and the DIE offset into the lower, DWO does something 
>> just as DWZ will need to. DWARFFileOffset doesn't mean much if there are 
>> multiple files. We have many competing type uniquing/debug info size 
>> reduction strategies being employed here. I can't believe we have DWO, DWZ, 
>> and debug types... But we have to make them all work. We can't just use the 
>> absolute file offset because DWO used external files where the file offsets 
>> could be the same in the external .o files... Not sure how this works with 
>> DWZ or what the best option is. I will read up on DWZ so I can propose some 
>> viable options. But each new flavor of the day that gets added the DWARF 
>> parser is adding a bunch of logic and edge cases. If two technologies (DWZ + 
>> DWO, DWZ + debug_types, etc) are used together, we need to ensure they can.
>> Any other change requires a lot of changes to the DWARF parser and logic. So 
>> I actually like this feature.
> 
> I agree it is a fine quick&dirty hack. Just if my DWZ support gets accepted 
> later anyway then this `.debug_types` feature could be implemented by its 
> framework in a clean way (as a regular DIEs remapping which is required for 
> DWZ anyway).

Now that I understand the DWZ format, I will try and rework this patch by 
modifying DWARFDataExtractor so it works for both our cases. Then you should be 
able to just open your partial units as a normal DWARF file with no 
differences, we just tweak the DWARFDataExtractor when the main file loads the 
external file and nothing more needs to be done.

> 
>> If two technologies (DWZ + DWO, DWZ + debug_types, etc) are used together, 
>> we need to ensure they can.
> 
> `DWZ + DWO` do not make sense to me.  I haven't tried to use DWZ for DWO but 
> DWZ finds common DWARF subtrees typically across CUs so it would not find 
> much.
> 
> `DWZ + debug_types` is explicitly supported by the DWZ tool although that is 
> IMO for compatibility only, DWZ can make the common type references slightly 
> smaller than debug_types. I will sure need to implement debug_types support 
> into my DWZ-for-LLDB patchset later.

Ok, let me try to rework this patch so it can work for both and I'll let you 
know how i

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

2018-04-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

In https://reviews.llvm.org/D32167#1062614, @clayborg wrote:

> Found the DWZ stuff:
>  http://www.dwarfstd.org/ShowIssue.php?issue=120604.1&type=open


BTW the current DWZ tool  supports only 
DWARF-4 and the DWZ tags are non-standard GNU extensions.  Therefore 
`DW_FORM_GNU_ref_alt` in existing DWZ files is `DW_FORM_ref_alt` in the 
proposal you found. I am not aware of any real documentation for this DWARF-4 
GNU extension. But DWARF-5 further modified the proposal so it became 
`DW_FORM_ref_sup4`+`DW_FORM_ref_sup8` in the DWARF-5 standard.

My DWZ-for-LLDB patchset implements the DWARF-4 non-standard GNU extensions as 
it is used in RHEL-7 (and possibly RHEL-8, not sure yet) which will be 
supported at least till 2024. It is also present in all recent Fedoras. 
Currently the DWZ tool has not yet been ported to DWARF-5, Mark Wielaard from 
Red Hat plans to do so.


https://reviews.llvm.org/D32167



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


Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Jan Kratochvil via lldb-commits
On Tue, 10 Apr 2018 17:46:55 +0200, Greg Clayton wrote:
> > On Apr 7, 2018, at 2:04 AM, Jan Kratochvil  
> > wrote:
> > DW_TAG_partial_unit gets read in (by
> > DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded) as DWARFCompileUnit because
> > there is no quick enough way to find the difference.  It would require 
> > reading
> > the first DIE tag which means to read and decode .debug_abbrev for each unit
> > being scanned.
> 
> If there is no structural difference other than the first DW_TAG, is there
> a reason you would need to know the difference?

Not really, it also currently does work with DWARFCompileUnit.
Just one could assert that nobody tries to use DIEs directly from that
DWARFCompileUnit without their remapping to DWARFPartialUnit.


> It would be fine to just add extra methods on DWARFCompileUnit that do
> partial unit kind of things if it is a partial unit?

There are not really any extra methods.


> > DWARFPartialUnit is used only as a remapping of DWARFCompileUnit to a new
> > offset without any new data (there is stored only a new remapped offset 
> > whose
> > value is only made up internally in LLDB) at the moment someone uses
> > DW_TAG_imported_unit for it - at that moment we easily know that unit has to
> > be DW_TAG_partial_unit.
> 
> It is remapped to a new offset just to keep LLDB's lldb::user_id_t stuff
> happy?

Yes.

> A partial unit can refer to an external file on disk. The remapping is
> solely making a unique offset by adding an offset to the offset of the
> external file?

Not only that. Even when DWZ does not use any external file then one
DW_TAG_partial_unit is included into many DW_TAG_compile_unit and so DIEs from
that DW_TAG_partial_unit would no longer have unique lldb::user_id_t.
(Although maybe that can be solved by that AST import which I haven't tried.)


> > Therefore DWARFCompileUnit and DWARFTypeUnit both contain some their own 
> > data.
> > But DWARFPartialUnit is just a remapping of DWARFCompileUnit (containing
> > DW_TAG_partial_unit) to a new offset without any new data. Particularly
> > m_die_array is not in DWARFPartialUnit.
> 
> In reading the DWZ it sounded like you can only have 1 external debug info
> file and that external debug info file can't itself refer to another?

Right.

> Is this what the DWARFPartialUnit would be for and this is the only
> remapping that needs to happen?

One also needs to remap all the uses of DW_TAG_partial_unit, which is even
used recursively (one DW_TAG_partial_unit uses DW_TAG_imported_unit for
another DW_TAG_partial_unit etc.) otherwise there happen ambiguous
lldb::user_id_t again.


> > (1) Your DWARFTypeUnit patch - it makes it more aligned to LLVM DWARFUnits.
> > (2) My DWZ patch - it is a new feature with no counterpart in LLVM 
> > DWARFUnits.
> > (3) Replacement of LLDB DWARFUnits with LLVM DWARFUnits.
...
> We need to solve (1) first so we can move onto (2). (3) can wait IMHO, but
> if someone really wants to tackle that it is fine.

Great, thanks. So I would like to somehow adjust (1) so that (2) can be built
on top of it (and not having undo it or create similar parallel
functionality).


> I think I need to see any example of this DWZ so I can intelligently comment
> on thing. The brief description of DWZ at
> http://www.dwarfstd.org/ShowIssue.php?issue=120604.1&type=open
>  doesn't
> help me see how this is actually laid out on disk and who refers to who and
> how things are laid out in the DWARF itself.

You said in another mail you already have DWZ sample.  That dwarfstd reference
above I have commented at:
https://reviews.llvm.org/D32167#1063666


Thanks,
Jan
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

2018-04-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:595
+  uint64_t debug_info_size = get_debug_info_data().GetByteSize();
+  data_segment.m_data.OffsetData(debug_info_size);
+}

jankratochvil wrote:
> clayborg wrote:
> > jankratochvil wrote:
> > > I do not like this `DWARFDataExtractor::m_start` modification, it sort of 
> > > corrupts the `DataExtractor` and various operations stop working then - 
> > > such as `DWARFDataExtractor::GetByteSize()`. DWZ patch makes from current 
> > > `dw_offset_t` a virtual (remapped) offset and introduces new physical 
> > > file section offset which is looked up for data extraction. The file 
> > > offset is represented as `DWARFFileOffset` in D40474, instead of `bool 
> > > m_is_dwz;` there could be some `enum { DEBUG_INFO, DEBUG_TYPES, 
> > > DWZ_DEBUG_INFO } m_where;` instead.
> > This means that this diff doesn't affect all of the other DWARF code. 
> > Nothing in .debug_types will refer to anything else (not DW_FORM_ref_addr, 
> > or any external references). So this trick allows us to just treat 
> > .debug_info as if .debug_types was appended to the end since nothing in 
> > .debug_types refers to any DIE outside of its type unit. This also mirrors 
> > what will actually happen with DWARF5 when all of the data is contained 
> > inside of the .debug_info section. This allows each DIE to have a unique 
> > "ID". Any other change requires a lot of changes to the DWARF parser and 
> > logic. So I actually like this feature. We can fix the GetByteSize() if 
> > needed. Basically every object in DWARf right now must be able to make a 
> > unique 64 bit unsigned integer ID in some way that we can get back to that 
> > info and partially parse more. These are handed out as lldb::user_id_t 
> > values for types, functions and more. Each flavor of DWARF will encode what 
> > they want into here. The normal DWARF it is just the absolute offset within 
> > the .debug_info. With .debug_types we just add the size of the .debug_info 
> > to the ID. For DWARF in .o files on Darwin, we encode the compile unit 
> > index into the top 32 bits and the DIE offset into the lower, DWO does 
> > something just as DWZ will need to. DWARFFileOffset doesn't mean much if 
> > there are multiple files. We have many competing type uniquing/debug info 
> > size reduction strategies being employed here. I can't believe we have DWO, 
> > DWZ, and debug types... But we have to make them all work. We can't just 
> > use the absolute file offset because DWO used external files where the file 
> > offsets could be the same in the external .o files... Not sure how this 
> > works with DWZ or what the best option is. I will read up on DWZ so I can 
> > propose some viable options. But each new flavor of the day that gets added 
> > the DWARF parser is adding a bunch of logic and edge cases. If two 
> > technologies (DWZ + DWO, DWZ + debug_types, etc) are used together, we need 
> > to ensure they can.
> > Any other change requires a lot of changes to the DWARF parser and logic. 
> > So I actually like this feature.
> 
> I agree it is a fine quick&dirty hack. Just if my DWZ support gets accepted 
> later anyway then this `.debug_types` feature could be implemented by its 
> framework in a clean way (as a regular DIEs remapping which is required for 
> DWZ anyway).
> 
> > If two technologies (DWZ + DWO, DWZ + debug_types, etc) are used together, 
> > we need to ensure they can.
> 
> `DWZ + DWO` do not make sense to me.  I haven't tried to use DWZ for DWO but 
> DWZ finds common DWARF subtrees typically across CUs so it would not find 
> much.
> 
> `DWZ + debug_types` is explicitly supported by the DWZ tool although that is 
> IMO for compatibility only, DWZ can make the common type references slightly 
> smaller than debug_types. I will sure need to implement debug_types support 
> into my DWZ-for-LLDB patchset later.
> 
> For DWARF in .o files on Darwin, we encode the compile unit index into the 
> top 32 bits and the DIE offset into the lower

BTW that prevents implementing DWARF64 so I wanted to prevent using the 32/32 
split. Currently Fedora/RHEL already contains files with `.debug_info` size 
near the 32-bit limit:
```http://ftp.muni.cz/pub/linux/fedora/linux/development/rawhide/Everything/x86_64/debug/tree/Packages/q/qt5-qtwebkit-debuginfo-5.212.0-0.20.alpha2.fc29.x86_64.rpm
/usr/lib/debug/usr/lib64/libQt5WebKit.so.5.212.0-5.212.0-0.20.alpha2.fc29.x86_64.debug
.debug_info size = 0x9bff1b08 = 2.4GiB
```
Although that is because neither DWZ (as it would run out of memory) nor 
`.debug_types` (as it is expected DWZ will handle the duplicities) are applied 
and so all CUs are separate and even if it was >4GB there would still be no 
need for 64-bit `DW_FORM_ref_addr`. Still we are already close to the 32-bit 
limit, also shifting the split for example to 40/24 would make some limit on 
number of CUs etc.

Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Greg Clayton via lldb-commits


> On Apr 10, 2018, at 2:38 PM, Jan Kratochvil  wrote:
> 
> On Tue, 10 Apr 2018 17:46:55 +0200, Greg Clayton wrote:
>>> On Apr 7, 2018, at 2:04 AM, Jan Kratochvil  
>>> wrote:
>>> DW_TAG_partial_unit gets read in (by
>>> DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded) as DWARFCompileUnit because
>>> there is no quick enough way to find the difference.  It would require 
>>> reading
>>> the first DIE tag which means to read and decode .debug_abbrev for each unit
>>> being scanned.
>> 
>> If there is no structural difference other than the first DW_TAG, is there
>> a reason you would need to know the difference?
> 
> Not really, it also currently does work with DWARFCompileUnit.
> Just one could assert that nobody tries to use DIEs directly from that
> DWARFCompileUnit without their remapping to DWARFPartialUnit.

If it come down to really just getting a unique and lldb::user_id_t, there are 
probably easier ways. I always try to leave the DWARFCompileUnit and those 
level classes out of the loop if possible so no changes are needed for them 
when possible. Sounds like from what you said below there aren't any new 
methods needed on DWARFCompileUnit for it to be used as a DWARFPartialUnit and 
hopefully DWARFPartialUnit won't need to exist if we do things correctly. More 
details below...

> 
>> It would be fine to just add extra methods on DWARFCompileUnit that do
>> partial unit kind of things if it is a partial unit?
> 
> There are not really any extra methods.
> 
> 
>>> DWARFPartialUnit is used only as a remapping of DWARFCompileUnit to a new
>>> offset without any new data (there is stored only a new remapped offset 
>>> whose
>>> value is only made up internally in LLDB) at the moment someone uses
>>> DW_TAG_imported_unit for it - at that moment we easily know that unit has to
>>> be DW_TAG_partial_unit.
>> 
>> It is remapped to a new offset just to keep LLDB's lldb::user_id_t stuff
>> happy?
> 
> Yes.
> 
>> A partial unit can refer to an external file on disk. The remapping is
>> solely making a unique offset by adding an offset to the offset of the
>> external file?
> 
> Not only that. Even when DWZ does not use any external file then one
> DW_TAG_partial_unit is included into many DW_TAG_compile_unit and so DIEs from
> that DW_TAG_partial_unit would no longer have unique lldb::user_id_t.
> (Although maybe that can be solved by that AST import which I haven't tried.)

Why would we need to inline anything if the partial unit is along side the same 
compile units? Why wouldn't you just use DW_FORM_ref_addr to refer directly to 
any DIEs in a partial unit from a compile unit? I must be missing something. 
You you walk me through creating this kind of scenario? I have the multiple 
files that refer to an external file with one or more DW_TAG_partial_unit tags 
(or is there only ever just one DW_TAG_partial_unit?). But I would like to see 
this inlined DW_TAG_partial_unit along side DWARF. Why would and compile unit 
just not refer directly to the DIEs? They shouldn't have to import anything?
> 
> 
> 
>>> Therefore DWARFCompileUnit and DWARFTypeUnit both contain some their own 
>>> data.
>>> But DWARFPartialUnit is just a remapping of DWARFCompileUnit (containing
>>> DW_TAG_partial_unit) to a new offset without any new data. Particularly
>>> m_die_array is not in DWARFPartialUnit.
>> 
>> In reading the DWZ it sounded like you can only have 1 external debug info
>> file and that external debug info file can't itself refer to another?
> 
> Right.
> 
>> Is this what the DWARFPartialUnit would be for and this is the only
>> remapping that needs to happen?
> 
> One also needs to remap all the uses of DW_TAG_partial_unit, which is even
> used recursively (one DW_TAG_partial_unit uses DW_TAG_imported_unit for
> another DW_TAG_partial_unit etc.) otherwise there happen ambiguous
> lldb::user_id_t again.

If this is all in the same file, then the offsets are all in the .debug_info? 
What am I missing?
> 
> 
>>> (1) Your DWARFTypeUnit patch - it makes it more aligned to LLVM DWARFUnits.
>>> (2) My DWZ patch - it is a new feature with no counterpart in LLVM 
>>> DWARFUnits.
>>> (3) Replacement of LLDB DWARFUnits with LLVM DWARFUnits.
> ...
>> We need to solve (1) first so we can move onto (2). (3) can wait IMHO, but
>> if someone really wants to tackle that it is fine.
> 
> Great, thanks. So I would like to somehow adjust (1) so that (2) can be built
> on top of it (and not having undo it or create similar parallel
> functionality).
> 
> 
>> I think I need to see any example of this DWZ so I can intelligently comment
>> on thing. The brief description of DWZ at
>> http://www.dwarfstd.org/ShowIssue.php?issue=120604.1&type=open
>>  doesn't
>> help me see how this is actually laid out on disk and who refers to who and
>> how things are laid out in the DWARF itself.
> 
> You said in another mail you already have DWZ sample.  That dwarfstd reference
> above

Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Jan Kratochvil via lldb-commits
On Wed, 11 Apr 2018 00:22:45 +0200, Greg Clayton wrote:
> > On Apr 10, 2018, at 2:38 PM, Jan Kratochvil  
> > wrote:
> > Not only that. Even when DWZ does not use any external file then one
> > DW_TAG_partial_unit is included into many DW_TAG_compile_unit and so DIEs 
> > from
> > that DW_TAG_partial_unit would no longer have unique lldb::user_id_t.
> > (Although maybe that can be solved by that AST import which I haven't 
> > tried.)
> 
> Why would we need to inline anything if the partial unit is along side the
> same compile units? Why wouldn't you just use DW_FORM_ref_addr to refer
> directly to any DIEs in a partial unit from a compile unit?

DWZ also uses DW_FORM_ref_addr for exactly that.


> I must be missing something.

DWZ tries to keep the same structure of the DWARF files. If there is some
top-level (under DW_TAG_compile_unit) declaration then it is kept there - by
using DW_TAG_imported_unit.


> I have the multiple files that refer to an external file with one or more
> DW_TAG_partial_unit tags (or is there only ever just one
> DW_TAG_partial_unit?).

There are always many DW_TAG_partial_unit in both the DWZ-common symbol file
and in the main symbol file. Only main symbol file contains many
DW_TAG_compile_unit.


> But I would like to see this inlined DW_TAG_partial_unit along side DWARF.

You mean the real DWZ files? Here is some DWZ-compressed file:
https://people.redhat.com/jkratoch/rustgdbbug.tar.xz
main symbol file: rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug
DWZ-common symbol file: rustgdbbug/.dwz/rust-1.24.0-2.fc27.x86_64
At least recent binutils readelf -wi decodes it correctly.


> Why would and compile unit just not refer directly to the DIEs? They
> shouldn't have to import anything?

One cannot create new DIE under DW_TAG_compile_unit using just
DW_FORM_ref_addr. That is what DW_TAG_imported_unit is good for.


You are sort of right that one could move all declarations to a single
DW_TAG_partial_unit and use one DW_TAG_imported_unit for that in each
DW_TAG_compile_unit and it is done. But:
(1) GDB would run out of memory when expanding any single CU.
(2) Primitive types (and struct types in plain C) may differ between CUs.
(3) It would make a subtle change of the DWARF structure causing various
incompatibilities or even bugs in various tools (primarily GDB).


> > One also needs to remap all the uses of DW_TAG_partial_unit, which is even
> > used recursively (one DW_TAG_partial_unit uses DW_TAG_imported_unit for
> > another DW_TAG_partial_unit etc.) otherwise there happen ambiguous
> > lldb::user_id_t again.
> 
> If this is all in the same file, then the offsets are all in the
> .debug_info? What am I missing?

LLVM was asserting when I reported the same lldb::user_id_t referenced by
multiple CUs. From our discussion it looks to me that maybe I just did not
have the DWZ common symbol file with proper offset. Otherwise maybe I can
report the same lldb::user_id_t if multiple CUs reference the same DIE from
DW_TAG_partial_unit? As LLVM considers all CUs in a file as the same context.
I will try that again.


Thanks,
Jan
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-10 Thread Jan Kratochvil via lldb-commits
On Wed, 11 Apr 2018 00:53:20 +0200, Jan Kratochvil wrote:
> On Wed, 11 Apr 2018 00:22:45 +0200, Greg Clayton wrote:
> > If this is all in the same file, then the offsets are all in the
> > .debug_info? What am I missing?
> 
> LLVM was asserting when I reported the same lldb::user_id_t referenced by
> multiple CUs. From our discussion it looks to me that maybe I just did not
> have the DWZ common symbol file with proper offset. Otherwise maybe I can
> report the same lldb::user_id_t if multiple CUs reference the same DIE from
> DW_TAG_partial_unit? As LLVM considers all CUs in a file as the same context.
> I will try that again.

In such case if you are sure no real DIEs remapping (to make each inclusion of
DW_TAG_partial_unit unique) is needed then no remapping DWARFPartialUnit is
needed and your removal of my DWARFUnit abstraction by Data() (which
I incorrectly reverted) is also OK.

(Although I will believe it only after I code it; this is what I coded first
but I could have some other bugs there that time.)


Thanks,
Jan
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits