[Lldb-commits] [lldb] 58e9cc1 - Revert "[lldb] Remove redundant .c_str() and .get() calls"

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2022-12-19T13:52:10+05:00
New Revision: 58e9cc13e24f668a33abdae201d59a02e10c22c0

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

LOG: Revert "[lldb] Remove redundant .c_str() and .get() calls"

This reverts commit fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders/68/builds/44990
https://lab.llvm.org/buildbot/#/builders/96/builds/33160

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/API/SBCommandReturnObject.cpp
lldb/source/API/SBExpressionOptions.cpp
lldb/source/API/SBSourceManager.cpp
lldb/source/Breakpoint/BreakpointResolverScripted.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectHelp.cpp
lldb/source/Commands/CommandObjectMultiword.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/OptionArgParser.cpp
lldb/source/Interpreter/OptionValueArch.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
lldb/source/Plugins/Platform/Android/AdbClient.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.h
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
lldb/source/Target/LanguageRuntime.cpp
lldb/source/Target/TargetList.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlanStack.cpp
lldb/tools/debugserver/source/DNBTimer.h
lldb/tools/lldb-vscode/BreakpointBase.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 8bb8b85e22fbc..47040137078f6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2704,7 +2704,7 @@ void PruneThreadPlans();
   };
 
   void SetNextEventAction(Process::NextEventAction *next_event_action) {
-if (m_next_event_action_up)
+if (m_next_event_action_up.get())
   m_next_event_action_up->HandleBeingUnshipped();
 
 m_next_event_action_up.reset(next_event_action);

diff  --git a/lldb/source/API/SBCommandReturnObject.cpp 
b/lldb/source/API/SBCommandReturnObject.cpp
index bf9c66453c90b..7d2c102b3d8c1 100644
--- a/lldb/source/API/SBCommandReturnObject.cpp
+++ b/lldb/source/API/SBCommandReturnObject.cpp
@@ -292,7 +292,7 @@ void SBCommandReturnObject::PutCString(const char *string, 
int len) {
 return;
   } else if (len > 0) {
 std::string buffer(string, len);
-ref().AppendMessage(buffer);
+ref().AppendMessage(buffer.c_str());
   } else
 ref().AppendMessage(string);
 }

diff  --git a/lldb/source/API/SBExpressionOptions.cpp 
b/lldb/source/API/SBExpressionOptions.cpp
index 6ae57f24606af..d07acbc0aa2db 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -254,5 +254,5 @@ EvaluateExpressionOptions *SBExpressionOptions::get() const 
{
 }
 
 EvaluateExpressionOptions &SBExpressionOptions::ref() const {
-  r

[Lldb-commits] [PATCH] D140030: [lldb][TypeSystemClang][NFC] Make TemplateParameterInfos members private

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

That looks great. I have just one small nit. I think the test would read better 
if you made the names of temporary `TemplateArgument` objects more descriptive 
-- i.e. encode the kind and value information in the name. For example. 
`int47Arg` for an integer template argument whose value is 47, and `intTypeArg` 
for a type template argument (where `int` is the "value").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140030

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

[Lldb-commits] [PATCH] D140262: [lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:334
+struct ArgumentMetadata {
+  char const *const name = nullptr;
+};

This may depend on where you're going with this, but right now it seems like 
this `const` here is overkill, as one can express the notion of "const 
metadata" by making the whole ArgumentMetadata class const.

That obviously won't work if your actual goal is to have some kind of "mutable" 
metadata, but then I'd like to hear more about what is that going to be used 
for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140262

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

[Lldb-commits] [PATCH] D140051: [lldb] Add LTO dependency to lldb test suite

[David Spickett via Phabricator via lldb-commits]

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

> If you just run ninja check-lldb prior to this patch libLTO would not be 
> built.

Great, put that in the commit message.

What I was trying to get at was the meaning of depends on:

- depends on thing, so we should build the thing
- depends on thing, so we shouldn't run the test if thing isn't present

And confirm that the former was your intent, but we do a lot of the latter 
elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140051

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

[Lldb-commits] [lldb] 071c62c - [lldb] Modernize sprintf in FormatEntity.cpp

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-12-19T10:53:20+01:00
New Revision: 071c62c5d3eda2836174c0de82f6c55b082e26fc

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

LOG: [lldb] Modernize sprintf in FormatEntity.cpp

Avoid buffer overflows with large indexes, and spurious nul characters
with small ones.

Added: 


Modified: 
lldb/source/Core/FormatEntity.cpp

Removed: 




diff  --git a/lldb/source/Core/FormatEntity.cpp 
b/lldb/source/Core/FormatEntity.cpp
index c5cad95ecafe1..4f615a1106520 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -618,11 +618,8 @@ static bool DumpRegister(Stream &s, StackFrame *frame, 
RegisterKind reg_kind,
 static ValueObjectSP ExpandIndexedExpression(ValueObject *valobj, size_t index,
  bool deref_pointer) {
   Log *log = GetLog(LLDBLog::DataFormatters);
-  const char *ptr_deref_format = "[%d]";
-  std::string ptr_deref_buffer(10, 0);
-  ::sprintf(&ptr_deref_buffer[0], ptr_deref_format, index);
-  LLDB_LOGF(log, "[ExpandIndexedExpression] name to deref: %s",
-ptr_deref_buffer.c_str());
+  std::string name_to_deref = llvm::formatv("[{0}]", index);
+  LLDB_LOG(log, "[ExpandIndexedExpression] name to deref: {0}", name_to_deref);
   ValueObject::GetValueForExpressionPathOptions options;
   ValueObject::ExpressionPathEndResultType final_value_type;
   ValueObject::ExpressionPathScanEndReason reason_to_stop;
@@ -630,8 +627,7 @@ static ValueObjectSP ExpandIndexedExpression(ValueObject 
*valobj, size_t index,
   (deref_pointer ? ValueObject::eExpressionPathAftermathDereference
  : ValueObject::eExpressionPathAftermathNothing);
   ValueObjectSP item = valobj->GetValueForExpressionPath(
-  ptr_deref_buffer.c_str(), &reason_to_stop, &final_value_type, options,
-  &what_next);
+  name_to_deref, &reason_to_stop, &final_value_type, options, &what_next);
   if (!item) {
 LLDB_LOGF(log,
   "[ExpandIndexedExpression] ERROR: why stopping = %d,"



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

Re: [Lldb-commits] [EXTERNAL] [lldb] fbaf48b - [lldb] Remove redundant .c_str() and .get() calls

[Pavel Labath via lldb-commits]

(I fixed the test failures with 
071c62c5d3eda2836174c0de82f6c55b082e26fc, so this should be safe to 
reapply now.)


On 19/12/2022 01:09, Stella Stamenova via lldb-commits wrote:

I think this broke the windows lldb bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/27352

Can you address the failure or revert the change? I can revert it tomorrow 
otherwise.

Thanks,
-Stella

-Original Message-
From: lldb-commits  On Behalf Of Fangrui 
Song via lldb-commits
Sent: Saturday, December 17, 2022 5:16 PM
To: lldb-commits@lists.llvm.org
Subject: [EXTERNAL] [Lldb-commits] [lldb] fbaf48b - [lldb] Remove redundant 
.c_str() and .get() calls


Author: Fangrui Song
Date: 2022-12-18T01:15:25Z
New Revision: fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd

URL: 
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ffbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd&data=05%7C01%7CSTILIS%40microsoft.com%7Cbb5532e79e21420847da08dae0955e85%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638069229409870530%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=w2DRW2raXlSi3NYQL%2BeSqHZtuImPdR%2Bbi2EMqYzCZnQ%3D&reserved=0
DIFF: 
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ffbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.diff&data=05%7C01%7CSTILIS%40microsoft.com%7Cbb5532e79e21420847da08dae0955e85%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638069229409870530%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=GS4cYm4kPFClslEDMeaQl5Rpb26TVVf%2Fg0mAlaVsLcQ%3D&reserved=0

LOG: [lldb] Remove redundant .c_str() and .get() calls

Removing .c_str() has a semantics difference, but the use scenarios
likely do not matter as we don't have NUL in the strings.

Added:
 


Modified:
 lldb/include/lldb/Target/Process.h
 lldb/source/API/SBCommandReturnObject.cpp
 lldb/source/API/SBExpressionOptions.cpp
 lldb/source/API/SBSourceManager.cpp
 lldb/source/Breakpoint/BreakpointResolverScripted.cpp
 lldb/source/Commands/CommandObjectBreakpoint.cpp
 lldb/source/Commands/CommandObjectExpression.cpp
 lldb/source/Commands/CommandObjectHelp.cpp
 lldb/source/Commands/CommandObjectMultiword.cpp
 lldb/source/Commands/CommandObjectTarget.cpp
 lldb/source/Commands/CommandObjectThread.cpp
 lldb/source/Core/Disassembler.cpp
 lldb/source/Core/FormatEntity.cpp
 lldb/source/Core/IOHandlerCursesGUI.cpp
 lldb/source/Expression/REPL.cpp
 lldb/source/Interpreter/CommandInterpreter.cpp
 lldb/source/Interpreter/OptionArgParser.cpp
 lldb/source/Interpreter/OptionValueArch.cpp
 lldb/source/Interpreter/ScriptInterpreter.cpp
 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
 lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
 lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
 lldb/source/Plugins/Language/ObjC/Cocoa.cpp
 lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
 lldb/source/Plugins/Platform/Android/AdbClient.cpp
 lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
 lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
 lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.h
 lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
 lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
 lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
 lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
 lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h
 lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
 lldb/source/Target/LanguageRuntime.cpp
 lldb/source/Target/TargetList.cpp
 lldb/source/Target/Thread.cpp
 lldb/source/Target/ThreadPlanStack.cpp
 lldb/tools/debugserver/source/DNBTimer.h
 lldb/tools/lldb-vscode/BreakpointBase.cpp

Removed:
 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
inde

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Jannik Silvanus via Phabricator via lldb-commits]

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

LGTM, but let's wait for the other reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added a comment.

Just thoughts.

llvm::any_isa is usually paired with llvm::any_cast; replacing them with 
llvm::any_cast and nullptr check seems fine.
However,

- std::any uses RTTI, so it is unlikely to ever replace llvm::Any.
- llvm::any_isa<> is kind of convenient and is aligned with llvm::isa<>. Same 
for llvm::any_cast.
- With that in mind, introducing new llvm::any_cast_or_null to follow 
llvm::cast_or_null instead of changing the semantics of llvm::any_cast might be 
a better idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added inline comments.



Comment at: lldb/include/lldb/Core/RichManglingContext.h:90-91
 assert(parser.has_value());
-assert(llvm::any_isa(parser));
+assert(llvm::any_cast(&parser));
 return llvm::any_cast(parser);
   }

This is not intuitively clear. In assert you pass an address, but in 'return' 
below the object is passed by reference.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D140262: [lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:334
+struct ArgumentMetadata {
+  char const *const name = nullptr;
+};

labath wrote:
> This may depend on where you're going with this, but right now it seems like 
> this `const` here is overkill, as one can express the notion of "const 
> metadata" by making the whole ArgumentMetadata class const.
> 
> That obviously won't work if your actual goal is to have some kind of 
> "mutable" metadata, but then I'd like to hear more about what is that going 
> to be used for.
Fair point

The use-case is the follow-up PR here: https://reviews.llvm.org/D140262 which 
is still WIP

Depending on whether that works out we may or may not need this patch here in 
the first place. So I'm holding off of committing it for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140262

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

[Lldb-commits] [PATCH] D140262: [lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:334
+struct ArgumentMetadata {
+  char const *const name = nullptr;
+};

Michael137 wrote:
> labath wrote:
> > This may depend on where you're going with this, but right now it seems 
> > like this `const` here is overkill, as one can express the notion of "const 
> > metadata" by making the whole ArgumentMetadata class const.
> > 
> > That obviously won't work if your actual goal is to have some kind of 
> > "mutable" metadata, but then I'd like to hear more about what is that going 
> > to be used for.
> Fair point
> 
> The use-case is the follow-up PR here: https://reviews.llvm.org/D140262 which 
> is still WIP
> 
> Depending on whether that works out we may or may not need this patch here in 
> the first place. So I'm holding off of committing it for now
Whoops, wrong link: https://reviews.llvm.org/D140145


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140262

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Jannik Silvanus via Phabricator via lldb-commits]

jsilvanus added a comment.

In D139973#4004344 , @barannikov88 
wrote:

> - std::any uses RTTI, so it is unlikely to ever replace llvm::Any.

I just checked on Godbolt that gcc 12.2 with libstdc++, clang 15.0 with both 
libstd++ and libc++ and MSVC 19.33 support `std::any` without RTTI. 
But gcc and clang do not provide `std::any::type` without RTTI though.

Are you aware of any documentation on what can be relied on without RTTI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added a comment.

In D139973#4004408 , @jsilvanus wrote:

> I just checked on Godbolt that gcc 12.2 with libstdc++, clang 15.0 with both 
> libstd++ and libc++ and MSVC 19.33 support `std::any` without RTTI. 
> But gcc and clang do not provide `std::any::type` without RTTI though.
>
> Are you aware of any documentation on what can be relied on without RTTI?

It is surprising to me that std::any can work without RTTI. Never thought it 
could be implemented.
This "compare function pointer" trick is awesome, but it might not always work 
as explained in this 

 commit.
This question 

 however, suggest that any_cast doesn't always work with RTTI either, which is 
weird.
I don't know of any other potential issues. std::visitor can't be used with 
std::any in absense of std::any::type, but that's minor, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D140293: [lldb] Update custom commands to always be overrriden

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This is a follow-up patch to 6f7835f309b9.

As explained previously, when running from an IDE, it can happen that
the IDE imports some lldb scripts by itself. If the user also tries to
import these commands, lldb will show the following message:

  error: cannot add command: user command exists and force replace not set

This message is confusing to the user, because it suggests that the
command import failed and that the execution should stop. However, in
this case, lldb will continue the execution with the command added
previously by the user.

To prevent that, this patch updates every first-party lldb-packaged
custom commands to override commands that were pre-imported in lldb.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140293

Files:
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/bsd.py
  lldb/examples/python/cmdtemplate.py
  lldb/examples/python/crashlog.py
  lldb/examples/python/delta.py
  lldb/examples/python/diagnose_nsstring.py
  lldb/examples/python/diagnose_unwind.py
  lldb/examples/python/disassembly_mode.py
  lldb/examples/python/gdb_disassemble.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/jump.py
  lldb/examples/python/lldb_module_utils.py
  lldb/examples/python/memory.py
  lldb/examples/python/sources.py
  lldb/examples/python/stacks.py
  lldb/examples/python/step_and_print.py
  lldb/examples/python/types.py

Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -351,5 +351,5 @@
 
 def __lldb_init_module(debugger, internal_dict):
 debugger.HandleCommand(
-'command script add -f types.check_padding_command check_padding')
+'command script add -o -f types.check_padding_command check_padding')
 print('"check_padding" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/step_and_print.py
===
--- lldb/examples/python/step_and_print.py
+++ lldb/examples/python/step_and_print.py
@@ -21,4 +21,4 @@
 return "Does a step-over then runs frame variable passing the command args to it\n"
 
 def __lldb_init_module(debugger, unused):
-debugger.HandleCommand("command script add -c step_and_print.StepAndPrint sap")
+debugger.HandleCommand("command script add -o -c step_and_print.StepAndPrint sap")
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -64,5 +64,5 @@
 
 def __lldb_init_module(debugger, internal_dict):
 debugger.HandleCommand(
-"command script add -f stacks.stack_frames stack_frames")
+"command script add -o -f stacks.stack_frames stack_frames")
 print("A new command called 'stack_frames' was added, type 'stack_frames --help' for more information.")
Index: lldb/examples/python/sources.py
===
--- lldb/examples/python/sources.py
+++ lldb/examples/python/sources.py
@@ -27,5 +27,5 @@
 def __lldb_init_module(debugger, dict):
 # Add any commands contained in this module to LLDB
 debugger.HandleCommand(
-'command script add -f sources.info_sources info_sources')
+'command script add -o -f sources.info_sources info_sources')
 print('The "info_sources" command has been installed, type "help info_sources" or "info_sources --help" for detailed help.')
Index: lldb/examples/python/memory.py
===
--- lldb/examples/python/memory.py
+++ lldb/examples/python/memory.py
@@ -272,5 +272,5 @@
 def __lldb_init_module(debugger, internal_dict):
 memfind_command.__doc__ = create_memfind_options().format_help()
 debugger.HandleCommand(
-'command script add -f memory.memfind_command memfind')
+'command script add -o -f memory.memfind_command memfind')
 print('"memfind" command installed, use the "--help" option for detailed help')
Index: lldb/examples/python/lldb_module_utils.py
===
--- lldb/examples/python/lldb_module_utils.py
+++ lldb/examples/python/lldb_module_utils.py
@@ -183,9 +183,9 @@
 
 # Add any commands contained in this module to LLDB
 debugger.HandleCommand(
-'command script add -c %s.DumpLineTables %s' % (__name__,
+'command script add -o -c %s.DumpLineTables %s' % (__name__,
 DumpLineTables.command_name))
 debugger.HandleCommand(
-'command script add -c %s.DumpFiles %s' % (

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added inline comments.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:193
 
-  if (any_isa(IR)) {
-const Function *F = any_cast(IR);
-return F->getName().str();
+  if (const auto **F = any_cast(&IR)) {
+return (*F)->getName().str();

Redundant braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D140262: [lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:334
+struct ArgumentMetadata {
+  char const *const name = nullptr;
+};

Michael137 wrote:
> Michael137 wrote:
> > labath wrote:
> > > This may depend on where you're going with this, but right now it seems 
> > > like this `const` here is overkill, as one can express the notion of 
> > > "const metadata" by making the whole ArgumentMetadata class const.
> > > 
> > > That obviously won't work if your actual goal is to have some kind of 
> > > "mutable" metadata, but then I'd like to hear more about what is that 
> > > going to be used for.
> > Fair point
> > 
> > The use-case is the follow-up PR here: https://reviews.llvm.org/D140262 
> > which is still WIP
> > 
> > Depending on whether that works out we may or may not need this patch here 
> > in the first place. So I'm holding off of committing it for now
> Whoops, wrong link: https://reviews.llvm.org/D140145
Storing the is_default flag here makes sense to me, but I feel the same way 
about it's constness as I feel about the name field. Since `ArgumentMetadata` 
is just a dumb container, I don't think it makes sense enforcing constness at 
this level. If we want to, we can always make individual instances of this 
struct const as a whole.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140262

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think that the main reason we've arrived at such different conclusions is 
that I treat the "user IDs of DIEs" and and "user IDs of symbol files" as 
essentially two different things (namespaces if you will). Obviously, one needs 
the symbol file ID in order to compute the DIERef ID, but theoretically those 
two can use completely different encodings. With this take on things, I stand 
by my assertion that DIERef->user_id conversions are tightly controlled. The 
symbol file ID computations are a mess.

You, if I understand correctly, see the ID of a symbol file as a special case 
of an all-encompassing user id -- essentially a user_id (or a DIERef) pointing 
to the first byte of the symbol file. with this world view, the entirety of 
user ID computation is a mess. :)
I can definitely see the appeal of viewing the world that way. It's nice and 
uniform and unambiguous (since you can't have a DIE at offset zero) -- it's 
just not the view I had when I was writing this code a couple of years ago. :) 
And it has the disadvantage of obscuring the DIERef->user_id transition (for 
DIEs at least), and that's what I'm weight against the other advantages of that 
approach.

In D138618#4002747 , @ayermolo wrote:

> I guess main issue with GetUID is that we rely on an internal state of 
> SymbolFileDWARF to
>
> 1. figure out if we are dealing with dwo or oso with check for 
> GetDebugMapSymfile
> 2. get extra data GetDwoNum(), and GetID()
>
> We can either push that logic on the caller side of things (not I deal I 
> would think) and remove GetUID, or extend the constructor to be a bit more 
> explicit. This way all the bit settings are still consolidated, but the logic 
> of when to create what is still hidden in GetDebugMapSymfile.
>
> What do you think?

I'm not entirely sure what you mean by that, but I think either of those could 
be fine. Essentally, what I'm trying to achieve is to make sure is that if the 
DIERef<->user_id conversion is trivial, then it is always valid to perform it 
(i.e. there are no partially constructed DIERefs). Ideally, there wouldn't be 
partially constructed DIERefs in any case, but that is not as important if one 
is forced to provide that additional information in order to do the conversion.

However, I also want to throw out this alternative 
. This one goes in the completely opposite 
direction. Instead of centralizing the conversions, it federates it (which is I 
think is roughly what I had in mind when I worked on this last time). There is 
no single place which controls the conversion, but there are multiple 
**disjoint** places which do that:

- one for the OSO case. This includes the following problematic lines you've 
listed:

> GetOSOIndexFromUserID
> GetUID (1/2)
> Encode/Decode UID (1/2)
> return DecodedUID{
>
>   *dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};

- one for the DWO case:

> GetUID (1/2)
> Encode/Decode UID (1/2)

- one for Symbol File IDs (which is does a +1 on the internal index -- bacause 
the main object file has ID 0)

> oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
> SymbolFileDWARFDwo constructor
> GetDwoNum (cancels out the previous one)

And I don't think it's an obstacle for making the die offsets larger -- I've 
included comments on how I think that could happen.

It doesn't handle this one, which seems just wrong, and should be made to use 
GetUID/DecodeUID

> const dw_offset_t function_die_offset = func.GetID();


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+

mib wrote:
> labath wrote:
> > mib wrote:
> > > mib wrote:
> > > > mib wrote:
> > > > > labath wrote:
> > > > > > I am surprised that you want to go down the "run" path for this 
> > > > > > functionality. I think most of the launch functionality does not 
> > > > > > make sense for this use case (e.g., you can't provide arguments to 
> > > > > > these processes, when you "run" them, can you?), and it is not 
> > > > > > consistent with what the "process listing" functionality does for 
> > > > > > regular platforms.
> > > > > > 
> > > > > > OTOH, the "attach" flow makes perfect sense here -- you take the 
> > > > > > pid of an existing process, attach to it, and stop it at a random 
> > > > > > point in its execution. You can't customize anything about how that 
> > > > > > process is run (because it's already running) -- all you can do is 
> > > > > > choose how you want to select the target process.
> > > > > For now, there is no support for attaching to a scripted process, 
> > > > > because we didn't have any use for it quite yet: cripted processes 
> > > > > were mostly used for doing post-mortem debugging, so we "ran" them 
> > > > > artificially in lldb by providing some launch options (the name of 
> > > > > the class managing the process and an optional user-provided 
> > > > > dictionary) through the command line or using an `SBLaunchInfo` 
> > > > > object.
> > > > > 
> > > > > I guess I'll need to extend the `platform process launch/attach` 
> > > > > commands and `SBAttachInfo` object to also support these options 
> > > > > since they're required for the scripted process instantiation.
> > > > > 
> > > > > Note that we aren't really attaching to the real running process, 
> > > > > we're creating a scripted process that knows how to read memory to 
> > > > > mock the real process.
> > > > @labath, I'll do that work on a follow-up patch
> > > @labath here D139945 :) 
> > Thanks. However, are you still planning to use the launch path for your 
> > feature? Because if you're not, then I think this comment should say "Get a 
> > list of processes that **are running**" (or that **can be attached to**).
> > 
> > And if you are, then I'd like to hear your thoughts on the discrepancy 
> > between what "launching" means for scripted and non-scripted platforms.
> > 
> The way I see it is that the scripted platform will create a process with the 
> right process plugin. In the case of scripted processes, the 
> `ProcessLaunchInfo` argument should have the script class name set (which 
> automatically sets the process plugin name to "ScriptedProcess" in the launch 
> info). Once the process is instantiated (before the launch), the scripted 
> platform will need to redirect to process stop events through its event 
> multiplexer. So the way I see it essentially, running a regular process with 
> the scripted platform should be totally transparent.
> 
> Something that is also worth discussing IMO, is the discrepancy between 
> launching and attaching from the scripted platform:
> 
> One possibility could be that `platform process launch` would launch all the 
> scripted processes listed by the scripted platform and set them up with the 
> multiplexer, whereas `platform process attach` would just create a scripted 
> process individually. I know this doesn't match the current behavior of the 
> platform commands so if you guys think we should preserve the expected 
> behavior, I guess.
> 
> May be @jingham has some opinion about this ?
Before we do that, maybe we could take a step back. Could you explain why you 
chose to use the "launch" flow for this use case?

To me, it just seems confusing to be using "launching" for any of this, 
particularly given that "attaching" looks like a much better match for what is 
happening here:
- launch allows you to specify process cmdline arguments, attach does not - I 
don't think you will be able to specify cmdline arguments for these scripted 
processes
- launch allows you to specify env vars, attach does not -- ditto
- launch allows you to stop-at-entry, attach does not -- you cannot stop at 
entry for these processes, as they have been started already
- attach allows you to specify a pid, launch does not -- you (I think) want to 
be able to choose the process (pid) that you want to create a scripted process 
for

For me, the choice is obvious, particularly considering that there *is* an 
obvious equivalent for "launching" for the kernel co-debugging use case. One 
could actually have the kernel create a new process --and then it **would** 
make sense to specify cmdline arguments, environment, and all of the other 
launch flags. I don't expect anyone to actually support this, as creating a 
brand new process like this is going 

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I have a little example of a function pointer using ptrath in an arm64e 
(ARMv8.3) program, to show what I was talking about,

  * thread #1, queue = 'com.apple.main-thread', stop reason = instruction step 
into
  frame #0: 0x00013fa0 a.out`main + 40 at a.c:6
 1  int foo () { return 5; }
 2  
 3  int main()
 4  {
 5int (*func)() = foo;
  -> 6return func();
 7  }
  Target 0: (a.out) stopped.
  
  (lldb) p func
  (int (*)(...)) $0 = 0xb86800013f70 (actual=0x00013f70 a.out`foo 
at a.c:1)
  
  (lldb) reg read x8
x8 = 0xb86800013f70 (0x00013f70) a.out`foo at a.c:1
  
  (lldb) ima loo -a func
Address: a.out[0x00013f70] (a.out.__TEXT.__text + 0)
Summary: a.out`foo at a.c:1
  
  (lldb) br s -a func
  Breakpoint 2: where = a.out`foo at a.c:1, address = 0x00013f70
  
  (lldb) mem read -c 1 -s 4 -f x func
  0x13f70: 0x528000a0
  
  (lldb) x/1i func
  0x13f70: 0x528000a0   movw0, #0x5
  (lldb) 

(nb out of the box installs of macOS will not run arm64e binaries like this, 
the ABI is not finalized -- it's meant to prevent people from sharing builds 
that may stop working some day when the ABI changes)

This process isn't using TBI, but the high bits end up being used for signing 
so it's the same effect.  I located this in OptionArgParser::ToAddress() 
because we'll need this same behavior in

  if (expr_result == eExpressionCompleted) {
if (valobj_sp)
  valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
  valobj_sp->GetDynamicValueType(), true);
// Get the address to watch.
if (valobj_sp)
  addr = valobj_sp->GetValueAsUnsigned(fail_value, &success);
if (success) {
  if (error_ptr)
error_ptr->Clear();
  if (abi_sp)
addr = abi_sp->FixCodeAddress (addr);
  return addr;
} else {

What do you think about locating this change in ToAddress instead of 
Target::GetBreakableLoadAddress?  It looks like the one caller to 
GetBreakableLoadAddress is Target::CreateBreakpoint(addr_t addr) - which is 
probably called by an SBTarget method if we want to think of the most general 
purpose use case.  I think stripping non-addressable bits in 
OptionArgParser::ToAddress is the right thing to do - but I don't have an 
opinion about whether it should be done in Target::GetBreakableLoadAddress or 
not.

One thing to note is that I also have an SBValue method that needs to be 
upstreamed, SBValue::GetValueAsAddress().  So if someone has an SBValue of a 
function pointer and they want to take the value of that func ptr and call 
SBTarget::CreateBreakpoint on it, I would say "well yes, you need to use 
GetValueAsAddress before you pass that to CreateBreakpoint".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added a comment.

Related: https://reviews.llvm.org/D139974


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sebastian Neubauer via Phabricator via lldb-commits]

sebastian-ne updated this revision to Diff 483975.
sebastian-ne marked 2 inline comments as done.
sebastian-ne added a comment.

> It is surprising to me that std::any can work without RTTI. Never thought it 
> could be implemented.

It seems like libstdc++ and libc++ both implement it the way llvm::Any is 
implemented when RTTI is not available (actually, I think llvm::Any was copied 
from libstdc++ at some point).

> This "compare function pointer" trick is awesome, but it might not always 
> work as explained in this commit.
> This question however, suggest that any_cast doesn't always work with RTTI 
> either, which is weird.
> I don't know of any other potential issues. std::visitor can't be used with 
> std::any in absense of std::any::type, but that's minor, I think.

As you noticed, it is very difficult to implement Any correctly on every 
platform under any circumstances and compiler flags.
That is exactly what this patch aims at: Moving the responsibility to implement 
Any correctly to the standard library.

Unfortunatly, we can’t replace llvm::Any with std::any just yet, because of 
bugs in msvc that were only fixed recently.

> llvm::any_isa<> is kind of convenient and is aligned with llvm::isa<>. Same 
> for llvm::any_cast.

It is, however, there is no std::any_isa or std::any_cast_or_null, so the 
question gets wether we want to keep llvm::Any around as a wrapper of std::any 
once we can use it (in this case this patch would be obsolete)
or if we we want to remove llvm::Any and use std::any only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPE

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sebastian Neubauer via Phabricator via lldb-commits]

sebastian-ne added inline comments.



Comment at: lldb/include/lldb/Core/RichManglingContext.h:90-91
 assert(parser.has_value());
-assert(llvm::any_isa(parser));
+assert(llvm::any_cast(&parser));
 return llvm::any_cast(parser);
   }

barannikov88 wrote:
> This is not intuitively clear. In assert you pass an address, but in 'return' 
> below the object is passed by reference.
> 
I changed it to use an address + explicit dereference in the return. I hope 
that makes it clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> What do you think about locating this change in ToAddress instead of 
> Target::GetBreakableLoadAddress? It looks like the one caller to 
> GetBreakableLoadAddress is Target::CreateBreakpoint(addr_t addr) - which is 
> probably called by an SBTarget method if we want to think of the most general 
> purpose use case. I think stripping non-addressable bits in 
> OptionArgParser::ToAddress is the right thing to do - but I don't have an 
> opinion about whether it should be done in Target::GetBreakableLoadAddress or 
> not.

I had not checked the other lookup commands, so yeah this sounds good to me. 
Doing this fixing in fewer places is always better. Put up your change and I'll 
check the test from this patch against it.

> One thing to note is that I also have an SBValue method that needs to be 
> upstreamed, SBValue::GetValueAsAddress(). So if someone has an SBValue of a 
> function pointer and they want to take the value of that func ptr and call 
> SBTarget::CreateBreakpoint on it, I would say "well yes, you need to use 
> GetValueAsAddress before you pass that to CreateBreakpoint".

If I understand correctly, this would effectively run FixCodeAddress on the 
value? One thing that I found doing the SB API for MTE (which was shelved, but 
besides the point) is that there was no way to fix addresses via the API (or 
get access to the ABI plugin in general). I ended up just adding FixDataAddress 
to SBProcess as a temporary thing. So something like ValueAsAddress could be 
very useful.

We have this distinction (that doesn't mean much) between code and data fixes. 
For now a single method will work fine, I'm hoping the code/data difference 
never gets to a point where we have to be really careful about it.
(as you say "This process isn't using TBI, but the high bits end up being used 
for signing so it's the same effect.", so FixCode and FixData are the same 
thing in that case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D136938#4005138 , @DavidSpickett 
wrote:

>> What do you think about locating this change in ToAddress instead of 
>> Target::GetBreakableLoadAddress? It looks like the one caller to 
>> GetBreakableLoadAddress is Target::CreateBreakpoint(addr_t addr) - which is 
>> probably called by an SBTarget method if we want to think of the most 
>> general purpose use case. I think stripping non-addressable bits in 
>> OptionArgParser::ToAddress is the right thing to do - but I don't have an 
>> opinion about whether it should be done in Target::GetBreakableLoadAddress 
>> or not.
>
> I had not checked the other lookup commands, so yeah this sounds good to me. 
> Doing this fixing in fewer places is always better. Put up your change and 
> I'll check the test from this patch against it.

OK sounds good, I'll put up a phab.  I can't test it on macOS platforms because 
the bots won't be able to build & run arm64e (ARMv8.3 w/ ptrauth) binaries. :/

>> One thing to note is that I also have an SBValue method that needs to be 
>> upstreamed, SBValue::GetValueAsAddress(). So if someone has an SBValue of a 
>> function pointer and they want to take the value of that func ptr and call 
>> SBTarget::CreateBreakpoint on it, I would say "well yes, you need to use 
>> GetValueAsAddress before you pass that to CreateBreakpoint".
>
> If I understand correctly, this would effectively run FixCodeAddress on the 
> value? One thing that I found doing the SB API for MTE (which was shelved, 
> but besides the point) is that there was no way to fix addresses via the API 
> (or get access to the ABI plugin in general). I ended up just adding 
> FixDataAddress to SBProcess as a temporary thing. So something like 
> ValueAsAddress could be very useful.

Yeah my SBValue::GetValueAsAddress() simply passed it through FixCodeAddress 
which was correct enough for our current environment, but SBValue may have 
access to type information so maybe it could pick the correct Fix method to 
call given that information?

I've had some scripting people who also want methods to clear non-addressable 
bits in SBProcess, as they handle signed pointers in their own scripts, it's 
something I've been meaning to do for them -- or give them a way to get the 
address fixing bitmasks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

[Jason Molenda via Phabricator via lldb-commits]

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

I think we both agree that OptionArgParser::ToAddress needs to fix the address, 
but I don't have an opinion about whether we should also do it in 
GetBreakableLoadAddress.  I'm fine if you think we should do it here too, for 
other codepaths that can try to set a breakpoint on a signed function pointer 
address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

[Lldb-commits] [PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 accepted this revision.
barannikov88 added a comment.

In D139973#4005120 , @sebastian-ne 
wrote:

> the question gets wether we want to keep llvm::Any around as a wrapper of 
> std::any once we can use it (in this case this patch would be obsolete)
> or if we we want to remove llvm::Any and use std::any only.

Since there are not many uses of Any, it is hard to say which option would be 
preferable in the long term.
Perhaps, it would be possible to remove llvm::Any and use std::any with 
llvm::any_isa as an extension.
Anyway, this patch makes it possible to avoid double casting, which is an 
improvement by itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

[David Spickett via Phabricator via lldb-commits]

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

> I can't test it on macOS platforms because the bots won't be able to build & 
> run arm64e (ARMv8.3 w/ ptrauth) binaries. :/

Well our bots can't either but I've got QEMU locally is what I mean. We can run 
top byte ignore tests but top byte ignore being ignored by hardware sometimes 
makes tests pass anyway :)

> I don't have an opinion about whether we should also do it in 
> GetBreakableLoadAddress

I'll make a list of the paths to setting a breakpoint, to see which layer it 
makes the most sense to fix in. (which I'll get time to do sometime in January)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.
Herald added subscribers: jsetoain, JDevlieghere, ormris.

Worth trying on MSVC, which is the other more likely place to fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:513
   if (Opts.MaxBitIntWidth)
-MaxBitIntWidth = Opts.MaxBitIntWidth;
+MaxBitIntWidth = (unsigned)Opts.MaxBitIntWidth;
 

Nit: C-style casts are generally discouraged (there are several occurrences in 
this patch).



Comment at: llvm/lib/CodeGen/RegAllocGreedy.h:83
   public:
-ExtraRegInfo() = default;
+ExtraRegInfo() {}
 ExtraRegInfo(const ExtraRegInfo &) = delete;

Is it somehow different than ' = default'?



Comment at: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp:182
 
-  return ret;
+  return std::move(ret);
 }

clang-tidy would usually complain on this. Does it solve some gcc issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

Can you push `using OptionalFileEntryRef = CustomizableOptional;` 
and the renaming as a separate commit to make this patch smaller?
There is a smaller that this rename may be reverted.
205c0589f918f95d2f2c586a01bea2716d73d603 
 reverted 
a change related to `OptionalFileEntryRef`. I assume that your change is fine.




Comment at: mlir/include/mlir/IR/Visitors.h:49
   bool operator==(const WalkResult &rhs) const { return result == rhs.result; }
+  bool operator!=(const WalkResult &rhs) const { return result != rhs.result; }
 

Can be pushed separately


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 484077.
aeubanks added a comment.

check if type is template by looking at underlying clang Type, rather than 
looking for a '<'
properly handle typedefs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140240

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Symbol/TypeSystem.cpp
  lldb/test/API/lang/cpp/unique-types4/Makefile
  lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
  lldb/test/API/lang/cpp/unique-types4/main.cpp

Index: lldb/test/API/lang/cpp/unique-types4/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/main.cpp
@@ -0,0 +1,23 @@
+namespace ns {
+
+template  struct Foo {
+  static T value;
+};
+
+template  using Bar = Foo;
+
+using FooInt = Foo;
+using FooDouble = Foo;
+
+} // namespace ns
+
+ns::Foo a;
+ns::Foo b;
+ns::Bar c;
+ns::Bar d;
+ns::FooInt e;
+ns::FooDouble f;
+
+int main() {
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
@@ -0,0 +1,31 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase4(TestBase):
+def do_test(self, debug_flags):
+"""Test that we display the correct template instantiation."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+# FIXME: these should successfully print the values
+self.expect("print ns::Foo::value", substrs=["no member named"], error=True)
+self.expect("print ns::Foo::value", substrs=["no member named"], error=True)
+self.expect("print ns::Bar::value", substrs=["no member named"], error=True)
+self.expect("print ns::Bar::value", substrs=["no member named"], error=True)
+self.expect("print ns::FooDouble::value", substrs=["Couldn't lookup symbols"], error=True)
+self.expect("print ns::FooInt::value", substrs=["Couldn't lookup symbols"], error=True)
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/unique-types4/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -116,6 +116,10 @@
   return CompilerType(weak_from_this(), type);
 }
 
+bool TypeSystem::IsTemplateType(lldb::opaque_compiler_type_t type) {
+  return false;
+}
+
 size_t TypeSystem::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
bool expand_pack) {
   return 0;
Index: lldb/source/Symbol/Type.cpp
===
--- lldb/source/Symbol/Type.cpp
+++ lldb/source/Symbol/Type.cpp
@@ -393,6 +393,10 @@
   return GetForwardCompilerType().IsAggregateType();
 }
 
+bool Type::IsTemplateType() {
+  return GetForwardCompilerType().IsTemplateType();
+}
+
 lldb::TypeSP Type::GetTypedefType() {
   lldb::TypeSP type_sp;
   if (IsTypedef()) {
Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -260,6 +260,13 @@
   return false;
 }
 
+bool CompilerType::IsTemplateType() const {
+  if (IsValid())
+if (auto type_system_sp = GetTypeSystem())
+  return type_system_sp->IsTemplateType(m_type);
+  return false;
+}
+
 bool CompilerType::IsTypedefType() const {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/Type

[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added inline comments.



Comment at: lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py:15
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", 
lldb.SBFileSpec("main.cpp"))
+# FIXME: these should successfully print the values
+self.expect("print ns::Foo::value", substrs=["no member 
named"], error=True)

this makes handling of -gsimple-template-names the same as the current handling 
of -gno-simple-template-names

I haven't dug too deep into the existing difference between 
`FooDouble`/`FooInt` vs the others

for the "no member named" errors, I believe clang is attempting to get the 
uninstantiated declarations of `Foo`/`Bar`, which the dwarf knows nothing about


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140240

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

[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py:15
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", 
lldb.SBFileSpec("main.cpp"))
+# FIXME: these should successfully print the values
+self.expect("print ns::Foo::value", substrs=["no member 
named"], error=True)

aeubanks wrote:
> this makes handling of -gsimple-template-names the same as the current 
> handling of -gno-simple-template-names
> 
> I haven't dug too deep into the existing difference between 
> `FooDouble`/`FooInt` vs the others
> 
> for the "no member named" errors, I believe clang is attempting to get the 
> uninstantiated declarations of `Foo`/`Bar`, which the dwarf knows nothing 
> about
Yup that's exactly what it is. For `expr Foo`, `clang::Sema`'s template 
lookup would ask LLDB to give it type `Foo`, but we can't service that request 
from DWARF currently


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140240

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

[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

[Michael Buch via Phabricator via lldb-commits]

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

look reasonable to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140240

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Benjamin Kramer via Phabricator via lldb-commits]

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



Comment at: clang/lib/Basic/TargetInfo.cpp:513
   if (Opts.MaxBitIntWidth)
-MaxBitIntWidth = Opts.MaxBitIntWidth;
+MaxBitIntWidth = (unsigned)Opts.MaxBitIntWidth;
 

barannikov88 wrote:
> Nit: C-style casts are generally discouraged (there are several occurrences 
> in this patch).
-> static_cast



Comment at: llvm/lib/CodeGen/RegAllocGreedy.h:83
   public:
-ExtraRegInfo() = default;
+ExtraRegInfo() {}
 ExtraRegInfo(const ExtraRegInfo &) = delete;

barannikov88 wrote:
> Is it somehow different than ' = default'?
It makes the class non-trivial, std::optional::emplace has issues with trivial 
default constructors :(



Comment at: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp:182
 
-  return ret;
+  return std::move(ret);
 }

barannikov88 wrote:
> clang-tidy would usually complain on this. Does it solve some gcc issue?
It does, this is a bug in GCC 7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Benjamin Kramer via Phabricator via lldb-commits]

bkramer marked an inline comment as done.
bkramer added a comment.

In D140332#4005988 , @MaskRay wrote:

> Can you push `using OptionalFileEntryRef = 
> CustomizableOptional;` and the renaming as a separate commit to 
> make this patch smaller?
> There is a smaller that this rename may be reverted.
> 205c0589f918f95d2f2c586a01bea2716d73d603 
>  
> reverted a change related to `OptionalFileEntryRef`. I assume that your 
> change is fine.

Yeah, I'll split this into smaller pieces once we agree on the direction and 
just submit the alias. I expect a revert or two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

[Sergei Barannikov via Phabricator via lldb-commits]

barannikov88 accepted this revision.
barannikov88 added inline comments.



Comment at: llvm/lib/CodeGen/RegAllocGreedy.h:83
   public:
-ExtraRegInfo() = default;
+ExtraRegInfo() {}
 ExtraRegInfo(const ExtraRegInfo &) = delete;

bkramer wrote:
> barannikov88 wrote:
> > Is it somehow different than ' = default'?
> It makes the class non-trivial, std::optional::emplace has issues with 
> trivial default constructors :(
Ah, right, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140332

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

[Lldb-commits] [lldb] 2916b99 - [ADT] Alias llvm::Optional to std::optional

[Benjamin Kramer via lldb-commits]

Author: Benjamin Kramer
Date: 2022-12-20T01:01:46+01:00
New Revision: 2916b99182752b1aece8cc4479d8d6a20b5e02da

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

LOG: [ADT] Alias llvm::Optional to std::optional

This avoids the continuous API churn when upgrading things to use
std::optional and makes trivial string replace upgrades possible.

I tested this with GCC 7.5, the oldest supported GCC I had around.

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

Added: 


Modified: 
clang/include/clang/Basic/LLVM.h
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
flang/include/flang/Lower/Runtime.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/source/Symbol/Type.cpp
llvm/include/llvm/ADT/Optional.h
llvm/include/llvm/ADT/STLForwardCompat.h
llvm/include/llvm/Support/raw_ostream.h
llvm/include/llvm/Testing/Support/SupportHelpers.h
llvm/lib/CodeGen/RegAllocGreedy.h
llvm/unittests/ADT/CMakeLists.txt
llvm/unittests/Support/TypeTraitsTest.cpp
mlir/include/mlir/Bindings/Python/PybindAdaptors.h
mlir/include/mlir/Support/LLVM.h
mlir/lib/Bindings/Python/PybindUtils.h
mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Removed: 
llvm/unittests/ADT/OptionalTest.cpp



diff  --git a/clang/include/clang/Basic/LLVM.h 
b/clang/include/clang/Basic/LLVM.h
index 5d4d72630970b..7ffc4c403473b 100644
--- a/clang/include/clang/Basic/LLVM.h
+++ b/clang/include/clang/Basic/LLVM.h
@@ -37,7 +37,7 @@ namespace llvm {
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
-  template  class Optional;
+  template  using Optional = std::optional;
   template  class Expected;
 
   template

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7ced94a7dc371..f9e76d85efdde 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -481,7 +481,7 @@ Optional 
ExprEngine::getPendingInitLoop(ProgramStateRef State,
   const CXXConstructExpr *E,
   const LocationContext *LCtx) 
{
   const unsigned *V = State->get({E, LCtx->getStackFrame()});
-  return V ? Optional(*V) : std::nullopt;
+  return V ? std::make_optional(*V) : std::nullopt;
 }
 
 ProgramStateRef ExprEngine::removePendingInitLoop(ProgramStateRef State,
@@ -510,7 +510,7 @@ ExprEngine::getIndexOfElementToConstruct(ProgramStateRef 
State,
  const LocationContext *LCtx) {
   const unsigned *V =
   State->get({E, LCtx->getStackFrame()});
-  return V ? Optional(*V) : std::nullopt;
+  return V ? std::make_optional(*V) : std::nullopt;
 }
 
 ProgramStateRef
@@ -530,7 +530,7 @@ ExprEngine::getPendingArrayDestruction(ProgramStateRef 
State,
 
   const unsigned *V =
   State->get(LCtx->getStackFrame());
-  return V ? Optional(*V) : std::nullopt;
+  return V ? std::make_optional(*V) : std::nullopt;
 }
 
 ProgramStateRef ExprEngine::setPendingArrayDestruction(
@@ -600,7 +600,7 @@ ExprEngine::getObjectUnderConstruction(ProgramStateRef 
State,
const LocationContext *LC) {
   ConstructedObjectKey Key(Item, LC->getStackFrame());
   const SVal *V = State->get(Key);
-  return V ? Optional(*V) : std::nullopt;
+  return V ? std::make_optional(*V) : std::nullopt;
 }
 
 ProgramStateRef

diff  --git a/flang/include/flang/Lower/Runtime.h 
b/flang/include/flang/Lower/Runtime.h
index 64e9a16fd7988..da01d1a87cccd 100644
--- a/flang/include/flang/Lower/Runtime.h
+++ b/flang/include/flang/Lower/Runtime.h
@@ -16,9 +16,10 @@
 #ifndef FORTRAN_LOWER_RUNTIME_H
 #define FORTRAN_LOWER_RUNTIME_H
 
+#include 
+
 namespace llvm {
-template 
-class Optional;
+template  using Optional = std::optional;
 }
 
 namespace mlir {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index 4efb5a2029e85..32960c2102eda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -190,7 +190,7 @@ static FormSize g_form_sizes[] = {
 llvm::Optional
 DWARFFormValue::GetFixedSize(dw_form_t form, const DWARFUnit *u) {
   if (form <= DW_FORM_ref_sig8 && g_form_sizes[form].valid)
-return g_form_sizes[form].size;
+return static_cast(g_form_sizes[form].size);
   if (form == DW_FORM_addr && u)
 return u->GetAddressByteSize();
   return std::nullopt;

diff  --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index da9eeadc0ecd2..048e37fb1db2f 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -343,7 +343,7 @@ Type 

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

[Alexander Yermolovich via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:11-58
+#include "AppleDWARFIndex.h"
+#include "DWARFASTParser.h"
+#include "DWARFASTParserClang.h"
+#include "DWARFCompileUnit.h"
+#include "DWARFDebugAbbrev.h"
+#include "DWARFDebugAranges.h"
+#include "DWARFDebugInfo.h"

JDevlieghere wrote:
> Unrelated change?
This was recommended in another diff.  Although it was related to DIERef.h. I 
can move it to the second diff, or it's own diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139955

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

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

[Enrico Loparco via Phabricator via lldb-commits]

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

Implement DAP (Debug Adapter Protocol) for Disassemble Requests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140358

Files:
  lldb/include/lldb/API/SBTarget.h
  lldb/source/API/SBTarget.cpp
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/DisassembledInstruction.cpp
  lldb/tools/lldb-vscode/DisassembledInstruction.h
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/VSCodeForward.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -102,6 +102,8 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+lldb::SBFrame g_curr_frame;
+
 lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
   switch (variablesReference) {
   case VARREF_LOCALS:
@@ -1451,8 +1453,7 @@
   // which may affect the outcome of tests.
   bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
 
-  g_vsc.debugger =
-  lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
+  g_vsc.debugger = lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
 
   // Start our event thread so we can receive events from the debugger, target,
@@ -1542,6 +1543,8 @@
   // The debug adapter supports 'logMessage' in breakpoint.
   body.try_emplace("supportsLogPoints", true);
 
+  body.try_emplace("supportsDisassembleRequest", true);
+
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
@@ -2117,6 +2120,191 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+std::vector
+_get_instructions_from_memory(lldb::addr_t start, uint64_t count,
+  lldb::addr_t end) {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+
+  lldb::SBError error;
+  std::vector buffer(count, 0);
+  const size_t bytes_read __attribute__((unused)) = process.ReadMemory(
+  start, static_cast(buffer.data()), count, error);
+  assert(bytes_read == count && error.Success() &&
+ "unable to read byte range from memory");
+
+  // If base_addr starts in the middle of an instruction,
+  // that first instruction will not be parsed correctly (negligible)
+  std::vector sb_instructions;
+  const auto base_addr = lldb::SBAddress(start, g_vsc.target);
+  lldb::SBInstructionList instructions =
+  g_vsc.target.GetInstructions(base_addr, buffer.data(), count);
+
+  for (size_t i = 0; i < instructions.GetSize(); i++) {
+auto instr = instructions.GetInstructionAtIndex(i);
+if (instr.GetAddress().GetLoadAddress(g_vsc.target) > end)
+  break;
+
+sb_instructions.emplace_back(instr);
+  }
+  return sb_instructions;
+}
+
+std::pair _get_frame_boundary() {
+  assert(g_curr_frame.IsValid());
+  auto function = g_curr_frame.GetFunction();
+
+  if (!function.IsValid())
+return std::make_pair<>(LLDB_INVALID_ADDRESS, LLDB_INVALID_ADDRESS);
+
+  return std::make_pair<>(
+  function.GetStartAddress().GetLoadAddress(g_vsc.target),
+  function.GetEndAddress().GetLoadAddress(g_vsc.target));
+}
+
+auto _handle_disassemble_positive_offset(lldb::addr_t base_addr,
+ int64_t instruction_offset,
+ uint64_t instruction_count) {
+  llvm::json::Array response_instructions;
+
+  auto start_addr = lldb::SBAddress(base_addr, g_vsc.target);
+  lldb::SBInstructionList instructions = g_vsc.target.ReadInstructions(
+  start_addr, instruction_offset + instruction_count);
+
+  std::vector dis_instructions;
+  const auto num_instrs_to_skip = static_cast(instruction_offset);
+  for (size_t i = num_instrs_to_skip; i < instructions.GetSize(); ++i) {
+lldb::SBInstruction instr = instructions.GetInstructionAtIndex(i);
+
+auto disass_instr =
+CreateDisassembledInstruction(DisassembledInstruction(instr));
+response_instructions.emplace_back(std::move(disass_instr));
+  }
+
+  return response_instructions;
+}
+
+auto _handle_disassemble_negative_offset(
+lldb::addr_t base_addr, int64_t instruction_offset,
+uint64_t instruction_count,
+std::optional memory_reference) {
+  llvm::json::Array response_instructions;
+
+  const auto bytes_per_instruction = g_vsc.target.GetMaximumOpcodeByteSize();
+  const auto bytes_offset = -instruction_offset * bytes_per_instruction;
+  auto start_addr = base_addr - bytes_offset;
+  const auto disassemble_bytes = instruction_count * bytes_per_instruction;
+
+  // G

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Currently in some cases lldb reports stop reason as "step out" or "step over" 
(from thread plan completion) over "breakpoint", if the user breakpoint happens 
to be set on the same address.
The part of 
https://github.com/llvm/llvm-project/commit/f08f5c99262ff9eaa08956334accbb2614b0f7a2
 seems to overwrite internal breakpoint detection logic, so that only the last 
breakpoint for the current stop address is considered.
Together with step-out plans not clearing its breakpoint until they are 
destrouyed, this creates a situation when there is a user breakpoint set for 
address, but internal breakpoint makes lldb report a plan completion stop 
reason instead of breakpoint.
This patch reverts that internal breakpoint detection logic to consider all 
breakpoints


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/step_out_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
  lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_1
+  return 1 + func_1();  // breakpoint_2
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // breakpoint_0
+  return 0;  // breakpoint_3
+}
Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
@@ -0,0 +1,85 @@
+"""
+Test that breakpoints do not affect stepping.
+Check for correct StopReason when stepping to the line with breakpoint
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepOverBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+src = lldb.SBFileSpec("main.cpp")
+
+# Create a target by the debugger.
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+# Setup four breakpoints
+self.lines = [line_number('main.cpp', "breakpoint_%i" % i) for i in range(4)]
+
+self.breakpoints = [self.target.BreakpointCreateByLocation(src, line) for line in self.lines]
+self.assertTrue(
+self.breakpoints[0] and self.breakpoints[0].GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+# Start debugging
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertIsNotNone(self.process, PROCESS_IS_VALID)
+self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[0])
+self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 0.")
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+"Expected stop reason to be step into/over/out for inactive breakpoint %i line." % breakpoint_idx)
+
+def check_step_out_conditional(self, condition):
+conditionStr = 'true' if condition else 'false'
+
+self.breakpoints[1].GetLocationAtIndex(0).SetCondition(conditionStr)
+self.breakpoints[2].GetLocationAtIndex(0).SetCondition(conditionStr)
+self.breakpoints[3].GetLocationAtIndex(0).SetCondition(conditionStr)
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# This step-over creates a step-out from `f