[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Nathan Lanza via Phabricator via lldb-commits]

lanza added a comment.

So the ABI plugin for Windows x64 seems to be necessary for proper unwinding. 
(Also, proper parsing of xdata and pdata sections might be necessary, too.) I 
suspect that this could be related to 
https://bugs.llvm.org/show_bug.cgi?id=32343, but would need to look more into 
this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [lldb] r360971 - [Docs] Fix headings in remote debugging

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu May 16 18:38:16 2019
New Revision: 360971

URL: http://llvm.org/viewvc/llvm-project?rev=360971=rev
Log:
[Docs] Fix headings in remote debugging

Add the proper headings instead of using just a bold font. Also add the
local ToC.

Modified:
lldb/trunk/docs/use/remote.rst

Modified: lldb/trunk/docs/use/remote.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/use/remote.rst?rev=360971=360970=360971=diff
==
--- lldb/trunk/docs/use/remote.rst (original)
+++ lldb/trunk/docs/use/remote.rst Thu May 16 18:38:16 2019
@@ -23,8 +23,11 @@ communicating with it over the loopback
 debugging this whole process is transparent to the user. The platform binary is
 not used in this case, since no file transfers are needed.
 
+.. contents::
+   :local:
+
 Preparation for Remote Debugging
-
+-
 
 While the process of actual debugging (stepping, backtraces, evaluating
 expressions) is same as in the local case, in the case of remote debugging,
@@ -32,7 +35,8 @@ more preparation is needed as the requir
 remote system automatically. Also, if the remote system runs a different OS or
 architecture, the server component needs to be compiled separately.
 
-**Remote system**
+Remote system
+*
 
 On Linux and Android, all required remote functionality is contained in the
 lldb-server binary. This binary combines the functionality of the platform and
@@ -64,7 +68,8 @@ originating from that address. Adding a
 will fork off a new process for every incoming connection, allowing multiple
 parallel debug sessions.
 
-**Local system**
+Local system
+
 
 On the local system, you need to let LLDB know that you intend to do remote
 debugging. This is achieved through the platform command and its sub-commands.
@@ -130,9 +135,11 @@ application needs additional files, you
 commands: get-file, put-file, mkdir, etc. The environment can be prepared
 further using the platform shell command.
 
-**Launching a locally built process on the remote machine**
+Launching a locally built process on the remote machine
+---
 
-*Install and run in the platform working directory*
+Install and run in the platform working directory
+*
 
 To launch a locally built process on the remote system in the platform working
 directory:
@@ -151,7 +158,8 @@ changed. LLDB will automatically launch
 allow you to debug this executable, connect to it and start your debug session
 for you.
 
-*Changing the platform working directory*
+Changing the platform working directory
+***
 
 You can change the platform working directory while connected to the platform
 with:
@@ -173,7 +181,8 @@ And you can verify it worked using "plat
 
 If we run again, the program will be installed into ``/usr/local/bin``.
 
-*Install and run by specifying a remote install path*
+Install and run by specifying a remote install path
+***
 
 If you want the "a.out" executable to be installed into "/bin/a.out" instead of
 the platform's current working directory, we can set the platform file
@@ -201,7 +210,8 @@ specification:
(lldb) script 
lldb.target.module['libbar.so'].SetPlatformFileSpec("/usr/local/lib/libbar.so")
(lldb) run
 
-*Attaching to a remote process*
+Attaching to a remote process
+*
 
 If you want to attach to a remote process, you can first list the processes on
 the remote system:


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

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

echristo's comments on the mail thread for this didn't make it to phab. I 
apologize for repeating more or less everything he said, I had looked on phab 
before reading email.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [lldb] r360967 - [Docs] Remove architectures from feature matrix

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu May 16 18:04:37 2019
New Revision: 360967

URL: http://llvm.org/viewvc/llvm-project?rev=360967=rev
Log:
[Docs] Remove architectures from feature matrix

This is outdated, there's a bunch of architectures missing. If we want
them to be part of this table they should be a separate row anyway.

Modified:
lldb/trunk/docs/status/status.rst

Modified: lldb/trunk/docs/status/status.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/status/status.rst?rev=360967=360966=360967=diff
==
--- lldb/trunk/docs/status/status.rst (original)
+++ lldb/trunk/docs/status/status.rst Thu May 16 18:04:37 2019
@@ -40,27 +40,26 @@ section below.
 
 Features Matrix
 ---
-+++-+--+--+
-| Feature| FreeBSD| Linux   | 
macOS(i386/x86_64 and ARM/Thumb) | Windows (i386)   |
-|| (x86_64)   | (x86_64 and PPC64le)|  
|  |
-+++=+==+==+
-| Backtracing| OK | OK  | OK   
| OK   |
-+++-+--+--+
-| Breakpoints| OK | OK  | OK   
| OK   |
-+++-+--+--+
-| C++11: | OK | OK  | OK   
| Unknown  |
-+++-+--+--+
-| Commandline lldb tool  | OK | OK  | OK   
| OK   |
-+++-+--+--+
-| Core file debugging| OK (ELF)   | OK (ELF)| OK 
(MachO)   | OK (Minidump)|
-+++-+--+--+
-| Debugserver (remote debugging) | Not ported | Not ported  | OK   
| Not ported   |
-+++-+--+--+
-| Disassembly| OK | OK  | OK   
| OK   |
-+++-+--+--+
-| Expression evaluation  | Unknown| Works with some bugs| OK   
| Works with some bugs |
-+++-+--+--+
-| JIT debugging  | Unknown| Symbolic debugging only | 
Untested | No   |
-+++-+--+--+
-| Objective-C 2.0:   | Unknown| Not applicable  | OK   
| Not applicable   |
-+++-+--+--+
++++-++--+
+| Feature| FreeBSD| Linux   | 
macOS  | Windows  |
++++=++==+
+| Backtracing| OK | OK  | OK   
  | OK   |
++++-++--+
+| Breakpoints| OK | OK  | OK   
  | OK   |
++++-++--+
+| C++11: | OK | OK  | OK   
  | Unknown  |

[Lldb-commits] [lldb] r360966 - [CommandInterpreter] Accept blanks after `all` or [0-9]+ for bt.

[Davide Italiano via lldb-commits]

Author: davide
Date: Thu May 16 18:03:21 2019
New Revision: 360966

URL: http://llvm.org/viewvc/llvm-project?rev=360966=rev
Log:
[CommandInterpreter] Accept blanks after `all` or [0-9]+ for bt.

Previously "bt all" would've failed as the regex didn't match
them.

Over the shoulder review by Jonas Devlieghere.



Added:
lldb/trunk/lit/Commands/command-backtrace.test
Modified:
lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Added: lldb/trunk/lit/Commands/command-backtrace.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Commands/command-backtrace.test?rev=360966=auto
==
--- lldb/trunk/lit/Commands/command-backtrace.test (added)
+++ lldb/trunk/lit/Commands/command-backtrace.test Thu May 16 18:03:21 2019
@@ -0,0 +1,12 @@
+# Check basic functionality of command bt.
+# RUN: %lldb -s %s 2>&1 | FileCheck %s
+
+# Make sure this is not rejected by the parser as invalid syntax.
+# Blank characters after the '1' are important, as we're testing the parser.
+bt 1  
+# CHECK: error: invalid target
+
+# Make sure this is not rejected by the parser as invalid syntax.
+# Blank characters after the '1' are important, as we're testing the parser.
+bt all   
+# CHECK: error: invalid target

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=360966=360965=360966=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Thu May 16 18:03:21 
2019
@@ -758,12 +758,12 @@ void CommandInterpreter::LoadCommandDict
 // command if you wanted to backtrace three frames you would do "bt -c 3"
 // but the intention is to have this emulate the gdb "bt" command and so
 // now "bt 3" is the preferred form, in line with gdb.
-if (bt_regex_cmd_up->AddRegexCommand("^([[:digit:]]+)$",
+if (bt_regex_cmd_up->AddRegexCommand("^([[:digit:]]+)\\s*$",
  "thread backtrace -c %1") &&
-bt_regex_cmd_up->AddRegexCommand("^-c ([[:digit:]]+)$",
+bt_regex_cmd_up->AddRegexCommand("^-c ([[:digit:]]+)\\s*$",
  "thread backtrace -c %1") &&
-bt_regex_cmd_up->AddRegexCommand("^all$", "thread backtrace all") &&
-bt_regex_cmd_up->AddRegexCommand("^$", "thread backtrace")) {
+bt_regex_cmd_up->AddRegexCommand("^all\\s*$", "thread backtrace all") 
&&
+bt_regex_cmd_up->AddRegexCommand("^\\s*$", "thread backtrace")) {
   CommandObjectSP command_sp(bt_regex_cmd_up.release());
   m_command_dict[command_sp->GetCommandName()] = command_sp;
 }


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

[Lldb-commits] [lldb] r360964 - [Docs] Unify sidebar padding

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu May 16 17:45:05 2019
New Revision: 360964

URL: http://llvm.org/viewvc/llvm-project?rev=360964=rev
Log:
[Docs] Unify sidebar padding

Unify the padding across list items and the list header.

Modified:
lldb/trunk/docs/_static/lldb.css

Modified: lldb/trunk/docs/_static/lldb.css
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/_static/lldb.css?rev=360964=360963=360964=diff
==
--- lldb/trunk/docs/_static/lldb.css (original)
+++ lldb/trunk/docs/_static/lldb.css Thu May 16 17:45:05 2019
@@ -39,7 +39,7 @@ table.mapping td.content {
 div.sphinxsidebar .caption {
   font-family: Helvetica, Verdana, sans-serif;
   font-size: 10pt;
-  font-weight:bold;
+  font-weight: bold;
   color: #fefefe;
   background: #606060;
   margin-bottom: 0;
@@ -53,9 +53,8 @@ div.sphinxsidebar a:hover {
 }
 
 div.sphinxsidebar li {
-  padding-left:5px;
-  padding-right:5px;
-  border-bottom:1px solid #fefefe;
+  padding-left: 7px;
+  border-bottom: 1px solid #fefefe;
 }
 
 div.sphinxsidebar li:hover {


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

[Lldb-commits] [PATCH] D62012: Make DWARFContext dwo-aware and port debug_info sections over

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:47
+m_data_debug_info);
+  else
+return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugInfo,

After the early return we can remove the else.


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

https://reviews.llvm.org/D62012



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

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 199943.
JDevlieghere added a comment.

Address code review feedback


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

https://reviews.llvm.org/D61994

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -81,6 +81,17 @@
 static constexpr uintptr_t DefaultValueTrue = true;
 static constexpr uintptr_t DefaultValueFalse = false;
 static constexpr const char *NoCStrDefault = nullptr;
+static constexpr const char *InitFileWarning =
+"There is a .lldbinit file in the current directory which is not being "
+"read.\n"
+"To silence this warning without sourcing in the local .lldbinit,\n"
+"add the following to the lldbinit file in your home directory:\n"
+"settings set target.load-cwd-lldbinit false\n"
+"To allow lldb to source .lldbinit files in the current working "
+"directory,\n"
+"set the value of this variable to true.  Only do so if you understand "
+"and\n"
+"accept the security risk.";
 
 static constexpr PropertyDefinition g_properties[] = {
 {"expand-regex-aliases", OptionValue::eTypeBoolean, NoGlobalSetting,
@@ -2091,99 +2102,120 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
+static FileSpec GetHomeInitFile(llvm::StringRef suffix = {}) {
+  std::string init_file_name = ".lldbinit";
+  if (!suffix.empty()) {
+init_file_name.append("-");
+init_file_name.append(suffix.str());
+  }
+
+  llvm::SmallString<128> buffer;
+  llvm::sys::path::home_directory(buffer);
+  llvm::sys::path::append(buffer, init_file_name);
+
+  FileSpec init_file(buffer.c_str());
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static FileSpec GetCwdInitFile() {
+  FileSpec init_file(".lldbinit");
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static LoadCWDlldbinitFile ShouldLoadCwdInitFile() {
+  lldb::TargetPropertiesSP properties = Target::GetGlobalProperties();
+  if (!properties)
+return eLoadCWDlldbinitFalse;
+  return properties->GetLoadCWDlldbinitFile();
+}
+
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject ) {
-  FileSpec init_file;
-  if (in_cwd) {
-lldb::TargetPropertiesSP properties = Target::GetGlobalProperties();
-if (properties) {
-  // In the current working directory we don't load any program specific
-  // .lldbinit files, we only look for a ".lldbinit" file.
-  if (m_skip_lldbinit_files)
-return;
+  assert(!m_skip_lldbinit_files);
 
-  LoadCWDlldbinitFile should_load = properties->GetLoadCWDlldbinitFile();
-  if (should_load == eLoadCWDlldbinitWarn) {
-FileSpec dot_lldb(".lldbinit");
-FileSystem::Instance().Resolve(dot_lldb);
-llvm::SmallString<64> home_dir_path;
-llvm::sys::path::home_directory(home_dir_path);
-FileSpec homedir_dot_lldb(home_dir_path.c_str());
-homedir_dot_lldb.AppendPathComponent(".lldbinit");
-FileSystem::Instance().Resolve(homedir_dot_lldb);
-if (FileSystem::Instance().Exists(dot_lldb) &&
-dot_lldb.GetDirectory() != homedir_dot_lldb.GetDirectory()) {
-  result.AppendErrorWithFormat(
-  "There is a .lldbinit file in the current directory which is not "
-  "being read.\n"
-  "To silence this warning without sourcing in the local "
-  ".lldbinit,\n"
-  "add the following to the lldbinit file in your home directory:\n"
-  "settings set target.load-cwd-lldbinit false\n"
-  "To allow lldb to source .lldbinit files in the current working "
-  "directory,\n"
-  "set the value of this variable to true.  Only do so if you "
-  "understand and\n"
-  "accept the security risk.");
-  result.SetStatus(eReturnStatusFailed);
-  return;
-}
-  } else if (should_load == eLoadCWDlldbinitTrue) {
-init_file.SetFile("./.lldbinit", FileSpec::Style::native);
-FileSystem::Instance().Resolve(init_file);
-  }
-}
-  } else {
-// If we aren't looking in the current working directory we are looking in
-// the home directory. We will first see if there is an application
-// specific ".lldbinit" file whose name is "~/.lldbinit" followed by a "-"
-// and the name of the program. If this file doesn't exist, we fall back to
-// just the "~/.lldbinit" file. We also obey any requests to not load the
-// init files.
-llvm::SmallString<64> home_dir_path;
-llvm::sys::path::home_directory(home_dir_path);
-

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Bob Haarman via Phabricator via lldb-commits]

inglorion added a comment.

Reverted in r360955.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [PATCH] D62041: [Docs] Remove SVN checkout from LLDB build steps

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/docs/resources/build.rst:242
 
-We first need to checkout the source trees into the appropriate locations. Both
-Clang and LLDB build as subprojects of LLVM. This means we will be checking out
-the source for both Clang and LLDB into the tools subdirectory of LLVM. We will
-be setting up a directory hierarchy looking something like this:
-
-::
-
-  llvm
-  |
-  `-- tools
-  |
-  +-- clang
-  |
-  `-- lldb
-
-For reference, we will call the root of the LLVM project tree $llvm, and the
-roots of the Clang and LLDB source trees $clang and $lldb respectively.
-
-Change to the directory where you want to do development work and checkout
-LLVM:
-
-::
-
-  > svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
-
-
-Now switch to LLVM’s tools subdirectory and checkout both Clang and LLDB:
-
-::
-
-  > cd $llvm/tools
-  > svn co http://llvm.org/svn/llvm-project/cfe/trunk clang
-  > svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
-
-In general, building the LLDB trunk revision requires trunk revisions of both
-LLVM and Clang.
-
 It is highly recommended that you build the system out of tree. Create a second
 build directory and configure the LLVM project tree to your specifications as

Let's remove the next paragraph as well and remove the whole section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62041



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

[Lldb-commits] [lldb] r360956 - Slightly update the macOS part of status.rst to be less out-of-date.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu May 16 16:39:08 2019
New Revision: 360956

URL: http://llvm.org/viewvc/llvm-project?rev=360956=rev
Log:
Slightly update the macOS part of status.rst to be less out-of-date.

Modified:
lldb/trunk/docs/status/status.rst

Modified: lldb/trunk/docs/status/status.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/status/status.rst?rev=360956=360955=360956=diff
==
--- lldb/trunk/docs/status/status.rst (original)
+++ lldb/trunk/docs/status/status.rst Thu May 16 16:39:08 2019
@@ -4,10 +4,11 @@ Status
 macOS
 -
 
-LLDB has matured a lot in the last year and can be used for C, C++ and
-Objective-C development for x86_64, i386 and ARM debugging. The entire public
-API is exposed though a framework on macOS which is used by Xcode, the lldb
-command line tool, and can also be used by Python. The entire public API is
+LLDB is the system debugger on macOS, iOS, tvOS, and watchOS and
+can be used for C, C++, Objective-C and Swift development for x86_64,
+i386, ARM, and AArch64 debugging. The entire public API is exposed
+through a macOS framework which is used by Xcode and the `lldb`
+command line tool. It can also be imported from Python. The entire public API 
is
 exposed through script bridging which allows LLDB to use an embedded Python
 script interpreter, as well as having a Python module named "lldb" which can be
 used from Python on the command line. This allows debug sessions to be


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

Re: [Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Eric Christopher via lldb-commits]

For now I think the compelling argument on revert is:

- this breaks currently working thinlto builds

whether or not this has uncovered a latent problem in thinlto, the
linking strategy used, or something else I think we should revert
until we can take a look and decide where the problems are and what
should be done about them.

Thanks

-eric

On Thu, May 16, 2019 at 4:17 PM Bob Haarman via Phabricator
 wrote:
>
> inglorion added a comment.
>
> I also think we should revert this change while we think about the approach. 
> I've been reluctant to do this because I was impressed by how many bytes of 
> output it saves, and I didn't want to lose that just because it doesn't play 
> nice with ThinLTO. However, after some more data gathering and talking to 
> people, I think that:
>
> 1. There is enough doubt that this is the right approach that we probably 
> should take some time to think about which way we really want to go.
>
> 2. In the meantime, a build configuration that used to work is broken.
>
> 3. To fix that while leaving this change in, we would have to build more 
> things on top of the change, which seems unwise if we're not sure that this 
> is the best approach.
>
> So I think we should revert and rethink.
>
> Reasons I'm not convinced that this is the right approach:
>
> 1. We have now identified multiple use cases that break with this change. 
> ThinLTO is one, -fmodules-debuginfo is another. The general theme here is 
> that the expectation is that if an object file is passed to the linker and 
> not between --start-lib and --end-lib or in a static library, the expectation 
> is that it makes it into the final link. -gc-sections asks to discard unused 
> sections, but it's not clear that this should also apply to debug 
> information, and we have seen some problems.
>
> 2. It's also not clear to me that this is as much of a win as I initially 
> thought. It is in the cases that have been demonstrated, of course, but I'm 
> not sure how well that generalizes. On a local Clang build I performed, for 
> example, it made exactly 0 bytes difference. Perhaps the big wins only happen 
> in pathological cases that are better addressed some other way.
>
> So I'm going to revert this to unbreak the Chromium/Android build, and then 
> we can think some more about how we can get the size win without the breakage.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54747/new/
>
> https://reviews.llvm.org/D54747
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Bob Haarman via Phabricator via lldb-commits]

inglorion added a comment.

I also think we should revert this change while we think about the approach. 
I've been reluctant to do this because I was impressed by how many bytes of 
output it saves, and I didn't want to lose that just because it doesn't play 
nice with ThinLTO. However, after some more data gathering and talking to 
people, I think that:

1. There is enough doubt that this is the right approach that we probably 
should take some time to think about which way we really want to go.

2. In the meantime, a build configuration that used to work is broken.

3. To fix that while leaving this change in, we would have to build more things 
on top of the change, which seems unwise if we're not sure that this is the 
best approach.

So I think we should revert and rethink.

Reasons I'm not convinced that this is the right approach:

1. We have now identified multiple use cases that break with this change. 
ThinLTO is one, -fmodules-debuginfo is another. The general theme here is that 
the expectation is that if an object file is passed to the linker and not 
between --start-lib and --end-lib or in a static library, the expectation is 
that it makes it into the final link. -gc-sections asks to discard unused 
sections, but it's not clear that this should also apply to debug information, 
and we have seen some problems.

2. It's also not clear to me that this is as much of a win as I initially 
thought. It is in the cases that have been demonstrated, of course, but I'm not 
sure how well that generalizes. On a local Clang build I performed, for 
example, it made exactly 0 bytes difference. Perhaps the big wins only happen 
in pathological cases that are better addressed some other way.

So I'm going to revert this to unbreak the Chromium/Android build, and then we 
can think some more about how we can get the size win without the breakage.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [PATCH] D62041: [Docs] Remove SVN checkout from LLDB build steps

[J. Ryan Stinnett via Phabricator via lldb-commits]

jryans created this revision.
jryans added a reviewer: JDevlieghere.
Herald added a project: LLDB.

This removes several older paragraphs in the LLDB build steps for Unix systems
which suggested checking out various components via SVN. Since there's a
separate page about getting the LLDB source which only mentions Git, it seems
appropriate to remove this older info from the build docs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62041

Files:
  lldb/docs/resources/build.rst


Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -239,43 +239,6 @@
 
 **Building LLDB**
 
-We first need to checkout the source trees into the appropriate locations. Both
-Clang and LLDB build as subprojects of LLVM. This means we will be checking out
-the source for both Clang and LLDB into the tools subdirectory of LLVM. We will
-be setting up a directory hierarchy looking something like this:
-
-::
-
-  llvm
-  |
-  `-- tools
-  |
-  +-- clang
-  |
-  `-- lldb
-
-For reference, we will call the root of the LLVM project tree $llvm, and the
-roots of the Clang and LLDB source trees $clang and $lldb respectively.
-
-Change to the directory where you want to do development work and checkout
-LLVM:
-
-::
-
-  > svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
-
-
-Now switch to LLVM’s tools subdirectory and checkout both Clang and LLDB:
-
-::
-
-  > cd $llvm/tools
-  > svn co http://llvm.org/svn/llvm-project/cfe/trunk clang
-  > svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
-
-In general, building the LLDB trunk revision requires trunk revisions of both
-LLVM and Clang.
-
 It is highly recommended that you build the system out of tree. Create a second
 build directory and configure the LLVM project tree to your specifications as
 outlined in LLVM’s Getting Started Guide. A typical build procedure might be:


Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -239,43 +239,6 @@
 
 **Building LLDB**
 
-We first need to checkout the source trees into the appropriate locations. Both
-Clang and LLDB build as subprojects of LLVM. This means we will be checking out
-the source for both Clang and LLDB into the tools subdirectory of LLVM. We will
-be setting up a directory hierarchy looking something like this:
-
-::
-
-  llvm
-  |
-  `-- tools
-  |
-  +-- clang
-  |
-  `-- lldb
-
-For reference, we will call the root of the LLVM project tree $llvm, and the
-roots of the Clang and LLDB source trees $clang and $lldb respectively.
-
-Change to the directory where you want to do development work and checkout
-LLVM:
-
-::
-
-  > svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm
-
-
-Now switch to LLVM’s tools subdirectory and checkout both Clang and LLDB:
-
-::
-
-  > cd $llvm/tools
-  > svn co http://llvm.org/svn/llvm-project/cfe/trunk clang
-  > svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
-
-In general, building the LLDB trunk revision requires trunk revisions of both
-LLVM and Clang.
-
 It is highly recommended that you build the system out of tree. Create a second
 build directory and configure the LLVM project tree to your specifications as
 outlined in LLVM’s Getting Started Guide. A typical build procedure might be:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Eric Christopher via lldb-commits]

Unlikely. I'm in the middle of getting some, but that's still going to
be a few months I think.

And yes, we should probably just revert for now.

I can unless someone beats me to it.

Thanks!

On Thu, May 16, 2019 at 2:29 PM Nico Weber via Phabricator
 wrote:
>
> thakis added a comment.
>
> This breaking both debug info in thinlto builds and fmodules-debuginfo is 
> probably enough to revert and go back to the drawing board.
>
> I suppose we don't have any debug info quality tests that use thinlto?
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54747/new/
>
> https://reviews.llvm.org/D54747
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r360945 - [Target] Stop linking against lldbPluginObjCLanguage

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Thu May 16 15:01:25 2019
New Revision: 360945

URL: http://llvm.org/viewvc/llvm-project?rev=360945=rev
Log:
[Target] Stop linking against lldbPluginObjCLanguage

Modified:
lldb/trunk/source/Target/CMakeLists.txt
lldb/trunk/source/Target/LanguageRuntime.cpp

Modified: lldb/trunk/source/Target/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/CMakeLists.txt?rev=360945=360944=360945=diff
==
--- lldb/trunk/source/Target/CMakeLists.txt (original)
+++ lldb/trunk/source/Target/CMakeLists.txt Thu May 16 15:01:25 2019
@@ -67,7 +67,6 @@ add_lldb_library(lldbTarget
 lldbSymbol
 lldbUtility
 lldbPluginExpressionParserClang
-lldbPluginObjCLanguage
 lldbPluginProcessUtility
 
   LINK_COMPONENTS

Modified: lldb/trunk/source/Target/LanguageRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/LanguageRuntime.cpp?rev=360945=360944=360945=diff
==
--- lldb/trunk/source/Target/LanguageRuntime.cpp (original)
+++ lldb/trunk/source/Target/LanguageRuntime.cpp Thu May 16 15:01:25 2019
@@ -7,10 +7,10 @@
 
//===--===//
 
 #include "lldb/Target/LanguageRuntime.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 


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

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

This breaking both debug info in thinlto builds and fmodules-debuginfo is 
probably enough to revert and go back to the drawing board.

I suppose we don't have any debug info quality tests that use thinlto?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

There's another use case worth considering here, which is -fmodules-debuginfo. 
In that situation, a standalone object file is emitted that contains nothing 
but DWARF .debug_info sections, and the object is fed to the linker. If the 
linker drops it with --gc-sections, it's not going to work. Here's a transcript 
of me trying to use this feature:
https://reviews.llvm.org/P8150

Note that t.o (the normal object) only contains a forward declaration of Foo, 
and the complete definition is provided in the debug-info-only module object 
file, t2.o in this case.

I think, before this change, passing an object with only .debug_info in it (and 
--gc-sections) used to retain that debug info. I think we need a way to keep 
doing that going forward. I think that might mean we need a new flag for this 
new behavior, or we should revert this patch altogether, and recommend that 
people who want smaller debug info use tools like dsymutil, dwz, DWP, or other 
format aware things that can remove unreferenced or duplicate DWARF.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [lldb] r360930 - Factor out switch statement into a helper function (NFC)

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu May 16 13:03:05 2019
New Revision: 360930

URL: http://llvm.org/viewvc/llvm-project?rev=360930=rev
Log:
Factor out switch statement into a helper function (NFC)

This addresses post-commit review feedback for https://reviews.llvm.org/D62015.

Modified:
lldb/trunk/include/lldb/Target/Language.h
lldb/trunk/source/Core/ValueObject.cpp
lldb/trunk/source/Target/Language.cpp

Modified: lldb/trunk/include/lldb/Target/Language.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Language.h?rev=360930=360929=360930=diff
==
--- lldb/trunk/include/lldb/Target/Language.h (original)
+++ lldb/trunk/include/lldb/Target/Language.h Thu May 16 13:03:05 2019
@@ -255,6 +255,9 @@ public:
 
   static bool LanguageIsC(lldb::LanguageType language);
 
+  /// Equivalent to \c LanguageIsC||LanguageIsObjC||LanguageIsCPlusPlus.
+  static bool LanguageIsCFamily(lldb::LanguageType language);
+
   static bool LanguageIsPascal(lldb::LanguageType language);
 
   // return the primary language, so if LanguageIsC(l), return eLanguageTypeC,

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=360930=360929=360930=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Thu May 16 13:03:05 2019
@@ -1115,17 +1115,11 @@ const char *ValueObject::GetObjectDescri
   if (const char *desc = get_object_description(native_language))
 return desc;
 
-  switch (native_language) {
-  case eLanguageTypeC:
-  case eLanguageTypeC_plus_plus:
-  case eLanguageTypeObjC:
-  case eLanguageTypeObjC_plus_plus:
-// Try the Objective-C language runtime. This fallback is necessary
-// for Objective-C++ and mixed Objective-C / C++ programs.
+  // Try the Objective-C language runtime. This fallback is necessary
+  // for Objective-C++ and mixed Objective-C / C++ programs.
+  if (Language::LanguageIsCFamily(native_language))
 return get_object_description(eLanguageTypeObjC);
-  default:
-return nullptr;
-  }
+  return nullptr;
 }
 
 bool ValueObject::GetValueAsCString(const lldb_private::TypeFormatImpl ,

Modified: lldb/trunk/source/Target/Language.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Language.cpp?rev=360930=360929=360930=diff
==
--- lldb/trunk/source/Target/Language.cpp (original)
+++ lldb/trunk/source/Target/Language.cpp Thu May 16 13:03:05 2019
@@ -273,6 +273,24 @@ bool Language::LanguageIsC(LanguageType
   }
 }
 
+bool Language::LanguageIsCFamily(LanguageType language) {
+  switch (language) {
+  case eLanguageTypeC:
+  case eLanguageTypeC89:
+  case eLanguageTypeC99:
+  case eLanguageTypeC11:
+  case eLanguageTypeC_plus_plus:
+  case eLanguageTypeC_plus_plus_03:
+  case eLanguageTypeC_plus_plus_11:
+  case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeObjC_plus_plus:
+  case eLanguageTypeObjC:
+return true;
+  default:
+return false;
+  }
+}
+
 bool Language::LanguageIsPascal(LanguageType language) {
   switch (language) {
   case eLanguageTypePascal83:


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

[Lldb-commits] [PATCH] D62021: Fix LLDB warnings when compiling with Clang 8.0

[Aleksandr Urakov via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62021



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

[Lldb-commits] [PATCH] D62015: Make sure GetObjectDescription falls back to the Objective-C runtime.

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB360929: Make sure GetObjectDescription falls back to the 
Objective-C runtime. (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62015?vs=199862=199883#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62015

Files:
  packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
  
packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
  packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
  source/Core/ValueObject.cpp

Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -1086,44 +1086,46 @@
 }
 
 const char *ValueObject::GetObjectDescription() {
-
   if (!UpdateValueIfNeeded(true))
-return NULL;
+return nullptr;
 
+  // Return cached value.
   if (!m_object_desc_str.empty())
 return m_object_desc_str.c_str();
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   Process *process = exe_ctx.GetProcessPtr();
-  if (process == NULL)
-return NULL;
-
-  StreamString s;
-
-  LanguageType language = GetObjectRuntimeLanguage();
-  LanguageRuntime *runtime = process->GetLanguageRuntime(language);
+  if (!process)
+return nullptr;
 
-  if (runtime == NULL) {
-// Aw, hell, if the things a pointer, or even just an integer, let's try
-// ObjC anyway...
-CompilerType compiler_type = GetCompilerType();
-if (compiler_type) {
-  bool is_signed;
-  if (compiler_type.IsIntegerType(is_signed) ||
-  compiler_type.IsPointerType()) {
-runtime = process->GetLanguageRuntime(eLanguageTypeObjC);
+  // Returns the object description produced by one language runtime.
+  auto get_object_description = [&](LanguageType language) -> const char * {
+if (LanguageRuntime *runtime = process->GetLanguageRuntime(language)) {
+  StreamString s;
+  if (runtime->GetObjectDescription(s, *this)) {
+m_object_desc_str.append(s.GetString());
+return m_object_desc_str.c_str();
   }
 }
-  }
+return nullptr;
+  };
 
-  if (runtime && runtime->GetObjectDescription(s, *this)) {
-m_object_desc_str.append(s.GetString());
+  // Try the native language runtime first.
+  LanguageType native_language = GetObjectRuntimeLanguage();
+  if (const char *desc = get_object_description(native_language))
+return desc;
+
+  switch (native_language) {
+  case eLanguageTypeC:
+  case eLanguageTypeC_plus_plus:
+  case eLanguageTypeObjC:
+  case eLanguageTypeObjC_plus_plus:
+// Try the Objective-C language runtime. This fallback is necessary
+// for Objective-C++ and mixed Objective-C / C++ programs.
+return get_object_description(eLanguageTypeObjC);
+  default:
+return nullptr;
   }
-
-  if (m_object_desc_str.empty())
-return NULL;
-  else
-return m_object_desc_str.c_str();
 }
 
 bool ValueObject::GetValueAsCString(const lldb_private::TypeFormatImpl ,
Index: packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
===
--- packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
+++ packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+OBJCXX_SOURCES := main.mm
+LDFLAGS = $(CFLAGS) -lobjc -framework CoreFoundation
+
+include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
===
--- packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
+++ packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
@@ -0,0 +1,12 @@
+#include 
+
+void stop() {}
+
+int main(int argc, char **argv)
+{
+  int value = 42;
+  CFNumberRef num;
+  num = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, );
+  stop(); // break here
+  return 0;
+}
Index: packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
===
--- packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
+++ packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
@@ -0,0 +1,24 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestObjCXXBridgedPO(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@skipUnlessDarwin
+def test_bridged_type_po(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.mm'))
+self.expect('po num',
+"did not get the Objective-C object 

[Lldb-commits] [lldb] r360929 - Make sure GetObjectDescription falls back to the Objective-C runtime.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu May 16 12:21:31 2019
New Revision: 360929

URL: http://llvm.org/viewvc/llvm-project?rev=360929=rev
Log:
Make sure GetObjectDescription falls back to the Objective-C runtime.

This fixes an unintended regression introduced by
https://reviews.llvm.org/D61451 by making sure the Objective-C runtime
is also tried when the "correct" language runtime failed to return an
object description.

rdar://problem/50791055

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

Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/

lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile

lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
Modified:
lldb/trunk/source/Core/ValueObject.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile?rev=360929=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile 
Thu May 16 12:21:31 2019
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+OBJCXX_SOURCES := main.mm
+LDFLAGS = $(CFLAGS) -lobjc -framework CoreFoundation
+
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py?rev=360929=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
 Thu May 16 12:21:31 2019
@@ -0,0 +1,24 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestObjCXXBridgedPO(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@skipUnlessDarwin
+def test_bridged_type_po(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.mm'))
+self.expect('po num',
+"did not get the Objective-C object description",
+substrs=['CFNumber', '0x', '42'])
+pointer_val = str(self.frame().FindVariable('num').GetValue())
+self.expect('po '+pointer_val,
+"did not get the Objective-C object description",
+substrs=['CFNumber', '0x', '42'])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm?rev=360929=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm 
Thu May 16 12:21:31 2019
@@ -0,0 +1,12 @@
+#include 
+
+void stop() {}
+
+int main(int argc, char **argv)
+{
+  int value = 42;
+  CFNumberRef num;
+  num = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, );
+  stop(); // break here
+  return 0;
+}

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=360929=360928=360929=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Thu May 16 12:21:31 2019
@@ -1086,44 +1086,46 @@ std::pairGetLanguageRuntime(language);
+  if (!process)
+return nullptr;
 
-  if (runtime == NULL) {
-// Aw, hell, if the things a pointer, or even just an integer, let's try
-// ObjC anyway...
-CompilerType compiler_type = GetCompilerType();
-if (compiler_type) {
-  bool is_signed;
-  if (compiler_type.IsIntegerType(is_signed) ||
-  compiler_type.IsPointerType()) {
-runtime = process->GetLanguageRuntime(eLanguageTypeObjC);
+  // Returns the object description produced by one language runtime.
+  auto get_object_description = [&](LanguageType language) -> const char * {
+if (LanguageRuntime *runtime = process->GetLanguageRuntime(language)) {
+  StreamString s;
+  if (runtime->GetObjectDescription(s, *this)) {
+m_object_desc_str.append(s.GetString());
+return m_object_desc_str.c_str();
   }
 }
-  }
+

[Lldb-commits] [PATCH] D62021: Fix LLDB warnings when compiling with Clang 8.0

[Alexandre Ganea via Phabricator via lldb-commits]

aganea created this revision.
aganea added reviewers: JDevlieghere, aleksandr.urakov, asmith, mgorny.
aganea added a project: LLDB.
Herald added subscribers: lldb-commits, abidh, aprantl.

Ditto. Please see warnings list:

  [2660/3259] Building CXX object 
tools\lldb\source\Host\CMakeFiles\lldbHost.dir\common\GetOptInc.cpp.obj
  F:\svn\lldb\source\Host\common\GetOptInc.cpp(100,17): warning: cast from 
'char *const *' to 'char **' drops const qualifier [-Wcast-qual]
((char **)nargv)[pos] = nargv[cstart];
  ^
  F:\svn\lldb\source\Host\common\GetOptInc.cpp(102,17): warning: cast from 
'char *const *' to 'char **' drops const qualifier [-Wcast-qual]
((char **)nargv)[cstart] = swap;
  ^
  2 warnings generated.
  [2677/3259] Building CXX object 
tools\lldb\source\Host\CMakeFiles\lldbHost.dir\common\MainLoop.cpp.obj
  F:\svn\lldb\source\Host\common\MainLoop.cpp(64,13): warning: unused function 
'SignalHandler' [-Wunused-function]
  static void SignalHandler(int signo, siginfo_t *info, void *) {
  ^
  1 warning generated.
  [2694/3259] Building CXX object 
tools\lldb\source\Host\CMakeFiles\lldbHost.dir\common\Socket.cpp.obj
  F:\svn\lldb\source\Host\common\Socket.cpp(398,17): warning: format specifies 
type 'int' but the argument has type 'lldb_private::NativeSocket' (aka 
'unsigned long long') [-Wformat]
  m_socket);
  ^~~~
  1 warning generated.
  [2704/3259] Building CXX object 
tools\lldb\source\Host\CMakeFiles\lldbHost.dir\windows\Windows.cpp.obj
  F:\svn\lldb\source\Host\windows\Windows.cpp(87,19): warning: cast from 'const 
char *' to 'char *' drops const qualifier [-Wcast-qual]
return ((char *)s);
^
  1 warning generated.
  [2706/3259] Building CXX object 
tools\lldb\source\Host\CMakeFiles\lldbHost.dir\windows\ProcessLauncherWindows.cpp.obj
  F:\svn\lldb\source\Host\windows\ProcessLauncherWindows.cpp(33,43): warning: 
cast from 'const wchar_t *' to 'char *' drops const qualifier [-Wcast-qual]
buffer.insert(buffer.end(), (char *)warg.c_str(),
^
  F:\svn\lldb\source\Host\windows\ProcessLauncherWindows.cpp(34,29): warning: 
cast from 'const wchar_t *' to 'char *' drops const qualifier [-Wcast-qual]
  (char *)(warg.c_str() + warg.size() + 1));
  ^
  2 warnings generated.
  [2944/3259] Building CXX object 
tools\lldb\source\Plugins\Process\Windows\Common...eFiles\lldbPluginProcessWindowsCommon.dir\x64\RegisterContextWindows_x64.cpp.obj
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(69,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(74,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(79,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(84,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(89,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(94,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(99,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(104,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(109,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(114,13):
 warning: missing field 'dynamic_size_dwarf_expr_bytes' initializer 
[-Wmissing-field-initializers]
   nullptr},
  ^
  
F:\svn\lldb\source\Plugins\Process\Windows\Common\x64\RegisterContextWindows_x64.cpp(119,13):
 warning: 

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")

xiaobai wrote:
> labath wrote:
> > sgraenitz wrote:
> > > compnerd wrote:
> > > > sgraenitz wrote:
> > > > > Can / Should we add this? Here?
> > > > This is fine to add assuming that you are building on Darwin since that 
> > > > implies clang and that will work.  I would say that if you really want 
> > > > to be pedantically correct, it would be better to do:
> > > > 
> > > > ```
> > > > if(CMAKE_CXX_COMPILER_ID MATCHES Clang)
> > > >   set(LLVM_ENABLE_MODULES_ON CACHE BOOL "")
> > > > endif()
> > > > ```
> > > > 
> > > > Note that the `MATCHES` is required here to match both `Clang` and 
> > > > `AppleClang`.
> > > That sounds reasonable. I would take your version and put it in the base 
> > > cache?
> > > 
> > > The other question was, whether `RelWithDebInfo` is a good default. 
> > > Personally, I use it far more often than other configurations. (Running 
> > > the test suite with a debug Clang is just too slow.) Moving to the base 
> > > cache too.
> > Are you sure that `CMAKE_CXX_COMPILER_ID` is available this early in the 
> > initialization ?
> I think RelWithDebInfo is an okay default. I personally don't like to set 
> build types in caches because I think it's reasonable to expect the user to 
> specify their build type, but if you mostly use that one build type then it's 
> fine.
I don't think `CMAKE_CXX_COMPILER_ID` is available at the cache processing 
stage of initialization, so this is probably not something you can do. Maybe 
you can guard with the condition that the OS is Darwin? Maybe that's not 
needed, since the cache is Apple-specific anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956



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

[Lldb-commits] [PATCH] D62015: Make sure GetObjectDescription falls back to the Objective-C runtime.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 199859.
aprantl added a comment.

Address review feedback.


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

https://reviews.llvm.org/D62015

Files:
  lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
  lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
  lldb/source/Core/ValueObject.cpp

Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1086,44 +1086,45 @@
 }
 
 const char *ValueObject::GetObjectDescription() {
-
   if (!UpdateValueIfNeeded(true))
-return NULL;
+return nullptr;
 
+  // Return cached value.
   if (!m_object_desc_str.empty())
 return m_object_desc_str.c_str();
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   Process *process = exe_ctx.GetProcessPtr();
-  if (process == NULL)
-return NULL;
-
-  StreamString s;
-
-  LanguageType language = GetObjectRuntimeLanguage();
-  LanguageRuntime *runtime = process->GetLanguageRuntime(language);
+  if (!process)
+return nullptr;
 
-  if (runtime == NULL) {
-// Aw, hell, if the things a pointer, or even just an integer, let's try
-// ObjC anyway...
-CompilerType compiler_type = GetCompilerType();
-if (compiler_type) {
-  bool is_signed;
-  if (compiler_type.IsIntegerType(is_signed) ||
-  compiler_type.IsPointerType()) {
-runtime = process->GetLanguageRuntime(eLanguageTypeObjC);
+  // Returns the object description produced by one language runtime.
+  auto get_object_description = [&](LanguageType language) -> const char * {
+if (LanguageRuntime *runtime = process->GetLanguageRuntime(language)) {
+  StreamString s;
+  if (runtime->GetObjectDescription(s, *this)) {
+m_object_desc_str.append(s.GetString());
+return m_object_desc_str.c_str();
   }
 }
+return nullptr;
+  };
+
+  // Try the native language runtime first.
+  if (const char *desc = get_object_description(GetObjectRuntimeLanguage()))
+return desc;
+
+  switch (language) {
+  eLanguageTypeC:
+  eLanguageTypeC_plus_plus:
+  eLanguageTypeObjC:
+  eLanguageTypeObjC_plus_plus:
+// Try the Objective-C language runtime. This fallback is necessary
+// for Objective-C++ and mixed Objective-C / C++ programs.
+return get_object_description(eLanguageTypeObjC);
+  default:
+return nullptr;
   }
-
-  if (runtime && runtime->GetObjectDescription(s, *this)) {
-m_object_desc_str.append(s.GetString());
-  }
-
-  if (m_object_desc_str.empty())
-return NULL;
-  else
-return m_object_desc_str.c_str();
 }
 
 bool ValueObject::GetValueAsCString(const lldb_private::TypeFormatImpl ,
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
@@ -0,0 +1,12 @@
+#include 
+
+void stop() {}
+
+int main(int argc, char **argv)
+{
+  int value = 42;
+  CFNumberRef num;
+  num = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, );
+  stop(); // break here
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
@@ -0,0 +1,20 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestObjCXXBridgedPO(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+	
+@skipUnlessDarwin
+def test_bridged_type_po(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.mm'))
+self.expect('po num', "got an Objective-C object description",
+substrs=['CFNumber', '0x', '42'])
+
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+OBJCXX_SOURCES := main.mm
+LDFLAGS = $(CFLAGS) -lobjc -framework CoreFoundation
+
+include $(LEVEL)/Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added inline comments.



Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")

compnerd wrote:
> sgraenitz wrote:
> > Can / Should we add this? Here?
> This is fine to add assuming that you are building on Darwin since that 
> implies clang and that will work.  I would say that if you really want to be 
> pedantically correct, it would be better to do:
> 
> ```
> if(CMAKE_CXX_COMPILER_ID MATCHES Clang)
>   set(LLVM_ENABLE_MODULES_ON CACHE BOOL "")
> endif()
> ```
> 
> Note that the `MATCHES` is required here to match both `Clang` and 
> `AppleClang`.
That sounds reasonable. I would take your version and put it in the base cache?

The other question was, whether `RelWithDebInfo` is a good default. Personally, 
I use it far more often than other configurations. (Running the test suite with 
a debug Clang is just too slow.) Moving to the base cache too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956



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

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Seems like an overall improvement to the structure of this code. At the high 
level, the structure feels easier to understand.

In D61994#1504336 , @labath wrote:

> I think this is a bit more readable. I've included some suggestions which I 
> think could make it even better.
>
> Since you're already rewriting this code, this might be a good time to raise 
> a point I wanted to bring up some day. Should we be using `FileSpec` for code 
> like this? The code already uses a combination of llvm and lldb path 
> utilities, and I would argue that we should use llvm all the way for code 
> like this (except that we go through the FileSystem abstraction for the 
> reproducer stuff). I have two reasons for that:
>
> - FileSpec is designed for efficient matching of abstract file paths which 
> may not even exist on the host system. As such, this code will result in a 
> bunch of strings being added to the global string pool for no reason. And in 
> this case, you're definitely working files which do exist (or may exist) on 
> the host. In fact, FileSpec now contains code which performs path 
> simplifications which are not completely sound given a concrete filesystem -- 
> it will simplify a path like `bar/../foo` to `foo`, which is not correct if 
> `bar` is a symlink.


+1

> - Since we started supporting windows paths, the FileSpec class offers more 
> freedom than it is needed here. Specifically, it gives you the ability to 
> return a foreign-syntax path as the "lldbinit" location. Therefore, in the 
> spirit of using the most specific type which is sufficient to accomplish a 
> given task, I think we should use a plain string here.

I'm especially concerned about windows support, so I also agree with this.




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2149-2150
+
+  LoadCWDlldbinitFile should_load = ShouldLoadCwdInitFile();
+  if (should_load == eLoadCWDlldbinitFalse) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);

labath wrote:
> Make this a `switch` ?
+1



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

labath wrote:
> JDevlieghere wrote:
> > This should be reflowed. 
> What might help readability is moving the long block of text to a separate 
> static variable declared before the function. That way the text does not 
> obscure the logic of the function.
I agree with Pavel, that would indeed make things more readable.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61994



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

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added inline comments.



Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:10
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr")
+set(LLDB_FRAMEWORK_INSTALL_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+

labath wrote:
> sgraenitz wrote:
> > Follow-up from: https://reviews.llvm.org/D61956?id=199645#inline-550727
> > 
> > > I believe the expected usage of this variable is to make it point to the 
> > > final resting place of the executables, ...
> > 
> > It's been a pragmatic decision. Maybe we can improve this. The rationale 
> > was, that the default configuration should give the user something that 
> > works without touching caches or overriding parameters. In a previous 
> > sketch I used a real-world destination like:
> > ```
> > set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr/)
> > ```
> > But then `ninja install` would fail by default due to lack of permissions 
> > in the install destination. Actual release configurations tend to be more 
> > complex anyway and vendors will have their own downstream repos / caches 
> > for it. Thus, choosing a good default for developers sounded like a good 
> > way forward. What do you think?
> > 
> > > Are you sure including {CMAKE_CURRENT_BINARY_DIR} here is a good idea? I 
> > > think llvm tries to avoid that generally, ...
> > 
> > What exactly do you mean? Having absolute paths, or paths into the 
> > build-tree, or the `CMAKE_CURRENT_BINARY_DIR` specifically? I don't see 
> > problems with the two last points. Am I missing something?
> > 
> > For the first: choosing an absolute path was for consistency with 
> > `LLDB_FRAMEWORK_INSTALL_DIR`. In the current build logic, they can both be 
> > absolute paths. Otherwise:
> > * if the install prefix is relative, it will be appended to the path of the 
> > root build directory
> > * if the framework install dir is relative, it will be appended to the 
> > install prefix
> > 
> > > Then, if you want to copy the to-be-installed files into a staging area 
> > > first, you're expected to do that with "DESTDIR=whatever ninja install".
> > 
> > [Clang cache 
> > scripts](https://github.com/llvm/llvm-project/blob/a8f88c38/clang/cmake/caches/Apple-stage1.cmake#L4)
> >  seem to accomplish it manually, which may look like this (but the default 
> > would again fail due to privileges):
> > ```
> > if($ENV{DESTDIR})
> >   set(CMAKE_INSTALL_PREFIX $ENV{DESTDIR})
> >   set(LLDB_FRAMEWORK_INSTALL_DIR "../../SharedFrameworks" CACHE STRING "")
> > else()
> >   set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr)
> >   set(LLDB_FRAMEWORK_INSTALL_DIR 
> > /Applications/Xcode.app/Contents/SharedFrameworks CACHE STRING "")
> > endif()
> > ```
> > 
> > Would you (and other reviewers) prefer this solution?
> > But then ninja install would fail by default due to lack of permissions in 
> > the install destination. Actual release configurations tend to be more 
> > complex anyway and vendors will have their own downstream repos / caches 
> > for it. Thus, choosing a good default for developers sounded like a good 
> > way forward. What do you think?
> 
> I don't think most developers actually run the "install" rule TBH. :) But if 
> they do, I think we should encourage them to do the right thing, and run 
> "DESTDIR=foo ninja install", which will work even with a real-world prefix. 
> At least that is the right thing to do in the linuxy world -- on a mac I 
> guess most people will not be able to install lldb to the system destination 
> anyway, so I'm not sure what would be a sensible default.
> 
> > Would you (and other reviewers) prefer this solution?
> 
> I don't care that much about this TBH. I just wanted to explain the 
> difference between the install prefix and destdir, because in my experience, 
> a lot of people get those two mixed up. ccing  @mgorny, in case I'm saying 
> something wrong, as he does a lot of building and installing..
> I just wanted to explain the difference between the install prefix and destdir

Yes, that was a good point: DESTDIR is the location of the install-tree on the 
build machine, PREFIX is the install location on the enduser machine. Right? 
Sorry, I didn't come back to it. The Clang cache script I referred to, doesn't 
respect that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956



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

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")

sgraenitz wrote:
> Can / Should we add this? Here?
This is fine to add assuming that you are building on Darwin since that implies 
clang and that will work.  I would say that if you really want to be 
pedantically correct, it would be better to do:

```
if(CMAKE_CXX_COMPILER_ID MATCHES Clang)
  set(LLVM_ENABLE_MODULES_ON CACHE BOOL "")
endif()
```

Note that the `MATCHES` is required here to match both `Clang` and `AppleClang`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956



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

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 199853.
sgraenitz added a comment.

Add back `CACHE STRING ""` for `CMAKE` variables


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956

Files:
  lldb/cmake/caches/Apple-lldb-base.cmake
  lldb/cmake/caches/Apple-lldb-macOS.cmake


Index: lldb/cmake/caches/Apple-lldb-macOS.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -0,0 +1,18 @@
+include(${CMAKE_CURRENT_LIST_DIR}/Apple-lldb-base.cmake)
+
+set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
+set(LLDB_NO_INSTALL_DEFAULT_RPATH ON CACHE BOOL "")
+
+# Release builds will use actual install destinations. Their relative locations
+# must align with INSTALL_RPATH settings. The locations of LLDB.framework, in
+# particular, must match the RPATHs of the installed lldb driver executable.
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr" 
CACHE STRING "")
+set(LLDB_FRAMEWORK_INSTALL_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+
+# Release builds may change these:
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(LLDB_USE_SYSTEM_DEBUGSERVER ON CACHE BOOL "")
+set(LLVM_EXTERNALIZE_DEBUGINFO OFF CACHE BOOL "")
+
+set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
Index: lldb/cmake/caches/Apple-lldb-base.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-base.cmake
@@ -0,0 +1,8 @@
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
+
+# Release builds set these explicitly:
+#set(LLDB_VERSION_MAJOR  CACHE STRING "")
+#set(LLDB_VERSION_MINOR 9 CACHE STRING "")
+#set(LLDB_VERSION_PATCH 9 CACHE STRING "")
+#set(LLDB_VERSION_SUFFIX git CACHE STRING "")


Index: lldb/cmake/caches/Apple-lldb-macOS.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -0,0 +1,18 @@
+include(${CMAKE_CURRENT_LIST_DIR}/Apple-lldb-base.cmake)
+
+set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
+set(LLDB_NO_INSTALL_DEFAULT_RPATH ON CACHE BOOL "")
+
+# Release builds will use actual install destinations. Their relative locations
+# must align with INSTALL_RPATH settings. The locations of LLDB.framework, in
+# particular, must match the RPATHs of the installed lldb driver executable.
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr" CACHE STRING "")
+set(LLDB_FRAMEWORK_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+
+# Release builds may change these:
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(LLDB_USE_SYSTEM_DEBUGSERVER ON CACHE BOOL "")
+set(LLVM_EXTERNALIZE_DEBUGINFO OFF CACHE BOOL "")
+
+set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
Index: lldb/cmake/caches/Apple-lldb-base.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-base.cmake
@@ -0,0 +1,8 @@
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
+
+# Release builds set these explicitly:
+#set(LLDB_VERSION_MAJOR  CACHE STRING "")
+#set(LLDB_VERSION_MINOR 9 CACHE STRING "")
+#set(LLDB_VERSION_PATCH 9 CACHE STRING "")
+#set(LLDB_VERSION_SUFFIX git CACHE STRING "")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Yet another option: we could install the framework tools to a fake location and 
copy them to their final destination (overwriting their build-tree pendants) in 
a manual install step at the very end of the install process. The problem here 
is, that there may be more things in the build-tree framework than we 
overwrite, and thus remain in the install-tree (thinking about Swift resources 
in swift-lldb for example).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [PATCH] D62015: Make sure GetObjectDescription falls back to the Objective-C runtime.

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

My one concern is how this will work in a swift-lldb where we have a swift 
ValueObject, the print description fails, and we fall back to the ObjC print 
description.  That's a change in behavior, right?  I imagine worst case is that 
the objc print description will fail to produce anything and we'll have the 
same behavior as we do today.




Comment at: 
lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py:16
+@skipIfWindows
+@skipIfNetBSD
+def test_bridged_type_po(self):

@skipUnlessDarwin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62015



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

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

I thought about one more option, but I don't think it's better than the current 
proposal: instead of deleting the build-tree resources in the very first 
install step, I could **install the framework manually** at this point and skip 
its regular install target. We would loose implicit stripping and RPATH 
replacement for the dylib though. Should be possible to do it manually, but if 
it breaks, we won't notice. So, it's not quite appealing.

In D61952#1504991 , @labath wrote:

> Because one might want to run the lldb executable independently of the test 
> suite?


Agreed, it would be inconvenient to have non-functional build results. The 
solution to the "install -> test" issue would be having both I guess.
**Adding this to the current patch still sounds like the most convenient 
approach to me.**

In D61952#1504941 , @sgraenitz wrote:

> In D61952#1504346 , @labath wrote:
>
> > For instance, what if we set the build output paths to be separate and 
> > disjoint locations for each target. Then use a separate target, or some 
> > POST_BUILD commands to copy/symlink the files to construct an build-tree 
> > framework, and have the install targets create the install-tree framework 
> > from the original build output paths?
>
>
> Ok, so you mean copying the entire framework to a staging directory, 
> something like a "test-tree". Then we add test resources and run the test 
> suite there. Sounds reasonable, I will investigate.


In fact, your above comment is more relevant here. I'd have to set all 
build-tree RPATHs to the staging framework instead of the actual target output. 
Sounds like a number of non-intuitive edits all over the CMake scripts..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [PATCH] D62015: Make sure GetObjectDescription falls back to the Objective-C runtime.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:1112
-  bool is_signed;
-  if (compiler_type.IsIntegerType(is_signed) ||
-  compiler_type.IsPointerType()) {

This check was redundant, it is also performed by the ObjC runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62015



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

[Lldb-commits] [PATCH] D62015: Make sure GetObjectDescription falls back to the Objective-C runtime.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, jasonmolenda.
Herald added a project: LLDB.

This fixes an unintended regression introduced by 
https://reviews.llvm.org/D61451 by making sure the Objective-C runtime is also 
tried when the "correct" language runtime failed to return an object 
description.

rdar://problem/50791055


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62015

Files:
  lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
  lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
  lldb/source/Core/ValueObject.cpp

Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1086,44 +1086,37 @@
 }
 
 const char *ValueObject::GetObjectDescription() {
-
   if (!UpdateValueIfNeeded(true))
-return NULL;
+return nullptr;
 
+  // Return cached value.
   if (!m_object_desc_str.empty())
 return m_object_desc_str.c_str();
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   Process *process = exe_ctx.GetProcessPtr();
-  if (process == NULL)
-return NULL;
-
-  StreamString s;
-
-  LanguageType language = GetObjectRuntimeLanguage();
-  LanguageRuntime *runtime = process->GetLanguageRuntime(language);
+  if (!process)
+return nullptr;
 
-  if (runtime == NULL) {
-// Aw, hell, if the things a pointer, or even just an integer, let's try
-// ObjC anyway...
-CompilerType compiler_type = GetCompilerType();
-if (compiler_type) {
-  bool is_signed;
-  if (compiler_type.IsIntegerType(is_signed) ||
-  compiler_type.IsPointerType()) {
-runtime = process->GetLanguageRuntime(eLanguageTypeObjC);
+  // Returns the object description produced by one language runtime.
+  auto get_object_description = [&](LanguageType language) -> const char * {
+if (LanguageRuntime *runtime = process->GetLanguageRuntime(language)) {
+  StreamString s;
+  if (runtime->GetObjectDescription(s, *this)) {
+m_object_desc_str.append(s.GetString());
+return m_object_desc_str.c_str();
   }
 }
-  }
+return nullptr;
+  };
 
-  if (runtime && runtime->GetObjectDescription(s, *this)) {
-m_object_desc_str.append(s.GetString());
-  }
+  // Try the native language runtime first.
+  if (const char *desc = get_object_description(GetObjectRuntimeLanguage()))
+return desc;
 
-  if (m_object_desc_str.empty())
-return NULL;
-  else
-return m_object_desc_str.c_str();
+  // Try the Objective-C language runtime. This fallback is necessary
+  // for Objective-C++ and mixed Objective-C / C++ programs.
+  return get_object_description(eLanguageTypeObjC);
 }
 
 bool ValueObject::GetValueAsCString(const lldb_private::TypeFormatImpl ,
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/main.mm
@@ -0,0 +1,12 @@
+#include 
+
+void stop() {}
+
+int main(int argc, char **argv)
+{
+  int value = 42;
+  CFNumberRef num;
+  num = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, );
+  stop(); // break here
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/TestObjCXXBridgedPO.py
@@ -0,0 +1,23 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestObjCXXBridgedPO(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@skipIfFreeBSD
+@skipIfLinux
+@skipIfWindows
+@skipIfNetBSD
+def test_bridged_type_po(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.mm'))
+self.expect('po num', "got an Objective-C object description",
+substrs=['CFNumber', '0x', '42'])
+
Index: lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objcxx/cxx-bridged-po/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+OBJCXX_SOURCES := main.mm
+LDFLAGS = $(CFLAGS) -lobjc -framework CoreFoundation
+
+include $(LEVEL)/Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r360916 - [IRExecutionUnit] Remove static_assert

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu May 16 09:54:41 2019
New Revision: 360916

URL: http://llvm.org/viewvc/llvm-project?rev=360916=rev
Log:
[IRExecutionUnit] Remove static_assert

This doesn't make sense on platforms other than 64 bit.

Modified:
lldb/trunk/source/Expression/IRExecutionUnit.cpp

Modified: lldb/trunk/source/Expression/IRExecutionUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/IRExecutionUnit.cpp?rev=360916=360915=360916=diff
==
--- lldb/trunk/source/Expression/IRExecutionUnit.cpp (original)
+++ lldb/trunk/source/Expression/IRExecutionUnit.cpp Thu May 16 09:54:41 2019
@@ -1023,8 +1023,6 @@ IRExecutionUnit::MemoryManager::getSymbo
 
 void *IRExecutionUnit::MemoryManager::getPointerToNamedFunction(
 const std::string , bool AbortOnFailure) {
-  static_assert(sizeof(void *) == 8, "");
-
   return (void *)getSymbolAddress(Name);
 }
 


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

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D61952#1504942 , @sgraenitz wrote:

> Actually, why not make the copy operations `PRE_BUILD` actions of the test 
> suite instead of `POST_BUILD` actions of their executables?


Because one might want to run the lldb executable independently of the test 
suite?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Actually, why not make the copy operations `PRE_BUILD` actions of the test 
suite instead of `POST_BUILD` actions of their executables?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

In D61952#1504346 , @labath wrote:

> I'm not very familiar with frameworks and complexities involved in creating 
> them, but the fact that we need to delete stuff from the build tree in order 
> to install properly makes me think that there is something fishy going on. Is 
> there no way to arrange things so that this can be avoided?


Well, maybe there are better ways to do it, but we have a number of 
restrictions.

> For instance, what if we set the build output paths to be separate and 
> disjoint locations for each target. Then use a separate target, or some 
> POST_BUILD commands to copy/symlink the files to construct an build-tree 
> framework, and have the install targets create the install-tree framework 
> from the original build output paths?

Ok, so you mean copying the entire framework to a staging directory, something 
like a "test-tree". Then we add test resources and run the test suite there. 
Sounds reasonable, I will investigate.
Certainly we had to consider ordering issues too, because all external headers 
and libraries must be copied in first, before copying to the test-tree.

In D61952#1503551 , @JDevlieghere 
wrote:

> How does this cleanup affect dependency tracking? Does the build dir become 
> unusable after running ninja install?


Good point. Ideally we'd just copy them in again before running the test suite. 
Unlike I expected, this doesn't work at the moment. Maybe I should get this 
done first:

  $ cmake -GNinja ...
  $ ninja check-lldb
  $ ninja install
  $ ninja check-lldb

Which other test suite invocation(s) should I verify?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [PATCH] D60962: [NativePDB] Extend .pdb files search folders

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Adrian is on vacation now, but given that he was just waiting until you resolve 
my comments (which you have), I think we don't have to wait for him.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60962



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

[Lldb-commits] [PATCH] D61482: [DWARF] Store compile unit IDs in the DIERef class

[Pavel Labath via Phabricator via lldb-commits]

labath abandoned this revision.
labath added a comment.

Obsoleted by D61908 .


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

https://reviews.llvm.org/D61482



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

[Lldb-commits] [PATCH] D61853: [FuncUnwinders] Use "symbol file" unwind plans for unwinding

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added inline comments.



Comment at: source/Symbol/FuncUnwinders.cpp:61-70
   if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetCompactUnwindUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetArmUnwindUnwindPlan(target))

clayborg wrote:
> I would love to at least make this entire function just be:
> ```
> std::lock_guard guard(m_mutex);
>  if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
> return plan_sp;
>   return nullptr;
> ```
> and let the symbol files do all of the work as they will know if one plan is 
> better than another?
> 
> Or do we need to add an ObjectFile component?:
> 
> ```
> std::lock_guard guard(m_mutex);
>  if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
> return plan_sp;
>  if (UnwindPlanSP plan_sp = GetObjectFileUnwindPlan(thread))
> return plan_sp;
>   return nullptr;
> ```
> 
> Since macho will always check EH frame, then .debug_frame, then  compact 
> unwind and never ARM. ELF would check EH frame, then .debug_frame, then ARM 
> unwind.
> 
> If we split it up GetObjectFileUnwindPlan would check:
> EH frame
> Apple compact unwind
> ARM compact unwind
> 
> and GetSymbolFileUnwindPlan would check:
> .debug_frame
> breakpad
> 
> I might also argue the ordering is wrong in the original function. The 
> .debug_frame should come first since it can be more complete than EH frame. 
> Granted most times the .debug_frame is the same as the EH frame, but if it 
> isn't we should be using that first. Then EH frame, then apple compact 
> unwind, then ARM unwind. That is why I put the symbol file check first in the 
> example code where we check the symbol file, then the object file.
I can see some appeal in this idea, but is there any practical benefit to doing 
that now? Doing it might be useful if we plan to customize the plan order in 
different symbol file plugins, but i don't think we want to do that now. So all 
of this code will end up in the base `SymbolFile` class anyway, and I don't see 
much difference between this being in `Symbol/FuncUnwinders` and 
`Symbol/SymbolFile`. In fact it might be better to keep it in a separate class, 
as symbol files have a lot of other responsibilities anyway.

Given a choice, I'd much rather work on being able to avoid passing the Target 
and Thread pointers into the unwindplan generation code, because I think it has 
more practical benefits. In fact if I moved everything over to SymbolFile right 
now, it would mean the plugin would have to start operating with Targets and 
Threads, which is something we've managed to avoid so far. Moving stuff to 
symbol file might be a smaller project than the other one, but it is too not 
without it's pitfalls. There are places now where we directly ask for a 
specific (usually eh-frame) unwind plan, which kind of works now, but it won't 
be possible if that is all hidden behind a "symbol file" facade. 


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

https://reviews.llvm.org/D61853



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

[Lldb-commits] [PATCH] D62011: Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D62011#1504877 , @labath wrote:

> Seems like a nice cleanup. Given that code code already assumed that the 
> DWARFUnit can't be null, I'd just delete the null checks..


I have rechecked it now and 2 of the 3 tests are valid, it can be NULL, callers 
are:

  DWARFDebugInfoEntry::DumpAttribute():
case DW_AT_abstract_origin:
case DW_AT_specification: {
  DWARFDIE abstract_die = form_value.Reference();
  GetName(abstract_die.GetCU(), abstract_die.GetOffset(), s);
case DW_AT_type: {
  DWARFDIE type_die = form_value.Reference();
  AppendTypeName(type_die.GetCU(), type_die.GetOffset(), s);

Sorry for not verifying it myself first.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62011



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

[Lldb-commits] [PATCH] D62011: Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 199838.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62011

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1550,15 +1550,15 @@
   if (GetDebugMapSymfile())
 return nullptr;
 
-  const char *dwo_name = cu_die.GetAttributeValueAsString(
-  this, _cu, DW_AT_GNU_dwo_name, nullptr);
+  const char *dwo_name =
+  cu_die.GetAttributeValueAsString(_cu, DW_AT_GNU_dwo_name, nullptr);
   if (!dwo_name)
 return nullptr;
 
   SymbolFileDWARFDwp *dwp_symfile = GetDwpSymbolFile();
   if (dwp_symfile) {
-uint64_t dwo_id = cu_die.GetAttributeValueAsUnsigned(this, _cu,
- DW_AT_GNU_dwo_id, 0);
+uint64_t dwo_id =
+cu_die.GetAttributeValueAsUnsigned(_cu, DW_AT_GNU_dwo_id, 0);
 std::unique_ptr dwo_symfile =
 dwp_symfile->GetSymbolFileForDwoId(_cu, dwo_id);
 if (dwo_symfile)
@@ -1568,8 +1568,8 @@
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
   if (dwo_file.IsRelative()) {
-const char *comp_dir = cu_die.GetAttributeValueAsString(
-this, _cu, DW_AT_comp_dir, nullptr);
+const char *comp_dir =
+cu_die.GetAttributeValueAsString(_cu, DW_AT_comp_dir, nullptr);
 if (!comp_dir)
   return nullptr;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -106,8 +106,7 @@
   void SetRangesBase(dw_addr_t ranges_base);
   void SetBaseObjOffset(dw_offset_t base_obj_offset);
   void SetStrOffsetsBase(dw_offset_t str_offsets_base);
-  void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
-  DWARFDebugAranges *debug_aranges);
+  void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges);
 
   lldb::ByteOrder GetByteOrder() const;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -274,23 +274,22 @@
 // m_die_array_mutex must be already held as read/write.
 void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry _die) {
   dw_addr_t addr_base = cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_addr_base, LLDB_INVALID_ADDRESS);
+  this, DW_AT_addr_base, LLDB_INVALID_ADDRESS);
   if (addr_base != LLDB_INVALID_ADDRESS)
 SetAddrBase(addr_base);
 
   dw_addr_t ranges_base = cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_rnglists_base, LLDB_INVALID_ADDRESS);
+  this, DW_AT_rnglists_base, LLDB_INVALID_ADDRESS);
   if (ranges_base != LLDB_INVALID_ADDRESS)
 SetRangesBase(ranges_base);
 
-  SetStrOffsetsBase(cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_str_offsets_base, 0));
+  SetStrOffsetsBase(
+  cu_die.GetAttributeValueAsUnsigned(this, DW_AT_str_offsets_base, 0));
 
-  uint64_t base_addr = cu_die.GetAttributeValueAsAddress(
-  m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  uint64_t base_addr = cu_die.GetAttributeValueAsAddress(this, DW_AT_low_pc,
+ LLDB_INVALID_ADDRESS);
   if (base_addr == LLDB_INVALID_ADDRESS)
-base_addr = cu_die.GetAttributeValueAsAddress(
-m_dwarf, this, DW_AT_entry_pc, 0);
+base_addr = cu_die.GetAttributeValueAsAddress(this, DW_AT_entry_pc, 0);
   SetBaseAddress(base_addr);
 
   std::unique_ptr dwo_symbol_file =
@@ -307,7 +306,7 @@
 return; // Can't fetch the compile unit DIE from the dwo file.
 
   uint64_t main_dwo_id =
-  cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, DW_AT_GNU_dwo_id, 0);
+  cu_die.GetAttributeValueAsUnsigned(this, DW_AT_GNU_dwo_id, 0);
   uint64_t sub_dwo_id =
   dwo_cu_die.GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0);
   if (main_dwo_id != sub_dwo_id)
@@ -323,13 +322,13 @@
   // DW_AT_* attributes standardized in DWARF v5 are also applicable to the main
   // unit in contrast.
   if (addr_base == LLDB_INVALID_ADDRESS)
-addr_base = cu_die.GetAttributeValueAsUnsigned(m_dwarf, this,
-  

[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360908: Minidump: Add support for the MemoryList stream 
(authored by labath, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61885?vs=199570=199837#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61885

Files:
  llvm/trunk/include/llvm/Object/Minidump.h
  llvm/trunk/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/trunk/lib/Object/Minidump.cpp
  llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/trunk/test/tools/obj2yaml/basic-minidump.yaml
  llvm/trunk/unittests/Object/MinidumpTest.cpp

Index: llvm/trunk/lib/Object/Minidump.cpp
===
--- llvm/trunk/lib/Object/Minidump.cpp
+++ llvm/trunk/lib/Object/Minidump.cpp
@@ -78,6 +78,8 @@
 MinidumpFile::getListStream(StreamType) const;
 template Expected>
 MinidumpFile::getListStream(StreamType) const;
+template Expected>
+MinidumpFile::getListStream(StreamType) const;
 
 Expected>
 MinidumpFile::getDataSlice(ArrayRef Data, size_t Offset, size_t Size) {
Index: llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
@@ -168,6 +168,8 @@
 
 Stream::StreamKind Stream::getKind(StreamType Type) {
   switch (Type) {
+  case StreamType::MemoryList:
+return StreamKind::MemoryList;
   case StreamType::ModuleList:
 return StreamKind::ModuleList;
   case StreamType::SystemInfo:
@@ -190,6 +192,8 @@
 std::unique_ptr Stream::create(StreamType Type) {
   StreamKind Kind = getKind(Type);
   switch (Kind) {
+  case StreamKind::MemoryList:
+return llvm::make_unique();
   case StreamKind::ModuleList:
 return llvm::make_unique();
   case StreamKind::RawContent:
@@ -353,6 +357,16 @@
   return "";
 }
 
+void yaml::MappingTraits::mapping(
+IO , MemoryListStream::entry_type ) {
+  MappingContextTraits::mapping(
+  IO, Range.Entry, Range.Content);
+}
+
+static void streamMapping(yaml::IO , MemoryListStream ) {
+  IO.mapRequired("Memory Ranges", Stream.Entries);
+}
+
 static void streamMapping(yaml::IO , ModuleListStream ) {
   IO.mapRequired("Modules", Stream.Entries);
 }
@@ -421,6 +435,9 @@
   if (!IO.outputting())
 S = MinidumpYAML::Stream::create(Type);
   switch (S->Kind) {
+  case MinidumpYAML::Stream::StreamKind::MemoryList:
+streamMapping(IO, llvm::cast(*S));
+break;
   case MinidumpYAML::Stream::StreamKind::ModuleList:
 streamMapping(IO, llvm::cast(*S));
 break;
@@ -444,6 +461,7 @@
   switch (S->Kind) {
   case MinidumpYAML::Stream::StreamKind::RawContent:
 return streamValidate(cast(*S));
+  case MinidumpYAML::Stream::StreamKind::MemoryList:
   case MinidumpYAML::Stream::StreamKind::ModuleList:
   case MinidumpYAML::Stream::StreamKind::SystemInfo:
   case MinidumpYAML::Stream::StreamKind::TextContent:
@@ -466,6 +484,10 @@
   support::ulittle32_t(File.allocateBytes(Data))};
 }
 
+static void layout(BlobAllocator , MemoryListStream::entry_type ) {
+  Range.Entry.Memory = layout(File, Range.Content);
+}
+
 static void layout(BlobAllocator , ModuleListStream::entry_type ) {
   M.Entry.ModuleNameRVA = File.allocateString(M.Name);
 
@@ -502,6 +524,9 @@
   Result.Location.RVA = File.tell();
   Optional DataEnd;
   switch (S.Kind) {
+  case Stream::StreamKind::MemoryList:
+DataEnd = layout(File, cast(S));
+break;
   case Stream::StreamKind::ModuleList:
 DataEnd = layout(File, cast(S));
 break;
@@ -566,6 +591,19 @@
 Stream::create(const Directory , const object::MinidumpFile ) {
   StreamKind Kind = getKind(StreamDesc.Type);
   switch (Kind) {
+  case StreamKind::MemoryList: {
+auto ExpectedList = File.getMemoryList();
+if (!ExpectedList)
+  return ExpectedList.takeError();
+std::vector Ranges;
+for (const MemoryDescriptor  : *ExpectedList) {
+  auto ExpectedContent = File.getRawData(MD.Memory);
+  if (!ExpectedContent)
+return ExpectedContent.takeError();
+  Ranges.push_back({MD, *ExpectedContent});
+}
+return llvm::make_unique(std::move(Ranges));
+  }
   case StreamKind::ModuleList: {
 auto ExpectedList = File.getModuleList();
 if (!ExpectedList)
Index: llvm/trunk/unittests/Object/MinidumpTest.cpp
===
--- llvm/trunk/unittests/Object/MinidumpTest.cpp
+++ llvm/trunk/unittests/Object/MinidumpTest.cpp
@@ -463,3 +463,51 @@
 EXPECT_EQ(0x08070605u, T.Context.RVA);
   }
 }
+
+TEST(MinidumpFile, getMemoryList) {
+  std::vector OneRange{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  32, 0, 0, 0,  // StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, 

[Lldb-commits] [PATCH] D61885: Minidump: Add support for the MemoryList stream

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. This is very straight-forward, so I don't think another set of eyes is 
needed (though I expect Greg is keeping at least one eye on this).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61885



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

[Lldb-commits] [PATCH] D62012: Make DWARFContext dwo-aware and port debug_info sections over

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: aprantl, clayborg, JDevlieghere.

The previous attempt and moving section handling over to DWARFContext
(D59611 ) failed because it did not take into 
account the dwo sections
correctly. All DWARFContexts (even those in SymbolFileDWARFDwo) used the
main module for loading the sections, but in the dwo scenario some
sections should come from the dwo file.

This patch fixes that by making the DWARFContext aware of whether it a
dwo context or a regular one. A dwo context gets two sections lists, and
it knows where to look for a particular type of a section. This isn't
fully consistent with how the llvm DWARFContext behaves, because that
one leaves it up to the user to know whether it should ask for a dwo
section or not. However, for the time being, it seems useful to have a
single entity which knows how to peice together the debug info in dwo
and non-dwo scenarios. The rough roadmap for the future is:

- port over the rest of the sections to DWARFContext
- find a way to get rid of SymbolFileDWARFDwo/Dwp/DwpDwo. This will likely 
involve adding the ability for the DWARFContext to spawn dwo sub-contexts, 
similarly to how it's done in llvm.
- get rid of the special handling of the "dwo" contexts by making sure 
everything knows whether it should ask for the .dwo version of the section or 
not (similarly to how llvm's DWARFUnits do that)

To demonstrate how the DWARFContext should behave in this new world, I
port the debug_info section (which is debug_info.dwo in the dwo file)
handling to DWARFContext. The rest of the sections will come in
subsequent patches.


https://reviews.llvm.org/D62012

Files:
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  source/Plugins/SymbolFile/DWARF/DWARFContext.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -47,7 +47,6 @@
 
   const lldb_private::DWARFDataExtractor _debug_abbrev_data() override;
   const lldb_private::DWARFDataExtractor _debug_addr_data() override;
-  const lldb_private::DWARFDataExtractor _debug_info_data() override;
   const lldb_private::DWARFDataExtractor _debug_str_data() override;
   const lldb_private::DWARFDataExtractor _debug_str_offsets_data() override;
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -21,8 +21,9 @@
 
 SymbolFileDWARFDwo::SymbolFileDWARFDwo(ObjectFileSP objfile,
DWARFUnit *dwarf_cu)
-: SymbolFileDWARF(objfile.get()), m_obj_file_sp(objfile),
-  m_base_dwarf_cu(dwarf_cu) {
+: SymbolFileDWARF(objfile.get(), objfile->GetSectionList(
+ /*update_module_section_list*/ false)),
+  m_obj_file_sp(objfile), m_base_dwarf_cu(dwarf_cu) {
   SetID(((lldb::user_id_t)dwarf_cu->GetID()) << 32);
 }
 
@@ -129,10 +130,6 @@
   return m_data_debug_addr.m_data;
 }
 
-const DWARFDataExtractor ::get_debug_info_data() {
-  return GetCachedSectionData(eSectionTypeDWARFDebugInfoDwo, m_data_debug_info);
-}
-
 const DWARFDataExtractor ::get_debug_str_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugStrDwo, m_data_debug_str);
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -84,7 +84,8 @@
 
   // Constructors and Destructors
 
-  SymbolFileDWARF(lldb_private::ObjectFile *ofile);
+  SymbolFileDWARF(lldb_private::ObjectFile *ofile,
+  lldb_private::SectionList *dwo_section_list);
 
   ~SymbolFileDWARF() override;
 
@@ -214,7 +215,6 @@
   virtual const lldb_private::DWARFDataExtractor _debug_abbrev_data();
   virtual const lldb_private::DWARFDataExtractor _debug_addr_data();
   const lldb_private::DWARFDataExtractor _debug_frame_data();
-  virtual const lldb_private::DWARFDataExtractor _debug_info_data();
   const lldb_private::DWARFDataExtractor _debug_line_data();
   const lldb_private::DWARFDataExtractor _debug_line_str_data();
   const lldb_private::DWARFDataExtractor _debug_macro_data();
@@ -315,6 +315,8 @@
 
   void DumpClangAST(lldb_private::Stream ) override;
 
+  lldb_private::DWARFContext () { 

[Lldb-commits] [PATCH] D62011: Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Seems like a nice cleanup. Given that code code already assumed that the 
DWARFUnit can't be null, I'd just delete the null checks..


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62011



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

[Lldb-commits] [PATCH] D62008: DWARF: Parse type units from the debug_types section

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 199831.
labath added a comment.

- don't bail out if you see the debug_types section
- rename a variable in the test to work around the old "Multiple internal 
symbols found for 'a'" problem


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

https://reviews.llvm.org/D62008

Files:
  lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
  lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp
  lit/SymbolFile/DWARF/debug-types-basic.test
  lit/SymbolFile/DWARF/debug-types-expressions.test
  lit/SymbolFile/DWARF/lit.local.cfg
  source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  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
@@ -440,20 +440,6 @@
 if (section_list == NULL)
   return 0;
 
-// On non Apple platforms we might have .debug_types debug info that is
-// created by using "-fdebug-types-section". LLDB currently will try to
-// load this debug info, but it causes crashes during debugging when types
-// are missing since it doesn't know how to parse the info in the
-// .debug_types type units. This causes all complex debug info types to be
-// unresolved. Because this causes LLDB to crash and since it really
-// doesn't provide a solid debuggiung experience, we should disable trying
-// to debug this kind of DWARF until support gets added or deprecated.
-if (section_list->FindSectionByName(ConstString(".debug_types"))) {
-  m_obj_file->GetModule()->ReportWarning(
-"lldb doesn’t support .debug_types debug info");
-  return 0;
-}
-
 uint64_t debug_abbrev_file_size = 0;
 uint64_t debug_info_file_size = 0;
 uint64_t debug_line_file_size = 0;
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -172,6 +172,10 @@
 protected:
   DWARFUnit(SymbolFileDWARF *dwarf, lldb::user_id_t uid);
 
+  llvm::Error ExtractHeader(SymbolFileDWARF *dwarf,
+const lldb_private::DWARFDataExtractor ,
+lldb::offset_t *offset_ptr);
+
   SymbolFileDWARF *m_dwarf = nullptr;
   std::unique_ptr m_dwo_symbol_file;
   const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/Object/Error.h"
 
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"
@@ -790,3 +791,54 @@
   return *m_func_aranges_up;
 }
 
+llvm::Error DWARFUnit::ExtractHeader(SymbolFileDWARF *dwarf,
+ const DWARFDataExtractor ,
+ lldb::offset_t *offset_ptr) {
+  m_offset = *offset_ptr;
+
+  dw_offset_t abbr_offset;
+  const DWARFDebugAbbrev *abbr = dwarf->DebugAbbrev();
+  if (!abbr)
+return llvm::make_error(
+"No debug_abbrev data");
+
+  m_length = data.GetDWARFInitialLength(offset_ptr);
+  m_version = data.GetU16(offset_ptr);
+
+  if (m_version == 5) {
+m_unit_type = data.GetU8(offset_ptr);
+m_addr_size = data.GetU8(offset_ptr);
+abbr_offset = data.GetDWARFOffset(offset_ptr);
+
+if (m_unit_type == llvm::dwarf::DW_UT_skeleton)
+  m_dwo_id = data.GetU64(offset_ptr);
+  } else {
+abbr_offset = data.GetDWARFOffset(offset_ptr);
+m_addr_size = data.GetU8(offset_ptr);
+  }
+
+  bool length_OK = data.ValidOffset(GetNextUnitOffset() - 1);
+  bool version_OK = SymbolFileDWARF::SupportedVersion(m_version);
+  bool abbr_offset_OK = dwarf->get_debug_abbrev_data().ValidOffset(abbr_offset);
+  bool addr_size_OK = (m_addr_size == 4) || (m_addr_size == 8);
+
+  if (!length_OK)
+return llvm::make_error(
+"Invalid compile unit length");
+  if (!version_OK)
+return llvm::make_error(
+"Unsupported compile unit version");
+  if (!abbr_offset_OK)
+return llvm::make_error(
+"Abbreviation offset for compile unit is not valid");
+  if (!addr_size_OK)
+return llvm::make_error(
+"Invalid compile unit address size");
+
+  m_abbrevs = abbr->GetAbbreviationDeclarationSet(abbr_offset);
+  if (!m_abbrevs)

[Lldb-commits] [PATCH] D62008: DWARF: Parse type units from the debug_types section

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

On Fedora 29 x86_64 I get:

  FAIL: LLDB :: SymbolFile/DWARF/debug-types-basic.test (123 of 1636)
   TEST 'LLDB :: SymbolFile/DWARF/debug-types-basic.test' 
FAILED 
  Script:
  --
  : 'RUN: at line 3';   
/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/clang --driver-mode=g++ 
-pthread -target x86_64-pc-linux 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
-g -fdebug-types-section -c -o 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-basic.test.tmp.o
  : 'RUN: at line 5';   
/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/ld.lld 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-basic.test.tmp.o
 -o 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-basic.test.tmp
  : 'RUN: at line 6';   
/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/lldb --no-lldbinit -S 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/lit-lldb-init 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-basic.test.tmp
 -s 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test
 -o exit | /home/jkratoch/redhat/llvm-monorepo-clangassert/bin/FileCheck 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  ld.lld: warning: cannot find entry symbol _start; defaulting to 0x201000
  warning: (x86_64) 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-basic.test.tmp
 lldb doesn’t support .debug_types debug info
  error: use of undeclared identifier 'E'
  
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test:10:10:
 error: CHECK: expected string not found in input
  # CHECK: struct A {
   ^
  :11:1: note: scanning from here
  no type was found matching 'A'
  ^
  :11:11: note: possible intended match here
  no type was found matching 'A'
^
  
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test:19:10:
 error: CHECK: expected string not found in input
  # CHECK: enum E {
   ^
  :13:1: note: scanning from here
  no type was found matching 'E'
  ^
  :14:15: note: possible intended match here
  (lldb) type lookup EC
^
  
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test:27:10:
 error: CHECK: expected string not found in input
  # CHECK: enum class EC {
   ^
  :15:1: note: scanning from here
  no type was found matching 'EC'
  ^
  :15:24: note: possible intended match here
  no type was found matching 'EC'
 ^
  
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-basic.test:38:16:
 error: CHECK-LABEL: expected string not found in input
  # CHECK-LABEL: print (EC) 1
 ^
  :17:1: note: scanning from here
  (lldb) exit
  ^
  :17:8: note: possible intended match here
  (lldb) exit
 ^
  
  --
  
  
  FAIL: LLDB :: SymbolFile/DWARF/debug-types-expressions.test (144 of 1636)
   TEST 'LLDB :: 
SymbolFile/DWARF/debug-types-expressions.test' FAILED 
  Script:
  --
  : 'RUN: at line 3';   
/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/clang --driver-mode=g++ 
-pthread 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp
-g -fdebug-types-section -o 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-expressions.test.tmp
  : 'RUN: at line 5';   
/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/lldb --no-lldbinit -S 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/lit-lldb-init 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-expressions.test.tmp
 -s 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-expressions.test
 -o exit | /home/jkratoch/redhat/llvm-monorepo-clangassert/bin/FileCheck 
/home/jkratoch/redhat/llvm-monorepo/lldb/lit/SymbolFile/DWARF/debug-types-expressions.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  warning: (x86_64) 
/home/jkratoch/redhat/llvm-monorepo-clangassert/tools/lldb/lit/SymbolFile/DWARF/Output/debug-types-expressions.test.tmp
 lldb doesn’t support .debug_types debug info
  error: Multiple internal symbols found for 'a'
  id = {0x0e8f}, range = [0x152e2348-0x152e2350), name="a"
  id = {0x03ee}, range = [0x152a1c78-0x152a1c80), name="a"
  id = {0x0b5d}, range = [0x152e2348-0x152e2350), name="a"
  id = {0x0d17}, range = [0x152e2348-0x152e2350), name="a"
  
  error: use of undeclared identifier 'a'
  

[Lldb-commits] [PATCH] D62008: DWARF: Parse type units from the debug_types section

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62008#1504845 , @jankratochvil 
wrote:

> On Fedora 29 x86_64 I get:
>
>   ...


Ah, sorry I forgot to include the part which removes the 
run-away-screaming-if-you-see-debug_types check. I had that in a separate 
commit and I didn't squash. I'll fix that in a second...


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

https://reviews.llvm.org/D62008



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

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D61956#1504761 , @sgraenitz wrote:

> Instead lldb could have something like `GreenDragon-lldb-base.cmake` that 
> provides and explains reasonable default settings for a range of 
> bots/environments. Actual build bot configs in zorg could then be handled the 
> same way as downstream repos.


Yeah, that's more or less what I had in mind. I didn't mean to include paths to 
various things, which are only valid on the single machine that the bot is 
running on, but more like if a bot is using some specific combo of 
LLVM_ENABLE_ASSERTS/LLVM_ENABLE_MODULES/LLVM_LINK_LLVM_DYLIB/... settings then 
these things might be helpful in figuring out why something is only failing 
there. Though I suppose these things are accessible through the buildbot UIs 
already, so it's already accessible in other ways. I think it still might be 
useful to have these configs accessible in a the repo for to make it easier to 
use them, but it's not a big deal either. The other llvm projects don't do that 
afaik, so it may be better to follow that example..

Having the buildbot config changes in the history is also an interesting 
question. On one hand, they do clutter the history, but on the other, they make 
it more obvious that something has changed -- if a bot mysteriously starts 
failing after one of my changes, i'd like to know that this could be due to a 
configuration change that happened at the same time -- this assumes that the 
configuration changes take effect instantly, and not after a bot reboot as is 
the case with buildbot (I'm not sure how greendragon stands on this front).




Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:10
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr")
+set(LLDB_FRAMEWORK_INSTALL_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+

sgraenitz wrote:
> Follow-up from: https://reviews.llvm.org/D61956?id=199645#inline-550727
> 
> > I believe the expected usage of this variable is to make it point to the 
> > final resting place of the executables, ...
> 
> It's been a pragmatic decision. Maybe we can improve this. The rationale was, 
> that the default configuration should give the user something that works 
> without touching caches or overriding parameters. In a previous sketch I used 
> a real-world destination like:
> ```
> set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr/)
> ```
> But then `ninja install` would fail by default due to lack of permissions in 
> the install destination. Actual release configurations tend to be more 
> complex anyway and vendors will have their own downstream repos / caches for 
> it. Thus, choosing a good default for developers sounded like a good way 
> forward. What do you think?
> 
> > Are you sure including {CMAKE_CURRENT_BINARY_DIR} here is a good idea? I 
> > think llvm tries to avoid that generally, ...
> 
> What exactly do you mean? Having absolute paths, or paths into the 
> build-tree, or the `CMAKE_CURRENT_BINARY_DIR` specifically? I don't see 
> problems with the two last points. Am I missing something?
> 
> For the first: choosing an absolute path was for consistency with 
> `LLDB_FRAMEWORK_INSTALL_DIR`. In the current build logic, they can both be 
> absolute paths. Otherwise:
> * if the install prefix is relative, it will be appended to the path of the 
> root build directory
> * if the framework install dir is relative, it will be appended to the 
> install prefix
> 
> > Then, if you want to copy the to-be-installed files into a staging area 
> > first, you're expected to do that with "DESTDIR=whatever ninja install".
> 
> [Clang cache 
> scripts](https://github.com/llvm/llvm-project/blob/a8f88c38/clang/cmake/caches/Apple-stage1.cmake#L4)
>  seem to accomplish it manually, which may look like this (but the default 
> would again fail due to privileges):
> ```
> if($ENV{DESTDIR})
>   set(CMAKE_INSTALL_PREFIX $ENV{DESTDIR})
>   set(LLDB_FRAMEWORK_INSTALL_DIR "../../SharedFrameworks" CACHE STRING "")
> else()
>   set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr)
>   set(LLDB_FRAMEWORK_INSTALL_DIR 
> /Applications/Xcode.app/Contents/SharedFrameworks CACHE STRING "")
> endif()
> ```
> 
> Would you (and other reviewers) prefer this solution?
> But then ninja install would fail by default due to lack of permissions in 
> the install destination. Actual release configurations tend to be more 
> complex anyway and vendors will have their own downstream repos / caches for 
> it. Thus, choosing a good default for developers sounded like a good way 
> forward. What do you think?

I don't think most developers actually run the "install" rule TBH. :) But if 
they do, I think we should encourage them to do the right thing, and run 
"DESTDIR=foo ninja install", which will work even with a real-world prefix. At 
least that is the right thing to do in the 

[Lldb-commits] [PATCH] D62011: Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 3 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:380
+DWARFExpression *frame_base) const {
+  if (cu == nullptr)
 return false;

Here is a NULL check.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1055
   const dw_offset_t die_offset, Stream ) {
-  if (dwarf2Data == NULL) {
+  if (cu == NULL) {
 s.PutCString("NULL");

Here is a NULL check.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1086
  Stream ) {
-  if (dwarf2Data == NULL) {
+  if (cu == NULL) {
 s.PutCString("NULL");

Here is a NULL check.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62011



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

[Lldb-commits] [PATCH] D62011: Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: clayborg, labath, JDevlieghere.
jankratochvil added a project: LLDB.
Herald added subscribers: jdoerfert, aprantl.

In D61502#1503247  @clayborg suggested 
that `SymbolFileDWARF *dwarf2Data` is really redundant in all the calls with 
also having `DWARFUnit *cu`.  So remove it.  I see no regressions.

There are some nullptr checks, I changed those needed from `dwarf2Data == 
nullptr` to `cu == nullptr` but then who knows if they can happen.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62011

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1550,15 +1550,15 @@
   if (GetDebugMapSymfile())
 return nullptr;
 
-  const char *dwo_name = cu_die.GetAttributeValueAsString(
-  this, _cu, DW_AT_GNU_dwo_name, nullptr);
+  const char *dwo_name =
+  cu_die.GetAttributeValueAsString(_cu, DW_AT_GNU_dwo_name, nullptr);
   if (!dwo_name)
 return nullptr;
 
   SymbolFileDWARFDwp *dwp_symfile = GetDwpSymbolFile();
   if (dwp_symfile) {
-uint64_t dwo_id = cu_die.GetAttributeValueAsUnsigned(this, _cu,
- DW_AT_GNU_dwo_id, 0);
+uint64_t dwo_id =
+cu_die.GetAttributeValueAsUnsigned(_cu, DW_AT_GNU_dwo_id, 0);
 std::unique_ptr dwo_symfile =
 dwp_symfile->GetSymbolFileForDwoId(_cu, dwo_id);
 if (dwo_symfile)
@@ -1568,8 +1568,8 @@
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
   if (dwo_file.IsRelative()) {
-const char *comp_dir = cu_die.GetAttributeValueAsString(
-this, _cu, DW_AT_comp_dir, nullptr);
+const char *comp_dir =
+cu_die.GetAttributeValueAsString(_cu, DW_AT_comp_dir, nullptr);
 if (!comp_dir)
   return nullptr;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -106,8 +106,7 @@
   void SetRangesBase(dw_addr_t ranges_base);
   void SetBaseObjOffset(dw_offset_t base_obj_offset);
   void SetStrOffsetsBase(dw_offset_t str_offsets_base);
-  void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
-  DWARFDebugAranges *debug_aranges);
+  void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges);
 
   lldb::ByteOrder GetByteOrder() const;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -274,23 +274,22 @@
 // m_die_array_mutex must be already held as read/write.
 void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry _die) {
   dw_addr_t addr_base = cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_addr_base, LLDB_INVALID_ADDRESS);
+  this, DW_AT_addr_base, LLDB_INVALID_ADDRESS);
   if (addr_base != LLDB_INVALID_ADDRESS)
 SetAddrBase(addr_base);
 
   dw_addr_t ranges_base = cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_rnglists_base, LLDB_INVALID_ADDRESS);
+  this, DW_AT_rnglists_base, LLDB_INVALID_ADDRESS);
   if (ranges_base != LLDB_INVALID_ADDRESS)
 SetRangesBase(ranges_base);
 
-  SetStrOffsetsBase(cu_die.GetAttributeValueAsUnsigned(
-  m_dwarf, this, DW_AT_str_offsets_base, 0));
+  SetStrOffsetsBase(
+  cu_die.GetAttributeValueAsUnsigned(this, DW_AT_str_offsets_base, 0));
 
-  uint64_t base_addr = cu_die.GetAttributeValueAsAddress(
-  m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  uint64_t base_addr = cu_die.GetAttributeValueAsAddress(this, DW_AT_low_pc,
+ LLDB_INVALID_ADDRESS);
   if (base_addr == LLDB_INVALID_ADDRESS)
-base_addr = cu_die.GetAttributeValueAsAddress(
-m_dwarf, this, DW_AT_entry_pc, 0);
+base_addr = cu_die.GetAttributeValueAsAddress(this, DW_AT_entry_pc, 0);
   SetBaseAddress(base_addr);
 
   std::unique_ptr dwo_symbol_file =
@@ -307,7 +306,7 @@
 return; // Can't fetch the compile unit DIE from the dwo file.
 
   uint64_t main_dwo_id =
-  cu_die.GetAttributeValueAsUnsigned(m_dwarf, this, DW_AT_GNU_dwo_id, 0);
+  

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Thanks for your input.

In D61956#1504327 , @labath wrote:

> I wouldn't want to over-proliferate these but in general I think this is a 
> good idea.


Agreed, caches for most common configurations only. Also we may not 
over-proliferate includes across caches.
In a previous sketch I had extra variants for development and release builds. I 
preferred to keep only one variant for now and make notes for release builds 
instead, because actual release configurations are likely more complex anyway.
For downstream repositories I am not sure what's the best approach, but I'd 
tend to: create their own cache files, include one upstream cache file and add 
downstream options.

> What I would find particularly useful is if we checked in the configurations 
> used by all the bots in here, as that would make it easier to figure out 
> what's going on if one of them breaks and the reason is not obvious...

Interesting idea. This would go in the direction of CI systems with checked-in 
build/test configurations (like `.travis.yml` or `.gitlab-ci.yml`)? In my 
experience this works great with docker containers, because they provide a 
stable environment, but I remember well that debugging Travis CI builds was a 
big time sink before containerization. Hence, I wonder whether it comes with 
drawbacks in our case. We don't use containers exclusively. We do have local 
differences between machines and bot configurations smooth them out explicitly. 
I am not fond of putting thing like this in a cache file:

  -DPYTHON_EXECUTABLE=/usr/bin/python2.7
  
-DPYTHON_LIBRARY=/System/Library/Frameworks/Python.framework/Versions/2.7/lib/libpython2.7.dylib

Furthermore, fixing bot configurations can be painful and often enough 
experimental changes are committed (e.g. 
https://github.com/llvm/llvm-zorg/commits/master/zorg/jenkins/jobs/jobs/lldb-cmake-standalone).
 Not sure if we want this in the LLDB history? I guess there's a reason why 
zorg is not part of the monorepo? :-) Also, I think fixing bot configurations 
typically goes hand in hand with changes on build bot scripts (which we don't 
have in the repo either) more than with changes on the codebase itself. I don't 
see it getting easier by moving the configurations to the codebase..

With all this considered, I wouldn't prefer to check in concrete caches for 
build bots in lldb. We could have them in zorg right?
Instead lldb could have something like `GreenDragon-lldb-base.cmake` that 
provides and explains reasonable default settings for a range of 
bots/environments. Actual build bot configs in zorg could then be handled the 
same way as downstream repos.

Might that be a way forward? We may brainstorm a list of candidates or just 
handle them as they come up.




Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:10
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr")
+set(LLDB_FRAMEWORK_INSTALL_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+

Follow-up from: https://reviews.llvm.org/D61956?id=199645#inline-550727

> I believe the expected usage of this variable is to make it point to the 
> final resting place of the executables, ...

It's been a pragmatic decision. Maybe we can improve this. The rationale was, 
that the default configuration should give the user something that works 
without touching caches or overriding parameters. In a previous sketch I used a 
real-world destination like:
```
set(CMAKE_INSTALL_PREFIX /Applications/Xcode.app/Contents/Developer/usr/)
```
But then `ninja install` would fail by default due to lack of permissions in 
the install destination. Actual release configurations tend to be more complex 
anyway and vendors will have their own downstream repos / caches for it. Thus, 
choosing a good default for developers sounded like a good way forward. What do 
you think?

> Are you sure including {CMAKE_CURRENT_BINARY_DIR} here is a good idea? I 
> think llvm tries to avoid that generally, ...

What exactly do you mean? Having absolute paths, or paths into the build-tree, 
or the `CMAKE_CURRENT_BINARY_DIR` specifically? I don't see problems with the 
two last points. Am I missing something?

For the first: choosing an absolute path was for consistency with 
`LLDB_FRAMEWORK_INSTALL_DIR`. In the current build logic, they can both be 
absolute paths. Otherwise:
* if the install prefix is relative, it will be appended to the path of the 
root build directory
* if the framework install dir is relative, it will be appended to the install 
prefix

> Then, if you want to copy the to-be-installed files into a staging area 
> first, you're expected to do that with "DESTDIR=whatever ninja install".

[Clang cache 
scripts](https://github.com/llvm/llvm-project/blob/a8f88c38/clang/cmake/caches/Apple-stage1.cmake#L4)
 seem to accomplish it manually, which may look like this (but the default 
would again 

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 199817.
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Rename macOS specific cache file.
Add settings and comments.
Fix install destinations by appending `Developer` and `SharedFrameworks` 
respectively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956

Files:
  lldb/cmake/caches/Apple-lldb-base.cmake
  lldb/cmake/caches/Apple-lldb-macOS.cmake


Index: lldb/cmake/caches/Apple-lldb-macOS.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -0,0 +1,18 @@
+include(${CMAKE_CURRENT_LIST_DIR}/Apple-lldb-base.cmake)
+
+set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
+set(LLDB_NO_INSTALL_DEFAULT_RPATH ON CACHE BOOL "")
+
+# Release builds will use actual install destinations. Their relative locations
+# must align with INSTALL_RPATH settings. The locations of LLDB.framework, in
+# particular, must match the RPATHs of the installed lldb driver executable.
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr")
+set(LLDB_FRAMEWORK_INSTALL_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+
+# Release builds may change these:
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11)
+set(LLDB_USE_SYSTEM_DEBUGSERVER ON CACHE BOOL "")
+set(LLVM_EXTERNALIZE_DEBUGINFO OFF CACHE BOOL "")
+
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
Index: lldb/cmake/caches/Apple-lldb-base.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-base.cmake
@@ -0,0 +1,8 @@
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
+
+# Release builds set these explicitly:
+#set(LLDB_VERSION_MAJOR  CACHE STRING "")
+#set(LLDB_VERSION_MINOR 9 CACHE STRING "")
+#set(LLDB_VERSION_PATCH 9 CACHE STRING "")
+#set(LLDB_VERSION_SUFFIX git CACHE STRING "")


Index: lldb/cmake/caches/Apple-lldb-macOS.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -0,0 +1,18 @@
+include(${CMAKE_CURRENT_LIST_DIR}/Apple-lldb-base.cmake)
+
+set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
+set(LLDB_NO_INSTALL_DEFAULT_RPATH ON CACHE BOOL "")
+
+# Release builds will use actual install destinations. Their relative locations
+# must align with INSTALL_RPATH settings. The locations of LLDB.framework, in
+# particular, must match the RPATHs of the installed lldb driver executable.
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/Developer/usr")
+set(LLDB_FRAMEWORK_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/install/SharedFrameworks" CACHE STRING "")
+
+# Release builds may change these:
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11)
+set(LLDB_USE_SYSTEM_DEBUGSERVER ON CACHE BOOL "")
+set(LLVM_EXTERNALIZE_DEBUGINFO OFF CACHE BOOL "")
+
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
Index: lldb/cmake/caches/Apple-lldb-base.cmake
===
--- /dev/null
+++ lldb/cmake/caches/Apple-lldb-base.cmake
@@ -0,0 +1,8 @@
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
+
+# Release builds set these explicitly:
+#set(LLDB_VERSION_MAJOR  CACHE STRING "")
+#set(LLDB_VERSION_MINOR 9 CACHE STRING "")
+#set(LLDB_VERSION_PATCH 9 CACHE STRING "")
+#set(LLDB_VERSION_SUFFIX git CACHE STRING "")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Ah, yes, sure, it will just be moved on the `lldb-server` side. Thanks again 
for the clarification!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D53753#1504645 , @aleksandr.urakov 
wrote:

> In D53753#1504589 , @labath wrote:
>
> > When we evaluate an expression, we jit a bunch of opcodes into the inferior 
> > memory and then have it execute them. For that to work, we need to allocate 
> > some memory in order to store the opcodes. We cannot use the general 
> > expression engine to jit that expression, as we would be back to square 
> > one, so we manually set the PC to the entry point of the mmap function, and 
> > set the argument values as if it was being called. Then we just let the 
> > inferior loose and have it  allocate the memory for us and return it. For 
> > this to work, we need abi knowledge both to correctly set the arguments of 
> > mmap, and to retrieve its result.
>
>
> Got it, thanks! There's a different approach on Windows: for now we just call 
> `VirtualAllocEx`, which can allocate memory in another process. But it will 
> not work for the remote debugging case.


Sure it will. You just need to call that function from lldb-server in response 
to the `_M` packet. The `_M` packet is our primary method of allocating memory, 
and the `mmap` thingy is a fallback for platforms where allocating memory is 
not that easy.

(Actually, there is a relatively easy way to allocate memory from lldb-server 
on linux too, which is to (set up appropriate registers and then) have the 
inferior execute the `int 0x80` instruction. That would bypass the mmap 
function and jump straight to the kernel syscall. It would have the advantage 
of working in situations where the mmap libc function may not be available, but 
it's a bit of pain to set up so, i haven't tried yet to implement that.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

In D53753#1504589 , @labath wrote:

> When we evaluate an expression, we jit a bunch of opcodes into the inferior 
> memory and then have it execute them. For that to work, we need to allocate 
> some memory in order to store the opcodes. We cannot use the general 
> expression engine to jit that expression, as we would be back to square one, 
> so we manually set the PC to the entry point of the mmap function, and set 
> the argument values as if it was being called. Then we just let the inferior 
> loose and have it  allocate the memory for us and return it. For this to 
> work, we need abi knowledge both to correctly set the arguments of mmap, and 
> to retrieve its result.


Got it, thanks! There's a different approach on Windows: for now we just call 
`VirtualAllocEx`, which can allocate memory in another process. But it will not 
work for the remote debugging case.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D53753#1504539 , @aleksandr.urakov 
wrote:

> In D53753#1504361 , @labath wrote:
>
> > I'm not sure what exactly are the consequences of not using the correct ABI 
> > definition here. I think the case where the difference may start to become 
> > obvious is if you try to get argument values of a function for which you 
> > don't have debug info for.
>
>
> It sounds strange to me... If we don't have symbols for a function, then we 
> can't even know amount of its arguments, so how can we retrieve them? Also 
> e.g. on Windows x86 both stdcall, ccall, thiscall and fastcall are commonly 
> used, so it would be strange to use some "default" ABI...


That's good point. I may be misremembering things here. I never dealt with 
these things directly, and I'm just relaying what I remember from past 
discussions.

I had a brief look at the source code, and it looks like there's only a handful 
of callers to the `GetArgumentValues` method. The main use case seems to be 
when you already have some external knowledge that a certain function has some 
signature, but you may not have debug info for it (e.g. because it's a system 
function, and you don't have debug info for system libraries). AppleObjCRuntime 
seems to use that to extract some information about the exception being thrown..

So it's quite possible that this function is never actually called on windows..

> 
> 
>> (Also, we use the abi plugin to call mmap, and mmap takes 6 arguments).
> 
> Can you explain me, please, when does such mmap being called? Just for the 
> purpose of general education :)

When we evaluate an expression, we jit a bunch of opcodes into the inferior 
memory and then have it execute them. For that to work, we need to allocate 
some memory in order to store the opcodes. We cannot use the general expression 
engine to jit that expression, as we would be back to square one, so we 
manually set the PC to the entry point of the mmap function, and set the 
argument values as if it was being called. Then we just let the inferior loose 
and have it  allocate the memory for us and return it. For this to work, we 
need abi knowledge both to correctly set the arguments of mmap, and to retrieve 
its result.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

In D53753#1504361 , @labath wrote:

> I'm not sure what exactly are the consequences of not using the correct ABI 
> definition here. I think the case where the difference may start to become 
> obvious is if you try to get argument values of a function for which you 
> don't have debug info for.


It sounds strange to me... If we don't have symbols for a function, then we 
can't even know amount of its arguments, so how can we retrieve them? Also e.g. 
on Windows x86 both stdcall, ccall, thiscall and fastcall are commonly used, so 
it would be strange to use some "default" ABI...

But I don't know why the SysV ABI is used on Windows now (it was already used 
before my commit). It looks like nothing bad should happen if we will use it 
for expressions evaluation (and even with six arguments - r10 and r11 are 
volatile on Windows x64), but I have no objections to creating a separate ABI 
plugin.

> (Also, we use the abi plugin to call mmap, and mmap takes 6 arguments).

Can you explain me, please, when does such mmap being called? Just for the 
purpose of general education :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D61942: DWARFContext: Return empty data extractors instead of null pointers

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB360874: DWARFContext: Return empty data extractors 
instead of null pointers (authored by labath, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D61942?vs=199597=199787#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61942

Files:
  lit/SymbolFile/DWARF/debug_aranges-empty-section.s
  source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  source/Plugins/SymbolFile/DWARF/DWARFContext.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
@@ -42,8 +42,6 @@
 // Extract
 llvm::Error
 DWARFDebugAranges::extract(const DWARFDataExtractor _aranges_data) {
-  assert(debug_aranges_data.ValidOffset(0));
-
   lldb::offset_t offset = 0;
 
   DWARFDebugArangeSet set;
@@ -65,8 +63,8 @@
   }
 }
 set.Clear();
-}
-return llvm::ErrorSuccess();
+  }
+  return llvm::ErrorSuccess();
 }
 
 void DWARFDebugAranges::Dump(Log *log) const {
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -44,13 +44,10 @@
   assert(m_dwarf2Data);
 
   m_cu_aranges_up = llvm::make_unique();
-  const DWARFDataExtractor *debug_aranges_data =
+  const DWARFDataExtractor _aranges_data =
   m_context.getOrLoadArangesData();
-  if (debug_aranges_data) {
-llvm::Error error = m_cu_aranges_up->extract(*debug_aranges_data);
-if (error)
-  return std::move(error);
-  }
+  if (llvm::Error error = m_cu_aranges_up->extract(debug_aranges_data))
+return std::move(error);
 
   // Make a list of all CUs represented by the arange data in the file.
   std::set cus_with_data;
Index: source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -13,31 +13,32 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static const DWARFDataExtractor *
-LoadOrGetSection(Module , SectionType section_type,
- llvm::Optional ) {
-  if (extractor.hasValue())
-return extractor->GetByteSize() > 0 ? extractor.getPointer() : nullptr;
-
-  // Initialize to an empty extractor so that we always take the fast path going
-  // forward.
-  extractor.emplace();
-
-  const SectionList *section_list = module.GetSectionList();
+static DWARFDataExtractor LoadSection(Module ,
+  SectionType section_type) {
+  SectionList *section_list = module.GetSectionList();
   if (!section_list)
-return nullptr;
+return DWARFDataExtractor();
 
   auto section_sp = section_list->FindSectionByType(section_type, true);
   if (!section_sp)
-return nullptr;
+return DWARFDataExtractor();
 
-  section_sp->GetSectionData(*extractor);
-  return extractor.getPointer();
+  DWARFDataExtractor data;
+  section_sp->GetSectionData(data);
+  return data;
+}
+
+static const DWARFDataExtractor &
+LoadOrGetSection(Module , SectionType section_type,
+ llvm::Optional ) {
+  if (!extractor)
+extractor = LoadSection(module, section_type);
+  return *extractor;
 }
 
 DWARFContext::DWARFContext(Module ) : m_module(module) {}
 
-const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+const DWARFDataExtractor ::getOrLoadArangesData() {
   return LoadOrGetSection(m_module, eSectionTypeDWARFDebugAranges,
   m_data_debug_aranges);
 }
Index: source/Plugins/SymbolFile/DWARF/DWARFContext.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFContext.h
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.h
@@ -23,7 +23,7 @@
 public:
   explicit DWARFContext(Module );
 
-  const DWARFDataExtractor *getOrLoadArangesData();
+  const DWARFDataExtractor ();
 };
 } // namespace lldb_private
 
Index: lit/SymbolFile/DWARF/debug_aranges-empty-section.s
===
--- lit/SymbolFile/DWARF/debug_aranges-empty-section.s
+++ lit/SymbolFile/DWARF/debug_aranges-empty-section.s
@@ -0,0 +1,63 @@
+# Test that an empty .debug_aranges section doesn't confuse (or crash) us.
+
+# RUN: llvm-mc %s -triple x86_64-pc-linux -filetype=obj >%t
+# RUN: lldb %t -o "breakpoint set -n f" -b | FileCheck %s
+
+# CHECK: Breakpoint 1: where = {{.*}}`f, address = 0x0047
+
+	.text
+	.globl	f   # -- Begin function f
+	

[Lldb-commits] [PATCH] D61908: DWARF: Add ability to reference debug info coming from multiple sections

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360872: DWARF: Add ability to reference debug info coming 
from multiple sections (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61908?vs=199562=199784#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61908

Files:
  lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s
  lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -245,7 +245,7 @@
   }
 }
 
-DIERef ref(cu_offset, die.GetOffset());
+DIERef ref(unit.GetDebugSection(), cu_offset, die.GetOffset());
 switch (tag) {
 case DW_TAG_inlined_subroutine:
 case DW_TAG_subprogram:
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -153,7 +153,8 @@
   return DWARFDIE(cu, block_die);
 else
   return DWARFDIE(dwarf->DebugInfo()->GetUnit(
-  DIERef(cu->GetOffset(), block_die->GetOffset())),
+  DIERef(cu->GetDebugSection(), cu->GetOffset(),
+ block_die->GetOffset())),
   block_die);
   }
 }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -34,6 +34,10 @@
   /// Byte size of the compile unit header
   uint32_t GetHeaderByteSize() const override;
 
+  DIERef::Section GetDebugSection() const override {
+return DIERef::Section::DebugInfo;
+  }
+
 private:
   DWARFCompileUnit(SymbolFileDWARF *dwarf2Data, lldb::user_id_t uid);
   DISALLOW_COPY_AND_ASSIGN(DWARFCompileUnit);
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -24,7 +24,7 @@
   dw_offset_t cu_offset = m_cu->GetOffset();
   if (m_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
 cu_offset = m_cu->GetBaseObjOffset();
-  return DIERef(cu_offset, m_die->GetOffset());
+  return DIERef(m_cu->GetDebugSection(), cu_offset, m_die->GetOffset());
 }
 
 dw_tag_t DWARFBaseDIE::Tag() const {
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
@@ -13,12 +13,12 @@
 #include "SymbolFileDWARF.h"
 #include "SymbolFileDWARFDebugMap.h"
 
-DIERef::DIERef(const DWARFFormValue _value)
-: cu_offset(DW_INVALID_OFFSET), die_offset(DW_INVALID_OFFSET) {
+DIERef::DIERef(const DWARFFormValue _value) {
   if (form_value.IsValid()) {
 DWARFDIE die = form_value.Reference();
 die_offset = die.GetOffset();
 if (die) {
+  section = die.GetCU()->GetDebugSection();
   if (die.GetCU()->GetBaseObjOffset() != DW_INVALID_OFFSET)
 cu_offset = die.GetCU()->GetBaseObjOffset();
   else
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ 

[Lldb-commits] [lldb] r360872 - DWARF: Add ability to reference debug info coming from multiple sections

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 16 04:07:58 2019
New Revision: 360872

URL: http://llvm.org/viewvc/llvm-project?rev=360872=rev
Log:
DWARF: Add ability to reference debug info coming from multiple sections

Summary:
This patch adds the ability to precisely address debug info in
situations when a single file can have more than one debug-info-bearing
sections (as is the case with type units in DWARF v4).

The changes here can be classified into roughly three categories:
- the code which addresses a debug info by offset gets an additional
  argument, which specifies the section one should look into.
- the DIERef class also gets an additional member variable specifying
  the section. This way, code dealing with DIERefs can know which
  section is the object referring to.
- the user_id_t encoding steals one bit from the dwarf_id field to store
  the section. This means the total number of separate object files
  (apple .o, or normal .dwo) is limited to 2 billion, but that is fine
  as it's not possible to hit that number without switching to DWARF64
  anyway.

This patch is functionally equivalent to (and inspired by) the two
patches (D61503 and D61504) by Jan Kratochvil, but there are differences
in the implementation:
- it uses an enum instead of a bool flag to differentiate the sections
- it increases the size of DIERef struct instead of reducing the amount
  of addressable debug info
- it sets up DWARFDebugInfo to store the units in a single vector
  instead of two. This sets us up for the future in which type units can
  also live in the debug_info section, and I believe it's cleaner
  because there's no need for unit index remapping

There are no tests with this patch as this is essentially NFC until
we start parsing type units from the debug_types section.

Reviewers: JDevlieghere, clayborg, aprantl

Subscribers: arphaman, jankratochvil, lldb-commits

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

Modified:
lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s
lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Modified: lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s?rev=360872=360871=360872=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/array-sizes.s Thu May 16 04:07:58 2019
@@ -9,8 +9,8 @@
 # RUN: ld.lld %t.o -o %t
 # RUN: lldb-test symbols %t | FileCheck %s
 
-# CHECK: Variable{0x001e}, name = "X"
-# CHECK-SAME: type = {0033} 0x{{[0-9a-f]*}} (char [56])
+# CHECK: Variable{0x7fff001e}, name = "X"
+# CHECK-SAME: type = {7fff0033} 0x{{[0-9a-f]*}} (char [56])
 
 
 # Generated from "char X[47];"

Modified: lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s?rev=360872=360871=360872=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s Thu May 16 04:07:58 2019
@@ -4,7 +4,7 @@
 # RUN: ld.lld -m elf_x86_64 %t.o -o %t 
 # RUN: lldb-test symbols %t | FileCheck %s
 
-# CHECK: Variable{0x0011}, name = "color"
+# CHECK: Variable{0x7fff0011}, name = "color"
 # CHECK-SAME: location = DW_OP_addrx(0x0)
 
 .text

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp?rev=360872=360871=360872=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.cpp Thu May 16 04:07:58 
2019
@@ -13,12 +13,12 @@
 #include "SymbolFileDWARF.h"
 

[Lldb-commits] [PATCH] D61687: Update Python tests for lldb-server on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for explaining. I think this is good to go, sans the merging of the two 
tests I requested, and the comment fix that @clayborg noticed.




Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:208-212
+# In current implementation of llgs on Windows, as a response to '\x03' 
packet, the debugger
+# of the native process will trigger a call to DebugBreakProcess that will 
create a new thread
+# to handle the exception debug event. So one more stop thread will be 
notified to the
+# delegate, e.g. llgs.  So tests below to assert the stop threads number 
will all fail.
+@expectedFailureAll(oslist=["windows"])

Hui wrote:
> labath wrote:
> > labath wrote:
> > > Is this something that we consider to be a bug, or is it just how 
> > > debugging is supposed to work on windows? If it's a bug then fine, but if 
> > > not then we might consider adjusting the expectations in the test (or 
> > > just skipping it).
> > You marked this as done, but it's not obvious how is this comment resolved 
> > (or indeed, if it needs to be resolved). Can you elaborate?
> DebugBreakProcess is supposed to spawn a new thread in the debugged process 
> and then the thread exits after the irq is handled.
> 
> See below thread #1 is main thread of the debugged process and thread #2 is 
> the newly spawned.
> json string contains two stopped threads information.
> 
> looks like the stopped threads number is supposed to +1, but in order to be 
> consistent with Visual Studio,
> it shall be possible to only report the thread #1 stop info.
> 
> To modify the python script for Windows is at some cost. Maybe just skip them?
> 
> ```
> (lldb) process interrupt
>  GDBRemoteClientBase::Lock::Lock sent packet: \x03
> ...
>  <  16> send packet: $jThreadsInfo#c1
>  < 354> read packet: 
> $[{"name":"main.exe","reason":"trace","registers":{"16":"dc6b5a9af77f","6":"","7":"80fa1e8aca00"}],"tid":23108}],{"description":"Exception
>  0x8003 encountered at address 
> 0x7ffa1701e370","name":"main.exe","reason":"exception","registers":{"16":"71e30117fa7f","6":"","7":"28fa4f8aca00"}],"tid":23716}]]#f0
> 
> Process 27544 stopped
> * thread #1, name = 'main.exe', stop reason = trace
> frame #0: 0x7ff79a5a6bdc main.exe`main at main.cpp:7
>4{
>5
>6  printf("abc");
> -> 7  while(1);
>8  return 1;
>9}
>   thread #2, name = 'main.exe', stop reason = Exception 0x8003 
> encountered at address 0x7ffa1701e370
> frame #0: 0x7ffa1701e371
> ->  0x7ffa1701e371: retq
> 0x7ffa1701e372: int3
> 0x7ffa1701e373: int3
> 0x7ffa1701e374: int3
> ```
> 
Ok, I see. Hiding the thread sounds like the right thing to do, though I'm not 
sure if it should be done at lldb-server level. My feeling is it should be done 
at a higher level, because lldb-server should report the thing that the OS 
sees, and the OS definitely sees two threads here. It may be better to hide 
this in the higher levels of the debugger (unless the user has requested some 
sort of a verbose view).

If we go down that path, then I think it would be worth it to update these 
tests to have the correct expectation, but it does not have to happen now. We 
can leave them XFAILed for the time being. Thanks for explaining.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/inferior-crash/TestGdbRemoteAbort.py:40
 
+@skipIfWindows # For now the signo in T* packet is always 0.
 @llgs_test

Hui wrote:
> labath wrote:
> > labath wrote:
> > > Do you have any plans for changing this? Given that windows does not 
> > > support signals, maybe it should stay 0...
> > I'd like to hear your thoughts on this...
> > 
> > If the idea is for the signal number to stay zero, then the comment 
> > shouldn't say "for now" but instead say something to the effect that 
> > abort() does not cause a signal to be raised on windows because windows 
> > doesn't have signals. If the idea is to change this later, then I'd like to 
> > understand how/why.
> I think the signal number can just stay zero (treated as invalid) unless 
> there is any strategy that will
> need valid ones in the future for Windows user-mode debugging(or for 
> kernel-mode debugging?). 
> The proposed implementation of native process/thread for windows in D56233 is 
> mainly signal free. 
> 
> There will be several slight difference(worth mentioning) by always filling 
> with zero signo
> 
> (1) In remote debugging case, T* packet is to detail the stopped reason for a 
> thread.
> W or w/o a valid signal the messages shown on lldb  are slightly different.
> (StopReason::eStopReasonBreakpoint vs StopReason::eStopReasonException)
> 
> On Linux,
> 
>  thread #1, name = 'a.out', stop reason = signal SIGSEGV: invalid address 
> (fault address: 0x0)
> 
> On Windows,
>  thread #1, name = 

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D53753#1504320 , @aleksandr.urakov 
wrote:

> @lanza Hello! AFAIU, these values have nothing to do with the Microsoft x64 
> calling convention. They are used by `ABISysV_x86_64`, which specifies ABI 
> for working with code injectable into a process being debugged. It requires 
> six arguments to be passed through registers (see `GetArgumentValues`).
>
> If I understand right, `SymbolFile` retrieves the information about arguments 
> location on the stack, so there's no need to support different calling 
> conventions in LLDB for this purpose. Please, correct me if I'm wrong.


That definitely doesn't sound right. The reason ABISysV_x86_64 requires 6 
registers is because the "SysV" x86_64 ABI specifies 6 argument registers. If 
the microsoft ABI is different, then you probably need a new ABI plugin to 
correctly describe it. (Also, we use the abi plugin to call mmap, and mmap 
takes 6 arguments).

I'm not sure what exactly are the consequences of not using the correct ABI 
definition here. I think the case where the difference may start to become 
obvious is if you try to get argument values of a function for which you don't 
have debug info for. Probably the reason that expression evaluation works for 
you is that we only ever pass one argument into the generated expression (by 
arranging all inputs into a struct and then passing the struct pointer as an 
argument), and the first register argument happens to be the same for SysV and 
microsoft ABIs.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D61952: [CMake] Stabilize install process for LLDB.framework

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not very familiar with frameworks and complexities involved in creating 
them, but the fact that we need to delete stuff from the build tree in order to 
install properly makes me think that there is something fishy going on. Is 
there no way to arrange things so that this can be avoided? For instance, what 
if we set the build output paths to be separate and disjoint locations for each 
target. Then use a separate target, or some POST_BUILD commands to copy/symlink 
the files to construct an build-tree framework, and have the install targets 
create the install-tree framework from the original build output paths?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61952



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

[Lldb-commits] [lldb] r360868 - Simplify Triple::ppc64{, le} checks with Triple::isPPC64()

[Fangrui Song via lldb-commits]

Author: maskray
Date: Thu May 16 02:07:33 2019
New Revision: 360868

URL: http://llvm.org/viewvc/llvm-project?rev=360868=rev
Log:
Simplify Triple::ppc64{,le} checks with Triple::isPPC64()

While here, update some ppc64le specific check to isPPC64(), if it
applies to big-endian as well, in the hope that it will ease the support
of big-endian if people are interested in this area. The big-endian
variant is used by at least FreeBSD, Gentoo Linux, Adélie Linux, and
Void Linux.

Modified:
lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
lldb/trunk/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp
lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Modified: lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp?rev=360868=360867=360868=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp Thu May 16 
02:07:33 2019
@@ -69,10 +69,8 @@ lldb::ByteOrder ABISysV_ppc64::GetByteOr
 ABISP
 ABISysV_ppc64::CreateInstance(lldb::ProcessSP process_sp,
   const ArchSpec ) {
-  if (arch.GetTriple().getArch() == llvm::Triple::ppc64 ||
-  arch.GetTriple().getArch() == llvm::Triple::ppc64le) {
+  if (arch.GetTriple().isPPC64())
 return ABISP(new ABISysV_ppc64(process_sp));
-  }
   return ABISP();
 }
 

Modified: lldb/trunk/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp?rev=360868=360867=360868=diff
==
--- lldb/trunk/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp Thu May 
16 02:07:33 2019
@@ -35,11 +35,10 @@ void ArchitecturePPC64::Terminate() {
 }
 
 std::unique_ptr ArchitecturePPC64::Create(const ArchSpec ) {
-  if ((arch.GetMachine() != llvm::Triple::ppc64 &&
-   arch.GetMachine() != llvm::Triple::ppc64le) ||
-  arch.GetTriple().getObjectFormat() != 
llvm::Triple::ObjectFormatType::ELF)
-return nullptr;
-  return std::unique_ptr(new ArchitecturePPC64());
+  if (arch.GetTriple().isPPC64() &&
+  arch.GetTriple().getObjectFormat() == 
llvm::Triple::ObjectFormatType::ELF)
+return std::unique_ptr(new ArchitecturePPC64());
+  return nullptr;
 }
 
 ConstString ArchitecturePPC64::GetPluginName() { return GetPluginNameStatic(); 
}

Modified: 
lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp?rev=360868=360867=360868=diff
==
--- lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp Thu 
May 16 02:07:33 2019
@@ -55,23 +55,15 @@ EmulateInstruction *
 EmulateInstructionPPC64::CreateInstance(const ArchSpec ,
 InstructionType inst_type) {
   if (EmulateInstructionPPC64::SupportsEmulatingInstructionsOfTypeStatic(
-  inst_type)) {
-if (arch.GetTriple().getArch() == llvm::Triple::ppc64 ||
-arch.GetTriple().getArch() == llvm::Triple::ppc64le) {
+  inst_type))
+if (arch.GetTriple().isPPC64())
   return new EmulateInstructionPPC64(arch);
-}
-  }
 
   return nullptr;
 }
 
 bool EmulateInstructionPPC64::SetTargetTriple(const ArchSpec ) {
-  if (arch.GetTriple().getArch() == llvm::Triple::ppc64)
-return true;
-  else if (arch.GetTriple().getArch() == llvm::Triple::ppc64le)
-return true;
-
-  return false;
+  return arch.GetTriple().isPPC64();
 }
 
 static bool LLDBTableGetRegisterInfo(uint32_t reg_num, RegisterInfo _info) 
{

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=360868=360867=360868=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Thu May 16 
02:07:33 2019
@@ -1007,7 +1007,7 @@ NativeProcessLinux::SetupSoftwareSingleS
   // Arm mode
   error = SetSoftwareBreakpoint(next_pc, 4);
 }
-  } else if (m_arch.IsMIPS() || m_arch.GetMachine() == llvm::Triple::ppc64le)
+  } else if (m_arch.IsMIPS() || m_arch.GetTriple().isPPC64())
 

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is a bit more readable. I've included some suggestions which I 
think could make it even better.

Since you're already rewriting this code, this might be a good time to raise a 
point I wanted to bring up some day. Should we be using `FileSpec` for code 
like this? The code already uses a combination of llvm and lldb path utilities, 
and I would argue that we should use llvm all the way for code like this 
(except that we go through the FileSystem abstraction for the reproducer 
stuff). I have two reasons for that:

- FileSpec is designed for efficient matching of abstract file paths which may 
not even exist on the host system. As such, this code will result in a bunch of 
strings being added to the global string pool for no reason. And in this case, 
you're definitely working files which do exist (or may exist) on the host. In 
fact, FileSpec now contains code which performs path simplifications which are 
not completely sound given a concrete filesystem -- it will simplify a path 
like `bar/../foo` to `foo`, which is not correct if `bar` is a symlink.
- Since we started supporting windows paths, the FileSpec class offers more 
freedom than it is needed here. Specifically, it gives you the ability to 
return a foreign-syntax path as the "lldbinit" location. Therefore, in the 
spirit of using the most specific type which is sufficient to accomplish a 
given task, I think we should use a plain string here.




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2149-2150
+
+  LoadCWDlldbinitFile should_load = ShouldLoadCwdInitFile();
+  if (should_load == eLoadCWDlldbinitFalse) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);

Make this a `switch` ?



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

JDevlieghere wrote:
> This should be reflowed. 
What might help readability is moving the long block of text to a separate 
static variable declared before the function. That way the text does not 
obscure the logic of the function.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2191-2196
+const char *program_name = program_file_spec.GetFilename().AsCString();
+
+if (program_name) {
+  char program_init_file_name[PATH_MAX];
+  ::snprintf(program_init_file_name, sizeof(program_init_file_name),
+  "%s-%s", init_file.GetPath().c_str(), program_name);

This should be done with strings and stringrefs


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61994



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

[Lldb-commits] [lldb] r360865 - Simplify ArchSpec::IsMIPS()

[Fangrui Song via lldb-commits]

Author: maskray
Date: Thu May 16 01:37:32 2019
New Revision: 360865

URL: http://llvm.org/viewvc/llvm-project?rev=360865=rev
Log:
Simplify ArchSpec::IsMIPS()

Modified:
lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

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

Modified: lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp?rev=360865=360864=360865=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp Thu May 16 
01:37:32 2019
@@ -553,11 +553,8 @@ size_t ABISysV_mips64::GetRedZoneSize()
 
 ABISP
 ABISysV_mips64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
) {
-  const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
-  if ((arch_type == llvm::Triple::mips64) ||
-  (arch_type == llvm::Triple::mips64el)) {
+  if (arch.GetTriple().isMIPS64())
 return ABISP(new ABISysV_mips64(process_sp));
-  }
   return ABISP();
 }
 

Modified: lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp?rev=360865=360864=360865=diff
==
--- lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp (original)
+++ lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp Thu May 
16 01:37:32 2019
@@ -1178,10 +1178,7 @@ DisassemblerLLVMC::DisassemblerLLVMC(con
 break;
   }
 
-  if (triple.getArch() == llvm::Triple::mips ||
-  triple.getArch() == llvm::Triple::mipsel ||
-  triple.getArch() == llvm::Triple::mips64 ||
-  triple.getArch() == llvm::Triple::mips64el) {
+  if (arch.IsMIPS()) {
 uint32_t arch_flags = arch.GetFlags();
 if (arch_flags & ArchSpec::eMIPSAse_msa)
   features_str += "+msa,";
@@ -1219,10 +1216,7 @@ DisassemblerLLVMC::DisassemblerLLVMC(con
 if (!m_alternate_disasm_up)
   m_disasm_up.reset();
 
-  } else if (llvm_arch == llvm::Triple::mips ||
- llvm_arch == llvm::Triple::mipsel ||
- llvm_arch == llvm::Triple::mips64 ||
- llvm_arch == llvm::Triple::mips64el) {
+  } else if (arch.IsMIPS()) {
 /* Create alternate disassembler for MIPS16 and microMIPS */
 uint32_t arch_flags = arch.GetFlags();
 if (arch_flags & ArchSpec::eMIPSAse_mips16)

Modified: lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp?rev=360865=360864=360865=diff
==
--- lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
(original)
+++ lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp Thu 
May 16 01:37:32 2019
@@ -488,10 +488,7 @@ bool DYLDRendezvous::ReadSOEntryFromMemo
   const ArchSpec  = m_process->GetTarget().GetArchitecture();
   if ((arch.GetTriple().getOS() == llvm::Triple::FreeBSD ||
arch.GetTriple().getOS() == llvm::Triple::NetBSD) &&
-  (arch.GetMachine() == llvm::Triple::mips ||
-   arch.GetMachine() == llvm::Triple::mipsel ||
-   arch.GetMachine() == llvm::Triple::mips64 ||
-   arch.GetMachine() == llvm::Triple::mips64el)) {
+  arch.IsMIPS()) {
 addr_t mips_l_offs;
 if (!(addr = ReadPointer(addr, _l_offs)))
   return false;

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=360865=360864=360865=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Thu May 16 
01:37:32 2019
@@ -2185,11 +2185,7 @@ unsigned ObjectFileELF::ParseSymbols(Sym
* class
* accordingly.
   */
-  const llvm::Triple::ArchType llvm_arch = arch.GetMachine();
-  if (llvm_arch == llvm::Triple::mips ||
-  llvm_arch == llvm::Triple::mipsel ||
-  llvm_arch == llvm::Triple::mips64 ||
- 

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I wouldn't want to over-proliferate these, but in general I think this is a 
good idea. What I would find particularly useful is if we checked in the 
configurations used by all the bots in here, as that would make it easier to 
figure out what's going on if one of them breaks and the reason is not 
obvious...




Comment at: lldb/cmake/caches/Apple-lldb-osx.cmake:4
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/install/usr" CACHE 
STRING "")
+

Are you sure including `{CMAKE_CURRENT_BINARY_DIR}` here is a good idea? I 
think llvm tries to avoid that generally, but my experience from other projects 
tells me that the value of install prefix tends to end up embedded in the final 
binaries in various ways.

I believe the expected usage of this variable is to make it point to the final 
resting place of the executables, which in your case would be `/System` or 
whatever. Then, if you want to copy the to-be-installed files into a staging 
area first, you're expected to do that with "DESTDIR=whatever ninja install".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956



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

[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

@lanza Hello! AFAIU, these values have nothing to do with the Microsoft x64 
calling convention. They are used by `ABISysV_x86_64`, which specifies ABI for 
working with code injectable into a process being debugged. It requires six 
arguments to be passed through registers (see `GetArgumentValues`).

If I understand right, `SymbolFile` retrieves the information about arguments 
location on the stack, so there's no need to support different calling 
conventions in LLDB for this purpose. Please, correct me if I'm wrong.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53753



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

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

I withdraw from my approach and thanks to @teemperor I have found something 
else to work on. Thank you for this fruitful discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61737



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

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2118
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject ) {
+  if (!FileSystem::Instance().Exists(file)) {

This should also have an `assert(!m_skip_lldbinit_files);`



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

This should be reflowed. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61994



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

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, xiaobai, jingham.
Herald added a project: LLDB.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2118
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject ) {
+  if (!FileSystem::Instance().Exists(file)) {

This should also have an `assert(!m_skip_lldbinit_files);`



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

This should be reflowed. 


I was looking at the current implementation of `SourceInitFile` and there were 
a few things that made this function hard to read. The code to find the 
~/.lldbinit file is duplicated across the cwd and non-cwd branch. The 
`./.lldbinit` is once computed by resolving `.lldbinit` and once by resolving 
`./.lldbinit`. Furthermore it wasn't clear to me what happened when you're 
sourcing the `.lldbinit` file in the current working directory. Apparently we 
do nothing when we property to control that is set to warn (makes sense) and we 
don't care when the property is set to true (debatable). Finally, there were at 
least two branches where the status of the CommandReturnObject were not set.

Anyway, this is an attempt to simplify this code a bit. It's still more complex 
than I had hoped.

Please let me know if you think it's more readable or not.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D61994

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2091,100 +2091,130 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
+static FileSpec GetHomeInitFile(llvm::StringRef suffix = {}) {
+  llvm::SmallString<64> buffer;
+  llvm::sys::path::home_directory(buffer);
+  llvm::sys::path::append(buffer, ".lldbinit");
+
+  FileSpec init_file(buffer.c_str());
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static FileSpec GetCwdInitFile() {
+  FileSpec init_file(".lldbinit");
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static LoadCWDlldbinitFile ShouldLoadCwdInitFile() {
+  lldb::TargetPropertiesSP properties = Target::GetGlobalProperties();
+  if (!properties)
+return eLoadCWDlldbinitFalse;
+  return properties->GetLoadCWDlldbinitFile();
+}
+
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject ) {
-  FileSpec init_file;
-  if (in_cwd) {
-lldb::TargetPropertiesSP properties = Target::GetGlobalProperties();
-if (properties) {
-  // In the current working directory we don't load any program specific
-  // .lldbinit files, we only look for a ".lldbinit" file.
-  if (m_skip_lldbinit_files)
-return;
+  if (!FileSystem::Instance().Exists(file)) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return;
+  }
 
-  LoadCWDlldbinitFile should_load = properties->GetLoadCWDlldbinitFile();
-  if (should_load == eLoadCWDlldbinitWarn) {
-FileSpec dot_lldb(".lldbinit");
-FileSystem::Instance().Resolve(dot_lldb);
-llvm::SmallString<64> home_dir_path;
-llvm::sys::path::home_directory(home_dir_path);
-FileSpec homedir_dot_lldb(home_dir_path.c_str());
-homedir_dot_lldb.AppendPathComponent(".lldbinit");
-FileSystem::Instance().Resolve(homedir_dot_lldb);
-if (FileSystem::Instance().Exists(dot_lldb) &&
-dot_lldb.GetDirectory() != homedir_dot_lldb.GetDirectory()) {
-  result.AppendErrorWithFormat(
-  "There is a .lldbinit file in the current directory which is not "
-  "being read.\n"
-  "To silence this warning without sourcing in the local "
-  ".lldbinit,\n"
-  "add the following to the lldbinit file in your home directory:\n"
-  "settings set target.load-cwd-lldbinit false\n"
-  "To allow lldb to source .lldbinit files in the current working "
-  "directory,\n"
-  "set the value of this variable to true.  Only do so if you "
-  "understand and\n"
-  "accept the security risk.");
-  result.SetStatus(eReturnStatusFailed);
-  return;
-}
-  } else if (should_load == eLoadCWDlldbinitTrue) {
-init_file.SetFile("./.lldbinit", FileSpec::Style::native);
-FileSystem::Instance().Resolve(init_file);
-  }
-}
-  } else