date:20220106

[Lldb-commits] [lldb] 1919720 - [lldb] [debugserver] Simplify handling of arch specific files

2022-01-06 Thread Martin Storsjö via lldb-commits


Author: Martin Storsjö
Date: 2022-01-06T10:23:04+02:00
New Revision: 1919720fdd348ca568b235bf3f1357c198eccd15

URL: 
https://github.com/llvm/llvm-project/commit/1919720fdd348ca568b235bf3f1357c198eccd15
DIFF: 
https://github.com/llvm/llvm-project/commit/1919720fdd348ca568b235bf3f1357c198eccd15.diff

LOG: [lldb] [debugserver] Simplify handling of arch specific files

There are no duplicates among the include files, and all the
source files are wrapped in architecture ifdefs, so there's no harm
in including all of them, always.

This fixes builds if TARGET_TRIPLE is set to something else than the
build architecture.

This also allows building for multiple architectures at once by
setting CMAKE_OSX_ARCHITECTURES.

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt 
b/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
index ea4593fcf4511..8f44d1bfbb436 100644
--- a/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 



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

[Lldb-commits] [PATCH] D116625: [lldb] [debugserver] Simplify handling of arch specific files

2022-01-06 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1919720fdd34: [lldb] [debugserver] Simplify handling of arch 
specific files (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116625

Files:
  lldb/tools/debugserver/source/MacOSX/CMakeLists.txt


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-06 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D116707#3223998 , @JDevlieghere 
wrote:

> We shouldn't have to manage memory at this granularity. Why isn't 
> `xmlFreeDoc` cleaning this up?

Because xmlGetProp  
returns a value that's independent of the containing document.

That said, I agree this is a pretty bad encapsulation breakage (and will not 
compile in !LLDB_ENABLE_LIBXML builds).

One way to fix this (if we wanted to be fancy) is to have this function return 
a `std::unique_ptr` with a custom deleter (xmlFree), but given that this 
is hardly performance sensitive code, I would just copy the attribute value 
into a std::string inside `GetAttributeValue`, and then have it return *that* 
(after freeing the original string).

In either case, this function only has a handful of callers, so it should be 
pretty easy to update it to use a saner API.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

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

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

2022-01-06 Thread Chuanqi Xu via Phabricator via lldb-commits

ChuanqiXu updated this revision to Diff 397763.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D116351

Files:
  clang-tools-extra/docs/clang-doc.rst
  clang/docs/CommandGuide/clang.rst
  clang/www/c_status.html
  clang/www/cxx_status.html
  clang/www/get_involved.html
  clang/www/get_started.html
  clang/www/menu.html.incl
  libcxx/docs/index.rst
  libunwind/docs/index.rst
  lldb/docs/index.rst
  llvm/CMakeLists.txt
  llvm/docs/CommandGuide/llvm-install-name-tool.rst
  llvm/docs/CommandGuide/llvm-libtool-darwin.rst
  llvm/docs/CommandGuide/llvm-lipo.rst
  llvm/docs/CommandGuide/llvm-objcopy.rst
  llvm/docs/CommandGuide/llvm-objdump.rst
  llvm/docs/CommandGuide/llvm-otool.rst
  llvm/docs/CommandGuide/llvm-size.rst
  llvm/docs/CommandGuide/llvm-strings.rst
  llvm/docs/CommandGuide/llvm-strip.rst
  llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
  utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h

Index: utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
===
--- utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
+++ utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
@@ -24,7 +24,7 @@
 #include "llvm/Config/llvm-config.h"
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to 1 to enable backtraces, and to 0 otherwise. */
 #define ENABLE_BACKTRACES 1
@@ -332,7 +332,7 @@
 /* LTDL_SHLIB_EXT defined in Bazel */
 
 /* Define to the address where bug reports for this package should be sent. */
-#define PACKAGE_BUGREPORT "https://bugs.llvm.org/";
+#define PACKAGE_BUGREPORT "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to the full name of this package. */
 #define PACKAGE_NAME "LLVM"
Index: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
===
--- utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
+++ utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
@@ -20,7 +20,7 @@
 #define CLANG_CONFIG_H
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Default to -fPIE and -pie on Linux. */
 #define CLANG_DEFAULT_PIE_ON_LINUX 0
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -72,7 +72,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "ENABLE_BACKTRACES=1",
 "ENABLE_CRASH_OVERRIDES=1",
 "BACKTRACE_HEADER=execinfo.h",
@@ -120,7 +120,7 @@
 "LLVM_VERSION_INFO=",
 "LLVM_VERSION_PRINTER_SHOW_HOST_TARGET_INFO=1",
 "LLVM_WINDOWS_PREFER_FORWARD_SLASH=",
-"PACKAGE_BUGREPORT=https://bugs.llvm.org/";,
+"PACKAGE_BUGREPORT=https://github.com/llvm/llvm-project/issues/";,
 "PACKAGE_NAME=LLVM",
 "PACKAGE_STRING=LLVM ${llvm_version}git",
 "PACKAGE_VERSION=${llvm_version}git",
Index: llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
+++ llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
@@ -8,7 +8,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "CLANG_DEFAULT_PIE_ON_LINUX=",
 "CLANG_DEFAULT_LINKER=",
 "CLANG_DEFAULT_STD_C=",
Index: llvm/docs/CommandGuide/llvm-strip.rst
===
--- llvm/docs/CommandGuide/llvm-strip.rst
+++ llvm/docs/CommandGuide/llvm-strip.rst
@@ -194,7 +194,7 @@
 BUGS
 
 
-To report bugs, please visit .
+To report bugs, please visit .
 
 SEE ALSO
 
Index: llvm/docs/CommandGuide/llvm-strings.rst
===
--- llvm/docs/CommandGuide/llvm-strings.rst
+++ llvm/docs/CommandGuide/llvm-strings.rst
@@ -123,4 +123,4 @@
 BUGS
 
 
-To report bugs, please visit .
+To report bugs, please visit .
Index: llvm/docs/Command

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

2022-01-06 Thread James Henderson via Phabricator via lldb-commits

jhenderson accepted this revision.
jhenderson added a comment.

LGTM, from my point of view.


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

https://reviews.llvm.org/D116351

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

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

2022-01-06 Thread Chuanqi Xu via Phabricator via lldb-commits

ChuanqiXu added a comment.

It looks like @asl isn't here and many experienced guys accepted this. So I 
think it might should be good to commit this one.


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

https://reviews.llvm.org/D116351

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

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

2022-01-06 Thread Chuanqi Xu via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbce75e352be: Update Bug report URL to Github Issues 
(authored by ChuanqiXu).

Changed prior to commit:
  https://reviews.llvm.org/D116351?vs=397763&id=397817#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116351

Files:
  clang-tools-extra/docs/clang-doc.rst
  clang/docs/CommandGuide/clang.rst
  clang/www/c_status.html
  clang/www/cxx_status.html
  clang/www/get_involved.html
  clang/www/get_started.html
  clang/www/menu.html.incl
  libcxx/docs/index.rst
  libunwind/docs/index.rst
  lldb/docs/index.rst
  llvm/CMakeLists.txt
  llvm/docs/CommandGuide/llvm-install-name-tool.rst
  llvm/docs/CommandGuide/llvm-libtool-darwin.rst
  llvm/docs/CommandGuide/llvm-lipo.rst
  llvm/docs/CommandGuide/llvm-objcopy.rst
  llvm/docs/CommandGuide/llvm-objdump.rst
  llvm/docs/CommandGuide/llvm-otool.rst
  llvm/docs/CommandGuide/llvm-size.rst
  llvm/docs/CommandGuide/llvm-strings.rst
  llvm/docs/CommandGuide/llvm-strip.rst
  llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
  utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h

Index: utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
===
--- utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
+++ utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
@@ -24,7 +24,7 @@
 #include "llvm/Config/llvm-config.h"
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to 1 to enable backtraces, and to 0 otherwise. */
 #define ENABLE_BACKTRACES 1
@@ -332,7 +332,7 @@
 /* LTDL_SHLIB_EXT defined in Bazel */
 
 /* Define to the address where bug reports for this package should be sent. */
-#define PACKAGE_BUGREPORT "https://bugs.llvm.org/";
+#define PACKAGE_BUGREPORT "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to the full name of this package. */
 #define PACKAGE_NAME "LLVM"
Index: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
===
--- utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
+++ utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
@@ -20,7 +20,7 @@
 #define CLANG_CONFIG_H
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Default to -fPIE and -pie on Linux. */
 #define CLANG_DEFAULT_PIE_ON_LINUX 0
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -72,7 +72,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "ENABLE_BACKTRACES=1",
 "ENABLE_CRASH_OVERRIDES=1",
 "BACKTRACE_HEADER=execinfo.h",
@@ -120,7 +120,7 @@
 "LLVM_VERSION_INFO=",
 "LLVM_VERSION_PRINTER_SHOW_HOST_TARGET_INFO=1",
 "LLVM_WINDOWS_PREFER_FORWARD_SLASH=",
-"PACKAGE_BUGREPORT=https://bugs.llvm.org/";,
+"PACKAGE_BUGREPORT=https://github.com/llvm/llvm-project/issues/";,
 "PACKAGE_NAME=LLVM",
 "PACKAGE_STRING=LLVM ${llvm_version}git",
 "PACKAGE_VERSION=${llvm_version}git",
Index: llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
+++ llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
@@ -8,7 +8,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "CLANG_DEFAULT_PIE_ON_LINUX=",
 "CLANG_DEFAULT_LINKER=",
 "CLANG_DEFAULT_STD_C=",
Index: llvm/docs/CommandGuide/llvm-strip.rst
===
--- llvm/docs/CommandGuide/llvm-strip.rst
+++ llvm/docs/CommandGuide/llvm-strip.rst
@@ -194,7 +194,7 @@
 BUGS
 
 
-To report bugs, please visit .
+To report bugs, please visit .
 
 SEE ALSO
 
Index: llvm/docs/CommandGuide/llvm-strings.rst
===

[Lldb-commits] [lldb] 31c7165 - [lldb] Remove summary for signed char *

2022-01-06 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2022-01-06T19:52:24+01:00
New Revision: 31c7165a2bd69e07b916434a50826860132ba75f

URL: 
https://github.com/llvm/llvm-project/commit/31c7165a2bd69e07b916434a50826860132ba75f
DIFF: 
https://github.com/llvm/llvm-project/commit/31c7165a2bd69e07b916434a50826860132ba75f.diff

LOG: [lldb] Remove summary for signed char *

It conflicts with the summary for BOOL * (aka signed char *). This
partially reverts D112709.

Added: 


Modified: 
lldb/source/DataFormatters/FormatManager.cpp
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py

Removed: 




diff  --git a/lldb/source/DataFormatters/FormatManager.cpp 
b/lldb/source/DataFormatters/FormatManager.cpp
index 0ef5f0adc8327..f07bb9a7136a3 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -730,7 +730,7 @@ void FormatManager::LoadSystemFormatters() {
   GetCategory(m_system_category_name);
 
   sys_category_sp->GetRegexTypeSummariesContainer()->Add(
-  RegularExpression(R"(^((un)?signed )?char ?(\*|\[\])$)"), string_format);
+  RegularExpression(R"(^(unsigned )?char ?(\*|\[\])$)"), string_format);
 
   sys_category_sp->GetRegexTypeSummariesContainer()->Add(
   std::move(any_size_char_arr), string_array_format);

diff  --git 
a/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp 
b/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
index b6886ea7f2052..ff833da0b7a8a 100644
--- a/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -62,7 +62,7 @@ int main (int argc, char const *argv[])
 //%
 //% for c in ["", "const"]:
 //%   for v in ["", "volatile"]:
-//% for s in ["", "signed", "unsigned"]:
+//% for s in ["", "unsigned"]:
 //%   summary = '"'+c+v+s+'char"'
 //%   self.expect_var_path(c+v+s+"chararray", summary=summary)
 //% # These should be printed normally

diff  --git 
a/lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py 
b/lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py
index a95fa34b3766c..b866786634058 100644
--- a/lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py
+++ b/lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py
@@ -18,7 +18,7 @@ def test(self):
 lldb.SBFileSpec("main.c"))
 
 self.expect_var_path("c->flexible", type="char[]", 
summary='"contents"')
-self.expect_var_path("sc->flexible", type="signed char[]", 
summary='"contents"')
+# self.expect_var_path("sc->flexible", type="signed char[]", 
summary='"contents"')
 self.expect_var_path("uc->flexible", type="unsigned char[]", 
summary='"contents"')
 # TODO: Make this work
 self.expect("expr c->flexible", error=True,



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

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2022-01-06 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py:60-61
+self.assertEqual(
+[thread.GetFrameAtIndex(i).addr.GetLoadAddress(target)
+ for i in range(thread.GetNumFrames())],
+bt_expected)

labath wrote:
> If you want to be even more fancy :)
Neat. I didn't know threads are iterable like this. Unfortunately, swig makes 
it kinda hard to inspect stuff in a really Pythonic way.


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

https://reviews.llvm.org/D116255

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

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

2022-01-06 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thanks for the heads up. I've reverted the problematic parts in 31c7165a2b 
. I'll 
come with a proper fix soon-ish.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112709

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

[Lldb-commits] [lldb] 9b1d27b - [lldb] [Process/FreeBSDKernel] Support finding all processes

2022-01-06 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2022-01-06T21:53:28+01:00
New Revision: 9b1d27b2fa727a3a6f53a803d75beed50a1be999

URL: 
https://github.com/llvm/llvm-project/commit/9b1d27b2fa727a3a6f53a803d75beed50a1be999
DIFF: 
https://github.com/llvm/llvm-project/commit/9b1d27b2fa727a3a6f53a803d75beed50a1be999.diff

LOG: [lldb] [Process/FreeBSDKernel] Support finding all processes

Include the complete list of threads of all running processes
in the FreeBSDKernel plugin.  This makes it possible to inspect
the states (including partial register dumps from PCB) of all kernel
and userspace threads at the time of crash, or at the time of reading
/dev/mem first.

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

Added: 

lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/libfbsdvmcore-hacks.patch

lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/lldb-minimize-processes.patch

Modified: 
lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h

lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/README.rst
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/test.script
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2

lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2

lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-arm64-minidump.bz2

lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-i386-minidump.bz2

Removed: 

lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/libfbsdvmcore-print-offsets.patch



diff  --git 
a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp 
b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
index 339d33d251109..e3707365a9c3f 100644
--- a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
@@ -137,12 +137,115 @@ bool ProcessFreeBSDKernel::DoUpdateThreadList(ThreadList 
&old_thread_list,
   return false;
 }
 
-const Symbol *pcb_sym =
-GetTarget().GetExecutableModule()->FindFirstSymbolWithNameAndType(
-ConstString("dumppcb"));
-ThreadSP thread_sp(new ThreadFreeBSDKernel(
-*this, 1, pcb_sym ? pcb_sym->GetFileAddress() : LLDB_INVALID_ADDRESS));
-new_thread_list.AddThread(thread_sp);
+Status error;
+
+// struct field offsets are written as symbols so that we don't have
+// to figure them out ourselves
+int32_t offset_p_list = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_list"), 4, -1, error);
+int32_t offset_p_pid =
+ReadSignedIntegerFromMemory(FindSymbol("proc_off_p_pid"), 4, -1, 
error);
+int32_t offset_p_threads = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_threads"), 4, -1, error);
+int32_t offset_p_comm = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_comm"), 4, -1, error);
+
+int32_t offset_td_tid = ReadSignedIntegerFromMemory(
+FindSymbol("thread_off_td_tid"), 4, -1, error);
+int32_t offset_td_plist = ReadSignedIntegerFromMemory(
+FindSymbol("thread_off_td_plist"), 4, -1, error);
+int32_t offset_td_pcb = ReadSignedIntegerFromMemory(
+FindSymbol("thread_off_td_pcb"), 4, -1, error);
+int32_t offset_td_oncpu = ReadSignedIntegerFromMemory(
+FindSymbol("thread_off_td_oncpu"), 4, -1, error);
+int32_t offset_td_name = ReadSignedIntegerFromMemory(
+FindSymbol("thread_off_td_name"), 4, -1, error);
+
+// fail if we were not able to read any of the offsets
+if (offset_p_list == -1 || offset_p_pid == -1 || offset_p_threads == -1 ||
+offset_p_comm == -1 || offset_td_tid == -1 || offset_td_plist == -1 ||
+offset_td_pcb == -1 || offset_td_oncpu == -1 || offset_td_name == -1)
+  return false;
+
+// dumptid contains the thread-id of the crashing thread
+// dumppcb contains its PCB
+int32_t dumptid =
+ReadSignedIntegerFromMemory(FindSymbol("dumptid"), 4, -1, error);
+lldb::addr_t dumppcb = FindSymbol("dumppcb");
+
+// stoppcbs is an array of PCBs on all CPUs
+// each element is of size pcb_size
+int32_t pcbsize =
+ReadSignedIntegerFromMemory(FindSymbol("pcb_size"), 4, -1, error);
+ll

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2022-01-06 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rG9b1d27b2fa72: [lldb] [Process/FreeBSDKernel] Support finding 
all processes (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D116255?vs=396789&id=397968#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116255

Files:
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/README.rst
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/copy-sparse.py
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/libfbsdvmcore-hacks.patch
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/libfbsdvmcore-print-offsets.patch
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/lldb-minimize-processes.patch
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/test.script
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-minidump.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-arm64-minidump.bz2
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-i386-minidump.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/test.script
===
--- lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/test.script
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/test.script
@@ -1,5 +1,11 @@
 thread list
-register read pc
+register read --all
+bt
+thread select 2
+register read --all
+bt
+thread select 3
+register read --all
 bt
 p *(int*)&hz
 memory read &hz
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/lldb-minimize-processes.patch
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/tools/lldb-minimize-processes.patch
@@ -0,0 +1,85 @@
+diff --git a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
+index e3707365a9c3..c4a9c82f3c63 100644
+--- a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
 b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
+@@ -38,6 +38,8 @@ public:
+ 
+   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
+   lldb_private::Status &error) override;
++  size_t DoWriteMemory(lldb::addr_t vm_addr, const void *buf,
++   size_t size, Status &error) override;
+ 
+ private:
+   fvc_t *m_fvc;
+@@ -185,6 +187,7 @@ bool ProcessFreeBSDKernel::DoUpdateThreadList(ThreadList &old_thread_list,
+ // iterate through a linked list of all processes
+ // allproc is a pointer to the first list element, p_list field
+ // (found at offset_p_list) specifies the next element
++lldb::addr_t prev = 0;
+ for (lldb::addr_t proc =
+  ReadPointerFromMemory(FindSymbol("allproc"), error);
+  proc != 0 && proc != LLDB_INVALID_ADDRESS;
+@@ -195,6 +198,8 @@ bool ProcessFreeBSDKernel::DoUpdateThreadList(ThreadList &old_thread_list,
+   char comm[fbsd_maxcomlen + 1];
+   ReadCStringFromMemory(proc + offset_p_comm, comm, sizeof(comm), error);
+ 
++  bool interesting = false;
++
+   // iterate through a linked list of all process' threads
+   // the initial thread is found in process' p_threads, subsequent
+   // elements are linked via td_plist field
+@@ -231,6 +236,7 @@ bool ProcessFreeBSDKernel::DoUpdateThreadList(ThreadList &old_thread_list,
+   // NB: dumppcb can be LLDB_INVALID_ADDRESS if reading it failed
+   pcb_addr = dumppcb;
+   thread_desc += " (crashed)";
++  interesting = true;
+ } else if (oncpu != -1) {
+   // if we managed to read stoppcbs and pcb_size, use them to find
+   // the correct PCB
+@@ -239,13 +245,27 @@ bool ProcessFreeBSDKernel::DoUpdateThreadList(ThreadList &old_thread_list,
+   else
+ pcb_addr = LLDB_INVALID_ADDRESS;
+   thread_desc += llvm::formatv(" (on CPU {0})", oncpu);
++  interesting = true;

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

2022-01-06 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik created this revision.
shafik added reviewers: labath, jingham, JDevlieghere.
shafik requested review of this revision.

Both `close` and `closesocket` should return `0` on success so using `!!` looks 
incorrect. I replaced this will a more readable `== 0` check.


https://reviews.llvm.org/D116768

Files:
  lldb/source/Host/common/Socket.cpp


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

2022-01-06 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

AFAICT this fix is correct but I am not sure how to verify of test it. I ran 
the test suite and it passed but that does not mean this is being covered.


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

https://reviews.llvm.org/D116768

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

[Lldb-commits] [PATCH] D116772: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-06 Thread Lirong Yuan via Phabricator via lldb-commits

yuanzi created this revision.
yuanzi requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While running heap checker on a test that uses LLDB API, the following memory 
leak is found:

RAW: HeapChecker started...
RAW: Leak check _main_ detected leaks of 34 bytes in 4 objects
RAW: The 2 largest leaks:
RAW: Leak of 17 bytes in 2 objects allocated from:
 @ 0x7fb93bd20166 NewHook()
 @ 0x7fb929372a73 absl::base_internal::MallocHook::InvokeNewHookSlow()
 @ 0x5600d1046093 __libc_malloc
 @ 0x7fb974529c03 xmlStrdup
 @ 0x7fb9744c2a0b xmlGetProp
 @ 0x7fb9749d9ed6 lldb_private::XMLNode::GetAttributeValue()
 @ 0x7fb979043001 std::__u::__function::__policy_invoker<>::__call_impl<>()
 @ 0x7fb9749da06d lldb_private::XMLNode::ForEachChildElement()
 @ 0x7fb97903c54d 
lldb_private::process_gdb_remote::ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess()
 @ 0x7fb97902cfe4 
lldb_private::process_gdb_remote::ProcessGDBRemote::GetGDBServerRegisterInfo()
 @ 0x7fb97902c1d0 
lldb_private::process_gdb_remote::ProcessGDBRemote::BuildDynamicRegisterInfo()
 @ 0x7fb97902e92a 
lldb_private::process_gdb_remote::ProcessGDBRemote::SetThreadStopInfo()
 @ 0x7fb97902db18 
lldb_private::process_gdb_remote::ProcessGDBRemote::DoConnectRemote()
 @ 0x7fb97584965e lldb_private::Process::ConnectRemote()
 @ 0x7fb975839fa6 lldb_private::Platform::DoConnectProcess()
 @ 0x7fb97583a39e lldb_private::Platform::ConnectProcessSynchronous()
 @ 0x7fb97545b28b CommandObjectProcessConnect::DoExecute()
 @ 0x7fb9755a70c9 lldb_private::CommandObjectParsed::Execute()
 @ 0x7fb97559c0e9 lldb_private::CommandInterpreter::HandleCommand()
 @ 0x7fb975460145 lldb_private::CommandObjectRegexCommand::DoExecute()
 @ 0x7fb9755a72d2 lldb_private::CommandObjectRaw::Execute()
 @ 0x7fb97559c0e9 lldb_private::CommandInterpreter::HandleCommand()
 @ 0x7fb997a5f22e lldb::SBCommandInterpreter::HandleCommand()
 @ 0x7fb997a5ef9b lldb::SBCommandInterpreter::HandleCommand()

This change fixes the memory leaks by freeing memory after it is no longer in 
use.
Tested with `ninja check-lldb`:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116772

Files:
  lldb/include/lldb/Host/XML.h
  lldb/source/Host/common/XML.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4351,9 +4351,9 @@
 } else if (name == "osabi") {
   node.GetElementText(target_info.osabi);
 } else if (name == "xi:include" || name == "include") {
-  llvm::StringRef href = node.GetAttributeValue("href");
+  std::string href = node.GetAttributeValue("href");
   if (!href.empty())
-target_info.includes.push_back(href.str());
+target_info.includes.push_back(href);
 } else if (name == "feature") {
   feature_nodes.push_back(node);
 } else if (name == "groups") {
@@ -4392,9 +4392,9 @@
 const XMLNode &node) -> bool {
   llvm::StringRef name = node.GetName();
   if (name == "xi:include" || name == "include") {
-llvm::StringRef href = node.GetAttributeValue("href");
+std::string href = node.GetAttributeValue("href");
 if (!href.empty())
-  target_info.includes.push_back(href.str());
+  target_info.includes.push_back(href);
 }
 return true;
   });
@@ -4530,7 +4530,8 @@
   "Error finding library-list-svr4 xml element");
 
 // main link map structure
-llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm");
+std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+llvm::StringRef main_lm(main_lm_str);
 // FIXME: we're silently ignoring invalid data here
 if (!main_lm.empty())
   llvm::to_integer(main_lm, list.m_link_map);
@@ -4618,15 +4619,16 @@
 "library", [log, &list](const XMLNode &library) -> bool {
   LoadedModuleInfoList::LoadedModuleInfo module;
 
-  llvm::StringRef name = library.GetAttributeValue("name");
-  module.set_name(name.str());
+  std::string name = library.GetAttributeValue("name");
+  module.set_name(name);
 
   // The base address of a given library will be the address of its
   // first section. Most remotes send only one section for Windows
   // targets for example.
   const XMLNode §ion =
   library.FindFirstChildElementWithName("section");
-  llvm::StringRef address = section.GetAttributeValue("address");
+  std::string address_str = section.GetAttributeValue("address");
+  llvm::StringRef address(address_str);
   uint64_t address_value = LL

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-06 Thread Lirong Yuan via Phabricator via lldb-commits

yuanzi updated this revision to Diff 398009.
yuanzi edited the summary of this revision.
yuanzi added a comment.

Yeah since `xmlGetProp` calls `xmlGetPropNodeValueInternal`, which calls 
`xmlStrdup` rather than returning the content or value of the node directly, 
`xmlFreeDoc` could not clean it up.
https://github.com/tenderlove/libxml2/blob/master/tree.c

Agree that we need to update the API to return not just a reference to string 
(`llvm::StringRef`), but an object that can own string data (`std::string`).

This change fixes the memory leaks in "GetGDBServerRegisterInfoXMLAndProcess" 
by updating "GetAttributeValue" to return "std::string" instead of 
"llvm::StringRef".
Tested with "ninja check-lldb".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

Files:
  lldb/include/lldb/Host/XML.h
  lldb/source/Host/common/XML.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4351,9 +4351,9 @@
 } else if (name == "osabi") {
   node.GetElementText(target_info.osabi);
 } else if (name == "xi:include" || name == "include") {
-  llvm::StringRef href = node.GetAttributeValue("href");
+  std::string href = node.GetAttributeValue("href");
   if (!href.empty())
-target_info.includes.push_back(href.str());
+target_info.includes.push_back(href);
 } else if (name == "feature") {
   feature_nodes.push_back(node);
 } else if (name == "groups") {
@@ -4392,9 +4392,9 @@
 const XMLNode &node) -> bool {
   llvm::StringRef name = node.GetName();
   if (name == "xi:include" || name == "include") {
-llvm::StringRef href = node.GetAttributeValue("href");
+std::string href = node.GetAttributeValue("href");
 if (!href.empty())
-  target_info.includes.push_back(href.str());
+  target_info.includes.push_back(href);
 }
 return true;
   });
@@ -4530,7 +4530,8 @@
   "Error finding library-list-svr4 xml element");
 
 // main link map structure
-llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm");
+std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+llvm::StringRef main_lm(main_lm_str);
 // FIXME: we're silently ignoring invalid data here
 if (!main_lm.empty())
   llvm::to_integer(main_lm, list.m_link_map);
@@ -4618,15 +4619,16 @@
 "library", [log, &list](const XMLNode &library) -> bool {
   LoadedModuleInfoList::LoadedModuleInfo module;
 
-  llvm::StringRef name = library.GetAttributeValue("name");
-  module.set_name(name.str());
+  std::string name = library.GetAttributeValue("name");
+  module.set_name(name);
 
   // The base address of a given library will be the address of its
   // first section. Most remotes send only one section for Windows
   // targets for example.
   const XMLNode §ion =
   library.FindFirstChildElementWithName("section");
-  llvm::StringRef address = section.GetAttributeValue("address");
+  std::string address_str = section.GetAttributeValue("address");
+  llvm::StringRef address(address_str);
   uint64_t address_value = LLDB_INVALID_ADDRESS;
   llvm::to_integer(address, address_value);
   module.set_base(address_value);
Index: lldb/source/Host/common/XML.cpp
===
--- lldb/source/Host/common/XML.cpp
+++ lldb/source/Host/common/XML.cpp
@@ -130,28 +130,33 @@
 #endif
 }
 
-llvm::StringRef XMLNode::GetAttributeValue(const char *name,
-   const char *fail_value) const {
-  const char *attr_value = nullptr;
+std::string XMLNode::GetAttributeValue(const char *name,
+   const char *fail_value) const {
+  std::string attr_value;
 #if LLDB_ENABLE_LIBXML2
-
-  if (IsValid())
-attr_value = (const char *)xmlGetProp(m_node, (const xmlChar *)name);
-  else
-attr_value = fail_value;
+  if (IsValid()) {
+xmlChar *value = xmlGetProp(m_node, (const xmlChar *)name);
+if (value) {
+  attr_value = (const char *)value;
+  free(value);
+}
+  } else {
+if (fail_value)
+  attr_value = fail_value;
+  }
 #else
-  attr_value = fail_value;
+  if (fail_value)
+attr_value = fail_value;
 #endif
-  if (attr_value)
-return llvm::StringRef(attr_value);
-  else
-return llvm::StringRef();
+  return attr_value;
 }
 
 bool XMLNode::GetAttributeValueAsUnsig

[Lldb-commits] [lldb] b3bfd59 - [lldb] Compute fully qualified command names in FindCommandsForApropos

2022-01-06 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2022-01-06T19:26:57-08:00
New Revision: b3bfd595a548cd85b12e4e83729436cb73b26f29

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

LOG: [lldb] Compute fully qualified command names in FindCommandsForApropos

Fixes incomplete command names in `apropos` results.

The full command names given by `apropos` have come from command name string
literals given to `CommandObject` constructors. For most commands, this has
been accurate, but some commands have incorrect strings. This results in
`apropos` output that doesn't tell the user the full command name they might
want learn more about. These strings can be fixed.

There's a seperate issue that can't be fixed as easily: plugin commands. With
the way they're implemented, plugin commands have to exclude the root command
from their command name string. To illustrate, the `language objc` subcommand
has to set its command name string to "objc", which results in apropos printing
results as `objc ...` instead of `language objc ...`.

To fix both of these issues, this commit changes `FindCommandsForApropos` to
derive the fully qualified command name using the keys of subcommand maps.

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index e6f0d5f9c4d43..3efb59fc05647 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@ class CommandInterpreter : public Broadcaster,
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 085b06bce0eae..59c23716bf89c 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,12 +2841,10 @@ void CommandInterpreter::OutputHelpText(Stream &strm, 
llvm::StringRef word_text,
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2856,14 +2854,19 @@ void CommandInterpreter::FindCommandsForApropos(
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+std::string qualified_name =
+(command_name + " " + subcommand_name).str();
+commands_found.AppendString(qualified_name);
+  }
 }
   }
 }



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

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

2022-01-06 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3bfd595a548: [lldb] Compute fully qualified command names 
in FindCommandsForApropos (authored by kastiglione).

Changed prior to commit:
  https://reviews.llvm.org/D116491?vs=397681&id=398034#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116491

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


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,12 +2841,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2856,14 +2854,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
+  StringList subcommands_found;
+  FindCommandsForApropos(search_word, subcommands_found, commands_help,
+ multiword_cmd->GetSubcommandDictionary());
+  for (const auto &subcommand_name : subcommands_found) {
+std::string qualified_name =
+(command_name + " " + subcommand_name).str();
+commands_found.AppendString(qualified_name);
+  }
 }
   }
 }
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  CommandObject::CommandMap &command_map);
+  const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,12 +2841,10 @@
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, CommandObject::CommandMap &command_map) {
-  CommandObject::CommandMap::const_iterator pos;
-
-  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
-llvm::StringRef command_name = pos->first;
-CommandObject *cmd_obj = pos->second.get();
+StringList &commands_help, const CommandObject::CommandMap &command_map) {
+  for (const auto &pair : command_map) {
+llvm::StringRef command_name = pair.first;
+CommandObject *cmd_obj = pair.second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2856,14 +2854,19 @@
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(cmd_obj->GetCommandName());
+  commands_found.AppendString(command_name);
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (cmd_obj->IsMultiwordObject()) {
-  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
-  FindCommandsForApropos(search_word, commands_found, commands_help,
- cmd_multiword->GetSubcommandDictionary());
+if (auto *multiword_cmd =

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

2022-01-06 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, clayborg.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Ensure that errors in `frame variable` are reflected in result object.

The statistics for `frame variable` show invocations as being successful, even
when executing one of the error paths.

This change replaces `result.GetErrorStream()` with `result.AppendError()`,
which also sets the status to `eReturnStatusFailed`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116788

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendWarningWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   "variable expression path that "
-   "matches '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"unable to find any variable expression path that matches "
+"'%s'.",
+entry.c_str());
 }
   }
 }
@@ -680,7 +677,8 @@
   }
 }
   }
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (result.GetStatus() != eReturnStatusFailed)
+result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 if (m_option_variable.show_recognized_args) {


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression '%s'.\n",
-   entry.c_str());
+result.AppendWarningWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

2022-01-06 Thread Dave Lee via Phabricator via lldb-commits

kastiglione updated this revision to Diff 398040.
kastiglione added a comment.

s/warning/error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

Files:
  lldb/source/Commands/CommandObjectFrame.cpp


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-   "variable expression path that "
-   "matches '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"unable to find any variable expression path that matches "
+"'%s'.",
+entry.c_str());
 }
   }
 }
@@ -680,7 +677,8 @@
   }
 }
   }
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (result.GetStatus() != eReturnStatusFailed)
+result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 if (m_option_variable.show_recognized_args) {


Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -558,18 +558,16 @@
   }
 }
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression '%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());
   }
 } else {
   if (llvm::Error err = regex.GetError())
-result.GetErrorStream().Printf(
-"error: %s\n", llvm::toString(std::move(err)).c_str());
+result.AppendError(llvm::toString(std::move(err)));
   else
-result.GetErrorStream().Printf(
-"error: unknown regex error when compiling '%s'\n",
-entry.c_str());
+result.AppendErrorWithFormat(
+"unknown regex error when compiling '%s'", entry.c_str());
 }
   } else // No regex, either exact variable names or variable
  // expressions.
@@ -605,14 +603,13 @@
   valobj_sp->GetParent() ? entry.c_str() : nullptr);
   valobj_sp->Dump(output_stream, options);
 } else {
-  const char *error_cstr = error.AsCString(nullptr);
-  if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+  if (auto error_cstr = error.AsCString(nullptr))
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any "
-

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

2022-01-06 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectFrame.cpp:560-563
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());

this regex error is a weird edge case. For example, considering running:

```
frame var --regex matchesSomeVars doesntMatchAnyVars
```

if the `doesntMatchAnyVars` pattern has no matches, then the command prints an 
error, and the result would be marked as an error. But if the `matchesSomeVars` 
does have matches, then we have a partial success / partial failure. In such a 
case, should the result be marked success, or failure? I don't know, but I 
would lean to success since it does entirely fail. Maybe a user could expect 
some patterns to match and some to not match. For example: a user alias that 
prints any variables based on a set of patterns they're interested in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

2022-01-06 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a subscriber: jingham.
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectFrame.cpp:560-563
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());

kastiglione wrote:
> this regex error is a weird edge case. For example, considering running:
> 
> ```
> frame var --regex matchesSomeVars doesntMatchAnyVars
> ```
> 
> if the `doesntMatchAnyVars` pattern has no matches, then the command prints 
> an error, and the result would be marked as an error. But if the 
> `matchesSomeVars` does have matches, then we have a partial success / partial 
> failure. In such a case, should the result be marked success, or failure? I 
> don't know, but I would lean to success since it does entirely fail. Maybe a 
> user could expect some patterns to match and some to not match. For example: 
> a user alias that prints any variables based on a set of patterns they're 
> interested in.
I think this could be changed to a warning. @jingham what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [lldb] bd23dff - Revert "[lldb] Compute fully qualified command names in FindCommandsForApropos"

2022-01-06 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2022-01-06T20:15:19-08:00
New Revision: bd23dffc2c38b7cffd3811716fd373cd2b588c27

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

LOG: Revert "[lldb] Compute fully qualified command names in 
FindCommandsForApropos"

This reverts commit b3bfd595a548cd85b12e4e83729436cb73b26f29.

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 3efb59fc05647..e6f0d5f9c4d43 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -649,7 +649,7 @@ class CommandInterpreter : public Broadcaster,
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
   StringList &commands_help,
-  const CommandObject::CommandMap &command_map);
+  CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
   void PrintCommandOutput(Stream &stream, llvm::StringRef str);

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 59c23716bf89c..085b06bce0eae 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2841,10 +2841,12 @@ void CommandInterpreter::OutputHelpText(Stream &strm, 
llvm::StringRef word_text,
 
 void CommandInterpreter::FindCommandsForApropos(
 llvm::StringRef search_word, StringList &commands_found,
-StringList &commands_help, const CommandObject::CommandMap &command_map) {
-  for (const auto &pair : command_map) {
-llvm::StringRef command_name = pair.first;
-CommandObject *cmd_obj = pair.second.get();
+StringList &commands_help, CommandObject::CommandMap &command_map) {
+  CommandObject::CommandMap::const_iterator pos;
+
+  for (pos = command_map.begin(); pos != command_map.end(); ++pos) {
+llvm::StringRef command_name = pos->first;
+CommandObject *cmd_obj = pos->second.get();
 
 const bool search_short_help = true;
 const bool search_long_help = false;
@@ -2854,19 +2856,14 @@ void CommandInterpreter::FindCommandsForApropos(
 cmd_obj->HelpTextContainsWord(search_word, search_short_help,
   search_long_help, search_syntax,
   search_options)) {
-  commands_found.AppendString(command_name);
+  commands_found.AppendString(cmd_obj->GetCommandName());
   commands_help.AppendString(cmd_obj->GetHelp());
 }
 
-if (auto *multiword_cmd = cmd_obj->GetAsMultiwordCommand()) {
-  StringList subcommands_found;
-  FindCommandsForApropos(search_word, subcommands_found, commands_help,
- multiword_cmd->GetSubcommandDictionary());
-  for (const auto &subcommand_name : subcommands_found) {
-std::string qualified_name =
-(command_name + " " + subcommand_name).str();
-commands_found.AppendString(qualified_name);
-  }
+if (cmd_obj->IsMultiwordObject()) {
+  CommandObjectMultiword *cmd_multiword = cmd_obj->GetAsMultiwordCommand();
+  FindCommandsForApropos(search_word, commands_found, commands_help,
+ cmd_multiword->GetSubcommandDictionary());
 }
   }
 }



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

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-06 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Yes, that's the API I had in mind, but check out the inline comments for some 
problems/improvements.




Comment at: lldb/source/Host/common/XML.cpp:141
+  attr_value = (const char *)value;
+  free(value);
+}

this should be xmlFree, per the function documentation.



Comment at: lldb/source/Host/common/XML.cpp:157-159
+  std::string attr_str = GetAttributeValue(name, "");
+  llvm::StringRef attr(attr_str);
+  return llvm::to_integer(attr, value, base);

I don't see why we need these temporary variables. std::string is implicitly 
convertible to a StringRef, and you don't need the value afterwards, so you 
should be able to pass the GetAttributeValue straight into the to_integer 
function. Did that not work for some reason?



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4534
+std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+llvm::StringRef main_lm(main_lm_str);
 // FIXME: we're silently ignoring invalid data here

Same here. `.empty()` and `to_integer` should work on std::string as well. 
Constructing a StringRef might make sense if we needed to perform some more 
complex operations (ones which std::string does not support) but I don't see 
anything like that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

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

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

2022-01-06 Thread Pavel Labath via Phabricator via lldb-commits

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

Usually, the only thing one can do if a close fails is to log an error message, 
so it's not completely surprising that this has gone by unnoticed.

It might be nice to insert a close call to one of the existing Socket unit 
tests (or add a new one), but other than that, I am pretty sure this is fine.


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

https://reviews.llvm.org/D116768

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

[Lldb-commits] [lldb] 1919720 - [lldb] [debugserver] Simplify handling of arch specific files

[Lldb-commits] [PATCH] D116625: [lldb] [debugserver] Simplify handling of arch specific files

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

[Lldb-commits] [lldb] 31c7165 - [lldb] Remove summary for signed char *

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

[Lldb-commits] [lldb] 9b1d27b - [lldb] [Process/FreeBSDKernel] Support finding all processes

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

[Lldb-commits] [PATCH] D116772: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

[Lldb-commits] [lldb] b3bfd59 - [lldb] Compute fully qualified command names in FindCommandsForApropos

[Lldb-commits] [PATCH] D116491: [lldb] Compute fully qualified command names in FindCommandsForApropos

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

[Lldb-commits] [lldb] bd23dff - Revert "[lldb] Compute fully qualified command names in FindCommandsForApropos"

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

25 matches

Site Navigation

Mail list logo

Footer information