[Lldb-commits] [lldb] [lldb] Update dwim-print to show expanded objc instances (PR #117500)

2024-11-27 Thread Dave Lee via lldb-commits


@@ -87,7 +87,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
   m_expr_options.m_verbosity, m_format_options.GetFormat());
-  dump_options.SetHideRootName(suppress_result);
+  dump_options.SetHideRootName(suppress_result)
+  .SetExpandPointerTypeFlags(lldb::eTypeIsObjC);

kastiglione wrote:

Here's my thinking: most of the time, I want top level pointers to be expanded 
(as c++ references are). But each of the C based language has its 
considerations.

For ObjC, I think all top level objects should be expanded, and I can't think 
of any counter arguments.

For C++, I also want top level pointers to be expanded – however raw pointers 
are less common. In C++ I would like to see top level smart pointers be 
expanded.

For C, I am less sure what, if anything, to do about top level pointers. First, 
I don't write a lot of C and have less experience to form opinions. Second, in 
C a pointer might be just as likely to be a buffer/array, and expanding a 
buffer as a single pointer would be misleading.

Coming back to your question: I would like to see C++ raw pointers expanded, 
and hopefully smart pointers too. I would leave C pointers alone. I guess I 
need to raise this on the forums, and find some buy in. As part of that, we can 
determine should pointer expansion be specific to `dwim-print`, or should 
some/all of it apply to `frame var`/`expr` too.

https://github.com/llvm/llvm-project/pull/117500
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)

2024-11-27 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

Updated patch to have a local `symtab_command` structure in ObjectFileMachO 
with larger offset fields, and read the load command fields into this 
structure.  Simplifies the patch; the exiting slide calculations can remain 
unmodified now that 64-bit offsets are being used.

https://github.com/llvm/llvm-project/pull/117832
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)

2024-11-27 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/117832

>From 00a429c14d159ebc42ac7c3a7e98a91851ece236 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Tue, 26 Nov 2024 17:56:06 -0800
Subject: [PATCH 1/2] [lldb][Mach-O] Handle shared cache binaries correctly

The Mach-O load commands have an LC_SYMTAB / struct symtab_command
which represents the offset of the symbol table (nlist records) and
string table for this binary.  In a mach-o binary on disk, these are
file offsets.  If a mach-o binary is loaded in memory with all
segments consecutive, the `symoff` and `stroff` are the offsets from
the TEXT segment (aka the mach-o header) virtual address to the
virtual address of the start of these tables.

However, if a Mach-O binary is a part of the shared cache, then the
segments will be separated -- they will have different slide values.
And it is possible for the LINKEDIT segment to be greater than 4GB
away from the TEXT segment in the virtual address space, so these
32-bit offsets cannot express the offset from TEXT segment to these
tables.

Create separate uint64_t variables to track the offset to the
symbol table and string table, instead of reusing the 32-bit ones
in the symtab_command structure.

rdar://140432279
---
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 26 ++-
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 079fd905037d45..5f047d84d53e73 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2244,6 +2244,18 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   // code.
   typedef AddressDataArray FunctionStarts;
 
+  // The virtual address offset from TEXT to the symbol/string tables
+  // in the LINKEDIT section.  The LC_SYMTAB symtab_command `symoff` and
+  // `stroff` are uint32_t's that give the file offset in the binary.
+  // If the binary is laid down in memory with all segments consecutive,
+  // then these are the offsets from the mach-o header aka TEXT segment
+  // to the tables' virtual addresses.
+  // But if the binary is loaded in virtual address space with different
+  // slides for the segments (e.g. a shared cache), the LINKEDIT may be
+  // more than 4GB away from TEXT, and a 32-bit offset is not sufficient.
+  offset_t symbol_table_offset_from_TEXT = 0;
+  offset_t string_table_offset_from_TEXT = 0;
+
   // Record the address of every function/data that we add to the symtab.
   // We add symbols to the table in the order of most information (nlist
   // records) to least (function starts), and avoid duplicating symbols
@@ -2282,6 +2294,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   if (m_data.GetU32(&offset, &symtab_load_command.symoff, 4) ==
   nullptr) // fill in symoff, nsyms, stroff, strsize fields
 return;
+  string_table_offset_from_TEXT = symtab_load_command.stroff;
+  symbol_table_offset_from_TEXT = symtab_load_command.symoff;
   break;
 
 case LC_DYLD_INFO:
@@ -2403,9 +2417,9 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
 
   const addr_t linkedit_file_offset = linkedit_section_sp->GetFileOffset();
   const addr_t symoff_addr = linkedit_load_addr +
- symtab_load_command.symoff -
+ symbol_table_offset_from_TEXT -
  linkedit_file_offset;
-  strtab_addr = linkedit_load_addr + symtab_load_command.stroff -
+  strtab_addr = linkedit_load_addr + string_table_offset_from_TEXT -
 linkedit_file_offset;
 
   // Always load dyld - the dynamic linker - from memory if we didn't
@@ -2473,17 +2487,17 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   lldb::addr_t linkedit_offset = linkedit_section_sp->GetFileOffset();
   lldb::offset_t linkedit_slide =
   linkedit_offset - m_linkedit_original_offset;
-  symtab_load_command.symoff += linkedit_slide;
-  symtab_load_command.stroff += linkedit_slide;
+  symbol_table_offset_from_TEXT += linkedit_slide;
+  string_table_offset_from_TEXT += linkedit_slide;
   dyld_info.export_off += linkedit_slide;
   dysymtab.indirectsymoff += linkedit_slide;
   function_starts_load_command.dataoff += linkedit_slide;
   exports_trie_load_command.dataoff += linkedit_slide;
 }
 
-nlist_data.SetData(m_data, symtab_load_command.symoff,
+nlist_data.SetData(m_data, symbol_table_offset_from_TEXT,
nlist_data_byte_size);
-strtab_data.SetData(m_data, symtab_load_command.stroff,
+strtab_data.SetData(m_data, string_table_offset_from_TEXT,
 strtab_data_byte_size);
 
 // We shouldn't have exports data from both the LC_DYLD_INFO command

>From 5761360c2ec87187efe50e43c2ddd1dfe

[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)

2024-11-27 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/117832

>From 00a429c14d159ebc42ac7c3a7e98a91851ece236 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Tue, 26 Nov 2024 17:56:06 -0800
Subject: [PATCH 1/3] [lldb][Mach-O] Handle shared cache binaries correctly

The Mach-O load commands have an LC_SYMTAB / struct symtab_command
which represents the offset of the symbol table (nlist records) and
string table for this binary.  In a mach-o binary on disk, these are
file offsets.  If a mach-o binary is loaded in memory with all
segments consecutive, the `symoff` and `stroff` are the offsets from
the TEXT segment (aka the mach-o header) virtual address to the
virtual address of the start of these tables.

However, if a Mach-O binary is a part of the shared cache, then the
segments will be separated -- they will have different slide values.
And it is possible for the LINKEDIT segment to be greater than 4GB
away from the TEXT segment in the virtual address space, so these
32-bit offsets cannot express the offset from TEXT segment to these
tables.

Create separate uint64_t variables to track the offset to the
symbol table and string table, instead of reusing the 32-bit ones
in the symtab_command structure.

rdar://140432279
---
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 26 ++-
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 079fd905037d45..5f047d84d53e73 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2244,6 +2244,18 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   // code.
   typedef AddressDataArray FunctionStarts;
 
+  // The virtual address offset from TEXT to the symbol/string tables
+  // in the LINKEDIT section.  The LC_SYMTAB symtab_command `symoff` and
+  // `stroff` are uint32_t's that give the file offset in the binary.
+  // If the binary is laid down in memory with all segments consecutive,
+  // then these are the offsets from the mach-o header aka TEXT segment
+  // to the tables' virtual addresses.
+  // But if the binary is loaded in virtual address space with different
+  // slides for the segments (e.g. a shared cache), the LINKEDIT may be
+  // more than 4GB away from TEXT, and a 32-bit offset is not sufficient.
+  offset_t symbol_table_offset_from_TEXT = 0;
+  offset_t string_table_offset_from_TEXT = 0;
+
   // Record the address of every function/data that we add to the symtab.
   // We add symbols to the table in the order of most information (nlist
   // records) to least (function starts), and avoid duplicating symbols
@@ -2282,6 +2294,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   if (m_data.GetU32(&offset, &symtab_load_command.symoff, 4) ==
   nullptr) // fill in symoff, nsyms, stroff, strsize fields
 return;
+  string_table_offset_from_TEXT = symtab_load_command.stroff;
+  symbol_table_offset_from_TEXT = symtab_load_command.symoff;
   break;
 
 case LC_DYLD_INFO:
@@ -2403,9 +2417,9 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
 
   const addr_t linkedit_file_offset = linkedit_section_sp->GetFileOffset();
   const addr_t symoff_addr = linkedit_load_addr +
- symtab_load_command.symoff -
+ symbol_table_offset_from_TEXT -
  linkedit_file_offset;
-  strtab_addr = linkedit_load_addr + symtab_load_command.stroff -
+  strtab_addr = linkedit_load_addr + string_table_offset_from_TEXT -
 linkedit_file_offset;
 
   // Always load dyld - the dynamic linker - from memory if we didn't
@@ -2473,17 +2487,17 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   lldb::addr_t linkedit_offset = linkedit_section_sp->GetFileOffset();
   lldb::offset_t linkedit_slide =
   linkedit_offset - m_linkedit_original_offset;
-  symtab_load_command.symoff += linkedit_slide;
-  symtab_load_command.stroff += linkedit_slide;
+  symbol_table_offset_from_TEXT += linkedit_slide;
+  string_table_offset_from_TEXT += linkedit_slide;
   dyld_info.export_off += linkedit_slide;
   dysymtab.indirectsymoff += linkedit_slide;
   function_starts_load_command.dataoff += linkedit_slide;
   exports_trie_load_command.dataoff += linkedit_slide;
 }
 
-nlist_data.SetData(m_data, symtab_load_command.symoff,
+nlist_data.SetData(m_data, symbol_table_offset_from_TEXT,
nlist_data_byte_size);
-strtab_data.SetData(m_data, symtab_load_command.stroff,
+strtab_data.SetData(m_data, string_table_offset_from_TEXT,
 strtab_data_byte_size);
 
 // We shouldn't have exports data from both the LC_DYLD_INFO command

>From 5761360c2ec87187efe50e43c2ddd1dfe

[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)

2024-11-27 Thread Pavel Labath via lldb-commits

labath wrote:

Great. Can you turn that into a test?

https://github.com/llvm/llvm-project/pull/117808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,43 @@
+//===-- HostInfoAIX.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_HOST_AIX_HOSTINFOAIX_H_
+#define LLDB_HOST_AIX_HOSTINFOAIX_H_
+
+#include "lldb/Host/posix/HostInfoPosix.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VersionTuple.h"
+
+#include 
+#include 

labath wrote:

```suggestion
#include "lldb/Host/posix/HostInfoPosix.h"
#include "lldb/Utility/FileSpec.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/VersionTuple.h"
#include 
#include 
```

LLVM style doesn't put empty lines between includes. LLDB did not do that 
historically, but let's not propagate that into the new files.

https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

2024-11-27 Thread Bill Wendling via lldb-commits

bwendling wrote:

Fine.

https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

2024-11-27 Thread Bill Wendling via lldb-commits

https://github.com/bwendling closed 
https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

2024-11-27 Thread Bill Wendling via lldb-commits

https://github.com/bwendling updated 
https://github.com/llvm/llvm-project/pull/116719

>From 2dcf18163de2ccce959f46bf82df1fa40e3fd1fc Mon Sep 17 00:00:00 2001
From: Bill Wendling 
Date: Fri, 15 Nov 2024 15:41:48 -0800
Subject: [PATCH 1/9] [Clang] Improve Sema diagnostic performance for
 __builtin_counted_by_ref

Implement the sema checks with a placeholder. We then check for that
placeholder in all of the places we care to emit a diagnostic.
---
 clang/include/clang/AST/ASTContext.h  |   1 +
 clang/include/clang/AST/BuiltinTypes.def  |   3 +
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +-
 .../include/clang/Serialization/ASTBitCodes.h |   5 +-
 clang/lib/AST/ASTContext.cpp  |   4 +
 clang/lib/AST/NSAPI.cpp   |   1 +
 clang/lib/AST/Type.cpp|   3 +
 clang/lib/AST/TypeLoc.cpp |   1 +
 clang/lib/Sema/SemaChecking.cpp   |   1 +
 clang/lib/Sema/SemaDecl.cpp   |  11 ++
 clang/lib/Sema/SemaExpr.cpp   | 138 +-
 clang/lib/Sema/SemaStmt.cpp   |  10 ++
 clang/lib/Serialization/ASTCommon.cpp |   3 +
 clang/lib/Serialization/ASTReader.cpp |   3 +
 clang/test/Modules/no-external-type-id.cppm   |   2 +-
 clang/test/Sema/builtin-counted-by-ref.c  |  77 +-
 .../TypeSystem/Clang/TypeSystemClang.cpp  |   2 +
 17 files changed, 154 insertions(+), 113 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 89fcb6789d880a..39cad95d911a33 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1184,6 +1184,7 @@ class ASTContext : public RefCountedBase {
   CanQualType DependentTy, OverloadTy, BoundMemberTy, UnresolvedTemplateTy,
   UnknownAnyTy;
   CanQualType BuiltinFnTy;
+  CanQualType BuiltinCountedByRefTy;
   CanQualType PseudoObjectTy, ARCUnbridgedCastTy;
   CanQualType ObjCBuiltinIdTy, ObjCBuiltinClassTy, ObjCBuiltinSelTy;
   CanQualType ObjCBuiltinBoolTy;
diff --git a/clang/include/clang/AST/BuiltinTypes.def 
b/clang/include/clang/AST/BuiltinTypes.def
index 444be4311a7431..4eae6962a46de6 100644
--- a/clang/include/clang/AST/BuiltinTypes.def
+++ b/clang/include/clang/AST/BuiltinTypes.def
@@ -314,6 +314,9 @@ PLACEHOLDER_TYPE(UnknownAny, UnknownAnyTy)
 
 PLACEHOLDER_TYPE(BuiltinFn, BuiltinFnTy)
 
+// A placeholder type for __builtin_counted_by_ref.
+PLACEHOLDER_TYPE(BuiltinCountedByRef, BuiltinCountedByRefTy)
+
 // The type of a cast which, in ARC, would normally require a
 // __bridge, but which might be okay depending on the immediate
 // context.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..37fb44d4bf74cd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6673,7 +6673,7 @@ def err_builtin_counted_by_ref_must_be_flex_array_member 
: Error<
 def err_builtin_counted_by_ref_cannot_leak_reference : Error<
   "value returned by '__builtin_counted_by_ref' cannot be assigned to a "
   "variable, have its address taken, or passed into or returned from a 
function">;
-def err_builtin_counted_by_ref_invalid_lhs_use : Error<
+def err_builtin_counted_by_ref_invalid_use : Error<
   "value returned by '__builtin_counted_by_ref' cannot be used in "
   "%select{an array subscript|a binary}0 expression">;
 def err_builtin_counted_by_ref_has_side_effects : Error<
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index fd834c14ce790f..f4b71861968e77 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -,6 +,9 @@ enum PredefinedTypeIDs {
   /// \brief The '__ibm128' type
   PREDEF_TYPE_IBM128_ID = 74,
 
+  /// \brief The placeholder type for __builtin_counted_by_ref.
+  PREDEF_TYPE_BUILTIN_COUNTED_BY_REF = 75,
+
 /// OpenCL image types with auto numeration
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix)   
\
   PREDEF_TYPE_##Id##_ID,
@@ -1148,7 +1151,7 @@ enum PredefinedTypeIDs {
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 513;
+const unsigned NUM_PREDEF_TYPE_IDS = 514;
 
 // Ensure we do not overrun the predefined types we reserved
 // in the enum PredefinedTypeIDs above.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 14fbadbc35ae5d..06226afaf23aab 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1390,6 +1390,10 @@ void ASTContext::InitBuiltinTypes(const TargetInfo 
&Target,
   if (LangOpts.MatrixTypes)
 InitBuiltinType(IncompleteMatrixIdxTy, BuiltinType::IncompleteMatrixIdx);
 
+  // Placeholder for __builtin_counted_by_ref().
+  if (!LangOpts.CPlusPlus)
+InitBuiltinType(

[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

2024-11-27 Thread Aaron Ballman via lldb-commits


@@ -14690,6 +14690,13 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
 }
   }
 
+  // The result of __builtin_counted_by_ref cannot be assigned to a variable.
+  // It allows leaking and modification of bounds safety information.
+  if (IsBuiltinCountedByRef(VD->getInit()))
+Diag(VD->getInit()->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+<< VD->getInit()->getSourceRange();

AaronBallman wrote:

It is the same in every case. Each time, the logic is:
```
if (IsBuiltinCountedByRef(SomeExpr))
  Diag(SomeExpr->getExprLoc(), 
diag::err_builtin_counted_by_ref_cannot_leak_reference) << 
SomeExpr->getSourceRange();
```
This would require changing some later to this equivalent:
```
  auto CheckBuiltinCountedByRef = [&](const Expr *E) {
if (BinaryOperator::isAssignmentOp(Opc))
  CheckInvalidBuiltinCountedByRef(E);
else if (IsBuiltinCountedByRef(E))
Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
<< 1 << E->getSourceRange();
  };
```

https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

2024-11-27 Thread Aaron Ballman via lldb-commits

https://github.com/AaronBallman approved this pull request.

LGTM modulo the request for a helper function (that two reviewers asked for the 
same thing is a pretty reasonable indication that we should probably have the 
helper, but if you feel strongly about it, I won't block the PR over it).

https://github.com/llvm/llvm-project/pull/116719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -32,35 +34,44 @@ using namespace lldb_dap;
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
-: debug_adaptor_path(path), broadcaster("lldb-dap"),
-  exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
-  stop_at_entry(false), is_attach(false),
+DAP::DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode,
+ std::vector pre_init_commands)
+: debug_adaptor_path(path), broadcaster("lldb-dap"), log(log),
+  exception_breakpoints(), pre_init_commands(pre_init_commands),
+  focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), 
is_attach(false),
   enable_auto_variable_summaries(false),
   enable_synthetic_child_debugging(false),
   display_extended_backtrace(false),
   restarting_process_id(LLDB_INVALID_PROCESS_ID),
   configuration_done_sent(false), waiting_for_run_in_terminal(false),
   progress_event_reporter(
   [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
-  reverse_request_seq(0), repl_mode(repl_mode) {
-  const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
-  // Windows opens stdout and stdin in text mode which converts \n to 13,10
-  // while the value is just 10 on Darwin/Linux. Setting the file mode to 
binary
-  // fixes this.
-  int result = _setmode(fileno(stdout), _O_BINARY);
-  assert(result);
-  result = _setmode(fileno(stdin), _O_BINARY);
-  UNUSED_IF_ASSERT_DISABLED(result);
-  assert(result);
-#endif
-  if (log_file_path)
-log.reset(new std::ofstream(log_file_path));
-}
+  reverse_request_seq(0), repl_mode(repl_mode) {}
 
 DAP::~DAP() = default;
 
+llvm::Error DAP::ConfigureIO(int out_fd, int err_fd) {
+  llvm::Expected new_stdout_fd =
+  RedirectFd(out_fd, [this](llvm::StringRef data) {
+SendOutput(OutputType::Stdout, data);
+  });

labath wrote:

> once the DAP object is not used anymore

I don't think you can really guarantee that while there are detached forwarding 
threads floating around. I think we need to shut those down first. The tricky 
part is doing that in cross-platform way, but since we're already all-in on 
lldbHost, I think we could/should use lldb's Pipe::ReadWithTimeout to implement 
polled reads (which check for a "should terminate flag" in between). Not the 
most elegant solution, but I it's probably the best we can do with what we have 
right now. One of these days I'm going to implement a MainLoop<->Pipe 
integration that can multiplex reads (and other things), but it's not very easy 
due to very different Pipe APIs on windows vs. unix.

I also believe this parts should be a separate patch, as there's a lot of 
subtleness involved.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -180,8 +195,9 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) 
{
 
   if (host_port->hostname == "*")
 host_port->hostname = "0.0.0.0";
-  std::vector addresses = SocketAddress::GetAddressInfo(
-  host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, 
IPPROTO_TCP);
+  std::vector addresses =
+  SocketAddress::GetAddressInfo(host_port->hostname.c_str(), nullptr,
+AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);

labath wrote:

It looks like you reformated the whole file.That's usually not a good idea as 
it produces spurious diffs like this.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -904,11 +928,14 @@ bool StartDebuggingRequestHandler::DoExecute(
   "startDebugging",
   llvm::json::Object{{"request", request},
  {"configuration", std::move(*configuration)}},
-  [](llvm::Expected value) {
+  [](auto dap, auto value) {

labath wrote:

[llvm 
policy](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable)
 is approximately to use `auto` only when the type is obvious from the context. 
That definitely isn't the case here.

I'm pretty sure this file was written by someone unaware of that policy, but 
let's not make it worse. And this is definitely making it worse. llvm::Expected 
can look very similar to std::optional, but it has a very important difference 
(the error must be checked), so it's very important to make the type obvious.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,182 @@
+//===-- Socket.h *- C++ 
-*-===//

labath wrote:

Cool. Thanks.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -118,6 +124,15 @@ std::string TCPSocket::GetRemoteConnectionURI() const {
   return "";
 }
 
+std::string TCPSocket::GetListeningConnectionURI() const {

labath wrote:

Let's put these into a separate patch (with a test case). I also believe these 
should return a `vector` as the listening socket can have more than one 
address. You can ignore the additional addresses in the caller if you don't 
need them.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -5028,48 +5021,128 @@ int main(int argc, char *argv[]) {
 #endif
 
   // Initialize LLDB first before we do anything.
-  lldb::SBDebugger::Initialize();
+  lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();
+  if (error.Fail()) {
+llvm::errs() << "Failed to initialize LLDB: " << error.GetCString() << 
"\n";
+return EXIT_FAILURE;
+  }
 
   // Terminate the debugger before the C++ destructor chain kicks in.
   auto terminate_debugger =
   llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
 
-  DAP dap = DAP(program_path.str(), default_repl_mode);
+  auto HandleClient = [=](int out_fd, int err_fd, StreamDescriptor input,
+  StreamDescriptor output) {
+DAP dap = DAP(program_path, log, default_repl_mode, pre_init_commands);
+dap.input.descriptor = std::move(input);
+dap.output.descriptor = std::move(output);
+RegisterRequestCallbacks(dap);
+
+if (auto Err = dap.ConfigureIO(out_fd, err_fd)) {
+  if (log)
+*log << "configureIO failed: " << llvm::toStringWithoutConsuming(Err)
+ << "\n";
+  std::cerr << "failed to configureIO: " << llvm::toString(std::move(Err))

labath wrote:

let's choose whether to use llvm::errs() or std::cerr.

cerr is [effectively 
banned](https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden),
 so I'd prefer the former, but I could be convinced to stick to it if 
everything else uses that.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits

labath wrote:

> I can split this into smaller patches.

Please, do that. Multithreading and servers and stuff are very tricky to get 
right, and it'd be a lot easier to review this if it was in smaller chunks. I 
think there's a lot of prepwork so that the code is prepared to handle multiple 
connections. That stuff can land without actually supporting multiple 
connections.

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -52,11 +51,11 @@ struct StreamDescriptor {
 struct InputStream {
   StreamDescriptor descriptor;
 
-  bool read_full(std::ofstream *log, size_t length, std::string &text);
+  bool read_full(llvm::raw_ostream *log, size_t length, std::string &text);
 
-  bool read_line(std::ofstream *log, std::string &line);
+  bool read_line(llvm::raw_ostream *log, std::string &line);
 
-  bool read_expected(std::ofstream *log, llvm::StringRef expected);
+  bool read_expected(llvm::raw_ostream *log, llvm::StringRef expected);

labath wrote:

(Note I'm not saying this isn't a good change. I'd be happy to review something 
like this as a separate patch. I just don't want to mix this in with the rest 
of the changes, as there's a lot of things going on here already.)

https://github.com/llvm/llvm-project/pull/116392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove child_process_inherit from the socket classes (PR #117699)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/117699

>From 6471ca1dfdf32d448c21b277dc9ea8cd1623317f Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Sat, 19 Oct 2024 18:17:23 +0200
Subject: [PATCH] [lldb] Remove child_process_inherit from the socket classes

It's never set to true. Also, using inheritable FDs in a multithreaded
process pretty much guarantees descriptor leaks. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.
---
 lldb/include/lldb/Host/Socket.h   | 18 +++
 lldb/include/lldb/Host/common/TCPSocket.h |  5 +-
 lldb/include/lldb/Host/common/UDPSocket.h |  4 +-
 lldb/include/lldb/Host/linux/AbstractSocket.h |  2 +-
 lldb/include/lldb/Host/posix/DomainSocket.h   |  7 ++-
 lldb/source/Host/common/Socket.cpp| 53 +++
 lldb/source/Host/common/TCPSocket.cpp | 25 -
 lldb/source/Host/common/UDPSocket.cpp | 14 ++---
 lldb/source/Host/linux/AbstractSocket.cpp |  3 +-
 .../posix/ConnectionFileDescriptorPosix.cpp   | 10 ++--
 lldb/source/Host/posix/DomainSocket.cpp   | 28 --
 .../gdb-remote/GDBRemoteCommunication.cpp |  3 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 12 ++---
 lldb/unittests/Host/MainLoopTest.cpp  |  7 +--
 lldb/unittests/Host/SocketTest.cpp| 15 +++---
 .../Host/SocketTestUtilities.cpp  | 10 ++--
 lldb/unittests/debugserver/RNBSocketTest.cpp  |  2 +-
 .../tools/lldb-server/tests/TestClient.cpp|  2 +-
 18 files changed, 83 insertions(+), 137 deletions(-)

diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 0e542b05a085c6..e98797b36c8a5d 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -94,7 +94,6 @@ class Socket : public IOObject {
   static void Terminate();
 
   static std::unique_ptr Create(const SocketProtocol protocol,
-bool child_processes_inherit,
 Status &error);
 
   virtual Status Connect(llvm::StringRef name) = 0;
@@ -115,14 +114,13 @@ class Socket : public IOObject {
   // implemented separately because the caller may wish to manipulate or query
   // the socket after it is initialized, but before entering a blocking accept.
   static llvm::Expected>
-  TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-int backlog = 5);
+  TcpListen(llvm::StringRef host_and_port, int backlog = 5);
 
   static llvm::Expected>
-  TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  TcpConnect(llvm::StringRef host_and_port);
 
   static llvm::Expected>
-  UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  UdpConnect(llvm::StringRef host_and_port);
 
   static int GetOption(NativeSocket sockfd, int level, int option_name,
int &option_value);
@@ -154,8 +152,7 @@ class Socket : public IOObject {
   virtual std::string GetRemoteConnectionURI() const { return ""; };
 
 protected:
-  Socket(SocketProtocol protocol, bool should_close,
- bool m_child_process_inherit);
+  Socket(SocketProtocol protocol, bool should_close);
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
@@ -163,15 +160,12 @@ class Socket : public IOObject {
   static Status GetLastError();
   static void SetLastError(Status &error);
   static NativeSocket CreateSocket(const int domain, const int type,
-   const int protocol,
-   bool child_processes_inherit, Status 
&error);
+   const int protocol, Status &error);
   static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
-   socklen_t *addrlen,
-   bool child_processes_inherit, Status 
&error);
+   socklen_t *addrlen, Status &error);
 
   SocketProtocol m_protocol;
   NativeSocket m_socket;
-  bool m_child_processes_inherit;
   bool m_should_close_fd;
 };
 
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h 
b/lldb/include/lldb/Host/common/TCPSocket.h
index eefe0240fe4a95..ca36622691fe9a 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -17,9 +17,8 @@
 namespace lldb_private {
 class TCPSocket : public Socket {
 public:
-  TCPSocket(bool should_close, bool child_processes_inherit);
-  TCPSocket(NativeSocket socket, bool should_close,
-bool child_processes_inherit);
+  explicit TCPSocket(bool should_close);
+  TCPSocket(NativeSocket socket, bool should_close);
   ~TCPSocket() override;
 
   // returns port number or 0 if error
diff --git a/lldb/include/lldb/Host/common/UDPSocket.h 
b/lldb/include/lldb/Host/common/UDPSocket.h
index 7348010d02ada2..63012d4fb6025f 100644
---

[Lldb-commits] [lldb] [lldb][dotest] Add an arg to add DLL search paths. (PR #104514)

2024-11-27 Thread Kendal Harland via lldb-commits

https://github.com/kendalharland closed 
https://github.com/llvm/llvm-project/pull/104514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX edited 
https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Dhruv Srivastava (DhruvSrivastavaX)


Changes

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. https://github.com/llvm/llvm-project/issues/101657
The complete changes for porting are present in this draft PR:
https://github.com/llvm/llvm-project/pull/102601

This is how we have currently added the support for HostInfo for AIX. 
It is taken directly from its Linux counterpart. 
Please let me know your suggestions about how can we plan to merge this and I 
will plan accordingly, and drop it as required. Can we keep it as a separate 
entity for AIX or we can combine it with something else? If you, how would like 
me to proceed in a way that is best for the code base as a whole.

Review Request: @labath @DavidSpickett

---
Full diff: https://github.com/llvm/llvm-project/pull/117906.diff


3 Files Affected:

- (added) lldb/include/lldb/Host/aix/HostInfoAIX.h (+43) 
- (modified) lldb/source/Host/CMakeLists.txt (+5) 
- (added) lldb/source/Host/aix/HostInfoAIX.cpp (+213) 


``diff
diff --git a/lldb/include/lldb/Host/aix/HostInfoAIX.h 
b/lldb/include/lldb/Host/aix/HostInfoAIX.h
new file mode 100644
index 00..c797b36f3dcc8d
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/HostInfoAIX.h
@@ -0,0 +1,43 @@
+//===-- HostInfoAIX.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_HOST_AIX_HOSTINFOAIX_H_
+#define LLDB_HOST_AIX_HOSTINFOAIX_H_
+
+#include "lldb/Host/posix/HostInfoPosix.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VersionTuple.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class HostInfoAIX : public HostInfoPosix {
+  friend class HostInfoBase;
+
+public:
+  static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
+
+  static llvm::VersionTuple GetOSVersion();
+  static std::optional GetOSBuildString();
+  static llvm::StringRef GetDistributionId();
+  static FileSpec GetProgramFileSpec();
+
+protected:
+  static bool ComputeSupportExeDirectory(FileSpec &file_spec);
+  static bool ComputeSystemPluginsDirectory(FileSpec &file_spec);
+  static bool ComputeUserPluginsDirectory(FileSpec &file_spec);
+  static void ComputeHostArchitectureSupport(ArchSpec &arch_32,
+ ArchSpec &arch_64);
+};
+} // namespace lldb_private
+
+#endif // LLDB_HOST_AIX_HOSTINFOAIX_H_
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index c2e091ee8555b7..e0cd8569bf9575 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -133,6 +133,11 @@ else()
   openbsd/Host.cpp
   openbsd/HostInfoOpenBSD.cpp
   )
+
+  elseif (CMAKE_SYSTEM_NAME MATCHES "AIX")
+add_host_subdirectory(aix
+  aix/HostInfoAIX.cpp
+  )
   endif()
 endif()
 
diff --git a/lldb/source/Host/aix/HostInfoAIX.cpp 
b/lldb/source/Host/aix/HostInfoAIX.cpp
new file mode 100644
index 00..e43ab3afd94657
--- /dev/null
+++ b/lldb/source/Host/aix/HostInfoAIX.cpp
@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+struct HostInfoAIXFields {
+  llvm::once_flag m_distribution_once_flag;
+  std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
+  llvm::VersionTuple m_os_version;
+};
+} // namespace
+
+static HostInfoAIXFields *g_fields = nullptr;
+
+void HostInfoAIX::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
+
+  g_fields = new HostInfoAIXFields();
+}
+
+void HostInfoAIX::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
+llvm::VersionTuple HostInfoAIX::GetOSVersion() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
+struct utsname un;
+i

[Lldb-commits] [lldb] [lldb][dotest] Add an arg to add DLL search paths. (PR #104514)

2024-11-27 Thread Kendal Harland via lldb-commits

kendalharland wrote:

Superceded by https://github.com/swiftlang/llvm-project/pull/9370

https://github.com/llvm/llvm-project/pull/104514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)

2024-11-27 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

> Is changing `struct symtab_command` to have the right-sized fields a no-go?

Yes, this is one possibility, and maybe the best.  The current structure 
reflects the in-binary layout, so it can be read (with endian fixing) on one go.

https://github.com/llvm/llvm-project/pull/117832
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX created 
https://github.com/llvm/llvm-project/pull/117906

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. https://github.com/llvm/llvm-project/issues/101657
The complete changes for porting are present in this draft PR:
https://github.com/llvm/llvm-project/pull/102601

This is how we have currently added the support for HostInfo for AIX. 
It is taken directly from its Linux counterpart. 
Please let me know your suggestions about how can we plan to merge this and I 
will plan accordingly, and drop it as required. Can we keep it as a separate 
entity for AIX or we can combine it with something else? If you, how would like 
me to proceed in a way that is best for the code base as a whole.

Review Request: @labath @DavidSpickett

>From d05de47c87362b54760f65c294c30c80b2d5bc9b Mon Sep 17 00:00:00 2001
From: Dhruv-Srivastava 
Date: Wed, 27 Nov 2024 10:10:32 -0600
Subject: [PATCH] HostInfoAIX

---
 lldb/include/lldb/Host/aix/HostInfoAIX.h |  43 +
 lldb/source/Host/CMakeLists.txt  |   5 +
 lldb/source/Host/aix/HostInfoAIX.cpp | 213 +++
 3 files changed, 261 insertions(+)
 create mode 100644 lldb/include/lldb/Host/aix/HostInfoAIX.h
 create mode 100644 lldb/source/Host/aix/HostInfoAIX.cpp

diff --git a/lldb/include/lldb/Host/aix/HostInfoAIX.h 
b/lldb/include/lldb/Host/aix/HostInfoAIX.h
new file mode 100644
index 00..c797b36f3dcc8d
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/HostInfoAIX.h
@@ -0,0 +1,43 @@
+//===-- HostInfoAIX.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_HOST_AIX_HOSTINFOAIX_H_
+#define LLDB_HOST_AIX_HOSTINFOAIX_H_
+
+#include "lldb/Host/posix/HostInfoPosix.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VersionTuple.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class HostInfoAIX : public HostInfoPosix {
+  friend class HostInfoBase;
+
+public:
+  static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
+
+  static llvm::VersionTuple GetOSVersion();
+  static std::optional GetOSBuildString();
+  static llvm::StringRef GetDistributionId();
+  static FileSpec GetProgramFileSpec();
+
+protected:
+  static bool ComputeSupportExeDirectory(FileSpec &file_spec);
+  static bool ComputeSystemPluginsDirectory(FileSpec &file_spec);
+  static bool ComputeUserPluginsDirectory(FileSpec &file_spec);
+  static void ComputeHostArchitectureSupport(ArchSpec &arch_32,
+ ArchSpec &arch_64);
+};
+} // namespace lldb_private
+
+#endif // LLDB_HOST_AIX_HOSTINFOAIX_H_
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index c2e091ee8555b7..e0cd8569bf9575 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -133,6 +133,11 @@ else()
   openbsd/Host.cpp
   openbsd/HostInfoOpenBSD.cpp
   )
+
+  elseif (CMAKE_SYSTEM_NAME MATCHES "AIX")
+add_host_subdirectory(aix
+  aix/HostInfoAIX.cpp
+  )
   endif()
 endif()
 
diff --git a/lldb/source/Host/aix/HostInfoAIX.cpp 
b/lldb/source/Host/aix/HostInfoAIX.cpp
new file mode 100644
index 00..e43ab3afd94657
--- /dev/null
+++ b/lldb/source/Host/aix/HostInfoAIX.cpp
@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+struct HostInfoAIXFields {
+  llvm::once_flag m_distribution_once_flag;
+  std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
+  llvm::VersionTuple m_os_version;
+};
+} // namespace
+
+static HostInfoAIXFields *g_fields = nullptr;
+
+void HostInfoAIX::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
+
+  g_fields = new HostInfoAIXFields();
+}
+
+void HostInfoAIX::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  Host

[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)

2024-11-27 Thread via lldb-commits

cmtice wrote:

Here's the test case I used:
$ cat main.cpp
  struct A {
struct {
  int x = 1;
};
int y = 2;6
  } a;

  struct B {
// Anonymous struct inherits another struct.
struct : public A {
  int z = 3;
};
int w = 4;
A a;
  } b;

  return 0; // Set a breakpoint here
}
$ clang++ -g -O0 main.cpp
$ lldb a.out

Break at 'main' and step to the return statement. Then
(lldb) frame var b.x
LLDB crashes, trying to pop the empty vector.


https://github.com/llvm/llvm-project/pull/117808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] build: cleanup extraneous include paths (PR #117615)

2024-11-27 Thread Saleem Abdulrasool via lldb-commits

https://github.com/compnerd edited 
https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] build: cleanup extraneous include paths (PR #117615)

2024-11-27 Thread Saleem Abdulrasool via lldb-commits

https://github.com/compnerd edited 
https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Miro Bucko via lldb-commits

https://github.com/mbucko updated 
https://github.com/llvm/llvm-project/pull/11

>From 4ba15cd7e64f8f4a2a5d804a1f986aafff9f6d17 Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Tue, 26 Nov 2024 12:07:57 -0800
Subject: [PATCH] Add 'FindFirstSymbolWithNameAndType()' to ModuleList.

---
 lldb/include/lldb/Core/ModuleList.h |  9 +
 lldb/source/Core/ModuleList.cpp | 12 
 2 files changed, 21 insertions(+)

diff --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..b122b1ce1d84ff 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -353,6 +353,15 @@ class ModuleList {
 
   lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
 
+  /// Find the first symbol that matches \a name and \a symbol_type.
+  /// \param[in] name
+  /// The name of the symbol we are looking for.
+  /// \param[in] symbol_type
+  /// The type of symbol we are looking for.
+  const Symbol *
+  FindFirstSymbolWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type) const;
+
   void FindSymbolsWithNameAndType(ConstString name,
   lldb::SymbolType symbol_type,
   SymbolContextList &sc_list) const;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2b8ccab2406c6b..572e4d640fa2b1 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -524,6 +524,18 @@ void ModuleList::FindGlobalVariables(const 
RegularExpression ®ex,
 module_sp->FindGlobalVariables(regex, max_matches, variable_list);
 }
 
+const Symbol *
+ModuleList::FindFirstSymbolWithNameAndType(ConstString name,
+   lldb::SymbolType symbol_type) const 
{
+  std::lock_guard guard(m_modules_mutex);
+  for (const ModuleSP &module_sp : m_modules) {
+if (const Symbol *symbol =
+module_sp->FindFirstSymbolWithNameAndType(name, symbol_type))
+  return symbol;
+  }
+  return nullptr;
+}
+
 void ModuleList::FindSymbolsWithNameAndType(ConstString name,
 SymbolType symbol_type,
 SymbolContextList &sc_list) const {

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


[Lldb-commits] [lldb] 24593f1 - [lldb] build: cleanup extraneous include paths (#117615)

2024-11-27 Thread via lldb-commits

Author: Saleem Abdulrasool
Date: 2024-11-27T08:39:15-08:00
New Revision: 24593f1814dc02c7404526674838ccfb1c61d780

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

LOG: [lldb] build: cleanup extraneous include paths (#117615)

Clean up some unnecessary include paths. The use of `LibXml2::LibXml2`
with `target_link_libraries` on `libLLDBHost` ensures that the header
search path is properly propagated.

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 93ccd9c479c2b8..ee4c2630d32e25 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -239,10 +239,6 @@ if (LLDB_ENABLE_LZMA)
   include_directories(${LIBLZMA_INCLUDE_DIRS})
 endif()
 
-if (LLDB_ENABLE_LIBXML2)
-  include_directories(${LIBXML2_INCLUDE_DIR})
-endif()
-
 include_directories(BEFORE
   ${CMAKE_CURRENT_BINARY_DIR}/include
   ${CMAKE_CURRENT_SOURCE_DIR}/include
@@ -283,7 +279,6 @@ if (APPLE)
   find_library(FOUNDATION_LIBRARY Foundation)
   find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)
   find_library(SECURITY_LIBRARY Security)
-  include_directories(${LIBXML2_INCLUDE_DIR})
 endif()
 
 if( WIN32 AND NOT CYGWIN )



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


[Lldb-commits] [lldb] [lldb] build: cleanup extraneous include paths (PR #117615)

2024-11-27 Thread Saleem Abdulrasool via lldb-commits

https://github.com/compnerd closed 
https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Miro Bucko via lldb-commits

mbucko wrote:

> Do you have a link to a PR that exercises this new API?

I'm only using it in an internal branch

https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to show expanded objc instances (PR #117500)

2024-11-27 Thread Dave Lee via lldb-commits


@@ -87,7 +87,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
   m_expr_options.m_verbosity, m_format_options.GetFormat());
-  dump_options.SetHideRootName(suppress_result);
+  dump_options.SetHideRootName(suppress_result)
+  .SetExpandPointerTypeFlags(lldb::eTypeIsObjC);

kastiglione wrote:

it definitely should be considered, but I didn't want to impede this change 
with those possibly more debatable changes. I have more thoughts on this, I'll 
write them up later today or this week.

https://github.com/llvm/llvm-project/pull/117500
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX edited 
https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to show expanded objc instances (PR #117500)

2024-11-27 Thread Dave Lee via lldb-commits

https://github.com/kastiglione edited 
https://github.com/llvm/llvm-project/pull/117500
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+struct HostInfoAIXFields {
+  llvm::once_flag m_distribution_once_flag;
+  std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
+  llvm::VersionTuple m_os_version;
+};
+} // namespace
+
+static HostInfoAIXFields *g_fields = nullptr;
+
+void HostInfoAIX::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
+
+  g_fields = new HostInfoAIXFields();
+}
+
+void HostInfoAIX::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
+llvm::VersionTuple HostInfoAIX::GetOSVersion() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
+struct utsname un;
+if (uname(&un) != 0)
+  return;
+
+llvm::StringRef release = un.release;
+// The kernel release string can include a lot of stuff (e.g.
+// 4.9.0-6-amd64). We're only interested in the numbered prefix.
+release = release.substr(0, release.find_first_not_of("0123456789."));
+g_fields->m_os_version.tryParse(release);
+  });
+
+  return g_fields->m_os_version;
+}
+
+std::optional HostInfoAIX::GetOSBuildString() {
+  struct utsname un;
+  ::memset(&un, 0, sizeof(utsname));
+
+  if (uname(&un) < 0)
+return std::nullopt;
+
+  return std::string(un.release);
+}

labath wrote:

I think we should move these to HostInfoPOSIX. `struct utsname` is a [part of 
POSIX](https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/) so it 
should at least compile everywhere. OSes (like the BSDs do now)  that -- for 
whatever reason -- want to retrieve them in a different way, can still override 
those implementations in  their own classes.

Can you create a patch to move the linux implementation to the base class?

https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Remove child_process_inherit from the socket classes (PR #117699)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/117699
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+struct HostInfoAIXFields {
+  llvm::once_flag m_distribution_once_flag;
+  std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
+  llvm::VersionTuple m_os_version;
+};
+} // namespace
+
+static HostInfoAIXFields *g_fields = nullptr;
+
+void HostInfoAIX::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
+
+  g_fields = new HostInfoAIXFields();
+}
+
+void HostInfoAIX::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
+llvm::VersionTuple HostInfoAIX::GetOSVersion() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
+struct utsname un;
+if (uname(&un) != 0)
+  return;
+
+llvm::StringRef release = un.release;
+// The kernel release string can include a lot of stuff (e.g.
+// 4.9.0-6-amd64). We're only interested in the numbered prefix.
+release = release.substr(0, release.find_first_not_of("0123456789."));
+g_fields->m_os_version.tryParse(release);
+  });
+
+  return g_fields->m_os_version;
+}
+
+std::optional HostInfoAIX::GetOSBuildString() {
+  struct utsname un;
+  ::memset(&un, 0, sizeof(utsname));
+
+  if (uname(&un) < 0)
+return std::nullopt;
+
+  return std::string(un.release);
+}
+
+llvm::StringRef HostInfoAIX::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
+  // Try to run 'lbs_release -i', and use that response for the distribution
+  // id.
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
+Log *log = GetLog(LLDBLog::Host);
+LLDB_LOGF(log, "attempting to determine AIX distribution...");
+
+// check if the lsb_release command exists at one of the following paths
+const char *const exe_paths[] = {"/bin/lsb_release",
+ "/usr/bin/lsb_release"};
+
+for (size_t exe_index = 0;
+ exe_index < sizeof(exe_paths) / sizeof(exe_paths[0]); ++exe_index) {
+  const char *const get_distribution_info_exe = exe_paths[exe_index];
+  if (access(get_distribution_info_exe, F_OK)) {
+// this exe doesn't exist, move on to next exe
+LLDB_LOGF(log, "executable doesn't exist: %s",
+  get_distribution_info_exe);
+continue;
+  }
+
+  // execute the distribution-retrieval command, read output
+  std::string get_distribution_id_command(get_distribution_info_exe);
+  get_distribution_id_command += " -i";
+
+  FILE *file = popen(get_distribution_id_command.c_str(), "r");
+  if (!file) {
+LLDB_LOGF(log,
+  "failed to run command: \"%s\", cannot retrieve "
+  "platform information",
+  get_distribution_id_command.c_str());
+break;
+  }
+
+  // retrieve the distribution id string.
+  char distribution_id[256] = {'\0'};
+  if (fgets(distribution_id, sizeof(distribution_id) - 1, file) !=
+  nullptr) {
+LLDB_LOGF(log, "distribution id command returned \"%s\"",
+  distribution_id);
+
+const char *const distributor_id_key = "Distributor ID:\t";
+if (strstr(distribution_id, distributor_id_key)) {
+  // strip newlines
+  std::string id_string(distribution_id + strlen(distributor_id_key));
+  llvm::erase(id_string, '\n');
+
+  // lower case it and convert whitespace to underscores
+  std::transform(
+  id_string.begin(), id_string.end(), id_string.begin(),
+  [](char ch) { return tolower(isspace(ch) ? '_' : ch); });
+
+  g_fields->m_distribution_id = id_string;
+  LLDB_LOGF(log, "distribution id set to \"%s\"",
+g_fields->m_distribution_id.c_str());
+} else {
+  LLDB_LOGF(log, "failed to find \"%s\" field in \"%s\"",
+distributor_id_key, distribution_id);
+}
+  } else {
+LLDB_LOGF(log,
+  "failed to retrieve distribution id, \"%s\" returned no"
+  " lines",
+  get_distribution_id_command.c_str());
+  }
+
+  // clean up the file
+  p

[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+struct HostInfoAIXFields {
+  llvm::once_flag m_distribution_once_flag;
+  std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
+  llvm::VersionTuple m_os_version;
+};
+} // namespace
+
+static HostInfoAIXFields *g_fields = nullptr;
+
+void HostInfoAIX::Initialize(SharedLibraryDirectoryHelper *helper) {
+  HostInfoPosix::Initialize(helper);
+
+  g_fields = new HostInfoAIXFields();
+}
+
+void HostInfoAIX::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
+llvm::VersionTuple HostInfoAIX::GetOSVersion() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
+struct utsname un;
+if (uname(&un) != 0)
+  return;
+
+llvm::StringRef release = un.release;
+// The kernel release string can include a lot of stuff (e.g.
+// 4.9.0-6-amd64). We're only interested in the numbered prefix.
+release = release.substr(0, release.find_first_not_of("0123456789."));
+g_fields->m_os_version.tryParse(release);
+  });
+
+  return g_fields->m_os_version;
+}
+
+std::optional HostInfoAIX::GetOSBuildString() {
+  struct utsname un;
+  ::memset(&un, 0, sizeof(utsname));
+
+  if (uname(&un) < 0)
+return std::nullopt;
+
+  return std::string(un.release);
+}
+
+llvm::StringRef HostInfoAIX::GetDistributionId() {

labath wrote:

Are you sure you need this part? I don't think it's being used for anything on 
linux even, and I suspect linux is a lot more diverse than AIX. Unless you're 
have a very specific use case for this in mind, I'd like to delete (i.e. not 
include) this highly questionable piece of code.

https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,213 @@
+//===-- HostInfoAIX.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/aix/HostInfoAIX.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include "llvm/Support/Threading.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 

labath wrote:

```suggestion
#include "lldb/Host/aix/HostInfoAIX.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "llvm/Support/Threading.h"
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
```

https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][AIX] HostInfoAIX Support (PR #117906)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

I think most of this file could be deleted or moved into HostInfoPOSIX. If we 
can do everything in the inline comments, then that will leave us with a couple 
of small functions that are copied from linux. We can then think about whether 
we want to merge those somehow, or we might just decide to live with it.

https://github.com/llvm/llvm-project/pull/117906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] c1dff71 - [lldb] Remove child_process_inherit from the socket classes (#117699)

2024-11-27 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-11-28T08:27:36+01:00
New Revision: c1dff7152592f1beee9059ee8e2cb3cc68baea4d

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

LOG: [lldb] Remove child_process_inherit from the socket classes (#117699)

It's never set to true. Also, using inheritable FDs in a multithreaded
process pretty much guarantees descriptor leaks. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/include/lldb/Host/common/TCPSocket.h
lldb/include/lldb/Host/common/UDPSocket.h
lldb/include/lldb/Host/linux/AbstractSocket.h
lldb/include/lldb/Host/posix/DomainSocket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/common/UDPSocket.cpp
lldb/source/Host/linux/AbstractSocket.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Host/posix/DomainSocket.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/tools/lldb-server/lldb-platform.cpp
lldb/unittests/Host/MainLoopTest.cpp
lldb/unittests/Host/SocketTest.cpp
lldb/unittests/TestingSupport/Host/SocketTestUtilities.cpp
lldb/unittests/debugserver/RNBSocketTest.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 0e542b05a085c6..e98797b36c8a5d 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -94,7 +94,6 @@ class Socket : public IOObject {
   static void Terminate();
 
   static std::unique_ptr Create(const SocketProtocol protocol,
-bool child_processes_inherit,
 Status &error);
 
   virtual Status Connect(llvm::StringRef name) = 0;
@@ -115,14 +114,13 @@ class Socket : public IOObject {
   // implemented separately because the caller may wish to manipulate or query
   // the socket after it is initialized, but before entering a blocking accept.
   static llvm::Expected>
-  TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
-int backlog = 5);
+  TcpListen(llvm::StringRef host_and_port, int backlog = 5);
 
   static llvm::Expected>
-  TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  TcpConnect(llvm::StringRef host_and_port);
 
   static llvm::Expected>
-  UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
+  UdpConnect(llvm::StringRef host_and_port);
 
   static int GetOption(NativeSocket sockfd, int level, int option_name,
int &option_value);
@@ -154,8 +152,7 @@ class Socket : public IOObject {
   virtual std::string GetRemoteConnectionURI() const { return ""; };
 
 protected:
-  Socket(SocketProtocol protocol, bool should_close,
- bool m_child_process_inherit);
+  Socket(SocketProtocol protocol, bool should_close);
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
@@ -163,15 +160,12 @@ class Socket : public IOObject {
   static Status GetLastError();
   static void SetLastError(Status &error);
   static NativeSocket CreateSocket(const int domain, const int type,
-   const int protocol,
-   bool child_processes_inherit, Status 
&error);
+   const int protocol, Status &error);
   static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
-   socklen_t *addrlen,
-   bool child_processes_inherit, Status 
&error);
+   socklen_t *addrlen, Status &error);
 
   SocketProtocol m_protocol;
   NativeSocket m_socket;
-  bool m_child_processes_inherit;
   bool m_should_close_fd;
 };
 

diff  --git a/lldb/include/lldb/Host/common/TCPSocket.h 
b/lldb/include/lldb/Host/common/TCPSocket.h
index eefe0240fe4a95..ca36622691fe9a 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -17,9 +17,8 @@
 namespace lldb_private {
 class TCPSocket : public Socket {
 public:
-  TCPSocket(bool should_close, bool child_processes_inherit);
-  TCPSocket(NativeSocket socket, bool should_close,
-bool child_processes_inherit);
+  explicit TCPSocket(bool should_close);
+  TCPSocket(NativeSocket socket, bool should_close);
   ~TCPSocket() override;
 
   // returns port number or 0 if error

diff  --git a/lldb/include/lldb/Host/common/UDPSocket.h 
b/lldb/include/lldb/Host/common/UDPSocket.h
index 7348010d02ada2..63012d4fb6025f 100644
--- a/lldb/include/lldb/Host/

[Lldb-commits] [lldb] [lldb][AIX] Header Parsing for XCOFF Object File in AIX (PR #116338)

2024-11-27 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX updated 
https://github.com/llvm/llvm-project/pull/116338

>From 0c63800bdcbadcfceed4c9a81305eda7d5a15960 Mon Sep 17 00:00:00 2001
From: Dhruv-Srivastava 
Date: Fri, 15 Nov 2024 02:16:31 -0600
Subject: [PATCH 1/4] Added XCOFF Header Parsing

---
 .../ObjectFile/XCOFF/ObjectFileXCOFF.cpp  | 126 +-
 .../ObjectFile/XCOFF/ObjectFileXCOFF.h|  58 
 2 files changed, 181 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.cpp 
b/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.cpp
index 3be900f9a4bc9f..c06ece4347822d 100644
--- a/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.cpp
@@ -81,9 +81,44 @@ ObjectFile *ObjectFileXCOFF::CreateInstance(const 
lldb::ModuleSP &module_sp,
   if (!objfile_up)
 return nullptr;
 
+  // Cache xcoff binary.
+  if (!objfile_up->CreateBinary())
+return nullptr;
+
+  if (!objfile_up->ParseHeader())
+return nullptr;
+
   return objfile_up.release();
 }
 
+bool ObjectFileXCOFF::CreateBinary() {
+  if (m_binary)
+return true;
+
+  Log *log = GetLog(LLDBLog::Object);
+
+  auto binary = llvm::object::XCOFFObjectFile::createObjectFile(
+  llvm::MemoryBufferRef(toStringRef(m_data.GetData()),
+m_file.GetFilename().GetStringRef()),
+  file_magic::xcoff_object_64);
+  if (!binary) {
+LLDB_LOG_ERROR(log, binary.takeError(),
+   "Failed to create binary for file ({1}): {0}", m_file);
+return false;
+  }
+
+  // Make sure we only handle XCOFF format.
+  m_binary =
+  llvm::unique_dyn_cast(std::move(*binary));
+  if (!m_binary)
+return false;
+
+  LLDB_LOG(log, "this = {0}, module = {1} ({2}), file = {3}, binary = {4}",
+   this, GetModule().get(), GetModule()->GetSpecificationDescription(),
+   m_file.GetPath(), m_binary.get());
+  return true;
+}
+
 ObjectFile *ObjectFileXCOFF::CreateMemoryInstance(
 const lldb::ModuleSP &module_sp, WritableDataBufferSP data_sp,
 const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {
@@ -136,13 +171,92 @@ bool ObjectFileXCOFF::MagicBytesMatch(DataBufferSP 
&data_sp,
   return XCOFFHeaderSizeFromMagic(magic) != 0;
 }
 
-bool ObjectFileXCOFF::ParseHeader() { return false; }
+bool ObjectFileXCOFF::ParseHeader() {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+std::lock_guard guard(module_sp->GetMutex());
+lldb::offset_t offset = 0;
+
+if (ParseXCOFFHeader(m_data, &offset, m_xcoff_header)) {
+  m_data.SetAddressByteSize(GetAddressByteSize());
+  if (m_xcoff_header.auxhdrsize > 0)
+ParseXCOFFOptionalHeader(m_data, &offset);
+}
+return true;
+  }
+
+  return false;
+}
+
+bool ObjectFileXCOFF::ParseXCOFFHeader(lldb_private::DataExtractor &data,
+   lldb::offset_t *offset_ptr,
+   xcoff_header_t &xcoff_header) {
+  // FIXME: data.ValidOffsetForDataOfSize
+  xcoff_header.magic = data.GetU16(offset_ptr);
+  xcoff_header.nsects = data.GetU16(offset_ptr);
+  xcoff_header.modtime = data.GetU32(offset_ptr);
+  xcoff_header.symoff = data.GetU64(offset_ptr);
+  xcoff_header.auxhdrsize = data.GetU16(offset_ptr);
+  xcoff_header.flags = data.GetU16(offset_ptr);
+  xcoff_header.nsyms = data.GetU32(offset_ptr);
+  return true;
+}
+
+bool ObjectFileXCOFF::ParseXCOFFOptionalHeader(
+lldb_private::DataExtractor &data, lldb::offset_t *offset_ptr) {
+  lldb::offset_t init_offset = *offset_ptr;
+  // FIXME: data.ValidOffsetForDataOfSize
+  m_xcoff_aux_header.AuxMagic = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.Version = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.ReservedForDebugger = data.GetU32(offset_ptr);
+  m_xcoff_aux_header.TextStartAddr = data.GetU64(offset_ptr);
+  m_xcoff_aux_header.DataStartAddr = data.GetU64(offset_ptr);
+  m_xcoff_aux_header.TOCAnchorAddr = data.GetU64(offset_ptr);
+  m_xcoff_aux_header.SecNumOfEntryPoint = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.SecNumOfText = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.SecNumOfData = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.SecNumOfTOC = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.SecNumOfLoader = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.SecNumOfBSS = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.MaxAlignOfText = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.MaxAlignOfData = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.ModuleType = data.GetU16(offset_ptr);
+  m_xcoff_aux_header.CpuFlag = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.CpuType = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.TextPageSize = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.DataPageSize = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.StackPageSize = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.FlagAndTDataAlignment = data.GetU8(offset_ptr);
+  m_xcoff_aux_header.TextSize = data.GetU64(offset_p

[Lldb-commits] [lldb] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page (PR #117860)

2024-11-27 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere approved this pull request.

LGTM. Thanks for adding this documentation! 

https://github.com/llvm/llvm-project/pull/117860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to support limited variable expression paths (PR #117452)

2024-11-27 Thread Michael Buch via lldb-commits


@@ -151,10 +151,23 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 result.SetStatus(eReturnStatusSuccessFinishResult);
   };
 
-  // First, try `expr` as the name of a frame variable.
-  if (frame) {
-auto valobj_sp = frame->FindVariable(ConstString(expr));
-if (valobj_sp && valobj_sp->GetError().Success()) {
+  // First, try `expr` as a _limited_ frame variable expression path: only the
+  // dot operator (`.`) is permitted for this case.
+  //
+  // This is limited to support only unambiguous expression paths. Of note,
+  // expression paths are not attempted if the expression contain either the
+  // arrow operator (`->`) or the subscript operator (`[]`). This is because
+  // both operators can be overloaded in C++, and could result in ambiguity in
+  // how the expression is handled. Additionally, `*` and `&` are not 
supported.
+  bool try_variable_path = expr.find_first_of("*&->[]") == StringRef::npos;

Michael137 wrote:

Nit:
```suggestion
  const bool try_variable_path = expr.find_first_of("*&->[]") == 
StringRef::npos;
```

https://github.com/llvm/llvm-project/pull/117452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to support limited variable expression paths (PR #117452)

2024-11-27 Thread Michael Buch via lldb-commits


@@ -154,5 +163,25 @@ def test_preserves_persistent_variables(self):
 def test_missing_type(self):
 """The expected output of po opaque is its address (no error)"""
 self.build()
-lldbutil.run_to_source_breakpoint(self, "break here", 
lldb.SBFileSpec("main.c"))
+lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp")
+)
 self.expect("dwim-print -O -- opaque", substrs=["0x"])
+
+def test_variable_expression_path(self):
+"""Test dwim-print supports certain variable expression paths."""
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp")
+)
+self.runCmd("settings set auto-one-line-summaries false")
+self._expect_cmd("dwim-print w.s", "frame variable")
+self._expect_cmd("dwim-print wp->s", "expression")

Michael137 wrote:

Should we also add cases for `*`/`[]`/`&`? Or are those tested elsewhere?

https://github.com/llvm/llvm-project/pull/117452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to support limited variable expression paths (PR #117452)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/117452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to support limited variable expression paths (PR #117452)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 approved this pull request.

Makes sense to me!

https://github.com/llvm/llvm-project/pull/117452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page (PR #117860)

2024-11-27 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

The meat of this is how we execute expressions and deal with the aftermath. For 
most users this will never be a concern, so it functions more as a design doc 
than anything else.

---
Full diff: https://github.com/llvm/llvm-project/pull/117860.diff


1 Files Affected:

- (modified) lldb/docs/use/aarch64-linux.md (+51) 


``diff
diff --git a/lldb/docs/use/aarch64-linux.md b/lldb/docs/use/aarch64-linux.md
index 393838dc0bb4f5..425336ddd81e2f 100644
--- a/lldb/docs/use/aarch64-linux.md
+++ b/lldb/docs/use/aarch64-linux.md
@@ -229,3 +229,54 @@ bytes.
 
 `zt0`'s value and whether it is active or not will be saved prior to
 expression evaluation and restored afterwards.
+
+## Guarded Control Stack Extension (GCS)
+
+GCS support includes the following new registers:
+
+* `gcs_features_enabled`
+* `gcs_features_locked`
+* `gcspr_el0`
+
+These map to the registers ptrace provides. The first two have had a `gcs_`
+prefix added as their names are too generic without it.
+
+When the GCS is enabled the kernel allocates a memory region for it. This 
region
+has a special attribute that LLDB will detect and presents like this:
+```
+  (lldb) memory region --all
+  <...>
+  [0xf7a0-0xf7e0) rw-
+  shadow stack: yes
+  [0xf7e0-0xf7e1) ---
+```
+
+`shadow stack` is a generic term used in the kernel for secure stack
+extensions like GCS.
+
+### Expression Evaluation
+
+To execute an expression, LLDB must push the return address of the expression
+wrapper (usually the entry point of the program) to the Guarded Control Stack.
+It does this by decrementing `gcspr_el0` and writing to the location that
+`gcspr_el0` then points to (instead of using the GCS push instructions).
+
+After an expression finishes, LLDB will restore the contents of all 3 
registers,
+apart from the enable bit of `gcs_features_enabled`.
+
+This is because there are limits on how often and from where you can set this
+value. We cannot enable GCS from ptrace at all and it is expected that a 
process
+that has enabled GCS then disabled it, will not enable it again. The simplest
+choice was to not restore the enable bit at all. It's up to the user or
+program to manage that value.
+
+The return address that was pushed onto the Guarded Control Stack will be left
+in place. As will any values that were pushed to the stack by functions run
+during the expresison.
+
+When the process resumes, `gcspr_el0` will be pointing to the original entry
+on the stack. So the other values will have no effect and likely be overwritten
+by future function calls.
+
+LLDB does not track and restore changes to general memory during expressions,
+so not restoring the GCS contents fits with the current behaviour.

``




https://github.com/llvm/llvm-project/pull/117860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page (PR #117860)

2024-11-27 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/117860

The meat of this is how we execute expressions and deal with the aftermath. For 
most users this will never be a concern, so it functions more as a design doc 
than anything else.

>From 4838ed0ef8a62041981e61a8d405251bb32c147d Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 27 Aug 2024 15:22:10 +0100
Subject: [PATCH] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page

The meat of this is how we execute expressions and deal with the
aftermath. For most users this will never be a concern, so it
functions more as a design doc than anything else.
---
 lldb/docs/use/aarch64-linux.md | 51 ++
 1 file changed, 51 insertions(+)

diff --git a/lldb/docs/use/aarch64-linux.md b/lldb/docs/use/aarch64-linux.md
index 393838dc0bb4f5..425336ddd81e2f 100644
--- a/lldb/docs/use/aarch64-linux.md
+++ b/lldb/docs/use/aarch64-linux.md
@@ -229,3 +229,54 @@ bytes.
 
 `zt0`'s value and whether it is active or not will be saved prior to
 expression evaluation and restored afterwards.
+
+## Guarded Control Stack Extension (GCS)
+
+GCS support includes the following new registers:
+
+* `gcs_features_enabled`
+* `gcs_features_locked`
+* `gcspr_el0`
+
+These map to the registers ptrace provides. The first two have had a `gcs_`
+prefix added as their names are too generic without it.
+
+When the GCS is enabled the kernel allocates a memory region for it. This 
region
+has a special attribute that LLDB will detect and presents like this:
+```
+  (lldb) memory region --all
+  <...>
+  [0xf7a0-0xf7e0) rw-
+  shadow stack: yes
+  [0xf7e0-0xf7e1) ---
+```
+
+`shadow stack` is a generic term used in the kernel for secure stack
+extensions like GCS.
+
+### Expression Evaluation
+
+To execute an expression, LLDB must push the return address of the expression
+wrapper (usually the entry point of the program) to the Guarded Control Stack.
+It does this by decrementing `gcspr_el0` and writing to the location that
+`gcspr_el0` then points to (instead of using the GCS push instructions).
+
+After an expression finishes, LLDB will restore the contents of all 3 
registers,
+apart from the enable bit of `gcs_features_enabled`.
+
+This is because there are limits on how often and from where you can set this
+value. We cannot enable GCS from ptrace at all and it is expected that a 
process
+that has enabled GCS then disabled it, will not enable it again. The simplest
+choice was to not restore the enable bit at all. It's up to the user or
+program to manage that value.
+
+The return address that was pushed onto the Guarded Control Stack will be left
+in place. As will any values that were pushed to the stack by functions run
+during the expresison.
+
+When the process resumes, `gcspr_el0` will be pointing to the original entry
+on the stack. So the other values will have no effect and likely be overwritten
+by future function calls.
+
+LLDB does not track and restore changes to general memory during expressions,
+so not restoring the GCS contents fits with the current behaviour.

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


[Lldb-commits] [lldb] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page (PR #117860)

2024-11-27 Thread David Spickett via lldb-commits

DavidSpickett wrote:

This is actually one of the last patches for GCS, but I'm putting it up first 
so you have something to refer to for the actual code changes.

https://github.com/llvm/llvm-project/pull/117860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)

2024-11-27 Thread Michael Buch via lldb-commits

Michael137 wrote:

Looks like this broke the macOS CI:
```
/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/unittests/debugserver/RNBSocketTest.cpp:123:33:
 error: no matching member function for call to 'Accept'
Status err = server_socket->Accept(connected_socket);
 ~~~^~
```
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/16045/execution/node/97/log/

Mind fixing up that test?

https://github.com/llvm/llvm-project/pull/117691
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Linux] Mark memory regions used for shadow stacks (PR #117861)

2024-11-27 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/117861

This is intended for use with Arm's Guarded Control Stack extension (GCS). 
Which reuses some existing shadow stack support in Linux. It should also work 
with the x86 equivalent.

A "ss" flag is added to the "VmFlags" line of shadow stack memory regions in 
/proc//smaps. To keep the naming generic I've called it shadow stack 
instead of guarded control stack.

Also the wording is "shadow stack: yes" because the shadow stack region is just 
where it's stored. It's enabled for the whole process or it isn't. As opposed 
to memory tagging which can be enabled per region, so "memory tagging: enabled" 
fits better for that.

I've added a test case that is also intended to be the start of a set of tests 
for GCS. This should help me avoid duplicating the inline assembly needed.

Note that no special compiler support is needed for the test. However, for the 
intial enabling of GCS (assuming the libc isn't doing it) we do need to use an 
inline assembly version of prctl.

This is because as soon as you enable GCS, all returns are checked against the 
GCS. If the GCS is empty, the program will fault. In other words, you can never 
return from the function that enabled GCS, unless you push values onto it 
(which is possible but not needed here).

So you cannot use the libc's prctl wrapper for this reason. You can use that 
wrapper for anything else, as we do to check if GCS is enabled.

>From ecac70b8d7b00e729b3203c6d5e6aff827775467 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Mon, 19 Aug 2024 15:55:45 +0100
Subject: [PATCH] [lldb][Linux] Mark memory regions used for shadow stacks

This is intended for use with Arm's Guarded Control Stack extension
(GCS). Which reuses some existing shadow stack support in Linux.
It should also work with the x86 equivalent.

A "ss" flag is added to the "VmFlags" line of shadow stack memory
regions in /proc//smaps. To keep the naming generic I've called
it shadow stack instead of guarded control stack.

Also the wording is "shadow stack: yes" because the shadow stack
region is just where it's stored. It's enabled for the whole process
or it isn't. As opposed to memory tagging which can be enabled per
region, so "memory tagging: enabled" fits better for that.

I've added a test case that is also intended to be the start of
a set of tests for GCS. This should help me avoid duplicating the
inline assembly needed.

Note that no special compiler support is needed for the test.
However, for the intial enabling of GCS (assuming the libc isn't
doing it) we do need to use an inline assembly version of prctl.

This is because as soon as you enable GCS, all returns are checked
against the GCS. If the GCS is empty, the program will fault.
In other words, you can never return from the function that enabled
GCS, unless you push values onto it (which is possible but not needed
here).

So you cannot use the libc's prctl wrapper for this reason. You can
use that wrapper for anything else, as we do to check if GCS is
enabled.
---
 lldb/include/lldb/Target/MemoryRegionInfo.h   |  22 ++-
 .../Python/lldbsuite/test/lldbtest.py |   3 +
 lldb/source/Commands/CommandObjectMemory.cpp  |   3 +
 .../Plugins/Process/Utility/LinuxProcMaps.cpp |  15 +-
 .../GDBRemoteCommunicationClient.cpp  |   7 +-
 .../GDBRemoteCommunicationServerLLGS.cpp  |  12 +-
 lldb/source/Target/MemoryRegionInfo.cpp   |   5 +-
 lldb/test/API/linux/aarch64/gcs/Makefile  |   3 +
 .../linux/aarch64/gcs/TestAArch64LinuxGCS.py  |  63 +++
 lldb/test/API/linux/aarch64/gcs/main.c|  54 ++
 .../Process/Utility/LinuxProcMapsTest.cpp | 156 +-
 .../MemoryTagManagerAArch64MTETest.cpp|  10 +-
 .../GDBRemoteCommunicationClientTest.cpp  |   5 +-
 .../Process/minidump/MinidumpParserTest.cpp   |  92 ++-
 14 files changed, 304 insertions(+), 146 deletions(-)
 create mode 100644 lldb/test/API/linux/aarch64/gcs/Makefile
 create mode 100644 lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
 create mode 100644 lldb/test/API/linux/aarch64/gcs/main.c

diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 66a4b3ed1b42d5..3e1272c2bba214 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -27,13 +27,13 @@ class MemoryRegionInfo {
   MemoryRegionInfo() = default;
   MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
OptionalBool execute, OptionalBool shared,
-   OptionalBool mapped, ConstString name,
-   OptionalBool flash, lldb::offset_t blocksize,
-   OptionalBool memory_tagged, OptionalBool stack_memory)
+   OptionalBool mapped, ConstString name, OptionalBool flash,
+   lldb::offset_t blocksize, OptionalBool memory_tagged,
+   Optio

[Lldb-commits] [lldb] [lldb][Linux] Mark memory regions used for shadow stacks (PR #117861)

2024-11-27 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

This is intended for use with Arm's Guarded Control Stack extension (GCS). 
Which reuses some existing shadow stack support in Linux. It should also work 
with the x86 equivalent.

A "ss" flag is added to the "VmFlags" line of shadow stack memory regions in 
/proc//smaps. To keep the naming generic I've called it shadow stack 
instead of guarded control stack.

Also the wording is "shadow stack: yes" because the shadow stack region is just 
where it's stored. It's enabled for the whole process or it isn't. As opposed 
to memory tagging which can be enabled per region, so "memory tagging: enabled" 
fits better for that.

I've added a test case that is also intended to be the start of a set of tests 
for GCS. This should help me avoid duplicating the inline assembly needed.

Note that no special compiler support is needed for the test. However, for the 
intial enabling of GCS (assuming the libc isn't doing it) we do need to use an 
inline assembly version of prctl.

This is because as soon as you enable GCS, all returns are checked against the 
GCS. If the GCS is empty, the program will fault. In other words, you can never 
return from the function that enabled GCS, unless you push values onto it 
(which is possible but not needed here).

So you cannot use the libc's prctl wrapper for this reason. You can use that 
wrapper for anything else, as we do to check if GCS is enabled.

---

Patch is 39.11 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/117861.diff


14 Files Affected:

- (modified) lldb/include/lldb/Target/MemoryRegionInfo.h (+14-8) 
- (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+3) 
- (modified) lldb/source/Commands/CommandObjectMemory.cpp (+3) 
- (modified) lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp (+10-5) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+4-3) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
(+9-3) 
- (modified) lldb/source/Target/MemoryRegionInfo.cpp (+3-2) 
- (added) lldb/test/API/linux/aarch64/gcs/Makefile (+3) 
- (added) lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py (+63) 
- (added) lldb/test/API/linux/aarch64/gcs/main.c (+54) 
- (modified) lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp (+79-77) 
- (modified) lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp 
(+4-6) 
- (modified) 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (+4-1) 
- (modified) lldb/unittests/Process/minidump/MinidumpParserTest.cpp (+51-41) 


``diff
diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 66a4b3ed1b42d5..3e1272c2bba214 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -27,13 +27,13 @@ class MemoryRegionInfo {
   MemoryRegionInfo() = default;
   MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
OptionalBool execute, OptionalBool shared,
-   OptionalBool mapped, ConstString name,
-   OptionalBool flash, lldb::offset_t blocksize,
-   OptionalBool memory_tagged, OptionalBool stack_memory)
+   OptionalBool mapped, ConstString name, OptionalBool flash,
+   lldb::offset_t blocksize, OptionalBool memory_tagged,
+   OptionalBool stack_memory, OptionalBool shadow_stack)
   : m_range(range), m_read(read), m_write(write), m_execute(execute),
 m_shared(shared), m_mapped(mapped), m_name(name), m_flash(flash),
 m_blocksize(blocksize), m_memory_tagged(memory_tagged),
-m_is_stack_memory(stack_memory) {}
+m_is_stack_memory(stack_memory), m_is_shadow_stack(shadow_stack) {}
 
   RangeType &GetRange() { return m_range; }
 
@@ -55,6 +55,8 @@ class MemoryRegionInfo {
 
   OptionalBool GetMemoryTagged() const { return m_memory_tagged; }
 
+  OptionalBool IsShadowStack() const { return m_is_shadow_stack; }
+
   void SetReadable(OptionalBool val) { m_read = val; }
 
   void SetWritable(OptionalBool val) { m_write = val; }
@@ -77,6 +79,8 @@ class MemoryRegionInfo {
 
   void SetMemoryTagged(OptionalBool val) { m_memory_tagged = val; }
 
+  void SetIsShadowStack(OptionalBool val) { m_is_shadow_stack = val; }
+
   // Get permissions as a uint32_t that is a mask of one or more bits from the
   // lldb::Permissions
   uint32_t GetLLDBPermissions() const {
@@ -101,12 +105,13 @@ class MemoryRegionInfo {
   bool operator==(const MemoryRegionInfo &rhs) const {
 return m_range == rhs.m_range && m_read == rhs.m_read &&
m_write == rhs.m_write && m_execute == rhs.m_execute &&
-   m_shared == rhs.m_shared &&
-   m_mapped == rhs.m_mapped && m_name == rhs.m_name &&
-   m_flash == rhs.m_flash && 

[Lldb-commits] [lldb] 0723870 - [lldb] Add timeout argument to Socket::Accept (#117691)

2024-11-27 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-11-27T09:50:33+01:00
New Revision: 072387042021b0f74c24c617b940fe8157f9f1a5

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

LOG: [lldb] Add timeout argument to Socket::Accept (#117691)

Allows us to stop waiting for a connection if it doesn't come in a
certain amount of time. Right now, I'm keeping the status quo (infitnite
wait) in the "production" code, but using smaller (finite) values in
tests. (A lot of these tests create "loopback" connections, where a
really short wait is sufficient: on linux at least even a poll (0s wait)
is sufficient if the other end has connect()ed already, but this doesn't
seem to be the case on Windows, so I'm using a 1s wait in these cases).

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/unittests/Host/MainLoopTest.cpp
lldb/unittests/Host/SocketTest.cpp
lldb/unittests/TestingSupport/Host/SocketTestUtilities.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 14468c98ac5a3a..0e542b05a085c6 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -13,6 +13,7 @@
 #include 
 
 #include "lldb/Host/MainLoopBase.h"
+#include "lldb/Utility/Timeout.h"
 #include "lldb/lldb-private.h"
 
 #include "lldb/Host/SocketAddress.h"
@@ -108,7 +109,7 @@ class Socket : public IOObject {
 
   // Accept a single connection and "return" it in the pointer argument. This
   // function blocks until the connection arrives.
-  virtual Status Accept(Socket *&socket);
+  virtual Status Accept(const Timeout &timeout, Socket *&socket);
 
   // Initialize a Tcp Socket object in listening mode.  listen and accept are
   // implemented separately because the caller may wish to manipulate or query

diff  --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index d69eb608204033..63396f7b4abc9c 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -460,7 +460,8 @@ NativeSocket Socket::CreateSocket(const int domain, const 
int type,
   return sock;
 }
 
-Status Socket::Accept(Socket *&socket) {
+Status Socket::Accept(const Timeout &timeout, Socket *&socket) {
+  socket = nullptr;
   MainLoop accept_loop;
   llvm::Expected> expected_handles =
   Accept(accept_loop,
@@ -470,7 +471,15 @@ Status Socket::Accept(Socket *&socket) {
  });
   if (!expected_handles)
 return Status::FromError(expected_handles.takeError());
-  return accept_loop.Run();
+  if (timeout) {
+accept_loop.AddCallback(
+[](MainLoopBase &loop) { loop.RequestTermination(); }, *timeout);
+  }
+  if (Status status = accept_loop.Run(); status.Fail())
+return status;
+  if (socket)
+return Status();
+  return Status(std::make_error_code(std::errc::timed_out));
 }
 
 NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,

diff  --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 8a03e47ef3d900..903bfc50def3aa 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -543,7 +543,7 @@ lldb::ConnectionStatus 
ConnectionFileDescriptor::AcceptSocket(
 
   if (!error.Fail()) {
 post_listen_callback(*listening_socket);
-error = listening_socket->Accept(accepted_socket);
+error = listening_socket->Accept(/*timeout=*/std::nullopt, 
accepted_socket);
   }
 
   if (!error.Fail()) {

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 7eacd605362e7c..67b41b1e77a533 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1223,10 +1223,6 @@ 
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
   listen_socket.Listen("localhost:0", backlog).ToError())
 return error;
 
-  Socket *accept_socket = nullptr;
-  std::future accept_status = std::async(
-  std::launch::async, [&] { return listen_socket.Accept(accept_socket); });
-
   llvm::SmallString<32> remote_addr;
   llvm::raw_svector_ostream(remote_addr)
   << "connect://localhost:" << listen_socket.GetLocalPortNumber();
@@ -1238,10 +1234,15 @@ 
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
 

[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/117691
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e8a01e7 - [lldb] Make sure Blocks always have a parent (#117683)

2024-11-27 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-11-27T09:51:59+01:00
New Revision: e8a01e75e6b17e84710e7cc3bfb70d95a3a696a2

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

LOG: [lldb] Make sure Blocks always have a parent (#117683)

It's basically true already (except for a brief time during
construction). This patch makes sure the objects are constructed with a
valid parent and enforces this in the type system, which allows us to
get rid of some nullptr checks.

Added: 


Modified: 
lldb/include/lldb/Symbol/Block.h
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/Block.h 
b/lldb/include/lldb/Symbol/Block.h
index c9c4d5ad767d7e..7c7a66de831998 100644
--- a/lldb/include/lldb/Symbol/Block.h
+++ b/lldb/include/lldb/Symbol/Block.h
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
   typedef RangeVector RangeList;
   typedef RangeList::Entry Range;
 
-  /// Construct with a User ID \a uid, \a depth.
-  ///
-  /// Initialize this block with the specified UID \a uid. The \a depth in the
-  /// \a block_list is used to represent the parent, sibling, and child block
-  /// information and also allows for partial parsing at the block level.
+  // Creates a block representing the whole function. Only meant to be used 
from
+  // the Function class.
+  Block(Function &function, lldb::user_id_t function_uid);
+
+  ~Block() override;
+
+  /// Creates a block with the specified UID \a uid.
   ///
   /// \param[in] uid
   /// The UID for a given block. This value is given by the
@@ -56,19 +58,7 @@ class Block : public UserID, public SymbolContextScope {
   /// information data that it parses for further or more in
   /// depth parsing. Common values would be the index into a
   /// table, or an offset into the debug information.
-  ///
-  /// \see BlockList
-  Block(lldb::user_id_t uid);
-
-  /// Destructor.
-  ~Block() override;
-
-  /// Add a child to this object.
-  ///
-  /// \param[in] child_block_sp
-  /// A shared pointer to a child block that will get added to
-  /// this block.
-  void AddChild(const lldb::BlockSP &child_block_sp);
+  lldb::BlockSP CreateChild(lldb::user_id_t uid);
 
   /// Add a new offset range to this block.
   void AddRange(const Range &range);
@@ -317,10 +307,6 @@ class Block : public UserID, public SymbolContextScope {
   const Declaration *decl_ptr,
   const Declaration *call_decl_ptr);
 
-  void SetParentScope(SymbolContextScope *parent_scope) {
-m_parent_scope = parent_scope;
-  }
-
   /// Set accessor for the variable list.
   ///
   /// Called by the SymbolFile plug-ins after they have parsed the variable
@@ -364,7 +350,7 @@ class Block : public UserID, public SymbolContextScope {
 protected:
   typedef std::vector collection;
   // Member variables.
-  SymbolContextScope *m_parent_scope;
+  SymbolContextScope &m_parent_scope;
   collection m_children;
   RangeList m_ranges;
   lldb::InlineFunctionInfoSP m_inlineInfoSP; ///< Inlined function information.
@@ -382,6 +368,8 @@ class Block : public UserID, public SymbolContextScope {
 private:
   Block(const Block &) = delete;
   const Block &operator=(const Block &) = delete;
+
+  Block(lldb::user_id_t uid, SymbolContextScope &parent_scope);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp 
b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index fadc19676609bf..df3bf157278daf 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -315,7 +315,8 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function 
&func) {
   if (record->InlineNestLevel == 0 ||
   record->InlineNestLevel <= last_added_nest_level + 1) {
 last_added_nest_level = record->InlineNestLevel;
-BlockSP block_sp = std::make_shared(It.GetBookmark().offset);
+BlockSP block_sp = blocks[record->InlineNestLevel]->CreateChild(
+It.GetBookmark().offset);
 FileSpec callsite_file;
 if (record->CallSiteFileNum < m_files->size())
   callsite_file = (*m_files)[record->CallSiteFileNum];
@@ -333,7 +334,6 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function 
&func) {
 }
 block_sp->FinalizeRanges();
 
-blocks[record->InlineNestLevel]->AddChild(block_sp);
 if (record->I

[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/117683
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] build: remove extraneous search paths for LibXml2 (PR #117615)

2024-11-27 Thread Pavel Labath via lldb-commits

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 commented:

This function is definitely due for a cleanup. Do you have a reduced test-case 
for this by any chance? One way I see the vector would get cleared is if the 
recursive call ended up failing in `GetIndexForRecordBase`. This does seem like 
a plausible crash, but it might also be hiding a different bug. So a test-case 
would be interesting to see

https://github.com/llvm/llvm-project/pull/117808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Linux] Mark memory regions used for shadow stacks (PR #117861)

2024-11-27 Thread David Spickett via lldb-commits

DavidSpickett wrote:

RFC here: 
https://discourse.llvm.org/t/rfc-aarch64-guarded-control-stack-support-for-lldb/83364

Should help you understand the test case.

https://github.com/llvm/llvm-project/pull/117861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Docs] Add Guarded Control Stack to AArch64 Linux page (PR #117860)

2024-11-27 Thread David Spickett via lldb-commits

DavidSpickett wrote:

RFC here: 
https://discourse.llvm.org/t/rfc-aarch64-guarded-control-stack-support-for-lldb/83364

https://github.com/llvm/llvm-project/pull/117860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Linux] Mark memory regions used for shadow stacks (PR #117861)

2024-11-27 Thread David Spickett via lldb-commits

DavidSpickett wrote:

> Also the wording is "shadow stack: yes" because the shadow stack region is 
> just where it's stored. It's enabled for the whole process or it isn't. As 
> opposed to memory tagging which can be enabled per region, so "memory 
> tagging: enabled" fits better for that.

Though now I read this I think maybe this is meaningless. You can have multiple 
shadow stacks using GCS, just like many regions could be memory tagged.

So I'll go with whatever makes sense to others, I'm too close to the details to 
decide really.

https://github.com/llvm/llvm-project/pull/117861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Michael Buch via lldb-commits


@@ -353,6 +353,10 @@ class ModuleList {
 
   lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
 
+  const Symbol *

Michael137 wrote:

Can we add a doxygen comment?

https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Michael Buch via lldb-commits


@@ -524,6 +524,19 @@ void ModuleList::FindGlobalVariables(const 
RegularExpression ®ex,
 module_sp->FindGlobalVariables(regex, max_matches, variable_list);
 }
 
+const Symbol *
+ModuleList::FindFirstSymbolWithNameAndType(ConstString name,
+   lldb::SymbolType symbol_type) const 
{
+  std::lock_guard guard(m_modules_mutex);
+  for (const ModuleSP &module_sp : m_modules) {
+const Symbol *symbol =
+module_sp->FindFirstSymbolWithNameAndType(name, symbol_type);
+if (symbol)
+  return symbol;

Michael137 wrote:

```suggestion
if (const Symbol *symbol =
module_sp->FindFirstSymbolWithNameAndType(name, symbol_type))
  return symbol;
```

https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] Add 'FindFirstSymbolWithNameAndType()' to ModuleList. (PR #117777)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 commented:

Do you have a link to a PR that exercises this new API?

https://github.com/llvm/llvm-project/pull/11
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] build: remove extraneous search paths for LibXml2 (PR #117615)

2024-11-27 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett edited 
https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb][Mach-O] Handle shared cache binaries correctly (PR #117832)

2024-11-27 Thread Michael Buch via lldb-commits

Michael137 wrote:

Is changing `struct symtab_command` to have the right-sized fields a no-go?

https://github.com/llvm/llvm-project/pull/117832
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] build: remove extraneous search paths for LibXml2 (PR #117615)

2024-11-27 Thread David Spickett via lldb-commits

DavidSpickett wrote:

I don't have any objections to this.

Please update the PR description before landing, it will be used as the commit 
message.


https://github.com/llvm/llvm-project/pull/117615
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 17b8785 - [lldb] Fix premature MainLoop wakeup on windows (#117756)

2024-11-27 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-11-27T09:34:53Z
New Revision: 17b87853c3b07b8e1c7f000c3818efab7fdd8883

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

LOG: [lldb] Fix premature MainLoop wakeup on windows (#117756)

The windows system APIs only take milliseconds. Make sure we round the
sleep interval (in nanoseconds) upwards.

Added: 


Modified: 
lldb/source/Host/windows/MainLoopWindows.cpp

Removed: 




diff  --git a/lldb/source/Host/windows/MainLoopWindows.cpp 
b/lldb/source/Host/windows/MainLoopWindows.cpp
index 0a5a35e9db9dde..f3ab2a710cd014 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -28,7 +28,7 @@ static DWORD 
ToTimeout(std::optional point) {
 return WSA_INFINITE;
 
   nanoseconds dur = (std::max)(*point - steady_clock::now(), nanoseconds(0));
-  return duration_cast(dur).count();
+  return ceil(dur).count();
 }
 
 MainLoopWindows::MainLoopWindows() {



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


[Lldb-commits] [lldb] [lldb] Fix premature MainLoop wakeup on windows (PR #117756)

2024-11-27 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett approved this pull request.

LGTM, thanks.

I'll keep an eye on the buildbot.

https://github.com/llvm/llvm-project/pull/117756
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Fix premature MainLoop wakeup on windows (PR #117756)

2024-11-27 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/117756
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to show expanded objc instances (PR #117500)

2024-11-27 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/117500
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Update dwim-print to show expanded objc instances (PR #117500)

2024-11-27 Thread Michael Buch via lldb-commits


@@ -87,7 +87,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
   m_expr_options.m_verbosity, m_format_options.GetFormat());
-  dump_options.SetHideRootName(suppress_result);
+  dump_options.SetHideRootName(suppress_result)
+  .SetExpandPointerTypeFlags(lldb::eTypeIsObjC);

Michael137 wrote:

Is this something we would want for `frame var`/`expr` too? In which case we 
could just check for ObjC in `ValueObjectPrinter::ShouldPrintChildren` directly?

https://github.com/llvm/llvm-project/pull/117500
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)

2024-11-27 Thread Michael Buch via lldb-commits

Michael137 wrote:

> Fixed in 
> [284d4e0](https://github.com/llvm/llvm-project/commit/284d4e0a7a789848b7af7f85158ccf522d70c6f0).
>  FWIW, I did not get any emails for that breakage.

Thanks! Yea IIRC email notifications for green dragon are disabled at the 
moment (@JDevlieghere @adrian-prantl any idea why?)

https://github.com/llvm/llvm-project/pull/117691
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Add timeout argument to Socket::Accept (PR #117691)

2024-11-27 Thread Pavel Labath via lldb-commits

labath wrote:

Fixed in 284d4e0a7a789848b7af7f85158ccf522d70c6f0. FWIW, I did not get any 
emails for that breakage.

https://github.com/llvm/llvm-project/pull/117691
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 284d4e0 - [lldb] Fix RNBSocketTest for #117691

2024-11-27 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2024-11-27T13:16:41+01:00
New Revision: 284d4e0a7a789848b7af7f85158ccf522d70c6f0

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

LOG: [lldb] Fix RNBSocketTest for #117691

Added: 


Modified: 
lldb/unittests/debugserver/RNBSocketTest.cpp

Removed: 




diff  --git a/lldb/unittests/debugserver/RNBSocketTest.cpp 
b/lldb/unittests/debugserver/RNBSocketTest.cpp
index 75f236b4d41800..fe2e79e1fedf97 100644
--- a/lldb/unittests/debugserver/RNBSocketTest.cpp
+++ b/lldb/unittests/debugserver/RNBSocketTest.cpp
@@ -120,7 +120,8 @@ void TestSocketConnect(const char *addr) {
 ASSERT_EQ(bye, goodbye);
   } else {
 Socket *connected_socket;
-Status err = server_socket->Accept(connected_socket);
+Status err =
+server_socket->Accept(std::chrono::seconds(10), connected_socket);
 if (err.Fail()) {
   llvm::errs() << err.AsCString();
   abort();



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