[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Jonas Devlieghere via lldb-commits]

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


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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

jimingham wrote:

Sounds good to me.

Jim


> On May 7, 2024, at 4:52 PM, royitaqi ***@***.***> wrote:
> 
> 
> Hi @jimingham ,
> 
> Thank you so much for the detailed explanation about the context and your 
> thoughts. I appreciate it.
> 
> Good to know about the automated scripts and that you personally never used 
> the existing transcript. It makes me feel that there are distinctively 
> different use cases, and for the ones where the user know they won't need the 
> transcript, they should be able to turn them off (and yes the text output can 
> be large if someone opt to print e.g. statistics dump).
> 
> As a matter of fact, that's a better design since then I can disable the 
> feature temporarily without having to mess up the settings I want when I do 
> do it.
> 
> This is also an interesting design aspect that I never thought about. This is 
> good learning.
> 
> --
> 
> Overall I feel what you said makes sense. I will open a follow-up PR 
> specifically for the settings, where we can discuss more (e.g. default on or 
> off; should the setting turn off the existing transcript; should we add a 
> interpreter.save-session-format; etc).
> 
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

jimingham wrote:

This wouldn't be a problem if we make sure that whoever makes the Streams for 
use by the command interpreter consults GetUseColor when it creates the stream, 
and then we can rely on the stream to tell us what to do.  If you are calling 
stream API's that add the color for you, that should just work.  If you are 
doing a Print by hand with the color codes or not, you should consult the 
stream.  For that we'll have to add Stream::GetUseColor.  If we always get that 
right, the only problem will be if there are persistent streams, we may need to 
change their state or replace them when GetUseColor changes.

That seems the most principled way to solve this.

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

royitaqi wrote:

Hi @jimingham,

Thank you so much for the detailed explanation about the context and your 
thoughts. I appreciate it.

Good to know about the automated scripts and that you personally never used the 
existing transcript. It makes me feel that there are distinctively different 
use cases, and for the ones where the user know they won't need the transcript, 
they should be able to turn them off (and yes the text output can be large if 
someone opt to print e.g. statistics dump).

> As a matter of fact, that's a better design since then I can disable the 
> feature temporarily without having to mess up the settings I want when I do 
> do it.

This is also an interesting design aspect that I never thought about. This is 
good learning.

--

Overall I feel what you said makes sense. I will open a follow-up PR 
specifically for the settings, where we can discuss more (e.g. default on or 
off; should the setting turn off the existing transcript; should we add a 
`interpreter.save-session-format`; etc).

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Chelsea Cassanova via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

chelcassanova wrote:

This means that only the debugger tells you whether or not you're using colours 
then? `BreakpointOptions::GetDescription()` doesn't seem to have a reference to 
the debugger, target or interpreter so I'm wondering what the next best options 
to determine if we're using colours is. 

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

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[Greg Clayton via lldb-commits]

https://github.com/clayborg commented:

LGTM. 

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

royitaqi wrote:

@jimingham 

> Adding debuggers, or querying them is controlled by the debugger list mutex.  
> Manipulating the I/O handlers is governed by the io handler stack mutex, 
> there's a lock around the interrupt requests.  Debugger doesn't seem like a 
> class that is not locking the lists of things it controls as a general rule.

This is fair. I somehow missed them the last time I was searching for mutex in 
the same code.

--

With the above, I'm on the fence. Since I have less knowledge and context, at 
this point I will depend on @jimingham & @JDevlieghere you guys aligning with 
each other and pointing me in the right direction. LMK. Either ways it should 
be an easy change in the PR (since we already have thread-safety implemented).

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

jimingham wrote:

It seems odd you can't ask the stream whether it is using colors or not.  
Because if I'm running with use-colors on, but I want to use the description of 
a breakpoint w/o colors - say to put it in a log - then I would correctly make 
an SBStream which is supposed to not use colors, and pass that to the 
description.  If you ONLY check whether the debugger is using colors, and 
override the stream's setting, then you will do the wrong thing.

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

jimingham wrote:

Note, you can get this output both when lldb decides to print in the terminal, 
and when someone does:

```
stream = lldb.SBStream()
bkpt.GetDescription(stream, eDescriptionLevelWhatever)

```

At present SBStream's are backed by StreamString objects which get constructed 
with use_color -> false.  So you should be okay there.

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Chelsea Cassanova via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

chelcassanova wrote:

Ah, so using `Printf` with `ansi::FormatAnsiTerminalCodes` instead? And yes 
using this with `--no-use-colors` does show the colour here so it looks like 
this isn't being checked at the stream level.

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

jimingham wrote:



> On May 7, 2024, at 2:51 PM, royitaqi ***@***.***> wrote:
> 
> 
> @royitaqi commented on this pull request.
> 
> In lldb/include/lldb/Core/Debugger.h 
> :
> 
> > @@ -731,8 +747,11 @@ class Debugger : public 
> > std::enable_shared_from_this,
>lldb::TargetSP m_dummy_target_sp;
>Diagnostics::CallbackID m_diagnostics_callback_id;
>  
> -  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
> -  void *m_destroy_callback_baton = nullptr;
> +  std::recursive_mutex m_destroy_callback_mutex;
> BTW:
> 
> I think you need to at least protect adding.
> 
> To me, it's either none, or all.
> 
IME you don't want to just keep adding mutexes willy-nilly, you do want to know 
that the resource could be contended for by operations on more than one thread. 
My only point was that we couldn't think of a plausible reason to lock when 
iterating for destroy, but it's easy to see that you need to lock adding and 
removing since there's nothing that keeps that from happening.

> If we decide to protect adding (multiple adds from different threads not 
> during destroy), then we should just protect all of add/remove/destroy, 
> because any of these operations can happen at the same time from multiple 
> threads.
> 
> FWIW I agree with what @JDevlieghere  said:
> 
> I'd say let's not overthink this and if we want to change that, let's do it 
> holistically for the whole class instead of piecemeal.
> 
I didn't really understand that statement.  Adding debuggers, or querying them 
is controlled by the debugger list mutex.  Manipulating the I/O handlers is 
governed by the io handler stack mutex, there's a lock around the interrupt 
requests.  Debugger doesn't seem like a class that is not locking the lists of 
things it controls as a general rule.

When you have a collection that can be accessed from multiple threads, adding 
and removing, then you really do need to protect those accesses.  I'm not sure 
it's good reasoning to say "That's not likely, let's wait till it crashes on 
someone to add the protection".

Jim

> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Jonas Devlieghere via lldb-commits]

@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);

JDevlieghere wrote:

This is going to regex search for disabled and then replace it with its colored 
variant. That seems unnecessarily inefficient. You know whether you're going to 
put enabled or disabled, so you could print it colored right away. 

You might also have to check the debugger setting for whether colors are 
enabled (`Debugger::GetUseColor()`). Sometimes it's set at the stream level: 
the stream might have colors disabled if it was created somewhere were the 
setting was checked). I don't know if that's always going to be the case here. 
If it's not, we should check the setting here. To find out I would run lldb 
with `--no-use-colors` and see if the colors still get printed. 

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

@@ -731,8 +747,11 @@ class Debugger : public 
std::enable_shared_from_this,
   lldb::TargetSP m_dummy_target_sp;
   Diagnostics::CallbackID m_diagnostics_callback_id;
 
-  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
-  void *m_destroy_callback_baton = nullptr;
+  std::recursive_mutex m_destroy_callback_mutex;

royitaqi wrote:

BTW:
> I think you need to at least protect adding.

To me, it's either none, or all.

If we decide to protect adding (multiple adds from different threads not during 
destroy), then we should just protect all of add/remove/destroy, because any of 
these operations can happen at the same time from multiple threads.

FWIW I agree with what @JDevlieghere said:

> I'd say let's not overthink this and if we want to change that, let's do it 
> holistically for the whole class instead of piecemeal.


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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
   UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
 
   UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
   : m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}

ZequanWu wrote:

I just removed this dead ctor.

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

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[Jason Molenda via lldb-commits]

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

This looks good to me, I know there are other codepaths that handle this 
correctly, where we can backtrace out of a frameless function that faults into 
a trap handler and we have the entire register state available in the trap 
handler.  

Looking at this, I'm a little uncertain why we have 
`m_behaves_like_zeroth_frame` and `m_all_registers_available` which are both 
set to true under the same conditions, and then we sometimes use 
`m_behaves_like_zeroth_frame`, sometimes `m_all_registers_available `, and 
sometimes call `RegisterContextUnwind::BehavesLikeZerothFrame` which has a 
redundant check if the frame number is 0, sigh.  Looks like some accumulated 
nonsense that you shouldn't have to deal with in this patch.

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

jimingham wrote:



> On May 7, 2024, at 11:28 AM, royitaqi ***@***.***> wrote:
> 
> 
> Hi @jimingham  ,
> 
> Sorry to come in a little late on this
> 
> No worries. Thanks for chiming in.
> 
> I think we need a setting to turn this off as well. If you aren't planning to 
> use the transcript, you shouldn't have to pay the cost for it.
> 
> Could you elaborate about why this is necessary? If we really need this, can 
> I add the setting in a follow-up PR?
> 
> --
> 
> FWIW, to help make the conversation faster (reduce rounds of communication), 
> I will write down my gut feeling below. Please kindly correct me if I'm wrong.
> 
> First, why I think the added cost is negligible:
> 
> The baseline is that there is already a m_transcript_stream, which doesn't 
> have any settings to turn off. This PR just doubles that cost (plus some 
> string split operations). It doesn't add huge cost on top of this baseline.
That's equally an argument for allowing me to turn that off as well - if I 
never plan to use `session save` then it's silly to take up memory for that 
either.  

> Maintaining transcript is a per-command operation, and commands are 
> relatively infrequently invoked. Here I assume the common use cases run tens 
> or at most hundreds of commands in a debug session. The above cost times 10x 
> ~ 100x isn't much.
People have automated scripts that run debug sessions, and you are also storing 
output text so this can get fairly big.  Plus if you start adding time stamps 
we're stopping to get the system clock on every command, it gets more 
expensive.  For people running lldb on big beefy machines, this is unlikely to 
matter for, but people also run lldb on low memory devices, where resources are 
tighter.


> The cost of the commands themselves (and the process) are much higher than 
> that of the transcript. E.g. resolving breakpoints and printing variables all 
> need to do actual work, rather than just recording info.
I don't think "unrelated thing X is bigger" is a a useful data point when 
considering whether it's good to do Y.


> Secondly, if we are going to add settings, there needs to be product design 
> considerations based on user usage/feedback, so we probably need more time to 
> gather data points after shipping this feature.
> 
> Prematurely add settings (which we cannot remove, I assume) can lead us down 
> to a more difficult road.

Not sure why a setting to turn off recording transcripts would ever be 
controversial.

> I think it will be the norm that we record transcripts, like bash/zsh records 
> command histories, so by default this should be on. In fact it is already, by 
> m_transcript_stream.
I guess.  I must admit I've used lldb since it existed and have never once used 
`session save`.  After all, my Terminal has a nice big buffer...  For my 
workflow that's all wasted memory.


> Besides just a binary on/off, settings can also allow more granular control. 
> E.g. verbosity; only log transcript for a subset of commands; limit the 
> memory footprint by using a cyclic buffer; etc. If we add these settings, 
> they will cause the on/off setting to be redundant and needs to be removed.
I don't see the force of this argument at all.  Having a control "whether to do 
X at all" and then another set "if I were to do X, how should I do it" is a 
pretty standard approach.  As a matter of fact, that's a better design since 
then I can disable the feature temporarily without having to mess up the 
settings I want when I do do it.

> (As mentioned in "Secondly") The above are all possibilities. We need user 
> usage/feedback to guide the design of these settings. So I think we should 
> ship this PR first then gather data points.
I really don't think this is controversial, and I'm not sure how you would 
gather data points. If it helps to get started, however, I vote there should be 
a way to turn this off.

But if you are running out of time for this feature and really need to get it 
off your plate, deferring the ability to turn it off to a subsequent PR is okay.

Jim

> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Alex Langford via lldb-commits]

@@ -400,6 +400,12 @@ friend class Breakpoint;
   /// Which options are set at this level.
   /// Drawn from BreakpointOptions::SetOptionsFlags.
   Flags m_set_flags;
+  /// Settings that allow the 'disabled' keyword to be displayed in red.
+  Stream::HighlightSettings m_disbaled_breakpoint_highlight_settings{
+  "disabled",
+  "\x1b[31m",
+  "\x1b[37m",

bulbazord wrote:

You don't need to hardcode these values yourself. Take a look at 
`AnsiTerminal.h`, there are some constants defined for this purpose.

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Alex Langford via lldb-commits]

@@ -400,6 +400,12 @@ friend class Breakpoint;
   /// Which options are set at this level.
   /// Drawn from BreakpointOptions::SetOptionsFlags.
   Flags m_set_flags;
+  /// Settings that allow the 'disabled' keyword to be displayed in red.
+  Stream::HighlightSettings m_disbaled_breakpoint_highlight_settings{

bulbazord wrote:

typo: `disbaled` -> `disabled`

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Alex Langford via lldb-commits]

https://github.com/bulbazord requested changes to this pull request.


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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Alex Langford via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
   UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
 
   UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
   : m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}

dwblaikie wrote:

Ah, cool - thanks, sorry I missed that.

If you wanted to remove the dead ctor either before or after this commit, that 
might be handy/nice.

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward 
declaration...
+# CHECK: (t2 >)  {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward 
declaration...
+# CHECK: (t1)  {}
+
+#--- lldb.cmd

ZequanWu wrote:

Yeah, that would work. I have an example at 
https://discourse.llvm.org/t/rfc-delay-definition-die-searching-when-parse-a-declaration-die-for-record-type/78526/15.

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Chelsea Cassanova (chelcassanova)


Changes

This commit adds colour highlighting to the `disabled` keyword in the 
breakpoint list so that it appears red (and should be easier to see in a large 
sea of breakpoints).

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


2 Files Affected:

- (modified) lldb/include/lldb/Breakpoint/BreakpointOptions.h (+6) 
- (modified) lldb/source/Breakpoint/BreakpointOptions.cpp (+2-1) 


``diff
diff --git a/lldb/include/lldb/Breakpoint/BreakpointOptions.h 
b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
index 7bf545717422f..3dc3a7190d03e 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointOptions.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
@@ -400,6 +400,12 @@ friend class Breakpoint;
   /// Which options are set at this level.
   /// Drawn from BreakpointOptions::SetOptionsFlags.
   Flags m_set_flags;
+  /// Settings that allow the 'disabled' keyword to be displayed in red.
+  Stream::HighlightSettings m_disbaled_breakpoint_highlight_settings{
+  "disabled",
+  "\x1b[31m",
+  "\x1b[37m",
+  };
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp 
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 6c6037dd9edd3..993de590b0d4f 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);
 
 if (m_one_shot)
   s->Printf("one-shot ");

``




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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward 
declaration...
+# CHECK: (t2 >)  {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward 
declaration...
+# CHECK: (t1)  {}
+
+#--- lldb.cmd

dwblaikie wrote:

Hmm - what about template parameters, even without simplified template names?

`t1` t2 should be a declaration, without looking up the definition DIE in 
this case? (though perhaps clang/LLDB doesn't do that correctly, in which case 
maybe `t1` might work)

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

[Lldb-commits] [lldb] [lldb][breakpointoptions] Make disabled keyword red (PR #91404)

[Chelsea Cassanova via lldb-commits]

https://github.com/chelcassanova created 
https://github.com/llvm/llvm-project/pull/91404

This commit adds colour highlighting to the `disabled` keyword in the 
breakpoint list so that it appears red (and should be easier to see in a large 
sea of breakpoints).

>From 4458e951ed4b8d226974000dae2705ad6ec6ed79 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Tue, 7 May 2024 14:20:14 -0700
Subject: [PATCH] [lldb][breakpointoptions] Make disabled keyword red

This commit adds colour highlighting to the `disabled` keyword in the
breakpoint list so that it appears red (and should be easier to see in a
large sea of breakpoints).
---
 lldb/include/lldb/Breakpoint/BreakpointOptions.h | 6 ++
 lldb/source/Breakpoint/BreakpointOptions.cpp | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointOptions.h 
b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
index 7bf545717422..3dc3a7190d03 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointOptions.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointOptions.h
@@ -400,6 +400,12 @@ friend class Breakpoint;
   /// Which options are set at this level.
   /// Drawn from BreakpointOptions::SetOptionsFlags.
   Flags m_set_flags;
+  /// Settings that allow the 'disabled' keyword to be displayed in red.
+  Stream::HighlightSettings m_disbaled_breakpoint_highlight_settings{
+  "disabled",
+  "\x1b[31m",
+  "\x1b[37m",
+  };
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp 
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 6c6037dd9edd..993de590b0d4 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -534,7 +534,8 @@ void BreakpointOptions::GetDescription(Stream *s,
 
 if (m_ignore_count > 0)
   s->Printf("ignore: %d ", m_ignore_count);
-s->Printf("%sabled ", m_enabled ? "en" : "dis");
+s->PutCStringColorHighlighted(m_enabled ? "enabled " : "disabled ",
+  m_disbaled_breakpoint_highlight_settings);
 
 if (m_one_shot)
   s->Printf("one-shot ");

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

@@ -731,8 +747,11 @@ class Debugger : public 
std::enable_shared_from_this,
   lldb::TargetSP m_dummy_target_sp;
   Diagnostics::CallbackID m_diagnostics_callback_id;
 
-  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
-  void *m_destroy_callback_baton = nullptr;
+  std::recursive_mutex m_destroy_callback_mutex;

jimingham wrote:

That's true of running the destroy chain callbacks, but you certainly can add a 
callback from more than one thread at a time.  I think you need to at least 
protect adding.

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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Zequan Wu via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward 
declaration...
+# CHECK: (t2 >)  {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward 
declaration...
+# CHECK: (t1)  {}
+
+#--- lldb.cmd

ZequanWu wrote:

I tried something like this:
```
struct Bar {
  int x;
  Bar(int x): x(x) {}
};
struct Foo {
  Bar* bar;
  Foo(int x) {
bar = new Bar(x);
  }
};
```
In another CU, print variable with type `Foo*`, but it still tries to complete 
both `Foo` and `Bar` because the looking-up  child count through a pointer 
issue.

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
   UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
 
   UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
   : m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}

ZequanWu wrote:

It's never used. It first gets assigned at 
https://github.com/llvm/llvm-project/pull/90663/files#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1929
 then updated to `false` at 
https://github.com/llvm/llvm-project/pull/90663/files#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1808
 when we find definition DIE.

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/90663

>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/8] [lldb][DWARF] Delay struct/class/union definition DIE
 searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 270 +++---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 105 ++-
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  14 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   5 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h |   2 +
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   |  95 +++---
 .../SymbolFile/DWARF/UniqueDWARFASTType.h |  21 +-
 7 files changed, 296 insertions(+), 216 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840a..7ad661c9a9d49 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
 static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
-   DWARFDIE die,
+   const DWARFDIE _ctx_die,
+   const DWARFDIE ,
const char *type_name_cstr) {
   auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
   if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang 
,
 type_name_cstr ? type_name_cstr : "", die.GetOffset());
   }
 
+  // By searching for the definition DIE of the decl_ctx type, we will either:
+  // 1. Found the the definition DIE and start its definition with
+  // TypeSystemClang::StartTagDeclarationDefinition.
+  // 2. Unable to find it, then need to forcefully complete it.
+  die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+  if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
   // We don't have a type definition and/or the import failed. We must
   // forcefully complete the type to avoid crashes.
   ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const 
SymbolContext ,
   if (tag == DW_TAG_typedef) {
 // DeclContext will be populated when the clang type is materialized in
 // Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+   decl_ctx_die, die, attrs.name.GetCString());
 
 if (attrs.type.IsValid()) {
   // Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  if (class_type->GetID() != decl_ctx_die.GetID() ||
-  IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
-  std::vector failures;
-
-  CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
-  // FIXME do something with these failures that's
-  // smarter than just dropping them on the ground.
-  // Unfortunately classes don't like having stuff added
-  // to them after their definitions are complete...
-
-  Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-  if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
-  }
-}
-  }
-
   if (attrs.specification.IsValid()) {
 // We have a specification which we are going to base our
 // function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
   }
 }
   }
+  // By here, we should have already completed the c++ class_type
+  // 

[Lldb-commits] [lldb] 8c4d798 - Add a missing check for nullptr

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2024-05-07T13:44:44-07:00
New Revision: 8c4d7989c2b4a7e251afc3b13002611646de90b6

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

LOG: Add a missing check for nullptr

This can't happen with Clang, but I've seen a crash report from the
Swift plugin where this happened.

rdar://126564844

Added: 


Modified: 
lldb/source/Expression/UserExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index 06fdb7007ced..b78f43995767 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -300,6 +300,8 @@ UserExpression::Evaluate(ExecutionContext _ctx,
 target->GetUserExpressionForLanguage(
 fixed_expression->c_str(), full_prefix, language, desired_type,
 options, ctx_obj, error));
+if (!fixed_expression_sp)
+  break;
 DiagnosticManager fixed_diagnostic_manager;
 parse_success = fixed_expression_sp->Parse(
 fixed_diagnostic_manager, exe_ctx, execution_policy,



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

[Lldb-commits] [lldb] 272ea28 - Remove else-after-break (NFC)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2024-05-07T13:44:44-07:00
New Revision: 272ea28bdec93b33527dc54edbdef8f43c51df47

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

LOG: Remove else-after-break (NFC)

Added: 


Modified: 
lldb/source/Expression/UserExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index 5658426c88912..06fdb7007cede 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -308,17 +308,16 @@ UserExpression::Evaluate(ExecutionContext _ctx,
   diagnostic_manager.Clear();
   user_expression_sp = fixed_expression_sp;
   break;
+}
+// The fixed expression also didn't parse. Let's check for any new
+// fixits we could try.
+if (!fixed_expression_sp->GetFixedText().empty()) {
+  *fixed_expression = fixed_expression_sp->GetFixedText().str();
 } else {
-  // The fixed expression also didn't parse. Let's check for any new
-  // Fix-Its we could try.
-  if (!fixed_expression_sp->GetFixedText().empty()) {
-*fixed_expression = fixed_expression_sp->GetFixedText().str();
-  } else {
-// Fixed expression didn't compile without a fixit, don't retry and
-// don't tell the user about it.
-fixed_expression->clear();
-break;
-  }
+  // Fixed expression didn't compile without a fixit, don't retry and
+  // don't tell the user about it.
+  fixed_expression->clear();
+  break;
 }
   }
 }



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

[Lldb-commits] [lldb] [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (PR #91390)

[Jonas Devlieghere via lldb-commits]

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

[Lldb-commits] [lldb] dad1109 - [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (#91390)

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2024-05-07T13:12:22-07:00
New Revision: dad11097096c05564758e539f9f03ef883365fdd

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

LOG: [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (#91390)

Alex pointed out in #91254 that we only need the custom target if we had
more than one target depending on it. This isn't the case upstream, but
on our downstream fork, we have a second dependency. Reintroduce the
target so that everything can depend on that, without the
single-dependency foot-gun.

Added: 


Modified: 
lldb/source/API/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 798a92874f13d..aa31caddfde3a 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -20,7 +20,7 @@ if(LLDB_ENABLE_LUA)
   set(lldb_lua_wrapper ${lua_bindings_dir}/LLDBWrapLua.cpp)
 endif()
 
-# Target to generate SBLanguages.h from Dwarf.def.
+# Generate SBLanguages.h from Dwarf.def.
 set(sb_languages_file
   ${CMAKE_CURRENT_BINARY_DIR}/../../include/lldb/API/SBLanguages.h)
 add_custom_command(
@@ -33,6 +33,8 @@ add_custom_command(
   DEPENDS ${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat/Dwarf.def
   WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
 )
+add_custom_target(lldb-sbapi-dwarf-enums
+  DEPENDS ${sb_languages_file})
 
 add_lldb_library(liblldb SHARED ${option_framework}
   SBAddress.cpp
@@ -113,7 +115,9 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}
   ${lldb_lua_wrapper}
-  ${sb_languages_file}
+
+  DEPENDS
+lldb-sbapi-dwarf-enums
 
   LINK_LIBS
 lldbBreakpoint



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

[Lldb-commits] [lldb] [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (PR #91390)

[Alex Langford via lldb-commits]

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

Thanks for taking care of that.

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

[Lldb-commits] [lldb] [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (PR #91390)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

Alex pointed out in #91254 that we only need the custom target if we 
had more than one target depending on it. This isn't the case upstream, but on 
our downstream fork, we have a second dependency. Reintroduce the target so 
that everything can depend on that, without the single-dependency foot-gun.

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


1 Files Affected:

- (modified) lldb/source/API/CMakeLists.txt (+6-2) 


``diff
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 798a92874f13..aa31caddfde3 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -20,7 +20,7 @@ if(LLDB_ENABLE_LUA)
   set(lldb_lua_wrapper ${lua_bindings_dir}/LLDBWrapLua.cpp)
 endif()
 
-# Target to generate SBLanguages.h from Dwarf.def.
+# Generate SBLanguages.h from Dwarf.def.
 set(sb_languages_file
   ${CMAKE_CURRENT_BINARY_DIR}/../../include/lldb/API/SBLanguages.h)
 add_custom_command(
@@ -33,6 +33,8 @@ add_custom_command(
   DEPENDS ${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat/Dwarf.def
   WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
 )
+add_custom_target(lldb-sbapi-dwarf-enums
+  DEPENDS ${sb_languages_file})
 
 add_lldb_library(liblldb SHARED ${option_framework}
   SBAddress.cpp
@@ -113,7 +115,9 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}
   ${lldb_lua_wrapper}
-  ${sb_languages_file}
+
+  DEPENDS
+lldb-sbapi-dwarf-enums
 
   LINK_LIBS
 lldbBreakpoint

``




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

[Lldb-commits] [lldb] [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC) (PR #91390)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/91390

Alex pointed out in #91254 that we only need the custom target if we had more 
than one target depending on it. This isn't the case upstream, but on our 
downstream fork, we have a second dependency. Reintroduce the target so that 
everything can depend on that, without the single-dependency foot-gun.

>From 3315ede7d03b8a0e5abf5db948e20a93f5530fb2 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 7 May 2024 13:05:34 -0700
Subject: [PATCH] [lldb] Reinstate lldb-sbapi-dwarf-enums target (NFC)

Alex pointed out in #91254 that we only need the custom target if we had
more than one target depending on it. This isn't the case upstream, but
on our downstream fork, we have a second dependency. Reintroduce the
target so that everything can depend on that, without the
single-dependency foot-gun.
---
 lldb/source/API/CMakeLists.txt | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 798a92874f13d..aa31caddfde3a 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -20,7 +20,7 @@ if(LLDB_ENABLE_LUA)
   set(lldb_lua_wrapper ${lua_bindings_dir}/LLDBWrapLua.cpp)
 endif()
 
-# Target to generate SBLanguages.h from Dwarf.def.
+# Generate SBLanguages.h from Dwarf.def.
 set(sb_languages_file
   ${CMAKE_CURRENT_BINARY_DIR}/../../include/lldb/API/SBLanguages.h)
 add_custom_command(
@@ -33,6 +33,8 @@ add_custom_command(
   DEPENDS ${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat/Dwarf.def
   WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
 )
+add_custom_target(lldb-sbapi-dwarf-enums
+  DEPENDS ${sb_languages_file})
 
 add_lldb_library(liblldb SHARED ${option_framework}
   SBAddress.cpp
@@ -113,7 +115,9 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}
   ${lldb_lua_wrapper}
-  ${sb_languages_file}
+
+  DEPENDS
+lldb-sbapi-dwarf-enums
 
   LINK_LIBS
 lldbBreakpoint

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
   UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
 
   UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
   : m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}

dwblaikie wrote:

Where is this ctor used? I can't find any calls to it.

And if it is never used, how is `m_is_forward_declaration` ever `false`, if 
it's initialized to true and this ctor is never called, I don't see any other 
place that assigns to it?

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -1632,27 +1632,34 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
-
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  DWARFDIE dwarf_die = GetDIE(die_it->second);
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  Type *type = nullptr;
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
+type = dwarf_ast->FindDefinitionTypeForDIE(dwarf_die);
+  if (!type)
+return false;
 
-Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
-if (log)
-  GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-  log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
-  dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
-  dwarf_die.Tag(), type->GetName().AsCString());
-assert(compiler_type);
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
-  return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+  die_it = GetForwardDeclCompilerTypeToDIE().find(
+  compiler_type_no_qualifiers.GetOpaqueQualType());
+  if (die_it != GetForwardDeclCompilerTypeToDIE().end()) {
+dwarf_die = GetDIE(die_it->getSecond());
+GetForwardDeclCompilerTypeToDIE().erase(die_it);
   }
-  return false;
+
+  Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
+  if (log)

dwblaikie wrote:

```
if (Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion))
```

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward 
declaration...
+# CHECK: (t2 >)  {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type 
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward 
declaration...
+# CHECK: (t1)  {}
+
+#--- lldb.cmd

dwblaikie wrote:

Might be better to test this without `-gsimple-template-names`, because it's a 
more generally valuable feature than that?

Oh, but that "look for the child count through a pointer" issue comes up and 
it's hard to reach this situation without `-gsimple-template-names`? (Maybe 
indirectly - if you print a struct with a pointer - perhaps that doesn't do the 
child count searching through the pointer?)

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[David Blaikie via lldb-commits]

@@ -16,61 +16,66 @@ using namespace lldb_private::plugin::dwarf;
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
   const lldb_private::Declaration ,
   const int32_t byte_size,
+  bool is_forward_declaration,
   UniqueDWARFASTType ) const {
   for (const UniqueDWARFASTType  : m_collection) {
 // Make sure the tags match
 if (udt.m_die.Tag() == die.Tag()) {
-  // Validate byte sizes of both types only if both are valid.
-  if (udt.m_byte_size < 0 || byte_size < 0 ||
-  udt.m_byte_size == byte_size) {
-// Make sure the file and line match
-if (udt.m_declaration == decl) {
-  // The type has the same name, and was defined on the same file and
-  // line. Now verify all of the parent DIEs match.
-  DWARFDIE parent_arg_die = die.GetParent();
-  DWARFDIE parent_pos_die = udt.m_die.GetParent();
-  bool match = true;
-  bool done = false;
-  while (!done && match && parent_arg_die && parent_pos_die) {
-const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
-  switch (parent_arg_tag) {
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type:
-  case DW_TAG_namespace: {
-const char *parent_arg_die_name = parent_arg_die.GetName();
-if (parent_arg_die_name ==
-nullptr) // Anonymous (i.e. no-name) struct
-{
-  match = false;
-} else {
-  const char *parent_pos_die_name = parent_pos_die.GetName();
-  if (parent_pos_die_name == nullptr ||
-  ((parent_arg_die_name != parent_pos_die_name) &&
-   strcmp(parent_arg_die_name, parent_pos_die_name)))
-match = false;
-}
-  } break;
-
-  case DW_TAG_compile_unit:
-  case DW_TAG_partial_unit:
-done = true;
-break;
-  default:
-break;
-  }
+  // If they are not both definition DIEs or both declaration DIEs, then
+  // don't check for byte size and declaration location, because 
declaration
+  // DIEs usually don't have those info.
+  bool matching_size_declaration =
+  udt.m_is_forward_declaration != is_forward_declaration
+  ? true
+  : (udt.m_byte_size < 0 || byte_size < 0 ||
+ udt.m_byte_size == byte_size) &&
+udt.m_declaration == decl;
+  if (!matching_size_declaration)
+continue;
+  // The type has the same name, and was defined on the same file and
+  // line. Now verify all of the parent DIEs match.
+  DWARFDIE parent_arg_die = die.GetParent();
+  DWARFDIE parent_pos_die = udt.m_die.GetParent();
+  bool match = true;
+  bool done = false;
+  while (!done && match && parent_arg_die && parent_pos_die) {
+const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
+const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
+if (parent_arg_tag == parent_pos_tag) {
+  switch (parent_arg_tag) {
+  case DW_TAG_class_type:
+  case DW_TAG_structure_type:
+  case DW_TAG_union_type:
+  case DW_TAG_namespace: {
+const char *parent_arg_die_name = parent_arg_die.GetName();
+if (parent_arg_die_name ==
+nullptr) // Anonymous (i.e. no-name) struct
+{

dwblaikie wrote:

The comment between the `)` and `{` creates some weird line wrapping, maybe put 
the comment inside the `{` block?

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

[Lldb-commits] [lldb] [lldb] Request crash report when prompting for a bug report on Darwin (PR #91371)

[Jonas Devlieghere via lldb-commits]

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

[Lldb-commits] [lldb] 057de4d - [lldb] Request crash report when prompting for a bug report on Darwin (#91371)

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2024-05-07T12:35:06-07:00
New Revision: 057de4d26425c8b9840912e40ce025626f45d8d6

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

LOG: [lldb] Request crash report when prompting for a bug report on Darwin 
(#91371)

On Darwin platforms, the system will generate a crash report in
~/Library/Logs/DiagnosticReports/ when a process crashes.

These reports are much more useful than the "pretty backtraces" printed
by LLVM and are preferred when filing bug reports on Darwin.

Added: 


Modified: 
lldb/tools/driver/Driver.cpp
lldb/tools/lldb-dap/lldb-dap.cpp

Removed: 




diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index a821699c5e2ec..14371da64f2f2 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -733,8 +733,14 @@ int main(int argc, char const *argv[]) {
   // Setup LLVM signal handlers and make sure we call llvm_shutdown() on
   // destruction.
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   // Parse arguments.
   LLDBOptTable T;

diff  --git a/lldb/tools/lldb-dap/lldb-dap.cpp 
b/lldb/tools/lldb-dap/lldb-dap.cpp
index cf52a22b18cc1..f35abd665e844 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4192,8 +4192,14 @@ int SetupStdoutStderrRedirection() {
 
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);



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

[Lldb-commits] [lldb] [lldb] Request crash report when prompting for a bug report on Darwin (PR #91371)

[Felipe de Azevedo Piovezan via lldb-commits]

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


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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

royitaqi wrote:

Hi @jimingham ,

> Sorry to come in a little late on this

No worries. Thanks for chiming in.

> I think we need a setting to turn this off as well. If you aren't planning to 
> use the transcript, you shouldn't have to pay the cost for it.

Could you elaborate about why this is necessary? If we really need this, can I 
add the setting in a follow-up PR?

--

FWIW, to help make the conversation faster (reduce rounds of communication), I 
will write down my gut feeling below. Please kindly correct me if I'm wrong.

**First**, why I think the added cost is negligible:
1. The baseline is that there is already a `m_transcript_stream`, which doesn't 
have any settings to turn off. This PR just doubles that cost (plus some string 
split operations). It doesn't add huge cost on top of this baseline.
2. Maintaining transcript is a per-command operation, and commands are 
relatively infrequently invoked. Here I assume the common use cases run tens or 
at most hundreds of commands in a debug session. The above cost times 10x ~ 
100x isn't much.
3. The cost of the commands themselves (and the process) are much higher than 
that of the transcript. E.g. resolving breakpoints and printing variables all 
need to do actual work, rather than just recording info.

**Secondly**, if we are going to add settings, there needs to be product design 
considerations _based on user usage/feedback_, so we probably need more time to 
gather data points after shipping this feature.
1. Prematurely add settings (which we cannot remove, I assume) can lead us down 
to a more difficult road.
2. I think it will be the norm that we record transcripts, like bash/zsh 
records command histories, so by default this should be on. In fact it is 
already, by `m_transcript_stream`.
3. Besides just a binary on/off, settings can also allow more granular control. 
E.g. verbosity; only log transcript for a subset of commands; limit the memory 
footprint by using a cyclic buffer; etc. If we add these settings, they will 
cause the on/off setting to be redundant and needs to be removed.
4. (As mentioned in "Secondly") The above are all possibilities. We need user 
usage/feedback to guide the design of these settings. So I think we should ship 
this PR first then gather data points.


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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Zequan Wu via lldb-commits]

ZequanWu wrote:

> > For example: 
> > https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L922-L927.
> >  When parsing function info, it validates low and hi address of the 
> > function: `GetMinRangeBase` returns the first range entry base and 
> > `GetMaxRangeEnd` returns the last range end. If low >= hi, it stops parsing 
> > this function.
> > This causes missing inline stack frames for us when debugging a core dump.
> 
> Gotcha, makes sense. Since you know how this fails, could you add a test to 
> make sure that scenario continues working?

Yeah, I updated the test 
`lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s` so that two ranges in 
`.debug_rnglists` are out of order and `image lookup -v -s lookup_rnglists` is 
still able to produce sorted ranges for the inner block:
Without this change:
```
 Blocks: id = {0x0030}, range = [0x-0x0004)
 Symbol: id = {0x0003}, range = 
[0x0001-0x0004), name="lookup_rnglists"
```
With this change:
```
 Blocks: id = {0x0030}, range = [0x-0x0004)
 id = {0x0046}, ranges = 
[0x0001-0x0002)[0x0003-0x0004)
 Symbol: id = {0x0003}, range = 
[0x0001-0x0004), name="lookup_rnglists"
```




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

[Lldb-commits] [lldb] [lldb] Request crash report when prompting for a bug report on Darwin (PR #91371)

[via lldb-commits]

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

LGTM

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

[Lldb-commits] [lldb] [lldb] Request crash report when prompting for a bug report on Darwin (PR #91371)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

On Darwin platforms, the system will generate a crash report in 
~/Library/Logs/DiagnosticReports/ when a process crashes.

These reports are much more useful than the "pretty backtraces" printed by LLVM 
and are preferred when filing bug reports on Darwin.

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


2 Files Affected:

- (modified) lldb/tools/driver/Driver.cpp (+6) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+6) 


``diff
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index a821699c5e2ec..14371da64f2f2 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -733,8 +733,14 @@ int main(int argc, char const *argv[]) {
   // Setup LLVM signal handlers and make sure we call llvm_shutdown() on
   // destruction.
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   // Parse arguments.
   LLDBOptTable T;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index cf52a22b18cc1..f35abd665e844 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4192,8 +4192,14 @@ int SetupStdoutStderrRedirection() {
 
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);

``




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

[Lldb-commits] [lldb] [lldb] Request crash report when prompting for a bug report on Darwin (PR #91371)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/91371

On Darwin platforms, the system will generate a crash report in 
~/Library/Logs/DiagnosticReports/ when a process crashes.

These reports are much more useful than the "pretty backtraces" printed by LLVM 
and are preferred when filing bug reports on Darwin.

>From 5821138c31b91b4e1df5757db23f571292f925a0 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 7 May 2024 11:15:22 -0700
Subject: [PATCH] [lldb] Request crash report when prompting for a bug report
 on Darwin

On Darwin platforms, the system will generate a crash report in
~/Library/Logs/DiagnosticReports/ when a process crashes.

These reports are much more useful than the "pretty backtraces" printed
by LLVM and are preferred when filing bug reports on Darwin.
---
 lldb/tools/driver/Driver.cpp | 6 ++
 lldb/tools/lldb-dap/lldb-dap.cpp | 6 ++
 2 files changed, 12 insertions(+)

diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index a821699c5e2ec2..14371da64f2f2f 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -733,8 +733,14 @@ int main(int argc, char const *argv[]) {
   // Setup LLVM signal handlers and make sure we call llvm_shutdown() on
   // destruction.
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   // Parse arguments.
   LLDBOptTable T;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index cf52a22b18cc14..f35abd665e8449 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4192,8 +4192,14 @@ int SetupStdoutStderrRedirection() {
 
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
+#if !defined(__APPLE__)
   llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
 " and include the crash backtrace.\n");
+#else
+  llvm::setBugReportMsg("PLEASE submit a bug report to " LLDB_BUG_REPORT_URL
+" and include the crash report from "
+"~/Library/Logs/DiagnosticReports/.\n");
+#endif
 
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);

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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Alex Langford via lldb-commits]

bulbazord wrote:

> For example: 
> https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L922-L927.
>  When parsing function info, it validates low and hi address of the function: 
> `GetMinRangeBase` returns the first range entry base and `GetMaxRangeEnd` 
> returns the last range end. If low >= hi, it stops parsing this function.
> 
> This causes missing inline stack frames for us when debugging a core dump.

Gotcha, makes sense. Since you know how this fails, could you add a test to 
make sure that scenario continues working? 

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

[Lldb-commits] [lldb] [lldb] Make SBType::GetDirectNestedType (mostly) work with typedefs (PR #91189)

[Michael Buch via lldb-commits]

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

LGTM

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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Zequan Wu via lldb-commits]

ZequanWu wrote:

> > Some places assume the ranges are already sorted but it's not. This fixes 
> > it.
> 
> What places assumed it was sorted? I assume this means there are some bugs 
> that this fixes? Can you give some more details about how you came to write 
> this patch?

For example: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L922-L927.
When parsing function info, it validates low and hi address of the function: 
`GetMinRangeBase` returns the first range entry base and `GetMaxRangeEnd` 
returns the last range end. If low >= hi, it stops parsing this function.

This causes missing inline stack frames for us when debugging a core dump.

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[Alex Langford via lldb-commits]

@@ -731,8 +747,11 @@ class Debugger : public 
std::enable_shared_from_this,
   lldb::TargetSP m_dummy_target_sp;
   Diagnostics::CallbackID m_diagnostics_callback_id;
 
-  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
-  void *m_destroy_callback_baton = nullptr;
+  std::recursive_mutex m_destroy_callback_mutex;

bulbazord wrote:

@JDevlieghere Yes, I don't think we need to synchronize across threads, that's 
a good point. I was thinking about what if any of the callbacks add, remove, or 
otherwise invoke other callbacks. But that shouldn't require a mutex.

@royitaqi I think you can remove the mutex :)

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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Alex Langford via lldb-commits]

bulbazord wrote:

> Some places assume the ranges are already sorted but it's not. This fixes it.

What places assumed it was sorted? I assume this means there are some bugs that 
this fixes? Can you give some more details about how you came to write this 
patch?

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[Jonas Devlieghere via lldb-commits]

@@ -731,8 +747,11 @@ class Debugger : public 
std::enable_shared_from_this,
   lldb::TargetSP m_dummy_target_sp;
   Diagnostics::CallbackID m_diagnostics_callback_id;
 
-  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
-  void *m_destroy_callback_baton = nullptr;
+  std::recursive_mutex m_destroy_callback_mutex;

JDevlieghere wrote:

@bulbazord But that's only if you call `Debugger::Destroy` (and therefore run 
the callbacks) on another thread, right? Given nothing else in `(SB)Debugger` 
provides such safety guarantees (besides the global debugger list) I'd say 
let's not overthink this and if we want to change that, let's do it 
holistically for the whole class instead of piecemeal. 

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

@@ -2044,6 +2052,15 @@ bool CommandInterpreter::HandleCommand(const char 
*command_line,
   m_transcript_stream << result.GetOutputData();
   m_transcript_stream << result.GetErrorData();
 
+  // Add output and error to the transcript item after splitting lines. In the
+  // future, other aspects of the command (e.g. perf) can be added, too.
+  transcript_item->AddItem(
+  "output", StructuredData::Array::SplitString(result.GetOutputData(), 
'\n',
+   -1, false));
+  transcript_item->AddItem(
+  "error", StructuredData::Array::SplitString(result.GetErrorData(), '\n',
+  -1, false));

royitaqi wrote:

See my rely to your other comment about I plan to do it as a separate, 
follow-up PR.

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

[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

[via lldb-commits]

@@ -766,6 +768,12 @@ class CommandInterpreter : public Broadcaster,
   CommandUsageMap m_command_usages;
 
   StreamString m_transcript_stream;
+
+  /// Contains a list of handled commands, output and error. Each element in
+  /// the list is a dictionary with three keys: "command" (string), "output"
+  /// (list of strings) and optionally "error" (list of strings). Each string
+  /// in "output" and "error" is a line (without EOL characters).

royitaqi wrote:

Yes, we definitely can. I plan to do that as a separate PR as a follow-up.

I want to land this PR sooner rather than later, in order to unblock my other 
work in Meta (to log this transcript into the `lldb_usage` table).

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

[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

royitaqi wrote:

Gentle ping to make progress on this PR. :)

@JDevlieghere , could you see [my rely to your 
comment](https://github.com/llvm/llvm-project/pull/89868#discussion_r1588655287)
 about why I think why I think signed int makes sense for `typedef int 
destroy_callback_token_t`?  I have no strong opinion. If you still think it 
should be unsigned, I can update the PR.

@JDevlieghere and @bulbazord , about the necessity of thread safety, could you 
see if [my 
proposal](https://github.com/llvm/llvm-project/pull/89868#discussion_r1588661446)
 is agreeable? TL;DR: currently the class isn't thread safe; let's skip thread 
safety in this PR; we can add it to the class in one go when in the future the 
class is decided to be thread safe.

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

ZequanWu wrote:

Will leave it open for few days in case anyone has more comments.

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

[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91170)

[Santhosh Kumar Ellendula via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK-NEXT: 
DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'

ZequanWu wrote:

Updated to use `CHECK`. Unfortunately, it doesn't work with `image dump ast` 
because this change doesn't affect  when type completion happens. So, after `p 
v1`, w/wo this change, lldb just creates `ClassTemplateSpecializationDecl` for 
`t1` with no definition. When doing `p v2', lldb tries to complete the 
`t1` decl. This change only moves the definition searching to the time 
when completing a type.

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Zequan Wu via lldb-commits]

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/90663

>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/7] [lldb][DWARF] Delay struct/class/union definition DIE
 searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 270 +++---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 105 ++-
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  14 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   5 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h |   2 +
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   |  95 +++---
 .../SymbolFile/DWARF/UniqueDWARFASTType.h |  21 +-
 7 files changed, 296 insertions(+), 216 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840a..7ad661c9a9d49 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
 static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
-   DWARFDIE die,
+   const DWARFDIE _ctx_die,
+   const DWARFDIE ,
const char *type_name_cstr) {
   auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
   if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang 
,
 type_name_cstr ? type_name_cstr : "", die.GetOffset());
   }
 
+  // By searching for the definition DIE of the decl_ctx type, we will either:
+  // 1. Found the the definition DIE and start its definition with
+  // TypeSystemClang::StartTagDeclarationDefinition.
+  // 2. Unable to find it, then need to forcefully complete it.
+  die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+  if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
   // We don't have a type definition and/or the import failed. We must
   // forcefully complete the type to avoid crashes.
   ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const 
SymbolContext ,
   if (tag == DW_TAG_typedef) {
 // DeclContext will be populated when the clang type is materialized in
 // Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+   decl_ctx_die, die, attrs.name.GetCString());
 
 if (attrs.type.IsValid()) {
   // Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  if (class_type->GetID() != decl_ctx_die.GetID() ||
-  IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
-  std::vector failures;
-
-  CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
-  // FIXME do something with these failures that's
-  // smarter than just dropping them on the ground.
-  // Unfortunately classes don't like having stuff added
-  // to them after their definitions are complete...
-
-  Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-  if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
-  }
-}
-  }
-
   if (attrs.specification.IsValid()) {
 // We have a specification which we are going to base our
 // function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
   }
 }
   }
+  // By here, we should have already completed the c++ class_type
+  // 

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Pavel Iliin via lldb-commits]

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

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Pavel Iliin via lldb-commits]

@@ -426,30 +532,40 @@ def FeatureSpecRestrict : 
SubtargetFeature<"specrestrict", "HasSpecRestrict",
   "true", "Enable architectural speculation restriction (FEAT_CSV2_2)">;
 
 def FeatureSB : Extension<"sb", "SB",
-  "Enable v8.5 Speculation Barrier (FEAT_SB)" >;
+  "Enable v8.5 Speculation Barrier (FEAT_SB)", [],
+  "FEAT_SB", "+sb", 470>;
 
 def FeatureSSBS : Extension<"ssbs", "SSBS",
-  "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)" >;
+  "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)", [],
+  "FEAT_SSBS", "", 490>;
 
 def FeaturePredRes : Extension<"predres", "PredRes",
-  "Enable v8.5a execution and data prediction invalidation instructions 
(FEAT_SPECRES)" >;
+  "Enable v8.5a execution and data prediction invalidation instructions 
(FEAT_SPECRES)", [],
+  "FEAT_PREDRES", "+predres", 480>;
 
-def FeatureCacheDeepPersist : Extension<"ccdp", "CCDP",
+def FeatureCacheDeepPersist : SubtargetFeature<"ccdp", "CCDP", "true",
 "Enable v8.5 Cache Clean to Point of Deep Persistence (FEAT_DPB2)" >;
 
+// NOTE: BTI now has posfeat/negfeat, which it didn't before
+let ArchExtKindSpelling = "AEK_NONE" in
 def FeatureBranchTargetId : Extension<"bti", "BTI",
-"Enable Branch Target Identification (FEAT_BTI)" >;
+"Enable Branch Target Identification (FEAT_BTI)", [],
+"FEAT_BTI", "+bti", 510>;

ilinpv wrote:

Would it be possible to reserve for FMV features additional bit used to 
optimize or not to optimize feature in FMV resolver? I would address comments 
in https://github.com/llvm/llvm-project/pull/90928 having this bit set by 
default and by command line option.

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

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Pavel Iliin via lldb-commits]

@@ -426,30 +532,40 @@ def FeatureSpecRestrict : 
SubtargetFeature<"specrestrict", "HasSpecRestrict",
   "true", "Enable architectural speculation restriction (FEAT_CSV2_2)">;
 
 def FeatureSB : Extension<"sb", "SB",
-  "Enable v8.5 Speculation Barrier (FEAT_SB)" >;
+  "Enable v8.5 Speculation Barrier (FEAT_SB)", [],
+  "FEAT_SB", "+sb", 470>;
 
 def FeatureSSBS : Extension<"ssbs", "SSBS",
-  "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)" >;
+  "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)", [],
+  "FEAT_SSBS", "", 490>;
 
 def FeaturePredRes : Extension<"predres", "PredRes",
-  "Enable v8.5a execution and data prediction invalidation instructions 
(FEAT_SPECRES)" >;
+  "Enable v8.5a execution and data prediction invalidation instructions 
(FEAT_SPECRES)", [],
+  "FEAT_PREDRES", "+predres", 480>;
 
-def FeatureCacheDeepPersist : Extension<"ccdp", "CCDP",
+def FeatureCacheDeepPersist : SubtargetFeature<"ccdp", "CCDP", "true",
 "Enable v8.5 Cache Clean to Point of Deep Persistence (FEAT_DPB2)" >;
 
+// NOTE: BTI now has posfeat/negfeat, which it didn't before
+let ArchExtKindSpelling = "AEK_NONE" in
 def FeatureBranchTargetId : Extension<"bti", "BTI",
-"Enable Branch Target Identification (FEAT_BTI)" >;
+"Enable Branch Target Identification (FEAT_BTI)", [],
+"FEAT_BTI", "+bti", 510>;
 
+let ArchExtKindSpelling = "AEK_RAND", MArchName = "rng" in
 def FeatureRandGen : Extension<"rand", "RandGen",
-"Enable Random Number generation instructions (FEAT_RNG)" >;
+"Enable Random Number generation instructions (FEAT_RNG)", [],
+"FEAT_RNG", "+rand", 10>;
 
+let MArchName = "memtag" in
 def FeatureMTE : Extension<"mte", "MTE",
-"Enable Memory Tagging Extension (FEAT_MTE, FEAT_MTE2)" >;
+"Enable Memory Tagging Extension (FEAT_MTE, FEAT_MTE2)", [],
+"FEAT_MEMTAG", "", 440>;

ilinpv wrote:

"+mte" extension FEAT_MTE, FEAT_MTE2 corresponds to FMV "memtag2", FMV "memtag" 
is FEAT_MTE only.

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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)


Changes

Dwarf 5 says "There is no requirement that the entries be ordered in any 
particular way" in 2.17.3 Non-Contiguous Address Ranges for rnglist. Some 
places assume the ranges are already sorted but it's not. This fixes it.

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


2 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (+1) 
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s (+3-3) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index dabc595427df..3a57ec970b07 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1062,6 +1062,7 @@ DWARFUnit::FindRnglistFromOffset(dw_offset_t offset) {
 ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
 llvm_range.HighPC - llvm_range.LowPC));
   }
+  ranges.Sort();
   return ranges;
 }
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
index 89b5d94c68c3..af8a1796f3ab 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
@@ -124,12 +124,12 @@ lookup_rnglists2:
 .Lrnglists_table_base0:
 .long   .Ldebug_ranges0-.Lrnglists_table_base0
 .Ldebug_ranges0:
-.byte   4   # DW_RLE_offset_pair
-.uleb128 .Lblock1_begin-rnglists  #   starting offset
-.uleb128 .Lblock1_end-rnglists#   ending offset
 .byte   4   # DW_RLE_offset_pair
 .uleb128 .Lblock2_begin-rnglists  #   starting offset
 .uleb128 .Lblock2_end-rnglists#   ending offset
+.byte   4   # DW_RLE_offset_pair
+.uleb128 .Lblock1_begin-rnglists  #   starting offset
+.uleb128 .Lblock1_end-rnglists#   ending offset
 .byte   0   # DW_RLE_end_of_list
 .Ldebug_rnglist_table_end0:
 

``




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

[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)

[Zequan Wu via lldb-commits]

https://github.com/ZequanWu created 
https://github.com/llvm/llvm-project/pull/91343

Dwarf 5 says "There is no requirement that the entries be ordered in any 
particular way" in 2.17.3 Non-Contiguous Address Ranges for rnglist. Some 
places assume the ranges are already sorted but it's not. This fixes it.

>From eb1b2faa457522e7c11fbd69acea7f7d736c7c74 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 7 May 2024 10:52:59 -0400
Subject: [PATCH] [lldb][DWARF] Sort ranges list in dwarf 5.

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp| 1 +
 lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index dabc595427df..3a57ec970b07 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1062,6 +1062,7 @@ DWARFUnit::FindRnglistFromOffset(dw_offset_t offset) {
 ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
 llvm_range.HighPC - llvm_range.LowPC));
   }
+  ranges.Sort();
   return ranges;
 }
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
index 89b5d94c68c3..af8a1796f3ab 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
@@ -124,12 +124,12 @@ lookup_rnglists2:
 .Lrnglists_table_base0:
 .long   .Ldebug_ranges0-.Lrnglists_table_base0
 .Ldebug_ranges0:
-.byte   4   # DW_RLE_offset_pair
-.uleb128 .Lblock1_begin-rnglists  #   starting offset
-.uleb128 .Lblock1_end-rnglists#   ending offset
 .byte   4   # DW_RLE_offset_pair
 .uleb128 .Lblock2_begin-rnglists  #   starting offset
 .uleb128 .Lblock2_end-rnglists#   ending offset
+.byte   4   # DW_RLE_offset_pair
+.uleb128 .Lblock1_begin-rnglists  #   starting offset
+.uleb128 .Lblock1_end-rnglists#   ending offset
 .byte   0   # DW_RLE_end_of_list
 .Ldebug_rnglist_table_end0:
 

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o 
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK:  (lldb) p v1
+# CHECK-NEXT: 
DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type 
(DW_TAG_structure_type) name = 't2'

labath wrote:

CHECK-NEXT on log output sounds like a very bad idea, as the test can be broken 
by adding a random log statement. For what you're trying to do, a plain CHECK 
should suffice (though i'd also consider if this can be checked using something 
less volatile than log messages -- for example, you might be able to use `image 
dump ast` to compare the clang ast of the type before/after completion).

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Pavel Labath via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)

[Pavel Labath via lldb-commits]

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


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

[Lldb-commits] [lldb] [lldb] Use add_custom_command for SBLanguages.h (PR #91254)

[Pavel Labath via lldb-commits]

labath wrote:

thanks

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

[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)

[David Spickett via lldb-commits]

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

[Lldb-commits] [lldb] b22a6f1 - [lldb] fix step in AArch64 trampoline (#90783)

[via lldb-commits]

Author: Vincent Belliard
Date: 2024-05-07T13:42:16+01:00
New Revision: b22a6f1eba8e27b2a21bf6b96a3bd349230cb80a

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

LOG: [lldb] fix step in AArch64 trampoline (#90783)

Detects AArch64 trampolines in order to be able to step in a function
through a trampoline on AArch64.

-

Co-authored-by: Vincent Belliard 

Added: 
lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..51e4b3e6728f23 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,6 +506,19 @@ 
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
   Target  = thread.GetProcess()->GetTarget();
   const ModuleList  = target.GetImages();
 
+  llvm::StringRef target_name = sym_name.GetStringRef();
+  // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) added to the function name. If we detect a
+  // trampoline with the prefix, we need to remove the prefix to find the
+  // function symbol.
+  if (target_name.consume_front("__AArch64ADRPThunk_") ||
+  target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+  sym_name = ConstString(target_name);
+}
+  }
   images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
   if (!target_symbols.GetSize())
 return thread_plan_sp;

diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, 
user_id_t start_id,
 bool symbol_size_valid =
 symbol.st_size != 0 || symbol.getType() != STT_FUNC;
 
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+  // On AArch64, trampolines are registered as code.
+  // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+  // way we will be able to detect the trampoline when we step in a 
function
+  // and step through the trampoline.
+  if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+  symbol_type = eSymbolTypeTrampoline;
+  is_trampoline = true;
+}
+  }
+}
+
 Symbol dc_symbol(
 i + start_id, // ID is the original symbol table index.
 mangled,
 symbol_type,// Type of this symbol
 is_global,  // Is this globally visible?
 false,  // Is this symbol debug info?
-false,  // Is this symbol a trampoline?
+is_trampoline,  // Is this symbol a trampoline?
 false,  // Is this symbol artificial?
 AddressRange(symbol_section_sp, // Section in which this symbol is
 // defined or null.

diff  --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc 
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here() {
+asm (
+  "adrp x16, step_here\n"
+  "add x16, x16, :lo12:step_here\n"
+  "br x16"
+);
+}
+
+extern "C" __attribute__((used)) int step_here() {
+return 47;
+}
+
+int main() {
+  return __AArch64ADRPThunk_step_here();
+}

diff  --git 
a/lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test 
b/lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
new file mode 100644
index 00..336a746fa3a418
--- /dev/null
+++ 

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[Pavel Labath via lldb-commits]

@@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
   }
 
   if (unwindplan_regloc.IsSame()) {
-if (!IsFrameZero() &&
+if (!m_all_registers_available &&
 (regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||

labath wrote:

`pc=` is probably still only valid for real frame zero, so we could make 
the `m_all_registers_available` check `lr`-only.

.. or drop the lr check entirely, since some non-ABI-respecting functions could 
actually preserve the value of lr even if they are not leaf functions.

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

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[Pavel Labath via lldb-commits]

@@ -467,7 +465,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
 m_pushed_regs[reg_num] = addr;
 const int32_t offset = addr - m_initial_sp;
 m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset,
- cant_replace);
+ /*can_replace=*/true);

labath wrote:

This could be narrowed down so that it only overwrites the `` rules, but 
I'm not sure it's necessary given that lines 464&465 ensure that the register 
can get pushed only once.

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

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

A leaf function may not store the link register to stack, but we it can still 
end up being a non-zero frame if it gets interrupted by a signal. Currently, we 
were unable to unwind past this function because we could not read the link 
register value.

To make this work, this patch:
- changes the function-entry unwind plan to include the `fp|lr = same` 
rules. This in turn necessitated an adjustment in the generic instruction 
emulation logic to ensure that `lr=[sp-X]` can override the `same` rule.
- allows the `same` rule for pc and lr in all 
`m_all_registers_available` frames (and not just frame zero).

The test verifies that we can unwind in a situation like this, and that the 
backtrace matches the one we computed before getting a signal.

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


6 Files Affected:

- (modified) lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp 
(+2) 
- (modified) 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
 (+1-3) 
- (modified) lldb/source/Target/RegisterContextUnwind.cpp (+3-3) 
- (added) lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c (+15) 
- (added) lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test (+24) 
- (modified) lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp 
(+20-4) 


``diff
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp 
b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 6ca4fb052457e..62ecac3e0831d 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -444,6 +444,8 @@ bool EmulateInstructionARM64::CreateFunctionEntryUnwind(
 
   // Our previous Call Frame Address is the stack pointer
   row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_arm64, 0);
+  row->SetRegisterLocationToSame(gpr_lr_arm64, /*must_replace=*/false);
+  row->SetRegisterLocationToSame(gpr_fp_arm64, /*must_replace=*/false);
 
   unwind_plan.AppendRow(row);
   unwind_plan.SetSourceName("EmulateInstructionARM64");
diff --git 
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
 
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
index c4a171ec7d01b..49edd40544e32 100644
--- 
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ 
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -424,8 +424,6 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
 log->PutString(strm.GetString());
   }
 
-  const bool cant_replace = false;
-
   switch (context.type) {
   default:
   case EmulateInstruction::eContextInvalid:
@@ -467,7 +465,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
 m_pushed_regs[reg_num] = addr;
 const int32_t offset = addr - m_initial_sp;
 m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset,
- cant_replace);
+ /*can_replace=*/true);
 m_curr_row_modified = true;
   }
 }
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 13e101413a477..e2d712cb72eae 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
   }
 
   if (unwindplan_regloc.IsSame()) {
-if (!IsFrameZero() &&
+if (!m_all_registers_available &&
 (regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||
  regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_RA)) {
   UnwindLogMsg("register %s (%d) is marked as 'IsSame' - it is a pc or "
-   "return address reg on a non-zero frame -- treat as if we "
-   "have no information",
+   "return address reg on a frame which does not have all "
+   "registers available -- treat as if we have no information",
regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
   return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
 } else {
diff --git a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c 
b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
new file mode 100644
index 0..9a751330623f4
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
@@ -0,0 +1,15 @@
+#include 
+#include 
+
+int __attribute__((naked)) signal_generating_add(int a, int b) {
+  asm("add w0, w1, w0\n\t"
+  "udf #0xdead\n\t"
+  "ret");
+}
+
+void sigill_handler(int) { _exit(0); }
+
+int main() {
+  signal(SIGILL, sigill_handler);
+  return signal_generating_add(42, 

[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/91321

A leaf function may not store the link register to stack, but we it can still 
end up being a non-zero frame if it gets interrupted by a signal. Currently, we 
were unable to unwind past this function because we could not read the link 
register value.

To make this work, this patch:
- changes the function-entry unwind plan to include the `fp|lr = ` rules. 
This in turn necessitated an adjustment in the generic instruction emulation 
logic to ensure that `lr=[sp-X]` can override the `` rule.
- allows the `` rule for pc and lr in all `m_all_registers_available` 
frames (and not just frame zero).

The test verifies that we can unwind in a situation like this, and that the 
backtrace matches the one we computed before getting a signal.

>From 55009debd94713dc893f4243c53aeb5979ee9113 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 7 May 2024 11:42:20 +
Subject: [PATCH] [lldb/aarch64] Fix unwinding when signal interrupts a leaf
 function

A leaf function may not store the link register to stack, but we it can
still end up being a non-zero frame if it gets interrupted by a signal.
Currently, we were unable to unwind past this function because we could
not read the link register value.

To make this work, this patch:
- changes the function-entry unwind plan to include the
  `fp|lr = ` rules. This in turn necessitated an adjustment in the
  generic instruction emulation logic to ensure that `lr=[sp-X]` can
  override the `` rule.
- allows the `` rule for pc and lr in all `m_all_registers_available`
  frames (and not just frame zero).

The test verifies that we can unwind in a situation like this, and that
the backtrace matches the one we computed before getting a signal.
---
 .../ARM64/EmulateInstructionARM64.cpp |  2 ++
 .../UnwindAssemblyInstEmulation.cpp   |  4 +---
 lldb/source/Target/RegisterContextUnwind.cpp  |  6 ++---
 .../Inputs/signal-in-leaf-function-aarch64.c  | 15 
 .../signal-in-leaf-function-aarch64.test  | 24 +++
 .../ARM64/TestArm64InstEmulation.cpp  | 24 +++
 6 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 
lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
 create mode 100644 lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test

diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp 
b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 6ca4fb052457e..62ecac3e0831d 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -444,6 +444,8 @@ bool EmulateInstructionARM64::CreateFunctionEntryUnwind(
 
   // Our previous Call Frame Address is the stack pointer
   row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_arm64, 0);
+  row->SetRegisterLocationToSame(gpr_lr_arm64, /*must_replace=*/false);
+  row->SetRegisterLocationToSame(gpr_fp_arm64, /*must_replace=*/false);
 
   unwind_plan.AppendRow(row);
   unwind_plan.SetSourceName("EmulateInstructionARM64");
diff --git 
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
 
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
index c4a171ec7d01b..49edd40544e32 100644
--- 
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ 
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -424,8 +424,6 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
 log->PutString(strm.GetString());
   }
 
-  const bool cant_replace = false;
-
   switch (context.type) {
   default:
   case EmulateInstruction::eContextInvalid:
@@ -467,7 +465,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
 m_pushed_regs[reg_num] = addr;
 const int32_t offset = addr - m_initial_sp;
 m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset,
- cant_replace);
+ /*can_replace=*/true);
 m_curr_row_modified = true;
   }
 }
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 13e101413a477..e2d712cb72eae 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
   }
 
   if (unwindplan_regloc.IsSame()) {
-if (!IsFrameZero() &&
+if (!m_all_registers_available &&
 (regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||
  regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_RA)) {
   UnwindLogMsg("register %s (%d) is marked as 'IsSame' - it is a pc or "
-   "return address reg on a non-zero frame -- treat as if we "
- 

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Tomas Matheson via lldb-commits]

@@ -11,42 +11,123 @@
 
 // A SubtargetFeature that can be toggled from the command line, and therefore
 // has an AEK_* entry in ArmExtKind.
+//
+// If Function MultiVersioning (FMV) properties are left at their defaults
+// (FEAT_INIT, no dependencies, priority 0) it indiates that this extension is
+// not an FMV feature, but can be enabled via the command line (-march, -mcpu,
+// etc).
+//
+// Conversely if the ArchExtKindSpelling is set to AEK_NONE, this indicates
+// that a feature is FMV-only, and can not be selected on the command line.
+// Such extensions should be added via FMVOnlyExtension.
 class Extension<
-  string TargetFeatureName,// String used for -target-feature.
+  string TargetFeatureName,// String used for -target-feature and 
-march, unless overridden.
   string Spelling, // The XYZ in HasXYZ and AEK_XYZ.
   string Desc, // Description.
-  list Implies = []  // List of dependent features.
+  list Implies = [], // List of dependent features.
+  // FMV properties
+  string _FMVBit = "FEAT_INIT",// FEAT_INIT is repurposed to indicate 
"not an FMV feature"
+  string _FMVDependencies = "",
+  int _FMVPriority = 0
 > : SubtargetFeature Implies>
 {
 string ArchExtKindSpelling = "AEK_" # Spelling; // ArchExtKind enum name.
+
+// In general, the name written on the command line should match the name
+// used for -target-feature. However, there are exceptions. Therefore we
+// add a separate field for this, to allow overriding it. Strongly prefer
+// not doing so.
+string MArchName = TargetFeatureName;
+
+// Function MultiVersioning (FMV) properties
+
+// A C++ expression giving the number of the bit in the FMV ABI.
+// Currently this is given as a value from the enum "CPUFeatures".
+// If this is not set, it indicates that this is not an FMV extension.
+string FMVBit = _FMVBit;
+
+// List of features that this feature depends on.
+// FIXME generate this from Implies.

tmatheson-arm wrote:

That would require the FMV dependencies being aligned with the existing 
`SubtargetFeature` dependencies, which I believe is being considered but is a 
WIP.

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

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Jonathan Thackray via lldb-commits]

@@ -11,42 +11,123 @@
 
 // A SubtargetFeature that can be toggled from the command line, and therefore
 // has an AEK_* entry in ArmExtKind.
+//
+// If Function MultiVersioning (FMV) properties are left at their defaults
+// (FEAT_INIT, no dependencies, priority 0) it indiates that this extension is
+// not an FMV feature, but can be enabled via the command line (-march, -mcpu,
+// etc).
+//
+// Conversely if the ArchExtKindSpelling is set to AEK_NONE, this indicates
+// that a feature is FMV-only, and can not be selected on the command line.
+// Such extensions should be added via FMVOnlyExtension.
 class Extension<
-  string TargetFeatureName,// String used for -target-feature.
+  string TargetFeatureName,// String used for -target-feature and 
-march, unless overridden.
   string Spelling, // The XYZ in HasXYZ and AEK_XYZ.
   string Desc, // Description.
-  list Implies = []  // List of dependent features.
+  list Implies = [], // List of dependent features.
+  // FMV properties
+  string _FMVBit = "FEAT_INIT",// FEAT_INIT is repurposed to indicate 
"not an FMV feature"
+  string _FMVDependencies = "",
+  int _FMVPriority = 0
 > : SubtargetFeature Implies>
 {
 string ArchExtKindSpelling = "AEK_" # Spelling; // ArchExtKind enum name.
+
+// In general, the name written on the command line should match the name
+// used for -target-feature. However, there are exceptions. Therefore we
+// add a separate field for this, to allow overriding it. Strongly prefer
+// not doing so.
+string MArchName = TargetFeatureName;
+
+// Function MultiVersioning (FMV) properties
+
+// A C++ expression giving the number of the bit in the FMV ABI.
+// Currently this is given as a value from the enum "CPUFeatures".
+// If this is not set, it indicates that this is not an FMV extension.
+string FMVBit = _FMVBit;
+
+// List of features that this feature depends on.
+// FIXME generate this from Implies.

jthackray wrote:

Presumably this FIXME is coming in a future PR?

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

[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

[via lldb-commits]

github-actions[bot] wrote:



@Awfa Congratulations on having your first Pull Request (PR) merged into the 
LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with 
a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself.
This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


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

[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

[David Spickett via lldb-commits]

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

[Lldb-commits] [lldb] 6aed0ab - [lldb] Have lldb-server assign ports to children in platform mode (#88845)

[via lldb-commits]

Author: Anthony Ha
Date: 2024-05-07T09:45:07+01:00
New Revision: 6aed0ab6547f577cceaccfc6d710f96b645c3af7

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

LOG: [lldb] Have lldb-server assign ports to children in platform mode (#88845)

Fixes #47549

`lldb-server`'s platform mode seems to have an issue with its
`--min-gdbserver-port` `--max-gdbserver-port` flags (and probably the
`--gdbserver-port` flag, but I didn't test it).

How the platform code seems to work is that it listens on a port, and
whenever there's an incoming connection, it forks the process to handle
the connection. To handle the port flags, the main process uses an
instance of the helper class
`GDBRemoteCommunicationServerPlatform::PortMap`, that can be configured
and track usages of ports. The child process handling the platform
connection, can then use the port map to allocate a port for the
gdb-server connection it will make (this is another process it spawns).

However, in the current code, this works only once. After the first
connection is handled by forking a child process, the main platform
listener code loops around, and then 'forgets' about the port map. This
is because this code:
```cpp
GDBRemoteCommunicationServerPlatform platform(
acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
if (!gdbserver_portmap.empty()) {
  platform.SetPortMap(std::move(gdbserver_portmap));
}
```
is within the connection listening loop. This results in the
`gdbserver_portmap` being moved into the platform object at the
beginning of the first iteration of the loop, but on the second
iteration, after the first fork, the next instance of the platform
object will not have its platform port mapped.
The result of this bug is that subsequent connections to the platform,
when spawning the gdb-remote connection, will be supplied a random port
- which isn't bounded by the `--min-gdbserver-port` and
`--max-gdbserver--port` parameters passed in by the user.

This PR fixes this issue by having the port map be maintained by the
parent platform listener process. On connection, the listener allocates
a single available port from the port map, associates the child process
pid with the port, and lets the connection handling child use that
single port number.

Additionally, when cleaning up child processes, the main listener
process tracks the child that exited to deallocate the previously
associated port, so it can be reused for a new connection.

Added: 


Modified: 
lldb/docs/use/qemu-testing.rst
lldb/tools/lldb-server/lldb-platform.cpp

Removed: 




diff  --git a/lldb/docs/use/qemu-testing.rst b/lldb/docs/use/qemu-testing.rst
index 6e282141864cc1..51a30b11717a87 100644
--- a/lldb/docs/use/qemu-testing.rst
+++ b/lldb/docs/use/qemu-testing.rst
@@ -172,6 +172,7 @@ forwarded for this to work.
 
 .. note::
   These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not shared across all the processes it may create,
+  Unfortunately this map is not cleaned up on Windows on connection close,
   and across a few uses you may run out of valid ports. To work around this,
   restart the platform every so often, especially after running a set of tests.
+  This is tracked here: https://github.com/llvm/llvm-project/issues/90923

diff  --git a/lldb/tools/lldb-server/lldb-platform.cpp 
b/lldb/tools/lldb-server/lldb-platform.cpp
index 3e126584eb25b4..cfd0a3797d810a 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -282,17 +282,12 @@ int main_platform(int argc, char *argv[]) {
 }
   }
 
-  do {
-GDBRemoteCommunicationServerPlatform platform(
-acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
-
-if (port_offset > 0)
-  platform.SetPortOffset(port_offset);
-
-if (!gdbserver_portmap.empty()) {
-  platform.SetPortMap(std::move(gdbserver_portmap));
-}
+  GDBRemoteCommunicationServerPlatform platform(
+  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  if (port_offset > 0)
+platform.SetPortOffset(port_offset);
 
+  do {
 const bool children_inherit_accept_socket = true;
 Connection *conn = nullptr;
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
@@ -301,13 +296,37 @@ int main_platform(int argc, char *argv[]) {
   exit(socket_error);
 }
 printf("Connection established.\n");
+
 if (g_server) {
   // Collect child zombie processes.
 #if !defined(_WIN32)
-  while (waitpid(-1, nullptr, WNOHANG) > 0)
-;
+  ::pid_t waitResult;
+  while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) {
+// waitResult is the child pid
+gdbserver_portmap.FreePortForProcess(waitResult);
+  }
 

[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Nope, that would be lovely and is closer than ever with the move to GitHub, but 
not here yet.

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

[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)

[Tomas Matheson via lldb-commits]

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

[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)

[Tomas Matheson via lldb-commits]

tmatheson-arm wrote:

Thanks for the comments everyone. Given that this requires an IR break and 
additions to the importer, and there is still some question about which way to 
go with the renaming (i.e. FEAT_ names or user-facing names) I think it makes 
sense to defer renaming until later. For now, I will add mechanisms to handle 
the various inconsistencies as in 
https://github.com/llvm/llvm-project/pull/90987.

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

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-backend-aarch64

Author: Tomas Matheson (tmatheson-arm)


Changes

Generate target features and FMVExtensions from tablegen.

Use MArchName/ArchKindEnumSpelling to avoid renamings.
Cases where there is simply a case difference are handled by
consistently uppercasing the AEK_ name in the emitted code.

Remove some Extensions which were not needed.
These had AEK entries but were never actually used for anything.
They are not present in Extensions[] data.

---

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


5 Files Affected:

- (modified) lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
(+1-1) 
- (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+8-131) 
- (modified) llvm/lib/Target/AArch64/AArch64Features.td (+183-44) 
- (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+4-4) 
- (modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+50-9) 


``diff
diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
index e154f544e7cc6e0..685d0a84ec28960 100644
--- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
+++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -59,7 +59,7 @@ fn:
   bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM
   rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3
   sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4
-  addqv   v0.8h, p0, z0.h   // AEK_SVE2p1 / AEK_SME2p1
+  addqv   v0.8h, p0, z0.h   // AEK_SVE2P1 / AEK_SME2P1
   rcwswp x0, x1, [x2]   // AEK_THE
   tcommit   // AEK_TME
 lbl:
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h 
b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 04fbaf07adfbcb5..1124420daf8d806 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62,
   "Number of features in CPUFeatures are limited to 62 entries");
 
 // Each ArchExtKind correponds directly to a possible -target-feature.
-enum ArchExtKind : unsigned {
-  AEK_NONE = 1,
-#define ARM_EXTENSION(NAME, ENUM) ENUM,
+#define EMIT_ARCHEXTKIND_ENUM
 #include "llvm/TargetParser/AArch64TargetParserDef.inc"
-  AEK_NUM_EXTENSIONS,
-
-  // FIXME temporary fixes for inconsistent naming.
-  AEK_F32MM = AEK_MATMULFP32,
-  AEK_F64MM = AEK_MATMULFP64,
-  AEK_FCMA = AEK_COMPLXNUM,
-  AEK_FP = AEK_FPARMV8,
-  AEK_FP16 = AEK_FULLFP16,
-  AEK_I8MM = AEK_MATMULINT8,
-  AEK_JSCVT = AEK_JS,
-  AEK_PROFILE = AEK_SPE,
-  AEK_RASv2 = AEK_RASV2,
-  AEK_RAND = AEK_RANDGEN,
-  AEK_SIMD = AEK_NEON,
-  AEK_SME2p1 = AEK_SME2P1,
-  AEK_SVE2p1 = AEK_SVE2P1,
-  AEK_SME_LUTv2 = AEK_SME_LUTV2,
 
-};
 using ExtensionBitset = Bitset;
 
 // Represents an extension that can be enabled with -march=+.
@@ -148,111 +128,8 @@ struct ExtensionInfo {
   1000; // Maximum priority for FMV feature
 };
 
-// NOTE: If adding a new extension here, consider adding it to ExtensionMap
-// in AArch64AsmParser too, if supported as an extension name by binutils.
-// clang-format off
-inline constexpr ExtensionInfo Extensions[] = {
-{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 
150},
-{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0},
-{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280},
-{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0},
-{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510},
-{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110},
-{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, 
"+aes,+sha2", 0},
-{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0},
-{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0},
-{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260},
-{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180},
-{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, 
"+dotprod,+fp-armv8,+neon", 104},
-{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190},
-{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200},
-{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290},
-{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, 
"+sve,+f32mm,+fullfp16,+fp-armv8,+neon", 350},
-{"f64mm", AArch64::AEK_F64MM, "+f64mm", "-f64mm", FEAT_SVE_F64MM, 
"+sve,+f64mm,+fullfp16,+fp-armv8,+neon", 360},
-{"fcma", AArch64::AEK_FCMA, "+complxnum", "-complxnum", FEAT_FCMA, 
"+fp-armv8,+neon,+complxnum", 220},
-{"flagm", AArch64::AEK_FLAGM, "+flagm", "-flagm", FEAT_FLAGM, "+flagm", 
20},
-{"flagm2", AArch64::AEK_NONE, {}, {}, FEAT_FLAGM2, "+flagm,+altnzcv", 30},
-

[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)

[Tomas Matheson via lldb-commits]

https://github.com/tmatheson-arm updated 
https://github.com/llvm/llvm-project/pull/90987

>From 4b8b776348438847c2eb238dac973e93fe93294e Mon Sep 17 00:00:00 2001
From: Tomas Matheson 
Date: Mon, 29 Apr 2024 19:57:17 +0100
Subject: [PATCH 1/3] [AArch64] move extension information into tablgen

Generate target features and FMVExtensions from tablegen.

Use MArchName/ArchKindEnumSpelling to avoid renamings.
Cases where there is simply a case difference are handled by
consistently uppercasing the AEK_ name in the emitted code.

Remove some Extensions which were not needed.
These had AEK entries but were never actually used for anything.
They are not present in Extensions[] data.
---
 .../command-disassemble-aarch64-extensions.s  |   2 +-
 .../llvm/TargetParser/AArch64TargetParser.h   | 139 +--
 llvm/lib/Target/AArch64/AArch64Features.td| 216 ++
 .../TargetParser/TargetParserTest.cpp |   8 +-
 llvm/utils/TableGen/ARMTargetDefEmitter.cpp   |  60 -
 5 files changed, 236 insertions(+), 189 deletions(-)

diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
index e154f544e7cc6..685d0a84ec289 100644
--- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
+++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -59,7 +59,7 @@ fn:
   bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM
   rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3
   sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4
-  addqv   v0.8h, p0, z0.h   // AEK_SVE2p1 / AEK_SME2p1
+  addqv   v0.8h, p0, z0.h   // AEK_SVE2P1 / AEK_SME2P1
   rcwswp x0, x1, [x2]   // AEK_THE
   tcommit   // AEK_TME
 lbl:
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h 
b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 04fbaf07adfbc..1124420daf8d8 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62,
   "Number of features in CPUFeatures are limited to 62 entries");
 
 // Each ArchExtKind correponds directly to a possible -target-feature.
-enum ArchExtKind : unsigned {
-  AEK_NONE = 1,
-#define ARM_EXTENSION(NAME, ENUM) ENUM,
+#define EMIT_ARCHEXTKIND_ENUM
 #include "llvm/TargetParser/AArch64TargetParserDef.inc"
-  AEK_NUM_EXTENSIONS,
-
-  // FIXME temporary fixes for inconsistent naming.
-  AEK_F32MM = AEK_MATMULFP32,
-  AEK_F64MM = AEK_MATMULFP64,
-  AEK_FCMA = AEK_COMPLXNUM,
-  AEK_FP = AEK_FPARMV8,
-  AEK_FP16 = AEK_FULLFP16,
-  AEK_I8MM = AEK_MATMULINT8,
-  AEK_JSCVT = AEK_JS,
-  AEK_PROFILE = AEK_SPE,
-  AEK_RASv2 = AEK_RASV2,
-  AEK_RAND = AEK_RANDGEN,
-  AEK_SIMD = AEK_NEON,
-  AEK_SME2p1 = AEK_SME2P1,
-  AEK_SVE2p1 = AEK_SVE2P1,
-  AEK_SME_LUTv2 = AEK_SME_LUTV2,
 
-};
 using ExtensionBitset = Bitset;
 
 // Represents an extension that can be enabled with -march=+.
@@ -148,111 +128,8 @@ struct ExtensionInfo {
   1000; // Maximum priority for FMV feature
 };
 
-// NOTE: If adding a new extension here, consider adding it to ExtensionMap
-// in AArch64AsmParser too, if supported as an extension name by binutils.
-// clang-format off
-inline constexpr ExtensionInfo Extensions[] = {
-{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 
150},
-{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0},
-{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280},
-{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0},
-{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510},
-{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110},
-{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, 
"+aes,+sha2", 0},
-{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0},
-{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0},
-{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260},
-{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180},
-{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, 
"+dotprod,+fp-armv8,+neon", 104},
-{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190},
-{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200},
-{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290},
-{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, 
"+sve,+f32mm,+fullfp16,+fp-armv8,+neon", 350},
-{"f64mm", AArch64::AEK_F64MM, "+f64mm", "-f64mm", FEAT_SVE_F64MM, 
"+sve,+f64mm,+fullfp16,+fp-armv8,+neon", 360},
-{"fcma", AArch64::AEK_FCMA, "+complxnum", "-complxnum", FEAT_FCMA, 
"+fp-armv8,+neon,+complxnum", 220},
-{"flagm", AArch64::AEK_FLAGM, "+flagm", "-flagm", FEAT_FLAGM, "+flagm", 
20},
-{"flagm2", AArch64::AEK_NONE, {}, {}, FEAT_FLAGM2,