[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

2023-05-24 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 525110.
Michael137 added a comment.

Fix expected string in test


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

https://reviews.llvm.org/D151268

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
@@ -1,12 +1,17 @@
 #include 
 #include 
 
+struct Foo {
+  int mem = 5;
+};
+
 int
 main()
 {
 std::shared_ptr nsp;
 std::shared_ptr isp(new int{123});
 std::shared_ptr ssp = std::make_shared("foobar");
+std::shared_ptr fsp = std::make_shared();
 
 std::weak_ptr nwp;
 std::weak_ptr iwp = isp;
@@ -15,6 +20,7 @@
 nsp.reset(); // Set break point at this line.
 isp.reset();
 ssp.reset();
+fsp.reset();
 
 return 0; // Set break point at this line.
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
@@ -33,6 +33,13 @@
 self.expect("frame variable iwp", substrs=['iwp = 123'])
 self.expect("frame variable swp", substrs=['swp = "foobar"'])
 
+self.expect("frame variable *nsp", substrs=['*nsp = '])
+self.expect("frame variable *isp", substrs=['*isp = 123'])
+self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
+self.expect("frame variable *fsp", substrs=['*fsp = (mem = 5)'])
+
+self.expect("frame variable fsp->mem", substrs=['(int) fsp->mem = 5'])
+
 self.runCmd("continue")
 
 self.expect("frame variable nsp", substrs=['nsp = nullptr'])
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -73,6 +73,15 @@
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
+private:
+
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr; // Underlying pointer (held, not owned)
+  ValueObject* m_obj_obj = nullptr; // Underlying object (held, not owned)
 };
 
 } // end of anonymous namespace
@@ -367,24 +376,48 @@
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-return lldb::ValueObjectSP();
-
   if (idx == 0)
-return valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
-  else
-return lldb::ValueObjectSP();
+return m_ptr_obj->GetSP();
+  if (idx == 1)
+return m_obj_obj->GetSP();
+
+  return lldb::ValueObjectSP();
 }
 
-bool LibStdcppSharedPtrSyntheticFrontEnd::Update() { return false; }
+bool LibStdcppSharedPtrSyntheticFrontEnd::Update() {
+  auto backend = m_backend.GetSP();
+  if (!backend)
+return false;
+
+  auto valobj_sp = backend->GetNonSyntheticValue();
+  if (!valobj_sp)
+return false;
+
+  auto ptr_obj_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
+  if (!ptr_obj_sp)
+return false;
+
+  m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+
+  if (m_ptr_obj) {
+Status error;
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
+if (error.Success()) {
+  m_obj_obj = obj_obj->Clone(ConstString("object")).get();
+}
+  }
+
+  return false;
+}
 
 bool LibStdcppSharedPtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
 ConstString name) {
-  if (name == "_M_ptr")
+  if (name == "pointer")
 return 0;
+  if (name == "object" || name == "$$dereference$$")
+return 1;
   return UINT32_MAX;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail

[Lldb-commits] [lldb] 44bb442 - [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

2023-05-24 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2023-05-24T13:01:11+01:00
New Revision: 44bb442fd5be3860e7819cb216621b5ea59970c3

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

LOG: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr 
formatter

This mimicks the implementation of the libstdcpp std::unique_ptr
formatter.

This has been attempted several years ago in
`0789722d85cf1f1fdbe2ffb2245ea0ba034a9f94` but was reverted in
`e7dd3972094c2f2fb42dc9d4d5344e54a431e2ce`.

The difference to the original patch is that we now maintain
a `$$dereference$$` member and we only store weak pointers
to the other children inside the synthetic frontend. This is
what the libc++ formatters do to prevent the recursion mentioned
in the revert commit.

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index c7f1c79422246..bd129d2f64060 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -73,6 +73,15 @@ class LibStdcppSharedPtrSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
+private:
+
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr; // Underlying pointer (held, not owned)
+  ValueObject* m_obj_obj = nullptr; // Underlying object (held, not owned)
 };
 
 } // end of anonymous namespace
@@ -367,24 +376,48 @@ size_t 
LibStdcppSharedPtrSyntheticFrontEnd::CalculateNumChildren() { return 1; }
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-return lldb::ValueObjectSP();
-
   if (idx == 0)
-return valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
-  else
-return lldb::ValueObjectSP();
+return m_ptr_obj->GetSP();
+  if (idx == 1)
+return m_obj_obj->GetSP();
+
+  return lldb::ValueObjectSP();
 }
 
-bool LibStdcppSharedPtrSyntheticFrontEnd::Update() { return false; }
+bool LibStdcppSharedPtrSyntheticFrontEnd::Update() {
+  auto backend = m_backend.GetSP();
+  if (!backend)
+return false;
+
+  auto valobj_sp = backend->GetNonSyntheticValue();
+  if (!valobj_sp)
+return false;
+
+  auto ptr_obj_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), 
true);
+  if (!ptr_obj_sp)
+return false;
+
+  m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+
+  if (m_ptr_obj) {
+Status error;
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
+if (error.Success()) {
+  m_obj_obj = obj_obj->Clone(ConstString("object")).get();
+}
+  }
+
+  return false;
+}
 
 bool LibStdcppSharedPtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
 ConstString name) {
-  if (name == "_M_ptr")
+  if (name == "pointer")
 return 0;
+  if (name == "object" || name == "$$dereference$$")
+return 1;
   return UINT32_MAX;
 }
 

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
index 3d37af31d66c5..13bcb68cfd1fd 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
@@ -33,6 +33,13 @@ def test_with_run_command(self):
 self.expect("frame variable iwp", substrs=['iwp = 123'])
 self.expect("frame variable swp", substrs=['swp = "foobar"'])
 
+self.expect("frame variable *nsp", substrs=['*nsp = '])
+self.expect("frame variable *isp", substrs=['*isp = 123'])
+self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
+self.expect("frame variable *fsp", substrs=['*fsp = (mem = 5)'])
+
+self.expect("frame var

[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

2023-05-24 Thread Michael Buch via Phabricator via lldb-commits
Michael137 closed this revision.
Michael137 added a comment.

Committed in https://reviews.llvm.org/rG44bb442fd5be


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

https://reviews.llvm.org/D151268

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


[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

2023-05-24 Thread Mark de Wever via Phabricator via lldb-commits
Mordante created this revision.
Mordante added reviewers: glandium, hans, thakis.
Herald added subscribers: libc-commits, bviyer, ekilmer, Moerafaat, zero9178, 
Enna1, bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, cota, teijeong, 
rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, jvesely, Joonsoo, 
liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle, 
mehdi_amini, mstorsjo, whisperity.
Herald added a reviewer: bollu.
Herald added a reviewer: ldionne.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: NoQ.
Herald added projects: libc-project, Flang, All.
Mordante requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, libcxx-commits, openmp-commits, 
lldb-commits, Sanitizers, cfe-commits, jplehr, yota9, sstefan1, 
stephenneuendorffer, nicolasvasilache, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
libunwind, MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.

This reverts commit d763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6 
.

Adds the patch by @hans from
https://github.com/llvm/llvm-project/issues/62719


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151344

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  cmake/Modules/CMakePolicy.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/crt/CMakeLists.txt
  flang/CMakeLists.txt
  flang/lib/Decimal/CMakeLists.txt
  flang/runtime/CMakeLists.txt
  libc/CMakeLists.txt
  libc/examples/hello_world/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm-libgcc/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/GettingStarted.rst
  llvm/docs/ReleaseNotes.rst
  mlir/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/docs/SupportAndFAQ.rst
  openmp/libompd/src/CMakeLists.txt
  openmp/libomptarget/plugins/remote/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/Modules/FindOpenMPTarget.cmake
  openmp/tools/Modules/README.rst
  polly/CMakeLists.txt
  pstl/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -1,16 +1,13 @@
 # This file handles building LLVM runtime sub-projects.
-cmake_minimum_required(VERSION 3.13.4)
-if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-  message(WARNING
-"Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-"minimum version of CMake required to build LLVM will become 3.20.0, and "
-"using an older CMake will become an error. Please upgrade your CMake to "
-"at least 3.20.0 now to avoid issues in the future!")
-endif()
-project(Runtimes C CXX ASM)
+cmake_minimum_required(VERSION 3.20.0)
 
 # Add path for custom and the LLVM build's modules to the CMake module path.
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
+include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
+
+project(Runtimes C CXX ASM)
+
 list(INSERT CMAKE_MODULE_PATH 0
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.13.4)
+cmake_minimum_required(VERSION 3.20.0)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,14 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.13.4)
-  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-message(WARNING
-  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-  "minimum version of CMake required to build LLVM will become 3.20.0, and "
-  "using an older CMake will become an error. Please upgrade your CMake to "
-  "at least 3.20.0 now to avoid issues in the fu

[Lldb-commits] [PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

2023-05-24 Thread Mark de Wever via Phabricator via lldb-commits
Mordante added a comment.

I've created D151344  @glandium @hans @thakis 
I really would appreciate when you can test the patch locally to avoid another 
revert round.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144509

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


[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

2023-05-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.
Herald added a subscriber: JDevlieghere.

Thank you for making another try for the treewide change (which is admittedly 
very painful and not many people do such work).
Can you include the original patch description to the summary? That is the main 
part and the message "This reverts commit " is secondary. Readers should not 
need to trace through the revert history to obtain the original motivation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

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


[Lldb-commits] [PATCH] D151351: [lldb] Prevent dwim-print from showing kNoResult error

2023-05-24 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added reviewers: aprantl, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Expression evaluation for `void` valued expressions sets an error using the 
`kNoResult`
code. Like the `expression` command, `dwim-print` should also not print such 
errors.

Before:

  (lldb) dwim-print (void)printf("hi\n")
  hi
  Error: 'unknown error'

After:

  (lldb) dwim-print (void)printf("hi\n")
  hi

rdar://109746544


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151351

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -131,3 +131,9 @@
 self.runCmd("type summary add -e -s 'stub summary' Structure")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_void_result(self):
+"""Test dwim-print does not surface an error message for void 
expressions."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/ExpressionVariable.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -135,7 +136,8 @@
 expr);
   }
 
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
 
   if (suppress_result)
 if (auto result_var_sp =


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -131,3 +131,9 @@
 self.runCmd("type summary add -e -s 'stub summary' Structure")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_void_result(self):
+"""Test dwim-print does not surface an error message for void expressions."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/ExpressionVariable.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -135,7 +136,8 @@
 expr);
   }
 
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
 
   if (suppress_result)
 if (auto result_var_sp =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e108638 - [lldb] Disable `watchpoint_callback.test` temporarily on darwin

2023-05-24 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2023-05-24T10:58:40-07:00
New Revision: e1086384e5841e861cd19d5d980394cfcf94ef98

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

LOG: [lldb] Disable `watchpoint_callback.test` temporarily on darwin

This test started failing on the green-dragon bot, but after some
investigation, it doesn't have anything to do with Lua.

If we use a variable watchpoint with a condition using a scope variable,
if we go out-of-scope, the watpoint remains active which can the
expression evaluator to fail to parse the watchpoint condition (because
of the missing varible bindings).

For now, we should disable this test until we come up with a fix for it.

rdar://109574319

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test

Removed: 




diff  --git a/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test 
b/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
index 11bbe03410774..d72496c25c2ba 100644
--- a/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,4 +1,4 @@
-# XFAIL: system-netbsd
+# XFAIL: system-netbsd, system-darwin
 # RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
 # RUN: %lldb -b -s %S/Inputs/watchpoint1.in --script-language lua %t 2>&1 | 
FileCheck %S/Inputs/watchpoint1.in
 # RUN: %lldb -b -s %S/Inputs/watchpoint2.in --script-language lua %t 2>&1 | 
FileCheck %S/Inputs/watchpoint2.in



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


[Lldb-commits] [PATCH] D151269: [lldb] Pass CMAKE_SYSROOT through to lldb tests

2023-05-24 Thread Daniel Thornburgh via Phabricator via lldb-commits
mysterymath added a comment.

In D151269#4366484 , @JDevlieghere 
wrote:

> The change itself looks fine, but isn't this an issue for the API tests too? 
> If so, how is the sys root passed to `dotest.py` and can the shell tests do 
> the same?

Ah, thanks for the heads up. I couldn't find anything in the API tests that 
pass through a sysroot, so this may also be something to deal with there as 
well.
It's not very straightforward to reproduce this on our end though; the 
sysrooted builders we're working with don't yet have a suitable Python for 
LLDB, but we'll likely run into this the moment we try include one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151269

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


[Lldb-commits] [PATCH] D151269: [lldb] Pass CMAKE_SYSROOT through to lldb tests

2023-05-24 Thread Daniel Thornburgh via Phabricator via lldb-commits
mysterymath updated this revision to Diff 525282.
mysterymath added a comment.

Restrict description to Shell tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151269

Files:
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.site.cfg.py.in


Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -13,6 +13,7 @@
 # Since it comes from the command line, it may have backslashes which
 # should not need to be escaped.
 config.lldb_lit_tools_dir = lit_config.substitute(r"@LLDB_LIT_TOOLS_DIR@")
+config.cmake_sysroot = lit_config.substitute("@CMAKE_SYSROOT@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -44,6 +44,8 @@
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
 if config.objc_gnustep_dir:
 
build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
+if config.cmake_sysroot:
+build_script_args.append('--sysroot={0}'.format(config.cmake_sysroot))
 
 lldb_init = _get_lldb_init_path(config)
 
@@ -140,6 +142,9 @@
 # The clang module cache is used for building inferiors.
 host_flags += ['-fmodules-cache-path={}'.format(config.clang_module_cache)]
 
+if config.cmake_sysroot:
+host_flags += ['--sysroot={}'.format(config.cmake_sysroot)]
+
 host_flags = ' '.join(host_flags)
 config.substitutions.append(('%clang_host', '%clang ' + host_flags))
 config.substitutions.append(('%clangxx_host', '%clangxx ' + host_flags))
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -61,6 +61,12 @@
 default=False,
 help='Include and link GNUstep libobjc2 (Windows and Linux 
only)')
 
+parser.add_argument('--sysroot',
+metavar='directory',
+dest='sysroot',
+required=False,
+help='If specified, a sysroot to be passed via --sysroot')
+
 if sys.platform == 'darwin':
 parser.add_argument('--apple-sdk',
 metavar='apple_sdk',
@@ -254,6 +260,7 @@
"--objc-gnustep specified without path to libobjc2"
 self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 'include') 
if args.objc_gnustep_dir else None
 self.objc_gnustep_lib = os.path.join(args.objc_gnustep_dir, 'lib') if 
args.objc_gnustep_dir else None
+self.sysroot = args.sysroot
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -679,6 +686,8 @@
 args.extend(['-fobjc-runtime=gnustep-2.0', '-I', 
self.objc_gnustep_inc])
 if sys.platform == "win32":
 args.extend(['-Xclang', '-gcodeview', '-Xclang', 
'--dependent-lib=msvcrtd'])
+elif self.sysroot:
+args.extend(['--sysroot', self.sysroot])
 
 if self.std:
 args.append('-std={0}'.format(self.std))
@@ -713,6 +722,8 @@
 args.extend(['-Wl,-rpath,' + self.objc_gnustep_lib])
 elif sys.platform == 'win32':
 args.extend(['-fuse-ld=lld-link', '-g', '-Xclang', 
'--dependent-lib=msvcrtd'])
+elif self.sysroot:
+args.extend(['--sysroot', self.sysroot])
 
 return ('linking', self._obj_file_names(), self._exe_file_name(), 
None, args)
 


Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -13,6 +13,7 @@
 # Since it comes from the command line, it may have backslashes which
 # should not need to be escaped.
 config.lldb_lit_tools_dir = lit_config.substitute(r"@LLDB_LIT_TOOLS_DIR@")
+config.cmake_sysroot = lit_config.substitute("@CMAKE_SYSROOT@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -44,6 +44,8 @@
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
 if config.objc_gnustep_dir:
 build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
+if config.cmake_sysroot:
+build_script_args.append('--sysroot

[Lldb-commits] [PATCH] D151269: [lldb] Pass CMAKE_SYSROOT through to LLDB shell tests

2023-05-24 Thread Daniel Thornburgh via Phabricator via lldb-commits
mysterymath updated this revision to Diff 525283.
mysterymath added a comment.

Fixed commit message typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151269

Files:
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.site.cfg.py.in


Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -13,6 +13,7 @@
 # Since it comes from the command line, it may have backslashes which
 # should not need to be escaped.
 config.lldb_lit_tools_dir = lit_config.substitute(r"@LLDB_LIT_TOOLS_DIR@")
+config.cmake_sysroot = lit_config.substitute("@CMAKE_SYSROOT@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -44,6 +44,8 @@
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
 if config.objc_gnustep_dir:
 
build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
+if config.cmake_sysroot:
+build_script_args.append('--sysroot={0}'.format(config.cmake_sysroot))
 
 lldb_init = _get_lldb_init_path(config)
 
@@ -140,6 +142,9 @@
 # The clang module cache is used for building inferiors.
 host_flags += ['-fmodules-cache-path={}'.format(config.clang_module_cache)]
 
+if config.cmake_sysroot:
+host_flags += ['--sysroot={}'.format(config.cmake_sysroot)]
+
 host_flags = ' '.join(host_flags)
 config.substitutions.append(('%clang_host', '%clang ' + host_flags))
 config.substitutions.append(('%clangxx_host', '%clangxx ' + host_flags))
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -61,6 +61,12 @@
 default=False,
 help='Include and link GNUstep libobjc2 (Windows and Linux 
only)')
 
+parser.add_argument('--sysroot',
+metavar='directory',
+dest='sysroot',
+required=False,
+help='If specified, a sysroot to be passed via --sysroot')
+
 if sys.platform == 'darwin':
 parser.add_argument('--apple-sdk',
 metavar='apple_sdk',
@@ -254,6 +260,7 @@
"--objc-gnustep specified without path to libobjc2"
 self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 'include') 
if args.objc_gnustep_dir else None
 self.objc_gnustep_lib = os.path.join(args.objc_gnustep_dir, 'lib') if 
args.objc_gnustep_dir else None
+self.sysroot = args.sysroot
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -679,6 +686,8 @@
 args.extend(['-fobjc-runtime=gnustep-2.0', '-I', 
self.objc_gnustep_inc])
 if sys.platform == "win32":
 args.extend(['-Xclang', '-gcodeview', '-Xclang', 
'--dependent-lib=msvcrtd'])
+elif self.sysroot:
+args.extend(['--sysroot', self.sysroot])
 
 if self.std:
 args.append('-std={0}'.format(self.std))
@@ -713,6 +722,8 @@
 args.extend(['-Wl,-rpath,' + self.objc_gnustep_lib])
 elif sys.platform == 'win32':
 args.extend(['-fuse-ld=lld-link', '-g', '-Xclang', 
'--dependent-lib=msvcrtd'])
+elif self.sysroot:
+args.extend(['--sysroot', self.sysroot])
 
 return ('linking', self._obj_file_names(), self._exe_file_name(), 
None, args)
 


Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -13,6 +13,7 @@
 # Since it comes from the command line, it may have backslashes which
 # should not need to be escaped.
 config.lldb_lit_tools_dir = lit_config.substitute(r"@LLDB_LIT_TOOLS_DIR@")
+config.cmake_sysroot = lit_config.substitute("@CMAKE_SYSROOT@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -44,6 +44,8 @@
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
 if config.objc_gnustep_dir:
 build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
+if config.cmake_sysroot:
+build_script_args.append('--sysroot={0}'.form

[Lldb-commits] [PATCH] D150996: LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2023-05-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150996

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


[Lldb-commits] [lldb] 6006d43 - LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2023-05-24 Thread Craig Topper via lldb-commits

Author: Craig Topper
Date: 2023-05-24T12:40:10-07:00
New Revision: 6006d43e2d7dda56844f1c3867baa981cfefb8ea

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

LOG: LLVM_FALLTHROUGH => [[fallthrough]]. NFC

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/AST/TemplateBase.cpp
clang/lib/Basic/SourceManager.cpp
clang/lib/Sema/SemaChecking.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
llvm/lib/Analysis/MemoryLocation.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCRegisterInfo.h
llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Removed: 




diff  --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index c21e9c861875..c46b3e3d0c50 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -327,7 +327,7 @@ void TemplateArgument::Profile(llvm::FoldingSetNodeID &ID,
 
   case TemplateExpansion:
 ID.AddInteger(TemplateArg.NumExpansions);
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case Template:
 ID.AddPointer(TemplateArg.Name);
 break;

diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index f4ddae17f578..6fa802a33a50 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -1312,7 +1312,7 @@ LineOffsetMapping 
LineOffsetMapping::get(llvm::MemoryBufferRef Buffer,
 if (*Buf == '\n') {
   ++Buf;
 }
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case '\n':
 LineOffsets.push_back(Buf - Start);
   };

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 6c2cbc60a81d..c02f4f5a5269 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3792,7 +3792,7 @@ bool Sema::CheckLoongArchBuiltinFunctionCall(const 
TargetInfo &TI,
   return Diag(TheCall->getBeginLoc(),
   diag::err_loongarch_builtin_requires_la64)
  << TheCall->getSourceRange();
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case LoongArch::BI__builtin_loongarch_cacop_w: {
 if (BuiltinID == LoongArch::BI__builtin_loongarch_cacop_w &&
 !TI.hasFeature("32bit"))

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 8d8af21e1994..7b74129f848e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -511,14 +511,14 @@ ClangExpressionParser::ClangExpressionParser(
 break;
   case lldb::eLanguageTypeC_plus_plus_20:
 lang_opts.CPlusPlus20 = true;
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case lldb::eLanguageTypeC_plus_plus_17:
 // FIXME: add a separate case for CPlusPlus14. Currently folded into C++17
 // because C++14 is the default standard for Clang but enabling CPlusPlus14
 // expression evaluatino doesn't pass the test-suite cleanly.
 lang_opts.CPlusPlus14 = true;
 lang_opts.CPlusPlus17 = true;
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
   case lldb::eLanguageTypeC_plus_plus_14:

diff  --git a/llvm/lib/Analysis/MemoryLocation.cpp 
b/llvm/lib/Analysis/MemoryLocation.cpp
index e839f9e0dfb2..0404b32be848 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -257,7 +257,7 @@ MemoryLocation MemoryLocation::getForArgument(const 
CallBase *Call,
 
 case LibFunc_memset_chk:
   assert(ArgIdx == 0 && "Invalid argument index for memset_chk");
-  LLVM_FALLTHROUGH;
+  [[fallthrough]];
 case LibFunc_memcpy_chk: {
   assert((ArgIdx == 0 || ArgIdx == 1) &&
  "Invalid argument index for memcpy_chk");

diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp 
b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2ad787053f83..a700eaedd6c7 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -15325,7 +15325,7 @@ const SCEV *ScalarEvolution::applyLoopGuards(const SCEV 
*Expr, const Loop *L) {
 if (RHS->getType()->isPointerTy())
   return;
 RHS = getUMaxExpr(RHS, One);
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case CmpInst::ICM

[Lldb-commits] [PATCH] D150996: LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2023-05-24 Thread Craig Topper via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6006d43e2d7d: LLVM_FALLTHROUGH => [[fallthrough]]. NFC 
(authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150996

Files:
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Sema/SemaChecking.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/lib/Analysis/MemoryLocation.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCRegisterInfo.h
  llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Index: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
===
--- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8487,7 +8487,7 @@
   Ops[1] = SafeRHS;
   return new VPWidenRecipe(*I, make_range(Ops.begin(), Ops.end()));
 }
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   }
   case Instruction::Add:
   case Instruction::And:
Index: llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
===
--- llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
+++ llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
@@ -55,7 +55,7 @@
   switch (Intrinsic) {
   case Intrinsic::spv_load:
 AlignIdx = 2;
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case Intrinsic::spv_store: {
 if (I.getNumOperands() >= AlignIdx + 1) {
   auto *AlignOp = cast(I.getOperand(AlignIdx));
Index: llvm/lib/Target/PowerPC/PPCRegisterInfo.h
===
--- llvm/lib/Target/PowerPC/PPCRegisterInfo.h
+++ llvm/lib/Target/PowerPC/PPCRegisterInfo.h
@@ -183,7 +183,7 @@
   case 'f':
 if (RegName[1] == 'p')
   return RegName + 2;
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case 'r':
   case 'v':
 if (RegName[1] == 's') {
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -10686,7 +10686,7 @@
   RetOps.push_back(Extract);
   return DAG.getMergeValues(RetOps, dl);
 }
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   }
   case Intrinsic::ppc_vsx_disassemble_pair: {
 int NumVecs = 2;
Index: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -3994,7 +3994,7 @@
   if (SRLConst && SRLConst->getSExtValue() == 16)
 return false;
 }
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case ISD::ROTL:
   case ISD::SHL:
   case ISD::AND:
Index: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -2529,7 +2529,7 @@
   return 2 * LT.first;
 if (!Ty->getScalarType()->isFP128Ty())
   return LT.first;
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case ISD::FMUL:
   case ISD::FDIV:
 // These nodes are marked as 'custom' just to lower them to SVE.
Index: llvm/lib/ProfileData/InstrProf.cpp
===
--- llvm/lib/ProfileData/InstrProf.cpp
+++ llvm/lib/ProfileData/InstrProf.cpp
@@ -1385,7 +1385,7 @@
   case 10ull:
 H.TemporalProfTracesOffset =
 read(Buffer, offsetOf(&Header::TemporalProfTracesOffset));
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case 9ull:
 H.BinaryIdOffset = read(Buffer, offsetOf(&Header::BinaryIdOffset));
 [[fallthrough]];
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2420,7 +2420,7 @@
   case OMPScheduleType::BaseRuntimeSimd:
 assert(!ChunkSize &&
"schedule type does not support user-defined chunk sizes");
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   case OMPScheduleType::BaseDynamicChunked:
   case OMPScheduleType::BaseGuidedChunked:
   case OMPScheduleType::BaseGuidedIterativeChunked:
Index: llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
===
--- llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
++

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: jingham, JDevlieghere, bulbazord, aprantl.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

If we use a variable watchpoint with a condition using a scope variable,
if we go out-of-scope, the watpoint remains active which can the
expression evaluator to fail to parse the watchpoint condition (because
of the missing varible bindings).

This was discovered after `watchpoint_callback.test` started failing on
the green dragon bot.

This patch should address that issue by setting an internal breakpoint
on the return addresss of the current frame when creating a variable
watchpoint. The breakpoint has a callback that will disable the watchpoint
if the the breakpoint execution context matches the watchpoint execution
context.

This is only enabled for local variables.

This patch also re-enables the failing test following e1086384e584 
.

rdar://109574319

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151366

Files:
  lldb/include/lldb/Breakpoint/Watchpoint.h
  lldb/source/Breakpoint/Watchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
  lldb/test/Shell/Watchpoint/Inputs/val.c
  lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
  lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test

Index: lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
@@ -0,0 +1,3 @@
+# XFAIL: system-netbsd
+# RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
+# RUN: %lldb -b -s %S/Inputs/watchpoint.in %t 2>&1 | FileCheck %S/Inputs/watchpoint.in
Index: lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
@@ -0,0 +1,10 @@
+breakpoint set -p "Break here"
+r
+watchpoint set variable val
+c
+# CHECK: Watchpoint 1 hit:
+# CHECK-NEXT: old value: 0
+# CHECK-NEXT: new value: 10
+# CHECK-NEXT: stop reason = watchpoint 1
+c
+# CHECK: exited with status = 0 (0x)
Index: lldb/test/Shell/Watchpoint/Inputs/val.c
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/Inputs/val.c
@@ -0,0 +1,7 @@
+int main() {
+  int val = 0;
+  // Break here
+  val++;
+  val++;
+  return 0;
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,4 +1,4 @@
-# XFAIL: system-netbsd, system-darwin
+# XFAIL: system-netbsd
 # RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
 # RUN: %lldb -b -s %S/Inputs/watchpoint1.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint1.in
 # RUN: %lldb -b -s %S/Inputs/watchpoint2.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint2.in
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -9,6 +9,7 @@
 #include "CommandObjectWatchpoint.h"
 #include "CommandObjectWatchpointCommand.h"
 
+#include 
 #include 
 
 #include "llvm/ADT/StringRef.h"
@@ -20,6 +21,7 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandOptionArgumentTable.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/StackFrame.h"
@@ -953,11 +955,15 @@
 if (watch_sp) {
   watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
   watch_sp->SetWatchVariable(true);
-  if (var_sp && var_sp->GetDeclaration().GetFile()) {
-StreamString ss;
-// True to show fullpath for declaration file.
-var_sp->GetDeclaration().DumpStopContext(&ss, true);
-watch_sp->SetDeclInfo(std::string(ss.GetString()));
+  if (var_sp) {
+if (var_sp->GetDeclaration().GetFile()) {
+  StreamString ss;
+  // True to show fullpath for declaration file.
+  var_sp->GetDeclaration().DumpStopContext(&ss, true);
+  watch_sp->SetDeclInfo(std::string(ss.GetString()));
+}
+if (var_sp->GetScope() == eValueTypeVariableLocal)
+  watch_sp->SetupVariableWatchpointDisabler(m_exe_ctx.GetFrameSP());
   }
   output_stream.Printf("Watchpoint created: ");
   watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull);
Index: lldb/sour

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

2023-05-24 Thread Mike Hommey via Phabricator via lldb-commits
glandium added a comment.

I haven't tested this patch, but it looks like it contains everything that 
unbreaks our builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

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


[Lldb-commits] [PATCH] D150805: Rate limit progress reporting

2023-05-24 Thread Sterling Augustine via Phabricator via lldb-commits
saugustine added a comment.

Any more thoughts on this from the reviewers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805

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


[Lldb-commits] [PATCH] D151381: [lldb][NFCI] Include in SBDefines for FILE * definition

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, jingham, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

There are a few API headers that use FILE * but do not include the
correct header for their definition. Instead of including  in each
of the headers manually, it seems easiest to include it in SBDefines to
get them all at once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151381

Files:
  lldb/include/lldb/API/SBDefines.h


Index: lldb/include/lldb/API/SBDefines.h
===
--- lldb/include/lldb/API/SBDefines.h
+++ lldb/include/lldb/API/SBDefines.h
@@ -15,6 +15,8 @@
 #include "lldb/lldb-types.h"
 #include "lldb/lldb-versioning.h"
 
+#include  // For FILE *
+
 #ifndef LLDB_API
 #if defined(_WIN32)
 #if defined(LLDB_IN_LIBLLDB)


Index: lldb/include/lldb/API/SBDefines.h
===
--- lldb/include/lldb/API/SBDefines.h
+++ lldb/include/lldb/API/SBDefines.h
@@ -15,6 +15,8 @@
 #include "lldb/lldb-types.h"
 #include "lldb/lldb-versioning.h"
 
+#include  // For FILE *
+
 #ifndef LLDB_API
 #if defined(_WIN32)
 #if defined(LLDB_IN_LIBLLDB)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:86
 
+bool Watchpoint::SetupVariableWatchpointDisabler(StackFrameSP frame_sp) const {
+  ThreadSP thread_sp = frame_sp->GetThread();

Should you also verify that the `frame_sp` you got isn't `nullptr`? If it'll 
never be null, consider passing a `StackFrame &` instead.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:103-104
+  TargetSP target_sp = exe_ctx.GetTargetSP();
+  if (!target_sp)
+return false;
+

Is it even possible to have an `ExecutionContext` created from a `StackFrame` 
that doesn't have a valid `Target`? I wonder if we should assert that we got 
one here if not.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:129-133
+  if (!baton)
+return false;
+
+  if (!context)
+return false;

nit: You can merge this into one:
```
if (!baton || !context)
  return false;
```



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:140-141
+
+  LLDB_LOGF(log, "Watchpoint::%s called by breakpoint %" PRIu64 ".%" PRIu64,
+__FUNCTION__, break_id, break_loc_id);
+

Any reason to not use `LLDB_LOG` here? You'll get `__FILE__` and `__FUNCTION__` 
included.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:152
+  if (!process_sp)
+return true;
+

This should be `return false;`



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:165
+process_sp->DisableWatchpoint(watch_sp.get());
+return false;
+  }

This should be `return true;` no? You could invert the condition to be 
consistent with the patterns above of "return false if some condition wasn't 
met".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151384: [lldb] Override GetVariable in ValueObjectSynthetic (NFC)

2023-05-24 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added a reviewer: jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Make `GetVariable` a passthrough function the the underlying value object in 
`ValueObjectSynthetic`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151384

Files:
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h


Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -82,6 +82,10 @@
   : lldb::eNoDynamicValues);
   }
 
+  lldb::VariableSP GetVariable() override {
+return m_parent != nullptr ? m_parent->GetVariable() : nullptr;
+  }
+
   ValueObject *GetParent() override {
 return ((m_parent != nullptr) ? m_parent->GetParent() : nullptr);
   }


Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -82,6 +82,10 @@
   : lldb::eNoDynamicValues);
   }
 
+  lldb::VariableSP GetVariable() override {
+return m_parent != nullptr ? m_parent->GetVariable() : nullptr;
+  }
+
   ValueObject *GetParent() override {
 return ((m_parent != nullptr) ? m_parent->GetParent() : nullptr);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 3 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:103-104
+  TargetSP target_sp = exe_ctx.GetTargetSP();
+  if (!target_sp)
+return false;
+

bulbazord wrote:
> Is it even possible to have an `ExecutionContext` created from a `StackFrame` 
> that doesn't have a valid `Target`? I wonder if we should assert that we got 
> one here if not.
There is no `ExecutionContext::IsValid` method, so if you created an 
`ExecutionContext` with a bogus frame, you might get a nullptr.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:165
+process_sp->DisableWatchpoint(watch_sp.get());
+return false;
+  }

bulbazord wrote:
> This should be `return true;` no? You could invert the condition to be 
> consistent with the patterns above of "return false if some condition wasn't 
> met".
We want to always `return false` since this is a breakpoint callback, we never 
want to stop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 5 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:140-141
+
+  LLDB_LOGF(log, "Watchpoint::%s called by breakpoint %" PRIu64 ".%" PRIu64,
+__FUNCTION__, break_id, break_loc_id);
+

bulbazord wrote:
> Any reason to not use `LLDB_LOG` here? You'll get `__FILE__` and 
> `__FUNCTION__` included.
I just wanted to stay consistent with the rest of the file. May we be should 
standardize the logging format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Just so I understand the limitations:
This works for

  int main() {
int val = 0;
// Break here
val++;
val++;
return 0;
  }

but not for

  int main() {
{
  int val = 0;
  // Break here
  val++;
  val++;
}
{
  int other = 42;
 printf("assuming that other and val aliases");
}
return 0;
  }

?

To be clear, I think this is still useful!




Comment at: lldb/include/lldb/Breakpoint/Watchpoint.h:93
 
+  struct WatchpointVariableContext {
+WatchpointVariableContext(lldb::watch_id_t watch_id,

Doxygen comment?



Comment at: lldb/include/lldb/Breakpoint/Watchpoint.h:109
+
+  /// Callback routine which disable the watchpoint set on a variable when it
+  /// goes out of scope.

s/which/to/



Comment at: lldb/source/Breakpoint/Watchpoint.cpp:86
 
+bool Watchpoint::SetupVariableWatchpointDisabler(StackFrameSP frame_sp) const {
+  ThreadSP thread_sp = frame_sp->GetThread();

bulbazord wrote:
> Should you also verify that the `frame_sp` you got isn't `nullptr`? If it'll 
> never be null, consider passing a `StackFrame &` instead.
I subjectively feel like I've received a crash log for every single unchecked 
dereference in the LLDB code so far ;-)



Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:973
 } else {
   result.AppendErrorWithFormat(
   "Watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64

Convert to early exit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151381: [lldb][NFCI] Include in SBDefines for FILE * definition

2023-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere 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/D151381/new/

https://reviews.llvm.org/D151381

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


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 525367.
mib marked 4 inline comments as done.
mib added a comment.

Address @bulbazord & @aprantl comments.


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

https://reviews.llvm.org/D151366

Files:
  lldb/include/lldb/Breakpoint/Watchpoint.h
  lldb/source/Breakpoint/Watchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
  lldb/test/Shell/Watchpoint/Inputs/val.c
  lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
  lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test

Index: lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
@@ -0,0 +1,3 @@
+# XFAIL: system-netbsd
+# RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
+# RUN: %lldb -b -s %S/Inputs/watchpoint.in %t 2>&1 | FileCheck %S/Inputs/watchpoint.in
Index: lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
@@ -0,0 +1,13 @@
+breakpoint set -p "Break here"
+r
+watchpoint set variable val
+watchpoint modify -c "val == 1"
+c
+# CHECK: Watchpoint 1 hit:
+# CHECK-NEXT: old value: 0
+# CHECK-NEXT: new value: 1
+# CHECK-NEXT: Process {{[0-9]+}} resuming
+# CHECK-NEXT: Process {{[0-9]+}} stopped
+# CHECK-NEXT: {{.*}} stop reason = watchpoint 1
+c
+# CHECK: Process {{[0-9]+}} exited with status = 0 (0x)
Index: lldb/test/Shell/Watchpoint/Inputs/val.c
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/Inputs/val.c
@@ -0,0 +1,7 @@
+int main() {
+  int val = 0;
+  // Break here
+  val++;
+  val++;
+  return 0;
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,4 +1,4 @@
-# XFAIL: system-netbsd, system-darwin
+# XFAIL: system-netbsd
 # RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
 # RUN: %lldb -b -s %S/Inputs/watchpoint1.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint1.in
 # RUN: %lldb -b -s %S/Inputs/watchpoint2.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint2.in
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -9,6 +9,7 @@
 #include "CommandObjectWatchpoint.h"
 #include "CommandObjectWatchpointCommand.h"
 
+#include 
 #include 
 
 #include "llvm/ADT/StringRef.h"
@@ -20,6 +21,7 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandOptionArgumentTable.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/StackFrame.h"
@@ -950,27 +952,32 @@
 error.Clear();
 WatchpointSP watch_sp =
 target->CreateWatchpoint(addr, size, &compiler_type, watch_type, error);
-if (watch_sp) {
-  watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
-  watch_sp->SetWatchVariable(true);
-  if (var_sp && var_sp->GetDeclaration().GetFile()) {
-StreamString ss;
-// True to show fullpath for declaration file.
-var_sp->GetDeclaration().DumpStopContext(&ss, true);
-watch_sp->SetDeclInfo(std::string(ss.GetString()));
-  }
-  output_stream.Printf("Watchpoint created: ");
-  watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull);
-  output_stream.EOL();
-  result.SetStatus(eReturnStatusSuccessFinishResult);
-} else {
+if (!watch_sp) {
   result.AppendErrorWithFormat(
   "Watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64
   ", variable expression='%s').\n",
   addr, (uint64_t)size, command.GetArgumentAtIndex(0));
   if (error.AsCString(nullptr))
 result.AppendError(error.AsCString());
+  return result.Succeeded();
+}
+
+watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
+watch_sp->SetWatchVariable(true);
+if (var_sp) {
+  if (var_sp->GetDeclaration().GetFile()) {
+StreamString ss;
+// True to show fullpath for declaration file.
+var_sp->GetDeclaration().DumpStopContext(&ss, true);
+watch_sp->SetDeclInfo(std::string(ss.GetString()));
+  }
+  if (var_sp->GetScope() == eValueTypeVariableLocal)
+watch_sp->SetupVariableWatchpointDisabler(m_exe_ctx.GetFrameSP());
 }
+output_stream.Printf("Watchpoint created: ");
+watch_sp->GetDescription(&output_stream, lldb::eDescripti

[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

2023-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM

The help string for the setting seems clear.  There's also some logic to handle 
the setting vrs. the values we find from the stub which you describe in the 
comment to the review, but it would be nice to see that in a comment in the 
code somewhere to help out future generations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151292

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


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D151366#4370123 , @aprantl wrote:

> Just so I understand the limitations:
> This works for
>
>   int main() {
> int val = 0;
> // Break here
> val++;
> val++;
> return 0;
>   }
>
> but not for
>
>   int main() {
> {
>   int val = 0;
>   // Break here
>   val++;
>   val++;
> }
> {
>   int other = 42;
>   printf("assuming that other and val aliases");
> }
> return 0;
>   }
>
> ?
>
> To be clear, I think this is still useful!

@aprantl Do you know if we can detect the end of the scope ? I'm not sure it's 
possible currently ... But even if we could do it, that wouldn't cover the 
following case:

  int main() {
int i = 0;
{
  back_in_scope:
  int val = 0;
  i++;
  // Break here
  val++;
  val++;
}
{
  if (i == 1)
goto back_in_scope;
  printf("assuming that other and val aliases");
}
return 0;
  }

If we disable the `val` variable watchpoint at the end of the scope, if we get 
back into it (because of a `goto` or whatever), the watchpoint won't trigger 
because it will disabled (by leaving the scope).

I think in order to solve these kind of problem we should do proper static 
analysis to be able to answer those questions.


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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

2023-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

The array approach is cool but makes it hard to be backwards compatible: an old 
lldb is going to error out when presented with more than one value. If you made 
this two separate options, a client can use `settings set -e` to set the 
setting if it exists and still have valid low memory addresses if it doesn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151292

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


[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

2023-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: JDevlieghere, mib.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This was just a thinko.  The API StackFrame::GetVariableList takes a bool for 
"get_file_globals" which if true will also find file statics and file globals.  
But we only were passing that as true if the ValueType was 
eValueTypeVariableGlobal, which meant that we never find file statics.  It's 
okay if we cast too wide a net when we do GetVariableList as later on we check 
against the ValueType to filter globals from statics.

There was a test that had a whole bunch of globals and tested FindValue on all 
of them, but had no statics.  So I just made one of the globals a file static, 
which verifies the fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151392

Files:
  lldb/source/API/SBFrame.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py
  lldb/test/API/python_api/process/main.cpp


Index: lldb/test/API/python_api/process/main.cpp
===
--- lldb/test/API/python_api/process/main.cpp
+++ lldb/test/API/python_api/process/main.cpp
@@ -3,7 +3,7 @@
 
 // This simple program is to test the lldb Python API related to process.
 
-char my_char = 'u';
+static char my_char = 'u';
 char my_cstring[] = "lldb.SBProcess.ReadCStringFromMemory() works!";
 char *my_char_ptr = (char *)"Does it work?";
 uint32_t my_uint32 = 12345;
Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -52,8 +52,8 @@
 "There should be a thread stopped due to breakpoint")
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the file static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory 
and
@@ -149,8 +149,8 @@
 "There should be a thread stopped due to breakpoint")
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # If the variable does not have a load address, there's no sense
Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -604,7 +604,8 @@
 stop_if_block_is_inlined_function,
 [frame](Variable *v) { return v->IsInScope(frame); },
 &variable_list);
-  if (value_type == eValueTypeVariableGlobal) {
+  if (value_type == eValueTypeVariableGlobal 
+  || value_type == eValueTypeVariableStatic) {
 const bool get_file_globals = true;
 VariableList *frame_vars = frame->GetVariableList(get_file_globals,
   nullptr);


Index: lldb/test/API/python_api/process/main.cpp
===
--- lldb/test/API/python_api/process/main.cpp
+++ lldb/test/API/python_api/process/main.cpp
@@ -3,7 +3,7 @@
 
 // This simple program is to test the lldb Python API related to process.
 
-char my_char = 'u';
+static char my_char = 'u';
 char my_cstring[] = "lldb.SBProcess.ReadCStringFromMemory() works!";
 char *my_char_ptr = (char *)"Does it work?";
 uint32_t my_uint32 = 12345;
Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -52,8 +52,8 @@
 "There should be a thread stopped due to breakpoint")
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the file static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory and
@@ -149,8 +149,8 @@
 "There should be a thread stopped due to breakpoint")
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

2023-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere 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/D151392/new/

https://reviews.llvm.org/D151392

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


[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:1099-1103
   // 64 - T1Sz is the highest bit used for auth.
   // The value we pass in to SetVirtualAddressableBits is
   // the number of bits used for addressing, so if
   // T1Sz is 25, then 64-25 == 39, bits 0..38 are used for
   // addressing, bits 39..63 are used for PAC/TBI or whatever.

I think this comment needs to be updated? It doesn't look like you're calling 
`SetVirtualAddressableBits` here anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151292

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


[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

I have no objections but you should probably wait for others to take another 
look.


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

https://reviews.llvm.org/D151366

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


[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, jingham, mib, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch introduces FileSpec::GetComponents, a method that splits a
FileSpec's path into its individual components. For example, given
/foo/bar/baz, you'll get back a vector of strings {"foo", "bar", baz"}.

The motivation here is to reduce the use of
`FileSpec::RemoveLastPathComponent`. Mutating a FileSpec is expensive,
so providing a way of doing this without mutation is useful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151399

Files:
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp

Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -504,3 +504,33 @@
   EXPECT_FALSE(win_noext.IsSourceImplementationFile());
   EXPECT_FALSE(exe.IsSourceImplementationFile());
 }
+
+TEST(FileSpecTest, TestGetComponents) {
+  std::pair> PosixTests[] = {
+  {"/", {}},
+  {"/foo", {"foo"}},
+  {"/foo/", {"foo"}},
+  {"/foo/bar", {"foo", "bar"}},
+  {"/llvm-project/lldb/unittests/Utility/FileSpecTest.cpp",
+   {"llvm-project", "lldb", "unittests", "Utility", "FileSpecTest.cpp"}},
+  };
+
+  for (const auto &pair : PosixTests) {
+FileSpec file_spec = PosixSpec(pair.first);
+EXPECT_EQ(file_spec.GetComponents(), pair.second);
+  }
+
+  std::pair> WindowsTests[] = {
+  {"C:\\", {"C:"}},
+  {"C:\\Windows\\", {"C:", "Windows"}},
+  {"C:\\Windows\\System32", {"C:", "Windows", "System32"}},
+  {"C:\\llvm-project\\lldb\\unittests\\Utility\\FileSpecTest.cpp",
+   {"C:", "llvm-project", "lldb", "unittests", "Utility",
+"FileSpecTest.cpp"}},
+  };
+
+  for (const auto &pair : WindowsTests) {
+FileSpec file_spec = WindowsSpec(pair.first);
+EXPECT_EQ(file_spec.GetComponents(), pair.second);
+  }
+}
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -463,6 +463,26 @@
   }
   return false;
 }
+
+std::vector FileSpec::GetComponents() const {
+  std::vector components;
+
+  llvm::SmallString<64> current_path;
+  GetPath(current_path, false);
+
+  auto begin = llvm::sys::path::begin(current_path, m_style);
+  auto end = llvm::sys::path::end(current_path);
+
+  for (auto iter = begin; iter != end; ++iter) {
+if (*iter == "/" || *iter == ".")
+  continue;
+
+components.push_back(iter->str());
+  }
+
+  return components;
+}
+
 /// Returns true if the filespec represents an implementation source
 /// file (files with a ".c", ".cpp", ".m", ".mm" (many more)
 /// extension).
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1236,13 +1236,9 @@
 // "UIFoundation" and "UIFoundation.framework" -- most likely the latter
 // will be the one we find there.
 
-FileSpec platform_pull_upart(platform_file);
-std::vector path_parts;
-path_parts.push_back(platform_pull_upart.GetFilename().AsCString());
-while (platform_pull_upart.RemoveLastPathComponent()) {
-  ConstString part = platform_pull_upart.GetFilename();
-  path_parts.push_back(part.AsCString());
-}
+std::vector path_components = platform_file.GetComponents();
+// We want the components in reverse order.
+std::reverse(path_components.begin(), path_components.end());
 const size_t path_parts_size = path_parts.size();
 
 size_t num_module_search_paths = module_search_paths_ptr->GetSize();
Index: lldb/include/lldb/Utility/FileSpec.h
===
--- lldb/include/lldb/Utility/FileSpec.h
+++ lldb/include/lldb/Utility/FileSpec.h
@@ -408,6 +408,17 @@
   /// A boolean value indicating whether the path was updated.
   bool RemoveLastPathComponent();
 
+  /// Gets the components of the FileSpec's path.
+  /// For example, given the path:
+  ///   /System/Library/PrivateFrameworks/UIFoundation.framework/UIFoundation
+  ///
+  /// This function returns:
+  ///   {"System", "Library", "PrivateFrameworks", "UIFoundation.framework",
+  ///   "UIFoundation"}
+  /// \return
+  ///   A std::vector of std::strings for each path component.
+  std::vector GetComponents() const;
+
 protected:
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);
___
lldb-commit

[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

2023-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/FileSpec.h:420
+  ///   A std::vector of std::strings for each path component.
+  std::vector GetComponents() const;
+

I'm surprised this returns a vector of `std::string`s and not 
`llvm::StringRef`s. I would expect all the components to be part of the 
FileSpec's stored path. Even with the file and directory stored as separate 
`ConstString`s, that should be possible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151399

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


[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

2023-05-24 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/include/lldb/Utility/FileSpec.h:420
+  ///   A std::vector of std::strings for each path component.
+  std::vector GetComponents() const;
+

JDevlieghere wrote:
> I'm surprised this returns a vector of `std::string`s and not 
> `llvm::StringRef`s. I would expect all the components to be part of the 
> FileSpec's stored path. Even with the file and directory stored as separate 
> `ConstString`s, that should be possible?
Yes, it is possible to do `std::vector` here, especially 
because they would be backed by `ConstString`s which live forever. I chose 
`std::string` here because of the possibility that we one day no longer use 
`ConstString` to store the path, in which case the vector's StringRefs would 
only be valid for the lifetime of the FileSpec (or until it gets mutated).

Maybe I'm thinking too far ahead or planning for a future that will never come 
though. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151399

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


[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

2023-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/FileSpec.h:420
+  ///   A std::vector of std::strings for each path component.
+  std::vector GetComponents() const;
+

bulbazord wrote:
> JDevlieghere wrote:
> > I'm surprised this returns a vector of `std::string`s and not 
> > `llvm::StringRef`s. I would expect all the components to be part of the 
> > FileSpec's stored path. Even with the file and directory stored as separate 
> > `ConstString`s, that should be possible?
> Yes, it is possible to do `std::vector` here, especially 
> because they would be backed by `ConstString`s which live forever. I chose 
> `std::string` here because of the possibility that we one day no longer use 
> `ConstString` to store the path, in which case the vector's StringRefs would 
> only be valid for the lifetime of the FileSpec (or until it gets mutated).
> 
> Maybe I'm thinking too far ahead or planning for a future that will never 
> come though. What do you think?
I think it's reasonable to expect the lifetime of things handed out by a 
FileSpec match the lifetime of the FileSpec, but that depends on how we want to 
deal with mutability. If we want to be able to mutate a FileSpec in place, then 
you're right, these things need to have their own lifetime. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151399

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