[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:497
 if (LLDB_ENABLE_CURSES)
-find_package(Curses REQUIRED)
 find_library(CURSES_PANEL_LIBRARY NAMES panel DOC "The curses panel 
library")
 if (NOT CURSES_PANEL_LIBRARY)

Isn't this redundant now?


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

https://reviews.llvm.org/D71306



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

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);

labath wrote:
> I'm not actually opposed to this (and it's nice to know that this could work 
> if we need it), but why do this as a unit test? It seems like this could be 
> tested equally well with a `script` command through the lldb driver. I was 
> imagining the unit tests would be most useful for the lower level 
> functionality -- roughly corresponding to the `Lua` class. Right now that 
> class is pretty simple and doesn't really need a unit test, but I expect it 
> will become more complex over time...
Yes, I just wanted to show that it's possible :-) I've also included a test for 
the Lua class. 


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

https://reviews.llvm.org/D71234



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

[Lldb-commits] [PATCH] D71235: [lldb/Lua] Generate Lua Bindings and Make Them Available to the Script Interpreter

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Rebase


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

https://reviews.llvm.org/D71235

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt
  lldb/scripts/lldb_lua.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/test/Shell/ScriptInterpreter/Lua/bindings.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -11,6 +11,8 @@
 
 using namespace lldb_private;
 
+extern "C" int luaopen_lldb(lua_State *L) { return 0; }
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/bindings.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/bindings.test
@@ -0,0 +1,6 @@
+# REQUIRES: lua
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+script
+debugger = lldb.SBDebugger.Create()
+print(string.format("debugger is valid: %s", debugger:IsValid()))
+# CHECK: debugger is valid: true
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -16,11 +16,16 @@
 
 namespace lldb_private {
 
+extern "C" {
+int luaopen_lldb(lua_State *L);
+}
+
 class Lua {
 public:
   Lua() : m_lua_state(luaL_newstate()) {
 assert(m_lua_state);
 luaL_openlibs(m_lua_state);
+luaopen_lldb(m_lua_state);
   }
 
   ~Lua() {
Index: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
@@ -1,3 +1,5 @@
+find_package(Lua REQUIRED)
+
 add_lldb_library(lldbPluginScriptInterpreterLua PLUGIN
   Lua.cpp
   ScriptInterpreterLua.cpp
Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -9,6 +9,11 @@
   set(lldb_python_wrapper ${lldb_scripts_dir}/LLDBWrapPython.cpp)
 endif()
 
+if(LLDB_ENABLE_LUA)
+  get_target_property(lldb_scripts_dir swig_wrapper_lua BINARY_DIR)
+  set(lldb_lua_wrapper ${lldb_scripts_dir}/LLDBWrapLua.cpp)
+endif()
+
 if(LLDB_BUILD_FRAMEWORK)
   set(option_install_prefix INSTALL_PREFIX ${LLDB_FRAMEWORK_INSTALL_DIR})
   set(option_framework FRAMEWORK)
@@ -85,6 +90,7 @@
   SBUnixSignals.cpp
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}
+  ${lldb_lua_wrapper}
 
   LINK_LIBS
 lldbBase
@@ -130,6 +136,19 @@
   endif ()
 endif()
 
+if(lldb_lua_wrapper)
+  add_dependencies(liblldb swig_wrapper_lua)
+  target_include_directories(liblldb PRIVATE ${LUA_INCLUDE_DIR})
+
+  if (MSVC)
+set_property(SOURCE ${lldb_lua_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
+  else()
+set_property(SOURCE ${lldb_lua_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " -w")
+  endif()
+
+  set_source_files_properties(${lldb_lua_wrapper} PROPERTIES GENERATED ON)
+endif()
+
 set_target_properties(liblldb
   PROPERTIES
   VERSION ${LLDB_VERSION}
Index: lldb/scripts/lldb_lua.swig
===
--- /dev/null
+++ lldb/scripts/lldb_lua.swig
@@ -0,0 +1,18 @@
+/*
+   lldb.swig
+
+   This is the input file for SWIG, to create the appropriate C++ wrappers and
+   functions for various scripting languages, to enable them to call the
+   liblldb Script Bridge functions.
+*/
+
+%module lldb
+
+%include "./headers.swig"
+
+%{
+using namespace lldb_private;
+using namespace lldb;
+%}
+
+%include "./interfaces.swig"
Index: lldb/scripts/CMakeLists.txt
===
--- lldb/scripts/CMakeLists.txt
+++ lldb/scripts/CMakeLists.txt
@@ -27,31 +27,57 @@
   set(DARWIN_EXTRAS "")
 endif()
 
-add_custom_command(
-  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
-  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
-  DEPENDS ${SWIG_SOURCES}
-  DEPENDS ${SWIG_INTERFACES}
-  DEPENDS ${SWIG_HEADERS}
-  COMMAND ${SWIG_EXECUTABLE} 
-  -c++
-  -shadow
-  -python
-  -features autodoc
-  -threads
-  -I${LLDB_SOURCE_DIR}/include
-  -I${CMAKE_CURRENT_SOURCE_DIR}
-  -D__STDC_LIMIT_MACROS
-  -D__STDC_CONSTANT_MACROS
-  ${DARWIN_EXTRAS}
-  -outdir ${CMAKE_CURRENT_BINARY_DIR}
-  -o ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
-  ${LLDB_SOURCE_DIR}/scripts/lldb.swig
-  VERBATIM
-  COMMENT "Builds LLDB Python wrapper")
-

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 234798.
JDevlieghere marked 12 inline comments as done.
JDevlieghere added a comment.

- Rebase
- Feedback Pavel


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

https://reviews.llvm.org/D71234

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/ScriptInterpreter/Lua/lua.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/unittests/ScriptInterpreter/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -0,0 +1,62 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+
+// Pretend Linux is the host platform.
+platform_linux::PlatformLinux::Initialize();
+ArchSpec arch("powerpc64-pc-linux");
+Platform::SetHostPlatform(
+platform_linux::PlatformLinux::CreateInstance(true, ));
+  }
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+"Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", ));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", ));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+  "error: lua failed attempting to evaluate 'nil = foo'"));
+}
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -0,0 +1,26 @@
+//===-- LuaTests.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/ScriptInterpreter/Lua/Lua.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(LuaTest, RunValid) {
+  Lua lua;
+  llvm::Error error = lua.Run("foo = 1");
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunInvalid) {
+  Lua lua;
+  llvm::Error error = lua.Run("nil = foo");
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt
@@ -0,0 +1,15 @@
+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+  ScriptInterpreterTests.cpp
+
+  LINK_LIBS
+lldbHost
+lldbPluginScriptInterpreterLua
+lldbPluginPlatformLinux
+LLVMTestingSupport
+  LINK_COMPONENTS
+Support
+  )
+
+target_include_directories(ScriptInterpreterLuaTests PRIVATE ${LUA_INCLUDE_DIR})

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good, thanks for tracking this down!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Johannes Altmanninger via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2285
 
-if (addr.IsValid()) {
+if (addr.IsSectionOffset()) {
   sc_list.Append(sc);

clayborg wrote:
> Checking if the address is section offset might not be enough. All darwin 
> systems have a __PAGEZERO segment that covers the first 4GB for 64 bit files 
> and the first 4096 bytes for 32 bit fiules. This section has no read, no 
> write no execute permissions. So maybe grab the section and verify it has 
> read + execute permissions? That way data symbol matches won't trigger. 
> 
> ```
> if (auto section_sp = addr.GetSection()) {
>   if (section_sp->GetPermissions() & ePermissionsExecutable) {
> sc_list.Append(sc);
> return true;
>   }
> }
> ```
Yep, that works nicely!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Johannes Altmanninger via Phabricator via lldb-commits]

johannes updated this revision to Diff 234795.
johannes edited the summary of this revision.
johannes added a comment.

check if the function's section is executable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
  lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll


Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
@@ -0,0 +1,28 @@
+; REQUIRES: lld
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: ld.lld %t.o %t.o -o %t
+; "foo" is defined in both compilation units, but there should be only one 
meaningful debuginfo entry
+; RUN: lldb-test symbols --find=function --name=foo --function-flags=full %t | 
FileCheck %s
+; CHECK: Function: {{.*}} "foo"
+; CHECK-NOT: Function: {{.*}} "foo"
+
+$foo = comdat any
+define void @foo() comdat !dbg !6 {
+entry:
+  ret void
+}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, isOptimized: 
false, runtimeVersion: 0, emissionKind: FullDebug, enums: !{}, imports: !{}, 
splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "inline-function-address.h", directory: "")
+!2 = !DIFile(filename: "inline-function-address.c", directory: "")
+!3 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!4 = !DISubroutineType(types: !{})
+!5 = !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: 
DIFlagPrototyped, spFlags: 0)
+!6 = distinct !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: 
DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, declaration: !5)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{}
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
@@ -0,0 +1,6 @@
+# REQUIRES: lld
+; RUN: llc %S/inline-function-address.ll -filetype=obj -o %t.o
+; RUN: ld.lld %t.o %t.o -o %t -shared
+; RUN: lldb-test symbols --find=function --name=foo --function-flags=full %t | 
FileCheck %s
+; CHECK: Function: {{.*}} "foo"
+; CHECK-NOT: Function: {{.*}} "foo"
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2282,9 +2282,12 @@
   addr = sc.function->GetAddressRange().GetBaseAddress();
 }
 
-if (addr.IsValid()) {
-  sc_list.Append(sc);
-  return true;
+
+if (auto section_sp = addr.GetSection()) {
+  if (section_sp->GetPermissions() & ePermissionsExecutable) {
+sc_list.Append(sc);
+return true;
+  }
 }
   }
 


Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
@@ -0,0 +1,28 @@
+; REQUIRES: lld
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: ld.lld %t.o %t.o -o %t
+; "foo" is defined in both compilation units, but there should be only one meaningful debuginfo entry
+; RUN: lldb-test symbols --find=function --name=foo --function-flags=full %t | FileCheck %s
+; CHECK: Function: {{.*}} "foo"
+; CHECK-NOT: Function: {{.*}} "foo"
+
+$foo = comdat any
+define void @foo() comdat !dbg !6 {
+entry:
+  ret void
+}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !{}, imports: !{}, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "inline-function-address.h", directory: "")
+!2 = !DIFile(filename: "inline-function-address.c", directory: "")
+!3 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!4 = !DISubroutineType(types: !{})
+!5 = !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: DIFlagPrototyped, spFlags: 0)
+!6 = distinct !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, declaration: !5)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{}
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
===
--- /dev/null
+++ 

[Lldb-commits] [PATCH] D71235: [lldb/Lua] Generate Lua Bindings and Make Them Available to the Script Interpreter

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Very cool BTW!


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

https://reviews.llvm.org/D71235



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

[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 234789.

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

https://reviews.llvm.org/D71306

Files:
  lldb/cmake/modules/FindCursesAndPanel.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/test/CMakeLists.txt

Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -144,6 +144,7 @@
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
   LLDB_ENABLE_PYTHON
+  LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_IS_64_BITS)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -24,40 +24,43 @@
   set(LLDB_LINKER_SUPPORTS_GROUPS ON)
 endif()
 
+function(add_optional_dependency variable description package found)
+  set(${variable} "Auto" CACHE STRING "${description} On, Off or Auto (default)")
+  string(TOUPPER "${${variable}}" ${variable})
+
+  if("${${variable}}" STREQUAL "AUTO")
+set(maybe_required)
+  elseif(${${variable}})
+set(maybe_required REQUIRED)
+  else()
+set(${variable} OFF PARENT_SCOPE)
+return()
+  endif()
+
+  find_package(${package} ${maybe_required})
+  set(${variable} ${${found}} PARENT_SCOPE)
+endfunction()
+
+add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support." LibEdit libedit_FOUND)
+add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support." CursesAndPanel CURSES_PANEL_FOUND)
+add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support." LibLZMA LIBLZMA_FOUND)
+
 set(default_enable_python ON)
-set(default_enable_libedit ON)
-set(default_enable_curses ON)
 
 # Temporary support the old LLDB_DISABLE_* variables
-if (DEFINED LLDB_DISABLE_CURSES)
-  if (LLDB_DISABLE_CURSES)
-set(default_enable_curses OFF)
-  endif()
-endif()
 if (DEFINED LLDB_DISABLE_PYTHON)
   if (LLDB_DISABLE_PYTHON)
 set(default_enable_python OFF)
   endif()
 endif()
 
-if(DEFINED LLVM_ENABLE_LIBEDIT AND NOT LLVM_ENABLE_LIBEDIT)
-  set(default_disable_libedit ON)
-endif()
-
-if(CMAKE_SYSTEM_NAME MATCHES "Windows")
-  set(default_enable_libedit OFF)
-  set(default_enable_curses OFF)
-elseif(CMAKE_SYSTEM_NAME MATCHES "Android")
+if(CMAKE_SYSTEM_NAME MATCHES "Android")
   set(default_enable_python OFF)
-  set(default_enable_libedit OFF)
-  set(default_enable_curses OFF)
 elseif(IOS)
   set(default_enable_python OFF)
 endif()
 
 option(LLDB_ENABLE_PYTHON "Enable Python scripting integration." ${default_enable_python})
-option(LLDB_ENABLE_LIBEDIT "Enable the use of editline." ${default_enable_libedit})
-option(LLDB_ENABLE_CURSES "Enable Curses integration." ${default_enable_curses})
 option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to locate Python." OFF)
 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
@@ -120,12 +123,9 @@
   add_definitions( -DHAVE_ROUND )
 endif()
 
-
+# Check if we libedit capable of handling wide characters (built with
+# '--enable-widec').
 if (LLDB_ENABLE_LIBEDIT)
-  find_package(LibEdit REQUIRED)
-
-  # Check if we libedit capable of handling wide characters (built with
-  # '--enable-widec').
   set(CMAKE_REQUIRED_LIBRARIES ${libedit_LIBRARIES})
   set(CMAKE_REQUIRED_INCLUDES ${libedit_INCLUDE_DIRS})
   check_symbol_exists(el_winsertstr histedit.h LLDB_EDITLINE_USE_WCHAR)
@@ -141,7 +141,6 @@
   set(CMAKE_EXTRA_INCLUDE_FILES)
 endif()
 
-
 # On Windows, we can't use the normal FindPythonLibs module that comes with CMake,
 # for a number of reasons.
 # 1) Prior to MSVC 2015, it is only possible to embed Python if python itself was
@@ -402,12 +401,9 @@
 set(LLDB_VERSION "${LLDB_VERSION_MAJOR}.${LLDB_VERSION_MINOR}.${LLDB_VERSION_PATCH}${LLDB_VERSION_SUFFIX}")
 message(STATUS "LLDB version: ${LLDB_VERSION}")
 
-find_package(LibLZMA)
-cmake_dependent_option(LLDB_ENABLE_LZMA "Support LZMA compression" ON "LIBLZMA_FOUND" OFF)
 if (LLDB_ENABLE_LZMA)
   include_directories(${LIBLZMA_INCLUDE_DIRS})
 endif()
-llvm_canonicalize_cmake_booleans(LLDB_ENABLE_LZMA)
 
 include_directories(BEFORE
   ${CMAKE_CURRENT_BINARY_DIR}/include
@@ -498,10 +494,10 @@
 endif()
 
 if (LLDB_ENABLE_CURSES)
-find_package(Curses REQUIRED)
 find_library(CURSES_PANEL_LIBRARY NAMES panel DOC "The curses panel library")
 if (NOT CURSES_PANEL_LIBRARY)
-message(FATAL_ERROR "A required curses' panel library not found.")
+  set(LLDB_ENABLE_CURSES OFF)
+  message(WARNING "Curses panel library not found.")
 endif ()
 endif ()
 
Index: lldb/cmake/modules/FindCursesAndPanel.cmake
===
--- /dev/null
+++ lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -0,0 +1,16 @@
+#.rst:
+# 

[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 234788.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

- Extract finding curses and panel into FindCursesAndPanel
- Include LZMA


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

https://reviews.llvm.org/D71306

Files:
  lldb/cmake/modules/FindCursesAndPanel.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -144,6 +144,7 @@
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
   LLDB_ENABLE_PYTHON
+  LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_IS_64_BITS)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -25,7 +25,7 @@
 endif()
 
 function(add_optional_dependency variable description package found)
-  set(${variable} "AUTO" CACHE STRING "${description} On, Off or Auto 
(default)")
+  set(${variable} "Auto" CACHE STRING "${description} On, Off or Auto 
(default)")
   string(TOUPPER "${${variable}}" ${variable})
 
   if("${${variable}}" STREQUAL "AUTO")
@@ -38,17 +38,12 @@
   endif()
 
   find_package(${package} ${maybe_required})
-  # We could set ${variable} directory to ${${found}} but then the value is
-  # TRUE/FALSE rather than ON/OFF.
-  if (${${found}})
-set(${variable} ON PARENT_SCOPE)
-  else()
-set(${variable} OFF PARENT_SCOPE)
-  endif()
+  set(${variable} ${${found}} PARENT_SCOPE)
 endfunction()
 
 add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support." LibEdit 
libedit_FOUND)
-add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support." Curses 
CURSES_FOUND)
+add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support." 
CursesAndPanel CURSES_PANEL_FOUND)
+add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support." 
LibLZMA LIBLZMA_FOUND)
 
 set(default_enable_python ON)
 
@@ -406,12 +401,9 @@
 set(LLDB_VERSION 
"${LLDB_VERSION_MAJOR}.${LLDB_VERSION_MINOR}.${LLDB_VERSION_PATCH}${LLDB_VERSION_SUFFIX}")
 message(STATUS "LLDB version: ${LLDB_VERSION}")
 
-find_package(LibLZMA)
-cmake_dependent_option(LLDB_ENABLE_LZMA "Support LZMA compression" ON 
"LIBLZMA_FOUND" OFF)
 if (LLDB_ENABLE_LZMA)
   include_directories(${LIBLZMA_INCLUDE_DIRS})
 endif()
-llvm_canonicalize_cmake_booleans(LLDB_ENABLE_LZMA)
 
 include_directories(BEFORE
   ${CMAKE_CURRENT_BINARY_DIR}/include
Index: lldb/cmake/modules/FindCursesAndPanel.cmake
===
--- /dev/null
+++ lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -0,0 +1,16 @@
+#.rst:
+# FindCursesAndPanel
+# ---
+#
+# Find the curses and panel library as a whole.
+
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+  set(CURSES_PANEL_FOUND TRUE)
+else()
+  find_package(Curses QUIET)
+  find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library" 
QUIET)
+  if(CURSES_FOUND AND PANEL_LIBRARIES)
+mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+  endif()
+endif()
+


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -144,6 +144,7 @@
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
   LLDB_ENABLE_PYTHON
+  LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_IS_64_BITS)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -25,7 +25,7 @@
 endif()
 
 function(add_optional_dependency variable description package found)
-  set(${variable} "AUTO" CACHE STRING "${description} On, Off or Auto (default)")
+  set(${variable} "Auto" CACHE STRING "${description} On, Off or Auto (default)")
   string(TOUPPER "${${variable}}" ${variable})
 
   if("${${variable}}" STREQUAL "AUTO")
@@ -38,17 +38,12 @@
   endif()
 
   find_package(${package} ${maybe_required})
-  # We could set ${variable} directory to ${${found}} but then the value is
-  # TRUE/FALSE rather than ON/OFF.
-  if (${${found}})
-set(${variable} ON PARENT_SCOPE)
-  else()
-set(${variable} OFF PARENT_SCOPE)
-  endif()
+  set(${variable} ${${found}} PARENT_SCOPE)
 endfunction()
 
 add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support." LibEdit libedit_FOUND)
-add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support." Curses CURSES_FOUND)
+add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support." CursesAndPanel CURSES_PANEL_FOUND)
+add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support." LibLZMA LIBLZMA_FOUND)
 
 

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D70840#1783351 , @labath wrote:

> I am strongly opposed to ArchSpec owing an Architecture object. The latter is 
> far more complicated -- it reads bytes from target memory and disassembles 
> them -- whereas ArchSpec just returns a bunch of constants. If anything, it 
> should be the other way around. That way the code which just fiddles with 
> triples does not need to depend on the whole world.


I know part of this re-org was to keep the ArchSpec class from accessing any 
Target stuff to keep each directory with source code from accessing stuff in 
another tree. So from that perspective I can see your objection. I don't think 
it adds anything at all to the code by splitting this up other than achieving 
this separation though. It makes sense to ask an architecture to do 
architecture specific things. No sure why they need to be split IMHO.

> Having an Architecture instance in the Module object (to ensure it's 
> independent of the target) seems /ok/, though I am not really convinced that 
> it is needed, as the &~1 logic seems like a good candidate for ArchSpec 
> (which already knows a lot about arm vs. thumb etc.

I would rather keep all architecture specific code in the Architecture plug-ins 
somehow. It keeps code modifications clearer as only on file changes that is 
architecture specific. If we inline stuff into ArchSpec we end up with a big 
switch statement for the core and every adds their code to the same function.

So not sure what the right solution is here. I still like folding the 
Architecture into ArchSpec myself, but I am open to any and all opinions.


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

https://reviews.llvm.org/D70840



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

BTW: is used to be that both DW_AT_low_pc and DW_AT_high_pc would be set to 
zero when a function was dead stripped. This was back when both the low and 
high pc used DW_FORM_addr (a file address). But then DWARF changed such that 
DW_AT_high_pc could be encoded as a data form: DW_FORM_data1, DW_FORM_data2, 
DW_FORM_data4, or DW_FORM_data8. This is used to mean it is an offset from the 
low PC. Seems the linkers now didn't have a relocation for the DW_AT_high_pc so 
they couldn't zero it out. This is sad because we can end up with many 
functions at address zero that didn't get linked, and if zero is a valid 
address, then our DWARF contains a bunch of useless info that only hides which 
function is the real function for address zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

It is sad that we can't tell if a DW_AT_low_pc with a value of zero is valid or 
not. Some shared libraries might be able to have a function at address zero, so 
we need to be careful here. My proposed fix above will check the section that 
contains the low PC to see if it has execute permissions, so that should cover 
all cases (executables and shared libraries). Not sure if there is more we can 
do.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2285
 
-if (addr.IsValid()) {
+if (addr.IsSectionOffset()) {
   sc_list.Append(sc);

Checking if the address is section offset might not be enough. All darwin 
systems have a __PAGEZERO segment that covers the first 4GB for 64 bit files 
and the first 4096 bytes for 32 bit fiules. This section has no read, no write 
no execute permissions. So maybe grab the section and verify it has read + 
execute permissions? That way data symbol matches won't trigger. 

```
if (auto section_sp = addr.GetSection()) {
  if (section_sp->GetPermissions() & ePermissionsExecutable) {
sc_list.Append(sc);
return true;
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [PATCH] D71372: [lldb] Add additional validation on return address in 'thread step-out'

[Greg Clayton via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71372



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

[Lldb-commits] [PATCH] D71372: [lldb] Add additional validation on return address in 'thread step-out'

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This looks fine to me.  We should let Greg have a final chance to weigh in and 
then I'll check it in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71372



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

[Lldb-commits] [PATCH] D71372: [lldb] Add additional validation on return address in 'thread step-out'

[Mark Mossberg via Phabricator via lldb-commits]

mossberg added a comment.

I believe all the comments should be addressed at this point; thanks very much 
for your reviews so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71372



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:45
+
+/// Reads a LEB128 variable-length unsigned integer, limited to 7 bits.
+llvm::Optional GetVaruint7(DataExtractor _header_data,

The LLVM coding style requests that doxygen comments should be on the 
declaration in the header file and not in the implementation.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:375
+/// These 64-bit addresses will be used to request code ranges for a specific
+/// module from the WebAssembly engine.
+

again.. in the header, or inside the function



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:452
+
+// Dump a single Wasm section header to the specified output stream.
+void ObjectFileWasm::DumpSectionHeader(llvm::raw_ostream ,

ditto



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:18
+
+/// \class ObjectFileWasm
+/// Generic Wasm object file reader.

This line is redundant and can be removed.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:25
+public:
+  // Static Functions
+  static void Initialize();

this comment is inconsistent with the others



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:115
+
+  /// Read a range of bytes from the Wasm module
+  DataExtractor ReadImageData(uint64_t offset, size_t size);

Please make sure to use full sentences that end in a `.` in all comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a7df3a3f940: [ASTImporter][LLDB] Modifying 
ImportDeclContext(...) to ensure that we complete… (authored by shafik).
Herald added projects: clang, LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71378

Files:
  clang/lib/AST/ASTImporter.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py

Index: lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
@@ -1,4 +1,4 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(__file__, globals(), [decorators.skipIf(bugnumber="rdar://53756116")])
+lldbinline.MakeInlineTest(__file__, globals(), [])
Index: lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
@@ -0,0 +1,39 @@
+// This is a reproducer for a crash in codegen. It happens when we have a
+// RecordDecl used in an expression and one of the FieldDecl are not complete.
+// This case happens when:
+// - A RecordDecl (E) has a FieldDecl which is a reference member variable
+// - The underlying type of the FieldDec is a TypedefDecl
+// - The typedef refers to a ClassTemplateSpecialization (DWrapper)
+// - The typedef is not present in the DeclContext of B
+// - The typedef shows up as a return value of a member function of E (f())
+template  struct DWrapper {};
+
+struct D {};
+
+namespace NS {
+typedef DWrapper DW;
+}
+
+struct B {
+  NS::DW spd;
+  int a = 0;
+};
+
+struct E {
+  E(B ) : b_ref(b) {}
+  NS::DW f() { return {}; };
+  void g() {
+return; //%self.expect("p b_ref", substrs=['(B) $0 =', '(spd = NS::DW', 'a = 0)'])
+  }
+
+  B _ref;
+};
+
+int main() {
+  B b;
+  E e(b);
+
+  e.g();
+
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), [])
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1734,7 +1734,34 @@
   Error ChildErrors = Error::success();
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-if (!ImportedOrErr) {
+
+// If we are in the process of ImportDefinition(...) for a RecordDecl we
+// want to make sure that we are also completing each FieldDecl. There
+// are currently cases where this does not happen and this is correctness
+// fix since operations such as code generation will expect this to be so.
+if (ImportedOrErr) {
+  FieldDecl *FieldFrom = dyn_cast_or_null(From);
+  Decl *ImportedDecl = (Decl*)*ImportedOrErr;
+  FieldDecl *FieldTo = dyn_cast_or_null(ImportedDecl);
+  if (FieldFrom && FieldTo) {
+const RecordType *RecordFrom = FieldFrom->getType()->getAs();
+const RecordType *RecordTo = FieldTo->getType()->getAs();
+if (RecordFrom && RecordTo) {
+  RecordDecl *FromRecordDecl = RecordFrom->getDecl();
+  RecordDecl *ToRecordDecl = RecordTo->getDecl();
+
+  if (FromRecordDecl->isCompleteDefinition() &&
+  !ToRecordDecl->isCompleteDefinition()) {
+Error Err = ImportDefinition(FromRecordDecl, ToRecordDecl);
+
+if (Err && AccumulateChildErrors)
+  ChildErrors =  joinErrors(std::move(ChildErrors), std::move(Err));
+else
+  consumeError(std::move(Err));
+  }
+}
+  }
+} else {
   if (AccumulateChildErrors)
 ChildErrors 

[Lldb-commits] [lldb] 6a7df3a - [ASTImporter][LLDB] Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton

[via lldb-commits]

Author: shafik
Date: 2019-12-19T11:16:54-08:00
New Revision: 6a7df3a3f940473088b1db1ccadafe52bb274b62

URL: 
https://github.com/llvm/llvm-project/commit/6a7df3a3f940473088b1db1ccadafe52bb274b62
DIFF: 
https://github.com/llvm/llvm-project/commit/6a7df3a3f940473088b1db1ccadafe52bb274b62.diff

LOG: [ASTImporter][LLDB] Modifying ImportDeclContext(...) to ensure that we 
complete each FieldDecl of a RecordDecl when we are importing the definiton

This fix was motivated by a crashes in expression parsing during code 
generation in which we had a RecordDecl that had incomplete FieldDecl. During 
code generation when computing the layout for the RecordDecl we crash because 
we have several incomplete FieldDecl.

This fixes the issue by assuring that during ImportDefinition(...) for a 
RecordDecl we also import the definitions for each FieldDecl.

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

Added: 

lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py

lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp

Modified: 
clang/lib/AST/ASTImporter.cpp

lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index b75a689ec275..f495c48803d1 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -1734,7 +1734,34 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, 
bool ForceImport) {
   Error ChildErrors = Error::success();
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-if (!ImportedOrErr) {
+
+// If we are in the process of ImportDefinition(...) for a RecordDecl we
+// want to make sure that we are also completing each FieldDecl. There
+// are currently cases where this does not happen and this is correctness
+// fix since operations such as code generation will expect this to be so.
+if (ImportedOrErr) {
+  FieldDecl *FieldFrom = dyn_cast_or_null(From);
+  Decl *ImportedDecl = (Decl*)*ImportedOrErr;
+  FieldDecl *FieldTo = dyn_cast_or_null(ImportedDecl);
+  if (FieldFrom && FieldTo) {
+const RecordType *RecordFrom = 
FieldFrom->getType()->getAs();
+const RecordType *RecordTo = FieldTo->getType()->getAs();
+if (RecordFrom && RecordTo) {
+  RecordDecl *FromRecordDecl = RecordFrom->getDecl();
+  RecordDecl *ToRecordDecl = RecordTo->getDecl();
+
+  if (FromRecordDecl->isCompleteDefinition() &&
+  !ToRecordDecl->isCompleteDefinition()) {
+Error Err = ImportDefinition(FromRecordDecl, ToRecordDecl);
+
+if (Err && AccumulateChildErrors)
+  ChildErrors =  joinErrors(std::move(ChildErrors), 
std::move(Err));
+else
+  consumeError(std::move(Err));
+  }
+}
+  }
+} else {
   if (AccumulateChildErrors)
 ChildErrors =
 joinErrors(std::move(ChildErrors), ImportedOrErr.takeError());

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
new file mode 100644
index ..f08c0dcbda98
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/TestCodegenCrashTypedefDeclNotInDeclContext.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), [])

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
new file mode 100644
index ..e4f6600eab2c
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
@@ -0,0 +1,39 @@
+// This is a reproducer for a crash in codegen. It happens when we have a
+// RecordDecl used in an expression and one of the FieldDecl are not complete.
+// This case happens when:
+// - A RecordDecl (E) has a FieldDecl which is a reference member variable
+// - The underlying type of the FieldDec is a TypedefDecl
+// - The typedef refers to a ClassTemplateSpecialization (DWrapper)
+// - The typedef is not present in the DeclContext of B
+// - The typedef shows up as a return value of a member function of E (f())
+template  struct DWrapper {};
+

[Lldb-commits] [PATCH] D71232: [lldb/Lua] Add Boilerplate for a Lua Script Interpreter

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67de896229c0: [lldb/Lua] Add Boilerplate for a Lua Script 
Interpreter (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71232

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Interpreter/OptionArgParser.cpp
  lldb/source/Plugins/ScriptInterpreter/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h

Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -0,0 +1,47 @@
+//===-- ScriptInterpreterLua.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_ScriptInterpreterLua_h_
+#define liblldb_ScriptInterpreterLua_h_
+
+#include "lldb/Interpreter/ScriptInterpreter.h"
+
+namespace lldb_private {
+
+class ScriptInterpreterLua : public ScriptInterpreter {
+public:
+  ScriptInterpreterLua(Debugger );
+
+  ~ScriptInterpreterLua() override;
+
+  bool ExecuteOneLine(
+  llvm::StringRef command, CommandReturnObject *result,
+  const ExecuteScriptOptions  = ExecuteScriptOptions()) override;
+
+  void ExecuteInterpreterLoop() override;
+
+  // Static Functions
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ScriptInterpreterSP CreateInstance(Debugger );
+
+  static lldb_private::ConstString GetPluginNameStatic();
+
+  static const char *GetPluginDescriptionStatic();
+
+  // PluginInterface protocol
+  lldb_private::ConstString GetPluginName() override;
+
+  uint32_t GetPluginVersion() override;
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_ScriptInterpreterLua_h_
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -0,0 +1,71 @@
+//===-- ScriptInterpreterLua.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/StreamFile.h"
+#include "lldb/Utility/Stream.h"
+#include "lldb/Utility/StringList.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+ScriptInterpreterLua::ScriptInterpreterLua(Debugger )
+: ScriptInterpreter(debugger, eScriptLanguageLua) {}
+
+ScriptInterpreterLua::~ScriptInterpreterLua() {}
+
+bool ScriptInterpreterLua::ExecuteOneLine(llvm::StringRef command,
+  CommandReturnObject *,
+  const ExecuteScriptOptions &) {
+  m_debugger.GetErrorStream().PutCString(
+  "error: the lua script interpreter is not yet implemented.\n");
+  return false;
+}
+
+void ScriptInterpreterLua::ExecuteInterpreterLoop() {
+  m_debugger.GetErrorStream().PutCString(
+  "error: the lua script interpreter is not yet implemented.\n");
+}
+
+void ScriptInterpreterLua::Initialize() {
+  static llvm::once_flag g_once_flag;
+
+  llvm::call_once(g_once_flag, []() {
+PluginManager::RegisterPlugin(GetPluginNameStatic(),
+  GetPluginDescriptionStatic(),
+  lldb::eScriptLanguageLua, CreateInstance);
+  });
+}
+
+void ScriptInterpreterLua::Terminate() {}
+
+lldb::ScriptInterpreterSP
+ScriptInterpreterLua::CreateInstance(Debugger ) {
+  return std::make_shared(debugger);
+}
+
+lldb_private::ConstString ScriptInterpreterLua::GetPluginNameStatic() {
+  static ConstString g_name("script-lua");
+  return g_name;
+}
+
+const char *ScriptInterpreterLua::GetPluginDescriptionStatic() {
+  return "Lua script interpreter";
+}
+
+lldb_private::ConstString ScriptInterpreterLua::GetPluginName() {
+  return GetPluginNameStatic();
+}
+
+uint32_t ScriptInterpreterLua::GetPluginVersion() { return 1; }
Index: 

[Lldb-commits] [lldb] 67de896 - [lldb/Lua] Add Boilerplate for a Lua Script Interpreter

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-19T10:13:51-08:00
New Revision: 67de896229c0f1918f50a48973b7ce0007a181a9

URL: 
https://github.com/llvm/llvm-project/commit/67de896229c0f1918f50a48973b7ce0007a181a9
DIFF: 
https://github.com/llvm/llvm-project/commit/67de896229c0f1918f50a48973b7ce0007a181a9.diff

LOG: [lldb/Lua] Add Boilerplate for a Lua Script Interpreter

This adds the boilerplate necessary to support the Lua script
interpreter. The interpreter is not functional yet and just reports that
it's not implemented.

Discussion on the mailing list:
http://lists.llvm.org/pipermail/lldb-dev/2019-December/015812.html

Differential revision: https://reviews.llvm.org/D71232

Added: 
lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h

Modified: 
lldb/cmake/modules/LLDBConfig.cmake
lldb/include/lldb/Host/Config.h.cmake
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Interpreter/OptionArgParser.cpp
lldb/source/Plugins/ScriptInterpreter/CMakeLists.txt

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index da006039b268..063c1fdaa376 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -25,6 +25,7 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT 
"${CMAKE_SYSTEM_NAME}" MATCHES "Darw
 endif()
 
 set(default_enable_python ON)
+set(default_enable_lua OFF) # Experimental
 set(default_enable_libedit ON)
 set(default_enable_curses ON)
 
@@ -49,13 +50,16 @@ if(CMAKE_SYSTEM_NAME MATCHES "Windows")
   set(default_enable_curses OFF)
 elseif(CMAKE_SYSTEM_NAME MATCHES "Android")
   set(default_enable_python OFF)
+  set(default_enable_lua OFF)
   set(default_enable_libedit OFF)
   set(default_enable_curses OFF)
 elseif(IOS)
   set(default_enable_python OFF)
+  set(default_enable_lua OFF)
 endif()
 
 option(LLDB_ENABLE_PYTHON "Enable Python scripting integration." 
${default_enable_python})
+option(LLDB_ENABLE_PYTHON "Enable Lua scripting integration." 
${default_enable_lua})
 option(LLDB_ENABLE_LIBEDIT "Enable the use of editline." 
${default_enable_libedit})
 option(LLDB_ENABLE_CURSES "Enable Curses integration." 
${default_enable_curses})
 option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to 
locate Python." OFF)

diff  --git a/lldb/include/lldb/Host/Config.h.cmake 
b/lldb/include/lldb/Host/Config.h.cmake
index 8012d790954e..e9065ed04caa 100644
--- a/lldb/include/lldb/Host/Config.h.cmake
+++ b/lldb/include/lldb/Host/Config.h.cmake
@@ -42,6 +42,8 @@
 
 #cmakedefine01 LLDB_ENABLE_LIBXML2
 
+#cmakedefine01 LLDB_ENABLE_LUA
+
 #cmakedefine01 LLDB_ENABLE_PYTHON
 
 #cmakedefine LLDB_PYTHON_HOME "${LLDB_PYTHON_HOME}"

diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 0a92365544f9..5640a16c8f44 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -213,6 +213,7 @@ enum DescriptionLevel {
 enum ScriptLanguage {
   eScriptLanguageNone,
   eScriptLanguagePython,
+  eScriptLanguageLua,
   eScriptLanguageDefault = eScriptLanguagePython,
   eScriptLanguageUnknown
 };

diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 35a02ec29515..06f1a6cd3b75 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -14,6 +14,10 @@
 #include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
 #endif
 
+#if LLDB_ENABLE_LUA
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#endif
+
 #include "lldb/Core/Debugger.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Initialization/SystemInitializerCommon.h"
@@ -187,6 +191,10 @@ llvm::Error SystemInitializerFull::Initialize() {
   ScriptInterpreterPython::Initialize();
 #endif
 
+#if LLDB_ENABLE_LUA
+  ScriptInterpreterLua::Initialize();
+#endif
+
   platform_freebsd::PlatformFreeBSD::Initialize();
   platform_linux::PlatformLinux::Initialize();
   platform_netbsd::PlatformNetBSD::Initialize();

diff  --git a/lldb/source/Interpreter/OptionArgParser.cpp 
b/lldb/source/Interpreter/OptionArgParser.cpp
index 14b81cd7b3d2..56d99a4220f0 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -127,6 +127,8 @@ lldb::ScriptLanguage OptionArgParser::ToScriptLanguage(
 
   if (s.equals_lower("python"))
 return eScriptLanguagePython;
+  if (s.equals_lower("lua"))
+return eScriptLanguageLua;
   if (s.equals_lower("default"))
 return eScriptLanguageDefault;
   if (s.equals_lower("none"))

diff  --git a/lldb/source/Plugins/ScriptInterpreter/CMakeLists.txt 
b/lldb/source/Plugins/ScriptInterpreter/CMakeLists.txt
index 

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/unittests/TestingSupport/SubsystemRAII.h:57
+///   @endcode
+template  class SubsystemRAII {
+  /// RAII for a single subsystem.

Should we write a test for this?


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

https://reviews.llvm.org/D71630



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

[Lldb-commits] [PATCH] D70458: [NFC] Refactor and improve comments in CommandObjectTarget

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth closed this revision.
amccarth added a comment.

This landed in 3b69f0c5550a229dd6d39e361182cdd7cecc36a4 
, but 
there was a typo in the patch description so the tools didn't automatically 
close this.


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

https://reviews.llvm.org/D70458



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Johannes Altmanninger via Phabricator via lldb-commits]

johannes updated this revision to Diff 234736.
johannes added a comment.

- Use `addr.IsSectionOffset()` as suggested.
- Add test that links two copies of a compilation unit and makes sure that lldb 
only resolves it once.

The fix seems to work when linking an executable, but it does not when creating 
a shared object file, as demonstrated by the XFAIL test. In the DWARF of the 
shared object the low_pc of one copy of "foo" is set to zero, which seems to be 
inside a section so it is considered valid.

  $ build/bin/llvm-dwarfdump 
build/tools/lldb/test/SymbolFile/DWARF/Output/inline-function-address-shared.test.tmp
  
build/tools/lldb/test/SymbolFile/DWARF/Output/inline-function-address-shared.test.tmp:
   file format ELF64-x86-64
  
  .debug_info contents:
  0x: Compile Unit: length = 0x003d version = 0x0004 abbr_offset = 
0x addr_size = 0x08 (next unit at 0x0041)
  
  0x000b: DW_TAG_compile_unit
DW_AT_producer("")
DW_AT_language(DW_LANG_C99)
DW_AT_name("inline-function-address.c")
DW_AT_stmt_list   (0x)
DW_AT_low_pc  (0x1270)
DW_AT_high_pc (0x1271)
  
  0x0026:   DW_TAG_subprogram
  DW_AT_name  ("foo")
  DW_AT_decl_file ("inline-function-address.h")
  DW_AT_decl_line (12)
  DW_AT_prototyped(true)
  DW_AT_declaration   (true)
  DW_AT_external  (true)
  
  0x002d:   DW_TAG_subprogram
  DW_AT_low_pc(0x1270)
  DW_AT_high_pc   (0x1271)
  DW_AT_frame_base(DW_OP_reg7 RSP)
  DW_AT_specification (0x0026 "foo")
  
  0x0040:   NULL
  0x0041: Compile Unit: length = 0x003d version = 0x0004 abbr_offset = 
0x0030 addr_size = 0x08 (next unit at 0x0082)
  
  0x004c: DW_TAG_compile_unit
DW_AT_producer("")
DW_AT_language(DW_LANG_C99)
DW_AT_name("inline-function-address.c")
DW_AT_stmt_list   (0x003b)
DW_AT_low_pc  (0x)
DW_AT_high_pc (0x0001)
  
  0x0067:   DW_TAG_subprogram
  DW_AT_name  ("foo")
  DW_AT_decl_file ("inline-function-address.h")
  DW_AT_decl_line (12)
  DW_AT_prototyped(true)
  DW_AT_declaration   (true)
  DW_AT_external  (true)
  
  0x006e:   DW_TAG_subprogram
  DW_AT_low_pc(0x)
  DW_AT_high_pc   (0x0001)
  DW_AT_frame_base(DW_OP_reg7 RSP)
  DW_AT_specification (0x0067 "foo")
  
  0x0081:   NULL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
  lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll


Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/inline-function-address.ll
@@ -0,0 +1,28 @@
+; REQUIRES: lld
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: ld.lld %t.o %t.o -o %t
+; "foo" is defined in both compilation units, but there should be only one 
meaningful debuginfo entry
+; RUN: lldb-test symbols --find=function --name=foo --function-flags=full %t | 
FileCheck %s
+; CHECK: Function: {{.*}} "foo"
+; CHECK-NOT: Function: {{.*}} "foo"
+
+$foo = comdat any
+define void @foo() comdat !dbg !6 {
+entry:
+  ret void
+}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, isOptimized: 
false, runtimeVersion: 0, emissionKind: FullDebug, enums: !{}, imports: !{}, 
splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "inline-function-address.h", directory: "")
+!2 = !DIFile(filename: "inline-function-address.c", directory: "")
+!3 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!4 = !DISubroutineType(types: !{})
+!5 = !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: 
DIFlagPrototyped, spFlags: 0)
+!6 = distinct !DISubprogram(name: "foo", file: !1, line: 12, type: !4, flags: 
DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, declaration: !5)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{}
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
Index: lldb/test/Shell/SymbolFile/DWARF/inline-function-address-shared.test
===

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

@clayborg - can you follow up on @labath's reply here?

And irrespectively if the ArchSpec vs Architecture design, can you (either of 
you) comment on the updated form of the patch?


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

https://reviews.llvm.org/D70840



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

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Pavel Labath via Phabricator via lldb-commits]

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

Yep, looks good.




Comment at: lldb/unittests/TestingSupport/SubsystemRAII.h:59
+  /// RAII for a single subsystem.
+  template  struct SubsystemRAIICase {
+

This doesn't have to be a template since it's already parameterized by the 
enclosing `T`. Or, you could actually move it outside the `SubsystemRAII` class 
(and possibly into a detail namespace) and save the compiler some work then 
Using the same class in different subsystem sequences.


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

https://reviews.llvm.org/D71630



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

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 234711.
teemperor added a comment.

- Added SFINAE spaghetti to handle llvm::Error return types.


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

https://reviews.llvm.org/D71630

Files:
  lldb/unittests/Core/MangledTest.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/Host/HostInfoTest.cpp
  lldb/unittests/Host/MainLoopTest.cpp
  lldb/unittests/Interpreter/TestCompletion.cpp
  lldb/unittests/Language/Highlighting/HighlighterTest.cpp
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
  lldb/unittests/ObjectFile/PECOFF/TestPECallFrameInfo.cpp
  lldb/unittests/Symbol/TestClangASTImporter.cpp
  lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
  lldb/unittests/Target/ModuleCacheTest.cpp
  lldb/unittests/TestingSupport/SubsystemRAII.h

Index: lldb/unittests/TestingSupport/SubsystemRAII.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/SubsystemRAII.h
@@ -0,0 +1,87 @@
+//===- SubsystemRAII.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UNITTESTS_TESTINGSUPPORT_SUBSYSTEMRAII_H
+#define LLDB_UNITTESTS_TESTINGSUPPORT_SUBSYSTEMRAII_H
+
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace lldb_private {
+
+template  class SubsystemRAII {};
+
+/// RAII for initializing and deinitializing LLDB subsystems.
+///
+/// This RAII takes care of calling the Initialize and Terminate functions for
+/// the subsystems specified by its template arguments. The ::Initialize
+/// functions are called on construction for each subsystem template parameter
+/// in the order in which they are passed as template parameters.
+/// The ::Terminate functions are called in the reverse order at destruction
+/// time.
+///
+/// If the ::Initialize function returns an llvm::Error this function handles
+/// the Error instance (by checking that there is no error).
+///
+/// Constructing this RAII in a scope like this:
+///
+///   @code{.cpp}
+///   {
+/// SubsystemRAII Subsystems;
+/// DoingTestWork();
+///   }
+///   @endcode
+///
+/// is equivalent to the following code:
+///
+///   @code{.cpp}
+///   {
+/// FileSystem::Initialize();
+/// HostInfo::Initialize();
+/// ASSERT_THAT_ERROR(Socket::Initialize(), llvm::Succeeded());
+///
+/// DoingTestWork();
+///
+/// Socket::Terminate();
+/// FileSystem::Terminate();
+/// HostInfo::Terminate();
+///   }
+///   @endcode
+template  class SubsystemRAII {
+  /// RAII for a single subsystem.
+  template  struct SubsystemRAIICase {
+
+/// Calls ::Initialize if it has a void return type.
+template 
+typename std::enable_if<
+std::is_same::value>::type
+CallInitialize() {
+  SubT::Initialize();
+}
+
+/// Calls ::Initialize if it has a llvm::Error return type and checks
+/// the Error instance for success.
+template 
+typename std::enable_if<
+std::is_same::value>::type
+CallInitialize() {
+  ASSERT_THAT_ERROR(SubT::Initialize(), llvm::Succeeded());
+}
+
+SubsystemRAIICase() { CallInitialize(); }
+~SubsystemRAIICase() { SubT::Terminate(); }
+  };
+
+  SubsystemRAIICase CurrentSubsystem;
+  SubsystemRAII RemainingSubsystems;
+};
+} // namespace lldb_private
+
+#endif // LLDB_UNITTESTS_TESTINGSUPPORT_SUBSYSTEMRAII_H
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ lldb/unittests/Target/ModuleCacheTest.cpp
@@ -6,6 +6,7 @@
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
+#include "TestingSupport/SubsystemRAII.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -20,23 +21,21 @@
 namespace {
 
 class ModuleCacheTest : public testing::Test {
-public:
-  static void SetUpTestCase();
+  SubsystemRAII
+  subsystems;
 
-  static void TearDownTestCase();
+public:
+  void SetUp() override;
 
 protected:
-  static FileSpec s_cache_dir;
-  static std::string s_test_executable;
+  FileSpec s_cache_dir;
+  std::string s_test_executable;
 
   void TryGetAndPut(const FileSpec _dir, const char *hostname,
 bool 

[Lldb-commits] [PATCH] D71699: [lldb] Increase the rate at which ConstString's memory allocator scales the memory chunks it allocates

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: llunak.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.
teemperor added a parent revision: D71654: [llvm] Add a way to speed up the 
speed in which BumpPtrAllocator increases slab sizes.

We currently do far more malloc calls than necessary in the ConstString 
BumpPtrAllocator. This is due to the 256 BumpPtrAllocators
our ConstString implementation uses internally which end up all just receiving 
a small share of the total allocated memory
and therefore keep allocating memory in small chunks for far too long. This 
patch fixes this by increasing the rate at which we increase the
memory chunk size so that our collection of BumpPtrAllocators behaves in total 
similar to a single BumpPtrAllocator.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71699

Files:
  lldb/source/Utility/ConstString.cpp


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -29,9 +29,36 @@
 
 class Pool {
 public:
+  /// The default BumpPtrAllocatorImpl slab size.
+  static const size_t AllocatorSlabSize = 4096 * 4;
+  /// Every Pool has its own allocator which receives an equal share of
+  /// the ConstString allocations. This means that when allocating many
+  /// ConstStrings, every allocator sees only its small share of allocations 
and
+  /// assumes LLDB only allocated a small amount of memory so far. In reality
+  /// LLDB allocated a total memory that is N times as large as what the
+  /// allocator sees (where N is the number of string pools). This causes that
+  /// the BumpPtrAllocator continues a long time to allocate memory in small
+  /// chunks which only makes sense when allocating a small amount of memory
+  /// (which is true from the perspective of a single allocator). On some
+  /// systems doing all these small memory allocations causes LLDB to spend
+  /// a lot of time in malloc, so we need to force all these allocators to
+  /// behave like one allocator in terms of scaling their memory allocations
+  /// with increased demand. To do this we set the growth delay for each single
+  /// allocator to a rate so that our pool of allocators scales their memory
+  /// allocations similar to a single BumpPtrAllocatorImpl.
+  ///
+  /// Currently we have 256 string pools and the normal growth delay of the
+  /// BumpPtrAllocatorImpl is 128 (i.e., the memory allocation size increases
+  /// every 128 full chunks), so by changing the delay to 1 we get a
+  /// total growth delay in our allocator collection of 256/1 = 256. This is
+  /// still only half as fast as a normal allocator but we can't go any faster
+  /// without decreasing the number of string pools.
+  static const size_t AllocatorGrowthDelay = 1;
+  typedef llvm::BumpPtrAllocatorImpl
+  Allocator;
   typedef const char *StringPoolValueType;
-  typedef llvm::StringMap
-  StringPool;
+  typedef llvm::StringMap StringPool;
   typedef llvm::StringMapEntry StringPoolEntryType;
 
   static StringPoolEntryType &


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -29,9 +29,36 @@
 
 class Pool {
 public:
+  /// The default BumpPtrAllocatorImpl slab size.
+  static const size_t AllocatorSlabSize = 4096 * 4;
+  /// Every Pool has its own allocator which receives an equal share of
+  /// the ConstString allocations. This means that when allocating many
+  /// ConstStrings, every allocator sees only its small share of allocations and
+  /// assumes LLDB only allocated a small amount of memory so far. In reality
+  /// LLDB allocated a total memory that is N times as large as what the
+  /// allocator sees (where N is the number of string pools). This causes that
+  /// the BumpPtrAllocator continues a long time to allocate memory in small
+  /// chunks which only makes sense when allocating a small amount of memory
+  /// (which is true from the perspective of a single allocator). On some
+  /// systems doing all these small memory allocations causes LLDB to spend
+  /// a lot of time in malloc, so we need to force all these allocators to
+  /// behave like one allocator in terms of scaling their memory allocations
+  /// with increased demand. To do this we set the growth delay for each single
+  /// allocator to a rate so that our pool of allocators scales their memory
+  /// allocations similar to a single BumpPtrAllocatorImpl.
+  ///
+  /// Currently we have 256 string pools and the normal growth delay of the
+  /// BumpPtrAllocatorImpl is 128 (i.e., the memory allocation size increases
+  /// every 128 full chunks), so by changing the delay to 1 we get a
+  /// total growth delay in our allocator collection of 256/1 = 256. This is
+  /// still only half as fast as a 

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols with the same address range but different binding

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil planned changes to this revision.
jankratochvil added a comment.

In D63540#1791017 , @labath wrote:

> then this for example means that the "less global" symbols will not be 
> reported through the Symtab::ForEachSymbolContainingFileAddress API, which 
> seems like a bad thing.


I agree, I was not aware of this function. Still sure the prioritization could 
be moved to `Symtab::FindSymbolContainingFileAddress`.

> I'm also not happy that this is supposed to be a replacement for the size 
> setting patch, as I believe (and I think we've agreed on that while reviewing 
> the original patch) that *not* fiddling with the sizes of those symbols is a 
> good thing. I don't think reverting a patch that has been sitting in the tree 
> for several months because it "breaks expression evaluation" is a good idea,

OK, I will try to recheck how to fix the arm32 issue with the previous patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols with the same address range but different binding

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Defining some sort of a preference based on symbol type seems like a good idea, 
but I don't think this is a good way to implement it. If I read this patch 
correctly, then this for example means that the "less global" symbols will not 
be reported through the Symtab::ForEachSymbolContainingFileAddress API, which 
seems like a bad thing.

I'm also not happy that this is supposed to be a replacement for the size 
setting patch, as I believe (and I think we've agreed on that while reviewing 
the original patch) that *not* fiddling with the sizes of those symbols is a 
good thing. I don't think reverting a patch that has been sitting in the tree 
for several months because it "breaks expression evaluation" is a good idea, 
when that patch has nothing to do with expression evaluation, the failure only 
happens on a target which has a lot of failures anyway, and the failure only 
happens on some machine that the original author does not have access to. (I 
should've said this a while ago, but I was hoping that this problem would be 
resolved easily..)

Anyway, I think @jankratochvil has done far more than could be expected of any 
committer to diagnose that problem, and I think it should be up to @omjavaid to 
explain out how the expression evaluation failures relate to sizeless symbols. 
At that point, we can re-evaluate whether our decision to *not* fiddle with 
size of these symbols was correct.




Comment at: lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s:10-11
+.byte   0
+global:
+globalX:
+.globl  globalX

I found these names pretty confusing. I'd suggest something like 
`case1_local`+`case1_global`, `case2_local`+`case2_weak`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

[Pavel Labath via Phabricator via lldb-commits]

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

I like where this is going, but I think this still needs some work wrt. the 
panel library (and being able to customize the dependency search more).




Comment at: lldb/cmake/modules/LLDBConfig.cmake:28
+function(add_optional_dependency variable description package found)
+  set(${variable} "AUTO" CACHE STRING "${description} On, Off or Auto 
(default)")
+  string(TOUPPER "${${variable}}" ${variable})

Use the same capitalization of "auto" as the description



Comment at: lldb/cmake/modules/LLDBConfig.cmake:41-47
+  # We could set ${variable} directory to ${${found}} but then the value is
+  # TRUE/FALSE rather than ON/OFF.
+  if (${${found}})
+set(${variable} ON PARENT_SCOPE)
+  else()
+set(${variable} OFF PARENT_SCOPE)
+  endif()

I don't care that much about that, but I don't see the advantage in 
canonicalizing to ON/OFF (the other variables don't go through the 
canonicalization process, so only their default values will be ON/OFF -- the 
actual values will be whatever flavour of "true" the user chooses to pass to 
cmake)



Comment at: lldb/cmake/modules/LLDBConfig.cmake:507
 if (NOT CURSES_PANEL_LIBRARY)
-message(FATAL_ERROR "A required curses' panel library not found.")
+  set(LLDB_ENABLE_CURSES OFF)
+  message(WARNING "Curses panel library not found.")

I was wondering above if the name we pass to the `find_package` provides 
sufficient customization, and I think this shows it doesn't. Ideally, the panel 
(sub)library would behave the same way as the curses library and respect all 
three values of the cache variable. This one does not support the "force on" 
mode.

Having the cache variable handling code centralized in a single function is a 
worthy goal, but I am not sure that this is really possible, given the current 
cmake abilities. e.g. we'd need some sort of a indirect call so we can 
implement custom searching logic, but there aren't any particularly nice ways 
of achieving that. The only way I know of achieving indirection is to put the 
logic in a separate file, and then use either an `include`, `find_package` or 
something else to load it.

If you want to go the file loading approach, or "outline" than function (at 
least some parts of it), then I think both are fine, but I don't think leaving 
this code in here is good, since the whole point of this exercise is to 
standardize things.


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

https://reviews.llvm.org/D71306



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

[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This is starting to look pretty good. I think that pretty soon you'll have to 
come up with some way of having a more persistent lua context (instead of 
creating a fresh one for each command), but that doesn't have to happen now.

Does the "multiline" script command do anything already? Is there anything to 
test with respect to that.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:19-21
+#ifndef LLDB_DISABLE_LIBEDIT
+#include "lldb/Host/Editline.h"
+#endif

No longer needed?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:25
+
+#define LUA_PROMPT ">>> "
+

it doesn't look like this is used (but if it is, it would be better off as a 
regular variable)



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:59
+private:
+  lua_State *m_lua_state = nullptr;
+};

The constructor initialized this member, and the desctructor asserts it to be 
not null. It doesn't look like this assignment here is actually helping 
anything..



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:74
+if (llvm::Error error = m_lua.Run(data)) {
+  fprintf(GetOutputFILE(), "%s", llvm::toString(std::move(error)).c_str());
+}

*GetOutputStreamFileSP() << llvm::toString(std::move(error))



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:78
+
+  ~IOHandlerLuaInterpreter() override {}
+

delete



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:118
+  IOHandlerSP io_handler_sp(new IOHandlerLuaInterpreter(debugger));
+  if (io_handler_sp) {
+debugger.PushIOHandler(io_handler_sp);

This if is not needed  (new always succeeds)



Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);

I'm not actually opposed to this (and it's nice to know that this could work if 
we need it), but why do this as a unit test? It seems like this could be tested 
equally well with a `script` command through the lldb driver. I was imagining 
the unit tests would be most useful for the lower level functionality -- 
roughly corresponding to the `Lua` class. Right now that class is pretty simple 
and doesn't really need a unit test, but I expect it will become more complex 
over time...


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

https://reviews.llvm.org/D71234



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

[Lldb-commits] [lldb] 200cce3 - [lldb][NFC] Change if statements in ClangASTImporter to follow LLVM code style

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-19T10:55:54+01:00
New Revision: 200cce345dcf114a1b1012bc9c68adef6c99a595

URL: 
https://github.com/llvm/llvm-project/commit/200cce345dcf114a1b1012bc9c68adef6c99a595
DIFF: 
https://github.com/llvm/llvm-project/commit/200cce345dcf114a1b1012bc9c68adef6c99a595.diff

LOG: [lldb][NFC] Change if statements in ClangASTImporter to follow LLVM code 
style

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Symbol/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTImporter.h 
b/lldb/include/lldb/Symbol/ClangASTImporter.h
index e0448249eba8..9c0a95beb1e8 100644
--- a/lldb/include/lldb/Symbol/ClangASTImporter.h
+++ b/lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -287,9 +287,8 @@ class ClangASTImporter {
   ASTContextMetadataSP(new ASTContextMetadata(dst_ctx));
   m_metadata_map[dst_ctx] = context_md;
   return context_md;
-} else {
-  return context_md_iter->second;
 }
+return context_md_iter->second;
   }
 
   ASTContextMetadataSP MaybeGetContextMetadata(clang::ASTContext *dst_ctx) {
@@ -297,8 +296,7 @@ class ClangASTImporter {
 
 if (context_md_iter != m_metadata_map.end())
   return context_md_iter->second;
-else
-  return ASTContextMetadataSP();
+return ASTContextMetadataSP();
   }
 
   ImporterDelegateSP GetDelegate(clang::ASTContext *dst_ctx,
@@ -313,9 +311,8 @@ class ClangASTImporter {
   ImporterDelegateSP(new ASTImporterDelegate(*this, dst_ctx, src_ctx));
   delegates[src_ctx] = delegate;
   return delegate;
-} else {
-  return delegate_iter->second;
 }
+return delegate_iter->second;
   }
 
 public:

diff  --git a/lldb/source/Symbol/ClangASTImporter.cpp 
b/lldb/source/Symbol/ClangASTImporter.cpp
index de5448d4317d..d856443b268a 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -674,9 +674,8 @@ bool 
ClangASTImporter::CompleteAndFetchChildren(clang::QualType type) {
   }
 }
 
-if (RecordDecl *record_decl = dyn_cast(origin_tag_decl)) {
+if (RecordDecl *record_decl = dyn_cast(origin_tag_decl))
   record_decl->setHasLoadedFieldsFromExternalStorage(true);
-}
 
 return true;
   }
@@ -706,9 +705,8 @@ bool 
ClangASTImporter::CompleteAndFetchChildren(clang::QualType type) {
   }
 
   return true;
-} else {
-  return false;
 }
+return false;
   }
 
   return true;
@@ -730,15 +728,12 @@ bool 
ClangASTImporter::RequireCompleteType(clang::QualType type) {
 if (ObjCInterfaceDecl *objc_interface_decl =
 objc_object_type->getInterface())
   return CompleteObjCInterfaceDecl(objc_interface_decl);
-else
-  return false;
+return false;
   }
-  if (const ArrayType *array_type = type->getAsArrayTypeUnsafe()) {
+  if (const ArrayType *array_type = type->getAsArrayTypeUnsafe())
 return RequireCompleteType(array_type->getElementType());
-  }
-  if (const AtomicType *atomic_type = type->getAs()) {
+  if (const AtomicType *atomic_type = type->getAs())
 return RequireCompleteType(atomic_type->getPointeeType());
-  }
 
   return true;
 }
@@ -748,8 +743,7 @@ ClangASTMetadata *ClangASTImporter::GetDeclMetadata(const 
clang::Decl *decl) {
 
   if (decl_origin.Valid())
 return ClangASTContext::GetMetadata(decl_origin.ctx, decl_origin.decl);
-  else
-return ClangASTContext::GetMetadata(>getASTContext(), decl);
+  return ClangASTContext::GetMetadata(>getASTContext(), decl);
 }
 
 ClangASTImporter::DeclOrigin
@@ -762,8 +756,7 @@ ClangASTImporter::GetDeclOrigin(const clang::Decl *decl) {
 
   if (iter != origins.end())
 return iter->second;
-  else
-return DeclOrigin();
+  return DeclOrigin();
 }
 
 void ClangASTImporter::SetDeclOrigin(const clang::Decl *decl,
@@ -777,9 +770,9 @@ void ClangASTImporter::SetDeclOrigin(const clang::Decl 
*decl,
   if (iter != origins.end()) {
 iter->second.decl = original_decl;
 iter->second.ctx = _decl->getASTContext();
-  } else {
-origins[decl] = DeclOrigin(_decl->getASTContext(), original_decl);
+return;
   }
+  origins[decl] = DeclOrigin(_decl->getASTContext(), original_decl);
 }
 
 void ClangASTImporter::RegisterNamespaceMap(const clang::NamespaceDecl *decl,
@@ -799,8 +792,7 @@ ClangASTImporter::GetNamespaceMap(const 
clang::NamespaceDecl *decl) {
 
   if (iter != namespace_maps.end())
 return iter->second;
-  else
-return NamespaceMapSP();
+  return NamespaceMapSP();
 }
 
 void ClangASTImporter::BuildNamespaceMap(const clang::NamespaceDecl *decl) {



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

[Lldb-commits] [PATCH] D71232: [lldb/Lua] Add Boilerplate for a Lua Script Interpreter

[Pavel Labath via Phabricator via lldb-commits]

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

Given the positive response to the RFC, I think we can start landing this stuff.




Comment at: lldb/cmake/modules/LLDBConfig.cmake:28
 set(default_disable_python OFF)
+set(default_disable_lua OFF)
 set(default_disable_curses OFF)

JDevlieghere wrote:
> mgorny wrote:
> > labath wrote:
> > > I think this will tick off some bots (and people) because it means that 
> > > the default configuration will not build unless one has (compatible?) lua 
> > > installed. Though I don't really like that, the usual way to handle 
> > > external dependencies in llvm is to detect their presence and 
> > > automatically disable the relevant functionality.
> > > 
> > > Now, that's not how things work in lldb right now, so it _may_ make sense 
> > > to do the same for lua (though it also may make sense to port everything 
> > > to the llvm style). However, the current lldb behavior has been a source 
> > > of friction in the past and I suspect a fresh build error might reignite 
> > > some of that.
> > > 
> > > Anyway, you have been warned...
> > Fixing this is one of the things at the far end of my todo. If you could 
> > look into replacing the disable logic with something better, a lot of 
> > people would really be grateful.
> I've changed the default for now and put up an "RFC" here: 
> https://reviews.llvm.org/D71306
That's cool. I think this can stay as off until we deal with the deal with the 
dependency problem


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

https://reviews.llvm.org/D71232



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

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

We have at least one Initialize function (Socket::Initialize) which can return 
an llvm::Error (and it is used in some unit tests). What's your plan to handle 
that?

Assuming we can come up with something there, I think this would be a great 
utility. I wouldn't be opposed to using this for the main initialization code 
too...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71630



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 234672.
jankratochvil added a comment.
Herald added subscribers: MaskRay, emaste.
Herald added a reviewer: espindola.

Changing the size of symbols turned out to be too invasive. Let's keep it 
intact.
Rather choose the best symbols from those which have the same address range.
Currently LLDB chooses randomly any symbol from those so make it more 
deterministic.  That should have no negative effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63540

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s
  lldb/test/Shell/SymbolFile/symbol-binding.test

Index: lldb/test/Shell/SymbolFile/symbol-binding.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/symbol-binding.test
@@ -0,0 +1,22 @@
+# Some targets do not have the .size directive.
+# RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/symbol-binding.s -c -o %t.o
+# RUN: %lldb %t.o -s %s -o quit | FileCheck %s
+
+image lookup --address 4
+# CHECK: Summary: symbol-binding.test.tmp.o`globalX
+image lookup --address 5
+# CHECK: Summary: symbol-binding.test.tmp.o`weakW
+image lookup --address 6
+# CHECK: Summary: symbol-binding.test.tmp.o`bothX
+image dump symtab
+# CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
+# CHECK-NEXT:--- -- --- --- -- -- -- -- --
+# CHECK-NEXT:[0]  1 Code0x00040x0001 0x global
+# CHECK-NEXT:[1]  2 Code0x00030x0001 0x sizeend
+# CHECK-NEXT:[2]  3 Code0x00010x0002 0x sizeful
+# CHECK-NEXT:[3]  4 Code0x00010x0002 0x sizeless
+# CHECK-NEXT:[4]  5 Code0x00050x0001 0x weak
+# CHECK-NEXT:[5]  6 Code0x00060x0001 0x0020 bothW
+# CHECK-NEXT:[6]  7   X Code0x00060x0001 0x0010 bothX
+# CHECK-NEXT:[7]  8   X Code0x00040x0001 0x0010 globalX
+# CHECK-NEXT:[8]  9 Code0x00050x0001 0x0020 weakW
Index: lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/Inputs/symbol-binding.s
@@ -0,0 +1,22 @@
+.text
+.byte   0
+sizeless:
+sizeful:
+.byte   0
+.byte   0
+sizeend:
+.size   sizeful, sizeend - sizeful
+.byte   0
+global:
+globalX:
+.globl  globalX
+.byte   0
+weak:
+weakW:
+.weak   weakW
+.byte   0
+bothW:
+.weak   bothW
+bothX:
+.globl  bothX
+.byte   0
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -928,7 +928,53 @@
 }
   }
 
+  // For each matching address range choose the most global symbol.
+  // Rewrite 'data' of all the less global symbols, Sort() below will
+  // discard those entries.
+  size_t b_ix;
+  for (size_t a_ix = 0; a_ix < num_entries; a_ix = b_ix) {
+FileRangeToIndexMap::Entry  = m_file_addr_to_index.GetEntryRef(a_ix);
+for (b_ix = a_ix + 1; b_ix < num_entries; ++b_ix) {
+  FileRangeToIndexMap::Entry  =
+  m_file_addr_to_index.GetEntryRef(b_ix);
+  if (a.GetRangeBase() != b.GetRangeBase() ||
+  a.GetByteSize() != b.GetByteSize())
+break;
+}
+if (b_ix == a_ix + 1)
+  continue;
+int val_max = -1;
+FileRangeToIndexMap::Entry::DataType val_max_data;
+for (size_t scan_ix = a_ix; scan_ix < b_ix; ++scan_ix) {
+  FileRangeToIndexMap::Entry  =
+  m_file_addr_to_index.GetEntryRef(scan_ix);
+  const Symbol _symbol = *SymbolAtIndex(scan.data);
+  // Store to 'val' how much preferred is this symbol.
+  int val;
+  if (scan_symbol.IsExternal())
+val = 3;
+  else if (scan_symbol.IsWeak())
+val = 2;
+  else if (scan_symbol.IsDebug())
+val = 0;
+  else
+val = 1;
+  if (val > val_max) {
+val_max = val;
+ 

[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

[Michał Górny via Phabricator via lldb-commits]

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

Besides missing LZMA, looks good to me. However, I suspect you may want to wait 
for a second opinion ;-).




Comment at: lldb/cmake/modules/LLDBConfig.cmake:410
 find_package(LibLZMA)
 cmake_dependent_option(LLDB_ENABLE_LZMA "Support LZMA compression" ON 
"LIBLZMA_FOUND" OFF)
 if (LLDB_ENABLE_LZMA)

Did you omit `LLDB_ENABLE_LZMA` on purpose or accidentally?


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

https://reviews.llvm.org/D71306



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