[Lldb-commits] [lldb] da0b62d - Revert a LIT typo fix in a RUN line

[Qiu Chaofan via lldb-commits]

Author: Qiu Chaofan
Date: 2021-10-09T11:29:44+08:00
New Revision: da0b62dfb3ca597f618b1f88226b440bc7ca3960

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

LOG: Revert a LIT typo fix in a RUN line

Commit 573531f changes the behavior of the test, revert it back.

Added: 


Modified: 
lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test

Removed: 




diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test 
b/lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test
index 4b378cc921975..aa2cbcafc9283 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/command_relative_import.test
@@ -8,9 +8,9 @@
 # RUN:-o 'command source %t/foo/magritte.in' \
 # RUN:-o 'command source %t/foo/zip.in' \
 # RUN:-o 'command source %t/foo/magritte.in' \
-# RUN:-o 'zip' \
+# RUN;-o 'zip' \
 # RUN:-o 'hello'
-# RUN:-o 'magritte' 2>&1 | FileCheck %s
+# RUN -o 'magritte' 2>&1 | FileCheck %s
 
 # The first time importing 'magritte' fails because we didn't pass -c.
 # CHECK: ModuleNotFoundError: No module named 'magritte'



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

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D109738#3052661 , @ted wrote:

> In D109738#3052624 , @jingham wrote:
>
>> That is clearly wrong: an address is never enough to restore a breakpoint 
>> from one session to the next even if you can turn ASLR off.  After all, the 
>> breakpoint could be in a dlopened library.
>
> This is usually true.
>
> In cases where you're debugging an embedded application that doesn't involve 
> shared libraries, it's not. The main area where I see this is when debugging 
> application startup code. My coworker who's responsible for that is the one 
> who reported the problem originally. In that case, the address of the 
> breakpoint is always valid on subsequent runs.
>
> On the other hand, when you're running under our RTOS, the application is a 
> PIE, and we support shared libraries, so the address is never valid on 
> subsequent runs.
>
> Maybe we need a flag to "break set" that means "this address is always valid; 
> don't garbage collect it".

I'd almost suggest making another breakpoint type (like --fixed-address or 
something).  That way you won't have to fight the normal address resolver, and 
just keep trying to write a breakpoint at that address on each load event till 
it succeeds.  Note, you could trivially do this with scripted breakpoints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D109738#3052624 , @jingham wrote:

> That is clearly wrong: an address is never enough to restore a breakpoint 
> from one session to the next even if you can turn ASLR off.  After all, the 
> breakpoint could be in a dlopened library.

This is usually true.

In cases where you're debugging an embedded application that doesn't involve 
shared libraries, it's not. The main area where I see this is when debugging 
application startup code. My coworker who's responsible for that is the one who 
reported the problem originally. In that case, the address of the breakpoint is 
always valid on subsequent runs.

On the other hand, when you're running under our RTOS, the application is a 
PIE, and we support shared libraries, so the address is never valid on 
subsequent runs.

Maybe we need a flag to "break set" that means "this address is always valid; 
don't garbage collect it".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

That discussion is extremely helpful - gives me good context for what I need to 
suport.  THANKS!



From: lldb-commits  on behalf of Jim 
Ingham via Phabricator via lldb-commits 
Sent: Friday, October 8, 2021 5:44:49 PM
To: vadi...@gmail.com; jing...@apple.com
Cc: djordje.todoro...@syrmia.com; lldb-commits@lists.llvm.org; 
liburd1...@outlook.com; quic_soura...@quicinc.com; h.imai@nitech.jp; 
serhiy.re...@gmail.com; david.spick...@linaro.org
Subject: [Lldb-commits] [PATCH] D109738 : 
[lldb] Fix bug 38317 - Address breakpoints don't work if set before the process 
launches

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257  
https://reviews.llvm.org/D109738#3002257, @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I think that address breakpoints should still function when
> section addresses aren't known at breakpoint creation time (for whatever
> 

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[David Millar via Phabricator via lldb-commits]

d-millar added subscribers: DavidSpickett, djtodoro, HirokiImai, jingham, 
ZeeZeeMorin, SouraVX, serhiy.redko, vadimcn, d-millar.
d-millar added a comment.

That discussion is extremely helpful - gives me good context for what I need to 
suport.  THANKS!



From: lldb-commits  on behalf of Jim 
Ingham via Phabricator via lldb-commits 
Sent: Friday, October 8, 2021 5:44:49 PM
To: vadi...@gmail.com; jing...@apple.com
Cc: djordje.todoro...@syrmia.com; lldb-commits@lists.llvm.org; 
liburd1...@outlook.com; quic_soura...@quicinc.com; h.imai@nitech.jp; 
serhiy.re...@gmail.com; david.spick...@linaro.org
Subject: [Lldb-commits] [PATCH] D109738 : 
[lldb] Fix bug 38317 - Address breakpoints don't work if set before the process 
launches

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257  
https://reviews.llvm.org/D109738#3002257, @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I 

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257 , @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I think that address breakpoints should still function when
> section addresses aren't known at breakpoint creation time (for whatever
> reason), if only on a best-effort basis.
>
> 3. If the address doesn't fall into any sections, then it would be fine to
>
>> assign it to the first section that shows up at that address, but I would
>> convert it to section relative at that point.
>
> This is what I am attempting to do with my patch.  Is there something else
> I should do to make sure I am not regressing other scenarios?

The fact that Ted's case doesn't work means that at some point we stopped 
recording address breakpoints as section relative in cases where we should.  
That to me is the primary bug, but maybe not so interesting to you.

More generally, using raw address breakpoints is fallible, and whenever you 
find yourself doing that 

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Sounds good.  When we started adding extension points for python, we made the 
implementations be free functions.  But then over time we realized it was often 
more convenient if you had a object managing the callback, that way it could 
more easily store state over the invocations, etc.  If doing free functions is 
harder in Java, I think it would be fine to only implement the "class with an 
invocation method" version of the callback.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

Discussing it with developers here, they agreed with your opinion that we 
really really ought to do 2.  

As you just noted, 3 is trickier.  Java doesn't really have a REPL.  It has 
jshell - we're not sure that counts.  In any case, am up for tackling 2.  Will 
keep you posted - might take me a bit.  In the meantime, happy to hear any 
further thoughts, advice, etc.

Thanks!



From: lldb-commits  on behalf of David 
Millar via lldb-commits 
Sent: Friday, October 8, 2021 2:44:06 PM
To: anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
teempe...@gmail.com; medismail.benn...@gmail.com; jo...@devlieghere.com; 
tedw...@quicinc.com; jmole...@apple.com; syaghm...@apple.com; 
jing...@apple.com; v...@apple.com; boris.ulasev...@gmail.com; 
lldb-commits@lists.llvm.org; h.imai@nitech.jp; bruce.mitche...@gmail.com; 
david.spick...@linaro.org; quic_soura...@quicinc.com; 
djordje.todoro...@syrmia.com; serhiy.re...@gmail.com; liburd1...@outlook.com; 
Jim Ingham
Subject: Re: [Lldb-commits] [PATCH] D111409 : 
proposed support for Java interface to Scripting Bridge

Just to clarify my use case:  I am one of the developers for a 
reverse-engineering tool called Ghidra.  Part of the tool is debugging-support 
to allow cross-over between dynamic and static analysis.  We currently support 
windbg/kd, the gcc MI2 interface, and direct Java debugging.  I have a working 
prototype for lldb, but it requires users to patch and rebuild lldb, which may 
be a heavy lift for some.  While it falls outside of my use case, I am quite 
willing to attempt implementing pieces 2 & 3 from Mr. Ingham's post.



From: Jim Ingham via Phabricator 
Sent: Friday, October 8, 2021 2:31:13 PM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
teempe...@gmail.com; medismail.benn...@gmail.com; jo...@devlieghere.com; 
tedw...@quicinc.com; jmole...@apple.com; syaghm...@apple.com; 
jing...@apple.com; v...@apple.com; boris.ulasev...@gmail.com; 
lldb-commits@lists.llvm.org; h.imai@nitech.jp; bruce.mitche...@gmail.com; 
david.spick...@linaro.org; quic_soura...@quicinc.com; 
djordje.todoro...@syrmia.com; serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409 : proposed support 
for Java interface to Scripting Bridge

jingham added a comment.

Support for a scripting language in lldb has three distinct pieces.

1. Making the SB API's available to the scripting languages
2. Adding all the callbacks so the scripting language can be used for 
breakpoint callbacks, data formatters, etc.
3. Adding a REPL for that language to the "script" command

This patch does #1, but not #2 & #3.  Do we care about whether new scripting 
languages will be able to provide the full "scripting language" experience?  
Does there need to be some plan for this before we add on the task of 
supporting the language?  Maybe it's okay to say "a REPL's too hard, and not 
worth it" but we should have a plan for adding in the callback interfaces?  Do 
we want to have somewhere you can advertise levels of support?  I would be sad 
to spend some time using a scripting language for lldb only to find it doesn't 
allow me to write breakpoint callbacks, for instance.

This is language #3 so it seems like a fit time to discuss this...

Repository:

  rLLDB LLDB

CHANGES SINCE LAST ACTION

  https://reviews.llvm.org/D111409/new/

https://reviews.llvm.org/D111409


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D111409#3052192 , @d-millar wrote:

> Just to clarify my use case:  I am one of the developers for a 
> reverse-engineering tool called Ghidra.  Part of the tool is 
> debugging-support to allow cross-over between dynamic and static analysis.  
> We currently support windbg/kd, the gcc MI2 interface, and direct Java 
> debugging.  I have a working prototype for lldb, but it requires users to 
> patch and rebuild lldb, which may be a heavy lift for some.  While it falls 
> outside of my use case, I am quite willing to attempt implementing pieces 2 & 
> 3 from Mr. Ingham's post.

That would be great!

To be clear, I was more asking questions than stating requirements.  It might 
be that the community feels it's sufficiently valuable to just have the API 
wrappers, and maintaining that in tree is fine provided someone is around to 
keep it up to date.  IMO, the callback affordances add much of the power in 
having a scripting language in the debugger, so it would be a shame not to take 
on the added complexity of another language and not have this bit working.  
Whether a REPL is possible is in part determined by whether the language in 
question is really amenable to that.  It's a huge convenience for developers, 
and it is pretty cool to be able to drop into the script interpreter and write 
a quick loop to find something in a bunch of objects.  But it isn't the core 
part of the job the extension language is doing.

> 
>
> From: Jim Ingham via Phabricator 
> Sent: Friday, October 8, 2021 2:31:13 PM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> teempe...@gmail.com; medismail.benn...@gmail.com; jo...@devlieghere.com; 
> tedw...@quicinc.com; jmole...@apple.com; syaghm...@apple.com; 
> jing...@apple.com; v...@apple.com; boris.ulasev...@gmail.com; 
> lldb-commits@lists.llvm.org; h.imai@nitech.jp; bruce.mitche...@gmail.com; 
> david.spick...@linaro.org; quic_soura...@quicinc.com; 
> djordje.todoro...@syrmia.com; serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409 : proposed support 
> for Java interface to Scripting Bridge
>
> jingham added a comment.
>
> Support for a scripting language in lldb has three distinct pieces.
>
> 1. Making the SB API's available to the scripting languages
> 2. Adding all the callbacks so the scripting language can be used for 
> breakpoint callbacks, data formatters, etc.
> 3. Adding a REPL for that language to the "script" command
>
> This patch does #1, but not #2 & #3.  Do we care about whether new scripting 
> languages will be able to provide the full "scripting language" experience?  
> Does there need to be some plan for this before we add on the task of 
> supporting the language?  Maybe it's okay to say "a REPL's too hard, and not 
> worth it" but we should have a plan for adding in the callback interfaces?  
> Do we want to have somewhere you can advertise levels of support?  I would be 
> sad to spend some time using a scripting language for lldb only to find it 
> doesn't allow me to write breakpoint callbacks, for instance.
>
> This is language #3 so it seems like a fit time to discuss this...
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

@JDevlieghere: ping.Or are you saying you wouldn't consider landing such a 
change at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

[Lldb-commits] [PATCH] D111452: [lldb/Plugins] Replace platform-specific macro with LLVM_PRETTY_FUNCTION (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG88a941ba64a3: [lldb/Plugins] Replace platform-specific macro 
with LLVM_PRETTY_FUNCTION (NFC) (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111452

Files:
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
@@ -60,7 +60,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_thread_id", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return LLDB_INVALID_THREAD_ID;
 
   return obj->GetIntegerValue(LLDB_INVALID_THREAD_ID);
@@ -70,7 +70,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_name", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetStringValue().str();
@@ -80,7 +80,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_state", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return eStateInvalid;
 
   return static_cast(obj->GetIntegerValue(eStateInvalid));
@@ -90,7 +90,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_queue", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetStringValue().str();
@@ -101,7 +101,7 @@
   StructuredData::DictionarySP dict =
   Dispatch("get_stop_reason", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, dict, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))
 return {};
 
   return dict;
@@ -116,7 +116,7 @@
   StructuredData::DictionarySP dict =
   Dispatch("get_register_info", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, dict, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))
 return {};
 
   return dict;
@@ -127,7 +127,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_register_context", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetAsString()->GetValue().str();
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -39,7 +39,7 @@
 using Locker = ScriptInterpreterPythonImpl::Locker;
 
 std::string caller_signature =
-llvm::Twine(__PRETTY_FUNCTION__ + llvm::Twine(" (") +
+llvm::Twine(LLVM_PRETTY_FUNCTION + llvm::Twine(" (") +
 llvm::Twine(method_name) + llvm::Twine(")"))
 .str();
 if (!m_object_instance_sp)
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -72,7 +72,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("is_alive", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetBooleanValue();
@@ -89,7 +89,7 @@
   "get_memory_region_containing_address", error, address);
 
   if (error.Fail()) {
-return ErrorWithMessage(__PRETTY_FUNCTION__,
+return ErrorWithMessage(LLVM_PRETTY_FUNCTION,
   error.AsCString(), error);
   }
 
@@ -101,7 +101,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_thread_with_id", error, tid);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if 

[Lldb-commits] [lldb] 88a941b - [lldb/Plugins] Replace platform-specific macro with LLVM_PRETTY_FUNCTION (NFC)

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-08T20:50:45+02:00
New Revision: 88a941ba64a3824e1a6bac178034e4fce548f6cb

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

LOG: [lldb/Plugins] Replace platform-specific macro with LLVM_PRETTY_FUNCTION 
(NFC)

This patch refactors Scripted Process and Scripted Thread related
classes to use LLVM_PRETTY_FUNCTION instead of the compiler macro.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Interpreter/ScriptedInterface.h
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/ScriptedInterface.h
index bd1ed11279f4..427fa3f4f793 100644
--- a/lldb/include/lldb/Interpreter/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/ScriptedInterface.h
@@ -9,16 +9,14 @@
 #ifndef LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
 #define LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
 
-#ifdef _MSC_VER
-#define __PRETTY_FUNCTION__ __FUNCSIG__
-#endif
-
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/Compiler.h"
+
 #include 
 
 namespace lldb_private {

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index 551a0f752c9c..58576251be14 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -233,7 +233,7 @@ bool ScriptedProcess::IsAlive() {
 size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
  Status ) {
   if (!m_interpreter)
-return GetInterface().ErrorWithMessage(__PRETTY_FUNCTION__,
+return GetInterface().ErrorWithMessage(LLVM_PRETTY_FUNCTION,
"No interpreter.", error);
 
   lldb::DataExtractorSP data_extractor_sp =
@@ -247,7 +247,7 @@ size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, 
void *buf, size_t size,
 
   if (!bytes_copied || bytes_copied == LLDB_INVALID_OFFSET)
 return GetInterface().ErrorWithMessage(
-__PRETTY_FUNCTION__, "Failed to copy read memory to buffer.", error);
+LLVM_PRETTY_FUNCTION, "Failed to copy read memory to buffer.", error);
 
   return size;
 }
@@ -304,7 +304,7 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList 
_thread_list,
 
   if (language != eScriptLanguagePython)
 return GetInterface().ErrorWithMessage(
-__PRETTY_FUNCTION__,
+LLVM_PRETTY_FUNCTION,
 llvm::Twine("ScriptInterpreter language (" +
 llvm::Twine(m_interpreter->LanguageToString(language)) +
 llvm::Twine(") not supported."))
@@ -315,7 +315,7 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList 
_thread_list,
   thread_sp = std::make_shared(*this, error);
 
   if (!thread_sp || error.Fail())
-return GetInterface().ErrorWithMessage(__PRETTY_FUNCTION__,
+return GetInterface().ErrorWithMessage(LLVM_PRETTY_FUNCTION,
  error.AsCString(), error);
 
   new_thread_list.AddThread(thread_sp);

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index 11db89bfc397..de1203300e4b 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -141,13 +141,13 @@ bool ScriptedThread::CalculateStopInfo() {
 
   if (!dict_sp->GetValueForKeyAsInteger("type", stop_reason_type))
 return GetInterface()->ErrorWithMessage(
-__PRETTY_FUNCTION__,
+LLVM_PRETTY_FUNCTION,
 "Couldn't find value for key 'type' in stop reason dictionary.", 
error);
 
   StructuredData::Dictionary *data_dict;
   if (!dict_sp->GetValueForKeyAsDictionary("data", data_dict))
 return GetInterface()->ErrorWithMessage(
-__PRETTY_FUNCTION__,
+LLVM_PRETTY_FUNCTION,
 "Couldn't find value for key 'type' in stop reason dictionary.", 
error);
 
   switch (stop_reason_type) {
@@ -171,7 +171,7 @@ bool ScriptedThread::CalculateStopInfo() {
   } break;
   default:
 return GetInterface()->ErrorWithMessage(
-__PRETTY_FUNCTION__,
+

[Lldb-commits] [PATCH] D111452: [lldb/Plugins] Replace platform-specific macro with LLVM_PRETTY_FUNCTION (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111452

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

[Lldb-commits] [PATCH] D111452: [lldb/Plugins] Replace platform-specific macro with LLVM_PRETTY_FUNCTION (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch refactors Scripted Process and Scripted Thread related
classes to use LLVM_PRETTY_FUNCTION instead of the compiler macro.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111452

Files:
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
@@ -60,7 +60,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_thread_id", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return LLDB_INVALID_THREAD_ID;
 
   return obj->GetIntegerValue(LLDB_INVALID_THREAD_ID);
@@ -70,7 +70,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_name", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetStringValue().str();
@@ -80,7 +80,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_state", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return eStateInvalid;
 
   return static_cast(obj->GetIntegerValue(eStateInvalid));
@@ -90,7 +90,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_queue", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetStringValue().str();
@@ -101,7 +101,7 @@
   StructuredData::DictionarySP dict =
   Dispatch("get_stop_reason", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, dict, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))
 return {};
 
   return dict;
@@ -116,7 +116,7 @@
   StructuredData::DictionarySP dict =
   Dispatch("get_register_info", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, dict, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))
 return {};
 
   return dict;
@@ -127,7 +127,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_register_context", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetAsString()->GetValue().str();
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -39,7 +39,7 @@
 using Locker = ScriptInterpreterPythonImpl::Locker;
 
 std::string caller_signature =
-llvm::Twine(__PRETTY_FUNCTION__ + llvm::Twine(" (") +
+llvm::Twine(LLVM_PRETTY_FUNCTION + llvm::Twine(" (") +
 llvm::Twine(method_name) + llvm::Twine(")"))
 .str();
 if (!m_object_instance_sp)
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -72,7 +72,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("is_alive", error);
 
-  if (!CheckStructuredDataObject(__PRETTY_FUNCTION__, obj, error))
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
 return {};
 
   return obj->GetBooleanValue();
@@ -89,7 +89,7 @@
   "get_memory_region_containing_address", error, address);
 
   if (error.Fail()) {
-return ErrorWithMessage(__PRETTY_FUNCTION__,
+return ErrorWithMessage(LLVM_PRETTY_FUNCTION,
   error.AsCString(), error);
   }
 
@@ -101,7 +101,7 @@
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_thread_with_id", error, tid);
 
-  if 

Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via lldb-commits]

Just to clarify my use case:  I am one of the developers for a 
reverse-engineering tool called Ghidra.  Part of the tool is debugging-support 
to allow cross-over between dynamic and static analysis.  We currently support 
windbg/kd, the gcc MI2 interface, and direct Java debugging.  I have a working 
prototype for lldb, but it requires users to patch and rebuild lldb, which may 
be a heavy lift for some.  While it falls outside of my use case, I am quite 
willing to attempt implementing pieces 2 & 3 from Mr. Ingham's post.


From: Jim Ingham via Phabricator 
Sent: Friday, October 8, 2021 2:31:13 PM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
teempe...@gmail.com; medismail.benn...@gmail.com; jo...@devlieghere.com; 
tedw...@quicinc.com; jmole...@apple.com; syaghm...@apple.com; 
jing...@apple.com; v...@apple.com; boris.ulasev...@gmail.com; 
lldb-commits@lists.llvm.org; h.imai@nitech.jp; bruce.mitche...@gmail.com; 
david.spick...@linaro.org; quic_soura...@quicinc.com; 
djordje.todoro...@syrmia.com; serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting 
Bridge

jingham added a comment.

Support for a scripting language in lldb has three distinct pieces.

1. Making the SB API's available to the scripting languages
2. Adding all the callbacks so the scripting language can be used for 
breakpoint callbacks, data formatters, etc.
3. Adding a REPL for that language to the "script" command

This patch does #1, but not #2 & #3.  Do we care about whether new scripting 
languages will be able to provide the full "scripting language" experience?  
Does there need to be some plan for this before we add on the task of 
supporting the language?  Maybe it's okay to say "a REPL's too hard, and not 
worth it" but we should have a plan for adding in the callback interfaces?  Do 
we want to have somewhere you can advertise levels of support?  I would be sad 
to spend some time using a scripting language for lldb only to find it doesn't 
allow me to write breakpoint callbacks, for instance.

This is language #3 so it seems like a fit time to discuss this...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Support for a scripting language in lldb has three distinct pieces.

1. Making the SB API's available to the scripting languages
2. Adding all the callbacks so the scripting language can be used for 
breakpoint callbacks, data formatters, etc.
3. Adding a REPL for that language to the "script" command

This patch does #1, but not #2 & #3.  Do we care about whether new scripting 
languages will be able to provide the full "scripting language" experience?  
Does there need to be some plan for this before we add on the task of 
supporting the language?  Maybe it's okay to say "a REPL's too hard, and not 
worth it" but we should have a plan for adding in the callback interfaces?  Do 
we want to have somewhere you can advertise levels of support?  I would be sad 
to spend some time using a scripting language for lldb only to find it doesn't 
allow me to write breakpoint callbacks, for instance.

This is language #3 so it seems like a fit time to discuss this...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

Ah, OK, after more digging, I realize I have probably provided only half of 
what you would like for this commit.  My primary intention was to expose the SB 
API so I could make calls from Java into it, but for it to be, in some sense, a 
full-fledged member of the API, it would make sense to enable Java scripting 
from lldb.  I will attempt to fill out that side of the equation.  Cheers, D



From: lldb-commits  on behalf of David 
Millar via lldb-commits 
Sent: Friday, October 8, 2021 1:22:07 PM
To: anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com; Raphael Isemann
Subject: Re: [Lldb-commits] [PATCH] D111409 : 
proposed support for Java interface to Scripting Bridge

I'm not sure I understand your testing strategy, in particular how it applies 
to the existing Lua and Python extensions.  I am looking at the files in 
lldb/unittests/ScriptInterpreter/Lua  Do you execute test from native 
Lua/Python environments or through C wrappers-only?  You mentioned a bot - is 
that code in the main repository?



From: Raphael Isemann via Phabricator 
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409 : proposed support 
for Java interface to Scripting Bridge

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can 
just do a `git diff -U99 > ~/java-patch.diff`, click the "Update Diff" 
button on the top right of this website and then select *just* this diff file 
that contains your changes. Phabricator will render the diff properly for you 
(-> it will hide all the diff context by default). There is need to attach a 
separate diff file or anything else (users can just download the diff you 
uploaded).

Regarding the tests: We would essentially just need some basic test that 
exercises the new API a bit so that we know this works. The test code itself 
will be straightforward, but we would need a nice way to (automatically) find 
the system JRE and then set it up to be able to run the test code.

In D111409#3051140  
https://reviews.llvm.org/D111409#3051140, @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.
>
> 
>
> From: Raphael Isemann via Phabricator 
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
> jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
> boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
> bruce.mitche...@gmail.com; david.spick...@linaro.org; 
> quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
> serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409  
> https://reviews.llvm.org/D111409: proposed support for Java interface to 
> Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110  
> https://reviews.llvm.org/D111409#3051110 
> https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git 
>> diff -U999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. 
> There is no need to include the raw diff as part of the patch itself (it just 
> makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure 
> out though how we can properly set up some Java tests for this and it would 
> be nice if we also find a bot that could actually run the tests for 

Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via lldb-commits]

I'm not sure I understand your testing strategy, in particular how it applies 
to the existing Lua and Python extensions.  I am looking at the files in 
lldb/unittests/ScriptInterpreter/Lua  Do you execute test from native 
Lua/Python environments or through C wrappers-only?  You mentioned a bot - is 
that code in the main repository?


From: Raphael Isemann via Phabricator 
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting 
Bridge

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can 
just do a `git diff -U99 > ~/java-patch.diff`, click the "Update Diff" 
button on the top right of this website and then select *just* this diff file 
that contains your changes. Phabricator will render the diff properly for you 
(-> it will hide all the diff context by default). There is need to attach a 
separate diff file or anything else (users can just download the diff you 
uploaded).

Regarding the tests: We would essentially just need some basic test that 
exercises the new API a bit so that we know this works. The test code itself 
will be straightforward, but we would need a nice way to (automatically) find 
the system JRE and then set it up to be able to run the test code.

In D111409#3051140 , @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.
>
> 
>
> From: Raphael Isemann via Phabricator 
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
> jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
> boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
> bruce.mitche...@gmail.com; david.spick...@linaro.org; 
> quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
> serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409 : proposed support 
> for Java interface to Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110  
> https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git 
>> diff -U999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. 
> There is no need to include the raw diff as part of the patch itself (it just 
> makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure 
> out though how we can properly set up some Java tests for this and it would 
> be nice if we also find a bot that could actually run the tests for us.
>
>
>
>
> Comment at: lldb/bindings/java/CMakeLists.txt:3
> + * IP: Apache License 2.0 with LLVM Exceptions
> + */
>
> +add_custom_command(
> 
>
> I don't think CMake accepts this as a comment and I think we anyway don't put 
> license headers in CMake scripts.
>
>
>
>
> Comment at: lldb/source/API/CMakeLists.txt:84
>
>   SBTrace.cpp
>
> +  SBTraceOptions.cpp
>
>   SBType.cpp
>
> 
>
> I think this is some conflict with one of the SBTrace patches.
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar updated this revision to Diff 378288.
d-millar added a comment.

FIx for potential conflicts with master.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

Files:
  lldb/bindings/java/java-typemaps.swig
  lldb/bindings/java/java.swig
  lldb/source/API/CMakeLists.txt


Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -81,7 +81,6 @@
   SBThreadCollection.cpp
   SBThreadPlan.cpp
   SBTrace.cpp
-  SBTraceOptions.cpp
   SBType.cpp
   SBTypeCategory.cpp
   SBTypeEnumMember.cpp
@@ -203,13 +202,7 @@
   set_target_properties(liblldb_exports PROPERTIES FOLDER "lldb misc")
 endif()
 
-if (MSVC)
-  # Only MSVC has the ABI compatibility problem and avoids using 
FindPythonLibs,
-  # so only it needs to explicitly link against ${Python3_LIBRARIES}
-  if (LLDB_ENABLE_PYTHON)
-target_link_libraries(liblldb PRIVATE ${Python3_LIBRARIES})
-  endif()
-else()
+if (NOT MSVC)
   set_target_properties(liblldb
 PROPERTIES
 OUTPUT_NAME lldb
Index: lldb/bindings/java/java.swig
===
--- lldb/bindings/java/java.swig
+++ lldb/bindings/java/java.swig
@@ -1,6 +1,3 @@
-/* ###
- * IP: Apache License 2.0 with LLVM Exceptions
- */
 /*
lldb.swig
 
Index: lldb/bindings/java/java-typemaps.swig
===
--- lldb/bindings/java/java-typemaps.swig
+++ lldb/bindings/java/java-typemaps.swig
@@ -1,6 +1,3 @@
-/* ###
- * IP: Apache License 2.0 with LLVM Exceptions
- */
 %include 
 %include 
 %include 


Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -81,7 +81,6 @@
   SBThreadCollection.cpp
   SBThreadPlan.cpp
   SBTrace.cpp
-  SBTraceOptions.cpp
   SBType.cpp
   SBTypeCategory.cpp
   SBTypeEnumMember.cpp
@@ -203,13 +202,7 @@
   set_target_properties(liblldb_exports PROPERTIES FOLDER "lldb misc")
 endif()
 
-if (MSVC)
-  # Only MSVC has the ABI compatibility problem and avoids using FindPythonLibs,
-  # so only it needs to explicitly link against ${Python3_LIBRARIES}
-  if (LLDB_ENABLE_PYTHON)
-target_link_libraries(liblldb PRIVATE ${Python3_LIBRARIES})
-  endif()
-else()
+if (NOT MSVC)
   set_target_properties(liblldb
 PROPERTIES
 OUTPUT_NAME lldb
Index: lldb/bindings/java/java.swig
===
--- lldb/bindings/java/java.swig
+++ lldb/bindings/java/java.swig
@@ -1,6 +1,3 @@
-/* ###
- * IP: Apache License 2.0 with LLVM Exceptions
- */
 /*
lldb.swig
 
Index: lldb/bindings/java/java-typemaps.swig
===
--- lldb/bindings/java/java-typemaps.swig
+++ lldb/bindings/java/java-typemaps.swig
@@ -1,6 +1,3 @@
-/* ###
- * IP: Apache License 2.0 with LLVM Exceptions
- */
 %include 
 %include 
 %include 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, but maybe give the test comment a FIXME.




Comment at: 
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:39
+ //% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
 //% 
self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
 //% 
self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))

If you align the start of these comments when merging this, then I'll address 
you from now on as "Pavel the Great".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111399

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar updated this revision to Diff 378282.
d-millar added a comment.

Another (hopefully cleaner) attempt.  If the addition of SBTraceOptions in 
lldb/source/API/CMakelists.txt conflicts, please feel free to delete it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

Files:
  lldb/CMakeLists.txt
  lldb/bindings/CMakeLists.txt
  lldb/bindings/java/CMakeLists.txt
  lldb/bindings/java/java-typemaps.swig
  lldb/bindings/java/java.swig
  lldb/cmake/modules/FindJavaAndSwig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/liblldb-private.exports
  lldb/source/API/liblldb.exports

Index: lldb/source/API/liblldb.exports
===
--- lldb/source/API/liblldb.exports
+++ lldb/source/API/liblldb.exports
@@ -1,4 +1,7 @@
 _ZN4lldb*
 _ZNK4lldb*
+_ZN12lldb*
+_ZNK12lldb*
 init_lld*
 PyInit__lldb*
+Java*
Index: lldb/source/API/liblldb-private.exports
===
--- lldb/source/API/liblldb-private.exports
+++ lldb/source/API/liblldb-private.exports
@@ -4,3 +4,4 @@
 _ZNK12lldb_private*
 init_lld*
 PyInit__lldb*
+Java*
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -736,6 +736,9 @@
   AddBoolConfigEntry(
   *config_up, "lua", LLDB_ENABLE_LUA,
   "A boolean value that indicates if lua support is enabled in LLDB");
+  AddBoolConfigEntry(
+  *config_up, "java", LLDB_ENABLE_JAVA,
+  "A boolean value that indicates if java support is enabled in LLDB");
   AddLLVMTargets(*config_up);
 
   SBStructuredData data;
Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -19,6 +19,11 @@
   set(lldb_lua_wrapper ${lua_bindings_dir}/LLDBWrapLua.cpp)
 endif()
 
+if(LLDB_ENABLE_JAVA)
+  get_target_property(java_bindings_dir swig_wrapper_java BINARY_DIR)
+  set(lldb_java_wrapper ${java_bindings_dir}/LLDBWrapJava.cpp)
+endif()
+
 add_lldb_library(liblldb SHARED ${option_framework}
   SBAddress.cpp
   SBAttachInfo.cpp
@@ -76,6 +81,7 @@
   SBThreadCollection.cpp
   SBThreadPlan.cpp
   SBTrace.cpp
+  SBTraceOptions.cpp
   SBType.cpp
   SBTypeCategory.cpp
   SBTypeEnumMember.cpp
@@ -92,6 +98,7 @@
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}
   ${lldb_lua_wrapper}
+  ${lldb_java_wrapper}
 
   LINK_LIBS
 lldbBase
@@ -160,6 +167,21 @@
   set_source_files_properties(${lldb_lua_wrapper} PROPERTIES GENERATED ON)
 endif()
 
+if(LLDB_ENABLE_JAVA)
+  add_dependencies(liblldb swig_wrapper_java)
+  target_include_directories(liblldb PRIVATE ${JAVA_INCLUDE_DIR})
+  target_include_directories(liblldb PRIVATE ${JAVA_INCLUDE_DIR}/darwin)
+
+  if (MSVC)
+set_property(SOURCE ${lldb_java_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
+  else()
+set_property(SOURCE ${lldb_java_wrapper} APPEND_STRING PROPERTY COMPILE_FLAGS " ")
+  endif()
+
+  set_source_files_properties(${lldb_java_wrapper} PROPERTIES GENERATED ON)
+endif()
+
+
 set_target_properties(liblldb
   PROPERTIES
   VERSION ${LLDB_VERSION}
@@ -181,7 +203,13 @@
   set_target_properties(liblldb_exports PROPERTIES FOLDER "lldb misc")
 endif()
 
-if (NOT MSVC)
+if (MSVC)
+  # Only MSVC has the ABI compatibility problem and avoids using FindPythonLibs,
+  # so only it needs to explicitly link against ${Python3_LIBRARIES}
+  if (LLDB_ENABLE_PYTHON)
+target_link_libraries(liblldb PRIVATE ${Python3_LIBRARIES})
+  endif()
+else()
   set_target_properties(liblldb
 PROPERTIES
 OUTPUT_NAME lldb
Index: lldb/include/lldb/Host/Config.h.cmake
===
--- lldb/include/lldb/Host/Config.h.cmake
+++ lldb/include/lldb/Host/Config.h.cmake
@@ -41,6 +41,8 @@
 
 #cmakedefine01 LLDB_ENABLE_LIBXML2
 
+#cmakedefine01 LLDB_ENABLE_JAVA
+
 #cmakedefine01 LLDB_ENABLE_LUA
 
 #cmakedefine01 LLDB_ENABLE_PYTHON
Index: lldb/cmake/modules/FindJavaAndSwig.cmake
===
--- /dev/null
+++ lldb/cmake/modules/FindJavaAndSwig.cmake
@@ -0,0 +1,32 @@
+#.rst:
+# FindJavaAndSwig
+# --
+#
+# Find Java and SWIG as a whole.
+
+#if(JAVA_LIBRARIES AND JAVA_INCLUDE_DIR AND SWIG_EXECUTABLE)
+if(SWIG_EXECUTABLE)
+  set(JAVAANDSWIG_FOUND TRUE)
+else()
+  find_package(SWIG 2.0)
+  if (SWIG_FOUND)
+find_package(Java 11.0)
+if(JAVA_FOUND AND SWIG_FOUND)
+  mark_as_advanced(
+JAVA_LIBRARIES
+JAVA_INCLUDE_DIR
+SWIG_EXECUTABLE)
+endif()
+  else()
+message(STATUS "SWIG 2 or later is required for Java support in LLDB but could not be found")
+  endif()
+
+  include(FindPackageHandleStandardArgs)
+  

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D107585#3051607 , 
@stella.stamenova wrote:

> This broke the windows lldb bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/10836

This should be fixed with f94c9af6227f49c5da5bf1c9f97fdf887d26774c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

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

[Lldb-commits] [lldb] f94c9af - [lldb] Fix windows build failure due to undefined macro

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-08T18:27:21+02:00
New Revision: f94c9af6227f49c5da5bf1c9f97fdf887d26774c

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

LOG: [lldb] Fix windows build failure due to undefined macro

This should fix a build failure on Windows caused by the macro
__PRETTY_FUNCTION__ not being defined.

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

https://reviews.llvm.org/D107585

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Interpreter/ScriptedInterface.h

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/ScriptedInterface.h
index 38164739bab5..bd1ed11279f4 100644
--- a/lldb/include/lldb/Interpreter/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/ScriptedInterface.h
@@ -9,6 +9,10 @@
 #ifndef LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
 #define LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
 
+#ifdef _MSC_VER
+#define __PRETTY_FUNCTION__ __FUNCSIG__
+#endif
+
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Log.h"



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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D111409#3051556 , @d-millar wrote:

> Hmmm, well, have tried running "git clang-format" with a few different 
> invocations, but seem to always get "No modified files to format."  
> Suggestions?

I was confused by the `patch` file, since it included the entire 
`SBDebugger.cpp`, I thought you were adding all that code.
Since clang-format mostly cares about C/Objective-C/C++ files, I guess you're 
fine with the formatting.  However, it looks like the `patch` file is still 
here.

To remove it from the diff, make sure you delete it from your local repository, 
add all your changes to the git index (`git add `) and make a commit. 
Then, create your new patch (`git show HEAD -U99 > mypatch.patch`) and 
update the differential by uploading the new patch (`mypatch.patch`). While 
doing so, you can review your changes and make sure you didn't included the 
`patch` file in the differential.

Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111435: [lldb] [DynamicRegisterInfo] Support setting from vector

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste.
mgorny requested review of this revision.

Add an overload of DynamicRegisterInfo::SetRegisterInfo() that accepts
a std::vector as an argument.  This moves the conversion
from DRI::Register to RegisterInfo directly into DynamicRegisterInfo,
and avoids the necessity of creating fully-compatible intermediate
RegisterInfo instances.

While the new method could technically reuse AddRegister(), the ultimate
goal is to replace AddRegister() with SetRegisterInfo() entirely.


https://reviews.llvm.org/D111435

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -379,6 +379,45 @@
   return m_regs.size();
 }
 
+size_t DynamicRegisterInfo::SetRegisterInfo(
+const std::vector ,
+const ArchSpec ) {
+  assert(!m_finalized);
+
+  for (auto it : llvm::enumerate(regs)) {
+uint32_t local_regnum = it.index();
+const DynamicRegisterInfo::Register  = it.value();
+
+assert(reg.name);
+assert(reg.set_name);
+
+for (uint32_t value_reg : reg.value_regs)
+  m_value_regs_map[local_regnum].push_back(value_reg);
+for (uint32_t invalidate_reg : reg.invalidate_regs)
+  m_invalidate_regs_map[local_regnum].push_back(invalidate_reg);
+
+struct RegisterInfo reg_info {
+  reg.name.AsCString(), reg.alt_name.AsCString(), reg.byte_size,
+  reg.byte_offset, reg.encoding, reg.format,
+  {reg.regnum_ehframe, reg.regnum_dwarf, reg.regnum_generic,
+   reg.regnum_remote, local_regnum},
+  // value_regs and invalidate_regs are filled by Finalize()
+  nullptr, nullptr
+};
+
+m_regs.push_back(reg_info);
+
+uint32_t set = GetRegisterSetIndexByName(reg.set_name, true);
+assert(set < m_sets.size());
+assert(set < m_set_reg_nums.size());
+assert(set < m_set_names.size());
+m_set_reg_nums[set].push_back(local_regnum);
+  };
+
+  Finalize(arch);
+  return m_regs.size();
+}
+
 void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
   ConstString _name) {
   assert(!m_finalized);
@@ -682,8 +721,9 @@
   return nullptr;
 }
 
-uint32_t DynamicRegisterInfo::GetRegisterSetIndexByName(ConstString _name,
-bool can_create) {
+uint32_t
+DynamicRegisterInfo::GetRegisterSetIndexByName(const ConstString _name,
+   bool can_create) {
   name_collection::iterator pos, end = m_set_names.end();
   for (pos = m_set_names.begin(); pos != end; ++pos) {
 if (*pos == set_name)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4502,32 +4502,7 @@
   if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
 abi_sp->AugmentRegisterInfo(registers);
 
-  for (auto it : llvm::enumerate(registers)) {
-uint32_t local_regnum = it.index();
-DynamicRegisterInfo::Register _reg_info = it.value();
-
-auto regs_with_sentinel = [](std::vector ) -> uint32_t * {
-  if (!vec.empty()) {
-vec.push_back(LLDB_INVALID_REGNUM);
-return vec.data();
-  }
-  return nullptr;
-};
-
-struct RegisterInfo reg_info {
-  remote_reg_info.name.AsCString(), remote_reg_info.alt_name.AsCString(),
-  remote_reg_info.byte_size, remote_reg_info.byte_offset,
-  remote_reg_info.encoding, remote_reg_info.format,
-  {remote_reg_info.regnum_ehframe, remote_reg_info.regnum_dwarf,
-   remote_reg_info.regnum_generic, remote_reg_info.regnum_remote,
-   local_regnum},
-  regs_with_sentinel(remote_reg_info.value_regs),
-  regs_with_sentinel(remote_reg_info.invalidate_regs),
-};
-m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
-  };
-
-  m_register_info_sp->Finalize(arch_to_use);
+  m_register_info_sp->SetRegisterInfo(registers, arch_to_use);
 }
 
 // query the target of gdb-remote for extended target information returns
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -53,6 +53,9 @@
   size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary ,
  const lldb_private::ArchSpec );
 
+  size_t SetRegisterInfo(const std::vector ,
+ const lldb_private::ArchSpec );
+
   void 

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D107585#3051607 , 
@stella.stamenova wrote:

> This broke the windows lldb bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/10836



  
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\source\Plugins\Process\scripted\ScriptedThread.cpp(144):
 error C2065: '__PRETTY_FUNCTION__': undeclared identifier

It looks like MSVC doesn't have the `__PRETTY_FUNCTION__` macro ... Should be 
an easy fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

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

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This broke the windows lldb bot:

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

Hmmm, well, have tried running "git clang-format" with a few different 
invocations, but seem to always get "No modified files to format."  Suggestions?



From: David Millar via Phabricator 
Sent: Friday, October 8, 2021 11:33:37 AM
To: David Millar; teempe...@gmail.com
Subject: [PATCH] D111409 : proposed support 
for Java interface to Scripting Bridge

d-millar added a comment.

Merde - I followed you instructions but forgot to run clang-format.  Give me a 
minute



From: Raphael Isemann via Phabricator 
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409  
https://reviews.llvm.org/D111409: proposed support for Java interface to 
Scripting Bridge

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can 
just do a `git diff -U99 > ~/java-patch.diff`, click the "Update Diff" 
button on the top right of this website and then select *just* this diff file 
that contains your changes. Phabricator will render the diff properly for you 
(-> it will hide all the diff context by default). There is need to attach a 
separate diff file or anything else (users can just download the diff you 
uploaded).

Regarding the tests: We would essentially just need some basic test that 
exercises the new API a bit so that we know this works. The test code itself 
will be straightforward, but we would need a nice way to (automatically) find 
the system JRE and then set it up to be able to run the test code.

In D111409#3051140  
https://reviews.llvm.org/D111409#3051140 
https://reviews.llvm.org/D111409#3051140, @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.
>
> 
>
> From: Raphael Isemann via Phabricator 
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
> jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
> boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
> bruce.mitche...@gmail.com; david.spick...@linaro.org; 
> quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
> serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409  
> https://reviews.llvm.org/D111409 https://reviews.llvm.org/D111409: proposed 
> support for Java interface to Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110  
> https://reviews.llvm.org/D111409#3051110 
> https://reviews.llvm.org/D111409#3051110 
> https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git 
>> diff -U999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. 
> There is no need to include the raw diff as part of the patch itself (it just 
> makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure 
> out though how we can properly set up some Java tests for this and it would 
> be nice if we also find a bot that could actually run the tests for us.
>
> Comment at: lldb/bindings/java/CMakeLists.txt:3
> + * IP: Apache License 2.0 with LLVM Exceptions
> + */
>
> +add_custom_command(
> 
>
> I don't think CMake accepts this as a comment and I think we anyway don't put 
> license headers in CMake scripts.
>
> Comment at: lldb/source/API/CMakeLists.txt:84
>
>   SBTrace.cpp
>
> +  SBTraceOptions.cpp
>
>   SBType.cpp
>
> 
>
> I think this is some conflict with one of the SBTrace patches.
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409

Repository:

  rLLDB LLDB

CHANGES SINCE LAST ACTION

  https://reviews.llvm.org/D111409/new/

https://reviews.llvm.org/D111409

Repository:

  rLLDB LLDB

CHANGES 

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 updated this revision to Diff 378255.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -72,6 +72,24 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, ResultForValidDLangName) {
+  ConstString mangled_name("_Dmain");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  ConstString expected_result("D main");
+  EXPECT_STREQ(expected_result.GetCString(), the_demangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidDLangName) {
+  ConstString mangled_name("_DDD");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  EXPECT_STREQ("", the_demangled.GetCString());
+}
+
+
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
   subsystems;
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -248,10 +248,8 @@
 
   // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeMSVC:
-return false;
-
-  // No filters for this scheme yet. Include all names in indexing.
   case Mangled::eManglingSchemeRustV0:
+  case Mangled::eManglingSchemeD:
 return false;
 
   // Don't try and demangle things we can't categorize.
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -45,6 +45,9 @@
   if (name.startswith("_R"))
 return Mangled::eManglingSchemeRustV0;
 
+  if (name.startswith("_D"))
+return Mangled::eManglingSchemeD;
+
   if (name.startswith("_Z"))
 return Mangled::eManglingSchemeItanium;
 
@@ -185,6 +188,19 @@
   return demangled_cstr;
 }
 
+static char *GetDLangDemangledStr(const char *M) {
+  char *demangled_cstr = llvm::dlangDemangle(M);
+
+  if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
+if (demangled_cstr && demangled_cstr[0])
+  LLDB_LOG(log, "demangled dlang: {0} -> \"{1}\"", M, demangled_cstr);
+else
+  LLDB_LOG(log, "demangled dlang: {0} -> error: failed to demangle", M);
+  }
+
+  return demangled_cstr;
+}
+
 // Explicit demangling for scheduled requests during batch processing. This
 // makes use of ItaniumPartialDemangler's rich demangle info
 bool Mangled::DemangleWithRichManglingInfo(
@@ -244,7 +260,8 @@
   }
 
   case eManglingSchemeRustV0:
-// Rich demangling scheme is not supported for Rust
+  case eManglingSchemeD:
+// Rich demangling scheme is not supported
 return false;
   }
   llvm_unreachable("Fully covered switch above!");
@@ -277,6 +294,9 @@
   case eManglingSchemeRustV0:
 demangled_name = GetRustV0DemangledStr(mangled_name);
 break;
+  case eManglingSchemeD:
+demangled_name = GetDLangDemangledStr(mangled_name);
+break;
   case eManglingSchemeNone:
 llvm_unreachable("eManglingSchemeNone was handled already");
   }
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -44,7 +44,8 @@
 eManglingSchemeNone = 0,
 eManglingSchemeMSVC,
 eManglingSchemeItanium,
-eManglingSchemeRustV0
+eManglingSchemeRustV0,
+eManglingSchemeD
   };
 
   /// Default constructor.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via lldb-commits]

Merde - I followed you instructions but forgot to run clang-format.  Give me a 
minute


From: Raphael Isemann via Phabricator 
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting 
Bridge

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can 
just do a `git diff -U99 > ~/java-patch.diff`, click the "Update Diff" 
button on the top right of this website and then select *just* this diff file 
that contains your changes. Phabricator will render the diff properly for you 
(-> it will hide all the diff context by default). There is need to attach a 
separate diff file or anything else (users can just download the diff you 
uploaded).

Regarding the tests: We would essentially just need some basic test that 
exercises the new API a bit so that we know this works. The test code itself 
will be straightforward, but we would need a nice way to (automatically) find 
the system JRE and then set it up to be able to run the test code.

In D111409#3051140 , @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.
>
> 
>
> From: Raphael Isemann via Phabricator 
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
> jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
> boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
> bruce.mitche...@gmail.com; david.spick...@linaro.org; 
> quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
> serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409 : proposed support 
> for Java interface to Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110  
> https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git 
>> diff -U999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. 
> There is no need to include the raw diff as part of the patch itself (it just 
> makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure 
> out though how we can properly set up some Java tests for this and it would 
> be nice if we also find a bot that could actually run the tests for us.
>
>
>
>
> Comment at: lldb/bindings/java/CMakeLists.txt:3
> + * IP: Apache License 2.0 with LLVM Exceptions
> + */
>
> +add_custom_command(
> 
>
> I don't think CMake accepts this as a comment and I think we anyway don't put 
> license headers in CMake scripts.
>
>
>
>
> Comment at: lldb/source/API/CMakeLists.txt:84
>
>   SBTrace.cpp
>
> +  SBTraceOptions.cpp
>
>   SBType.cpp
>
> 
>
> I think this is some conflict with one of the SBTrace patches.
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar updated this revision to Diff 378237.
d-millar added a comment.

supplying a better patch file


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

Files:
  lldb/bindings/java/CMakeLists.txt
  patch

Index: patch
===
--- patch
+++ /dev/null
@@ -1,2403 +0,0 @@
-diff --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
-index 594c769141b4..faf3846a0a16 100644
 a/lldb/CMakeLists.txt
-+++ b/lldb/CMakeLists.txt
-@@ -1,106 +1,106 @@
- cmake_minimum_required(VERSION 3.13.4)
- 
- # Add path for custom modules.
- set(CMAKE_MODULE_PATH
-   ${CMAKE_MODULE_PATH}
-   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
-   )
- 
- # If we are not building as part of LLVM, build LLDB as a standalone project,
- # using LLVM as an external library.
- if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
-   project(lldb)
-   include(LLDBStandalone)
- 
-   set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
-   set(CMAKE_CXX_STANDARD_REQUIRED YES)
-   set(CMAKE_CXX_EXTENSIONS NO)
- endif()
- 
- include(LLDBConfig)
- include(AddLLDB)
- 
- # Define the LLDB_CONFIGURATION_xxx matching the build type.
- if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
-   add_definitions(-DLLDB_CONFIGURATION_DEBUG)
- endif()
- 
- if (WIN32)
-   add_definitions(-D_ENABLE_EXTENDED_ALIGNED_STORAGE)
- endif()
- 
- if (LLDB_ENABLE_PYTHON)
-   if (NOT CMAKE_CROSSCOMPILING)
- execute_process(
-   COMMAND ${Python3_EXECUTABLE}
-   -c "import distutils.sysconfig; print(distutils.sysconfig.get_python_lib(True, False, ''))"
-   OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_RELATIVE_PATH
-   OUTPUT_STRIP_TRAILING_WHITESPACE)
- 
- file(TO_CMAKE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH} LLDB_PYTHON_DEFAULT_RELATIVE_PATH)
-   else ()
- if ("${LLDB_PYTHON_RELATIVE_PATH}" STREQUAL "")
-   message(FATAL_ERROR
- "Crosscompiling LLDB with Python requires manually setting
- LLDB_PYTHON_RELATIVE_PATH.")
- endif ()
-   endif ()
- 
-   set(LLDB_PYTHON_RELATIVE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH}
- CACHE STRING "Path where Python modules are installed, relative to install prefix")
- endif ()
- 
--if (LLDB_ENABLE_PYTHON OR LLDB_ENABLE_LUA)
-+if (LLDB_ENABLE_PYTHON OR LLDB_ENABLE_LUA OR LLDB_ENABLE_JAVA)
-   add_subdirectory(bindings)
- endif ()
- 
- # We need the headers generated by instrinsics_gen before we can compile
- # any source file in LLDB as the imported Clang modules might include
- # some of these generated headers. This approach is copied from Clang's main
- # CMakeLists.txt, so it should kept in sync the code in Clang which was added
- # in llvm-svn 308844.
- if(LLVM_ENABLE_MODULES)
-   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
- endif()
- 
- if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
-   set(LLVM_USE_HOST_TOOLS ON)
-   include(CrossCompile)
-   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)
- message(FATAL_ERROR
-   "Crosscompiling standalone requires the variables NATIVE_{CLANG,LLVM}_DIR
-   for building the native lldb-tblgen used during the build process.")
-   endif()
-   llvm_create_cross_target(lldb NATIVE "" Release
- -DLLVM_DIR=${NATIVE_LLVM_DIR}
- -DClang_DIR=${NATIVE_Clang_DIR})
- endif()
- 
- # TableGen
- add_subdirectory(utils/TableGen)
- 
- add_subdirectory(source)
- add_subdirectory(tools)
- add_subdirectory(docs)
- 
- if (LLDB_ENABLE_PYTHON)
-   if(LLDB_BUILD_FRAMEWORK)
- set(lldb_python_target_dir "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Resources/Python/lldb")
-   else()
- set(lldb_python_target_dir "${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_PYTHON_RELATIVE_PATH}/lldb")
-   endif()
-   get_target_property(lldb_python_bindings_dir swig_wrapper_python BINARY_DIR)
-   finish_swig_python("lldb-python" "${lldb_python_bindings_dir}" "${lldb_python_target_dir}")
- endif()
- 
- option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." ${LLVM_INCLUDE_TESTS})
- if(LLDB_INCLUDE_TESTS)
-   add_subdirectory(test)
-   add_subdirectory(unittests)
-   add_subdirectory(utils)
- endif()
- 
- if(LLDB_BUILT_STANDALONE AND NOT LLVM_ENABLE_IDE)
-   llvm_distribution_add_targets()
- endif()
-diff --git a/lldb/bindings/CMakeLists.txt b/lldb/bindings/CMakeLists.txt
-index 9759b069fdc4..780413104d7f 100644
 a/lldb/bindings/CMakeLists.txt
-+++ b/lldb/bindings/CMakeLists.txt
-@@ -1,40 +1,45 @@
- file(GLOB SWIG_INTERFACES interface/*.i)
- file(GLOB_RECURSE SWIG_SOURCES *.swig)
- file(GLOB SWIG_HEADERS
-   ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-   ${LLDB_SOURCE_DIR}/include/lldb/*.h
- )
- file(GLOB SWIG_PRIVATE_HEADERS
-   ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h
- )
- foreach(private_header ${SWIG_PRIVATE_HEADERS})
-   list(REMOVE_ITEM SWIG_HEADERS ${private_header})
- endforeach()
- 
- 

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

No problem, first time using Phabricator is always a bit confusing. You can 
just do a `git diff -U99 > ~/java-patch.diff`, click the "Update Diff" 
button on the top right of this website and then select *just* this diff file 
that contains your changes. Phabricator will render the diff properly for you 
(-> it will hide all the diff context by default). There is need to attach a 
separate diff file or anything else (users can just download the diff you 
uploaded).

Regarding the tests: We would essentially just need some basic test that 
exercises the new API a bit so that we know this works. The test code itself 
will be straightforward, but we would need a nice way to (automatically) find 
the system JRE and then set it up to be able to run the test code.

In D111409#3051140 , @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.
>
> 
>
> From: Raphael Isemann via Phabricator 
> Sent: Friday, October 8, 2021 10:46:50 AM
> To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
> medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
> jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
> boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
> bruce.mitche...@gmail.com; david.spick...@linaro.org; 
> quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
> serhiy.re...@gmail.com; liburd1...@outlook.com
> Cc: mgo...@gentoo.org
> Subject: [PATCH] D111409 : proposed support 
> for Java interface to Scripting Bridge
>
> teemperor added a comment.
>
> In D111409#3051110  
> https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
>
>> Apologies for the inclusion of that last file "patch" - that is the "git 
>> diff -U999" result, should that be useful.
>
> You can just upload that diff file and Phabricator will display it properly. 
> There is no need to include the raw diff as part of the patch itself (it just 
> makes this diff 100 times larger than it needs to be) :)
>
> Anyway, I think this seems like a reasonable thing to have. We have to figure 
> out though how we can properly set up some Java tests for this and it would 
> be nice if we also find a bot that could actually run the tests for us.
>
>
>
>
> Comment at: lldb/bindings/java/CMakeLists.txt:3
> + * IP: Apache License 2.0 with LLVM Exceptions
> + */
>
> +add_custom_command(
> 
>
> I don't think CMake accepts this as a comment and I think we anyway don't put 
> license headers in CMake scripts.
>
>
>
>
> Comment at: lldb/source/API/CMakeLists.txt:84
>
>   SBTrace.cpp
>
> +  SBTraceOptions.cpp
>
>   SBType.cpp
>
> 
>
> I think this is some conflict with one of the SBTrace patches.
>
> Repository:
>
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D111409/new/
>
> https://reviews.llvm.org/D111409




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D111409#3051140 , @d-millar wrote:

> Am obviously brand new to your process and a bit of an old dog when it comes 
> to learning new tricks.  Would you prefer I make a new submission with the 
> -U99 diff?   Also, am more than willing to help with the Java tests if 
> that would be useful.

Hi @d-millar,

Here are some ressources that will help you comply with the LLVM guidelines:

- Contributing to LLVM : https://llvm.org/docs/Contributing.html
- Code Reviews with Phabricator : https://llvm.org/docs/Phabricator.html

Do you mind updating this diff by removing the patch and running clang-format 
on your code ?

That would simplify code review for reviewers.

Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via lldb-commits]

Am obviously brand new to your process and a bit of an old dog when it comes to 
learning new tricks.  Would you prefer I make a new submission with the 
-U99 diff?   Also, am more than willing to help with the Java tests if that 
would be useful.


From: Raphael Isemann via Phabricator 
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoro...@apple.com; fallk...@yahoo.com; kkle...@redhat.com; 
medismail.benn...@gmail.com; jo...@devlieghere.com; tedw...@quicinc.com; 
jmole...@apple.com; syaghm...@apple.com; jing...@apple.com; v...@apple.com; 
boris.ulasev...@gmail.com; lldb-commits@lists.llvm.org; h.imai@nitech.jp; 
bruce.mitche...@gmail.com; david.spick...@linaro.org; 
quic_soura...@quicinc.com; djordje.todoro...@syrmia.com; 
serhiy.re...@gmail.com; liburd1...@outlook.com
Cc: mgo...@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting 
Bridge

teemperor added a comment.

In D111409#3051110 , @d-millar wrote:

> Apologies for the inclusion of that last file "patch" - that is the "git diff 
> -U999" result, should that be useful.

You can just upload that diff file and Phabricator will display it properly. 
There is no need to include the raw diff as part of the patch itself (it just 
makes this diff 100 times larger than it needs to be) :)

Anyway, I think this seems like a reasonable thing to have. We have to figure 
out though how we can properly set up some Java tests for this and it would be 
nice if we also find a bot that could actually run the tests for us.




Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */
+add_custom_command(

I don't think CMake accepts this as a comment and I think we anyway don't put 
license headers in CMake scripts.



Comment at: lldb/source/API/CMakeLists.txt:84
   SBTrace.cpp
+  SBTraceOptions.cpp
   SBType.cpp

I think this is some conflict with one of the SBTrace patches.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D111409#3051110 , @d-millar wrote:

> Apologies for the inclusion of that last file "patch" - that is the "git diff 
> -U999" result, should that be useful.

You can just upload that diff file and Phabricator will display it properly. 
There is no need to include the raw diff as part of the patch itself (it just 
makes this diff 100 times larger than it needs to be) :)

Anyway, I think this seems like a reasonable thing to have. We have to figure 
out though how we can properly set up some Java tests for this and it would be 
nice if we also find a bot that could actually run the tests for us.




Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */
+add_custom_command(

I don't think CMake accepts this as a comment and I think we anyway don't put 
license headers in CMake scripts.



Comment at: lldb/source/API/CMakeLists.txt:84
   SBTrace.cpp
+  SBTraceOptions.cpp
   SBType.cpp

I think this is some conflict with one of the SBTrace patches.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

Apologies for the inclusion of that last file "patch" - that is the "git diff 
-U999" result, should that be useful.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D111409

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

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

[David Millar via Phabricator via lldb-commits]

d-millar created this revision.
d-millar added a reviewer: LLDB.
Herald added subscribers: teemperor, mgorny.
d-millar requested review of this revision.

The patch include files necessary to extend the Scripting Bridge API with Java. 
 The edits follow the existing patterns for Python and Lua.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D111409

Files:
  lldb/CMakeLists.txt
  lldb/bindings/CMakeLists.txt
  lldb/bindings/java/CMakeLists.txt
  lldb/bindings/java/java-typemaps.swig
  lldb/bindings/java/java.swig
  lldb/cmake/modules/FindJavaAndSwig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/liblldb-private.exports
  lldb/source/API/liblldb.exports
  patch

Index: patch
===
--- /dev/null
+++ patch
@@ -0,0 +1,2403 @@
+diff --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
+index 594c769141b4..faf3846a0a16 100644
+--- a/lldb/CMakeLists.txt
 b/lldb/CMakeLists.txt
+@@ -1,106 +1,106 @@
+ cmake_minimum_required(VERSION 3.13.4)
+ 
+ # Add path for custom modules.
+ set(CMAKE_MODULE_PATH
+   ${CMAKE_MODULE_PATH}
+   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+   )
+ 
+ # If we are not building as part of LLVM, build LLDB as a standalone project,
+ # using LLVM as an external library.
+ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+   project(lldb)
+   include(LLDBStandalone)
+ 
+   set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+   set(CMAKE_CXX_STANDARD_REQUIRED YES)
+   set(CMAKE_CXX_EXTENSIONS NO)
+ endif()
+ 
+ include(LLDBConfig)
+ include(AddLLDB)
+ 
+ # Define the LLDB_CONFIGURATION_xxx matching the build type.
+ if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+   add_definitions(-DLLDB_CONFIGURATION_DEBUG)
+ endif()
+ 
+ if (WIN32)
+   add_definitions(-D_ENABLE_EXTENDED_ALIGNED_STORAGE)
+ endif()
+ 
+ if (LLDB_ENABLE_PYTHON)
+   if (NOT CMAKE_CROSSCOMPILING)
+ execute_process(
+   COMMAND ${Python3_EXECUTABLE}
+   -c "import distutils.sysconfig; print(distutils.sysconfig.get_python_lib(True, False, ''))"
+   OUTPUT_VARIABLE LLDB_PYTHON_DEFAULT_RELATIVE_PATH
+   OUTPUT_STRIP_TRAILING_WHITESPACE)
+ 
+ file(TO_CMAKE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH} LLDB_PYTHON_DEFAULT_RELATIVE_PATH)
+   else ()
+ if ("${LLDB_PYTHON_RELATIVE_PATH}" STREQUAL "")
+   message(FATAL_ERROR
+ "Crosscompiling LLDB with Python requires manually setting
+ LLDB_PYTHON_RELATIVE_PATH.")
+ endif ()
+   endif ()
+ 
+   set(LLDB_PYTHON_RELATIVE_PATH ${LLDB_PYTHON_DEFAULT_RELATIVE_PATH}
+ CACHE STRING "Path where Python modules are installed, relative to install prefix")
+ endif ()
+ 
+-if (LLDB_ENABLE_PYTHON OR LLDB_ENABLE_LUA)
++if (LLDB_ENABLE_PYTHON OR LLDB_ENABLE_LUA OR LLDB_ENABLE_JAVA)
+   add_subdirectory(bindings)
+ endif ()
+ 
+ # We need the headers generated by instrinsics_gen before we can compile
+ # any source file in LLDB as the imported Clang modules might include
+ # some of these generated headers. This approach is copied from Clang's main
+ # CMakeLists.txt, so it should kept in sync the code in Clang which was added
+ # in llvm-svn 308844.
+ if(LLVM_ENABLE_MODULES)
+   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
+ endif()
+ 
+ if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
+   set(LLVM_USE_HOST_TOOLS ON)
+   include(CrossCompile)
+   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)
+ message(FATAL_ERROR
+   "Crosscompiling standalone requires the variables NATIVE_{CLANG,LLVM}_DIR
+   for building the native lldb-tblgen used during the build process.")
+   endif()
+   llvm_create_cross_target(lldb NATIVE "" Release
+ -DLLVM_DIR=${NATIVE_LLVM_DIR}
+ -DClang_DIR=${NATIVE_Clang_DIR})
+ endif()
+ 
+ # TableGen
+ add_subdirectory(utils/TableGen)
+ 
+ add_subdirectory(source)
+ add_subdirectory(tools)
+ add_subdirectory(docs)
+ 
+ if (LLDB_ENABLE_PYTHON)
+   if(LLDB_BUILD_FRAMEWORK)
+ set(lldb_python_target_dir "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Resources/Python/lldb")
+   else()
+ set(lldb_python_target_dir "${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_PYTHON_RELATIVE_PATH}/lldb")
+   endif()
+   get_target_property(lldb_python_bindings_dir swig_wrapper_python BINARY_DIR)
+   finish_swig_python("lldb-python" "${lldb_python_bindings_dir}" "${lldb_python_target_dir}")
+ endif()
+ 
+ option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." ${LLVM_INCLUDE_TESTS})
+ if(LLDB_INCLUDE_TESTS)
+   add_subdirectory(test)
+   add_subdirectory(unittests)
+   add_subdirectory(utils)
+ endif()
+ 
+ if(LLDB_BUILT_STANDALONE AND NOT LLVM_ENABLE_IDE)
+   llvm_distribution_add_targets()
+ endif()
+diff --git a/lldb/bindings/CMakeLists.txt b/lldb/bindings/CMakeLists.txt
+index 9759b069fdc4..780413104d7f 100644
+--- a/lldb/bindings/CMakeLists.txt

[Lldb-commits] [PATCH] D111408: [lldb] [DynamicRegisterInfo] Remove non-const GetRegisterInfoAtIndex()

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste.
Herald added a subscriber: arphaman.
mgorny requested review of this revision.

https://reviews.llvm.org/D111408

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -691,12 +691,6 @@
   return nullptr;
 }
 
-RegisterInfo *DynamicRegisterInfo::GetRegisterInfoAtIndex(uint32_t i) {
-  if (i < m_regs.size())
-return _regs[i];
-  return nullptr;
-}
-
 const RegisterInfo *DynamicRegisterInfo::GetRegisterInfo(uint32_t kind,
  uint32_t num) const {
   uint32_t reg_index = ConvertRegisterKindToRegisterNumber(kind, num);
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -55,8 +55,6 @@
 
   const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(uint32_t i) const;
 
-  lldb_private::RegisterInfo *GetRegisterInfoAtIndex(uint32_t i);
-
   const lldb_private::RegisterSet *GetRegisterSet(uint32_t i) const;
 
   uint32_t GetRegisterSetIndexByName(lldb_private::ConstString _name,


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -691,12 +691,6 @@
   return nullptr;
 }
 
-RegisterInfo *DynamicRegisterInfo::GetRegisterInfoAtIndex(uint32_t i) {
-  if (i < m_regs.size())
-return _regs[i];
-  return nullptr;
-}
-
 const RegisterInfo *DynamicRegisterInfo::GetRegisterInfo(uint32_t kind,
  uint32_t num) const {
   uint32_t reg_index = ConvertRegisterKindToRegisterNumber(kind, num);
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -55,8 +55,6 @@
 
   const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(uint32_t i) const;
 
-  lldb_private::RegisterInfo *GetRegisterInfoAtIndex(uint32_t i);
-
   const lldb_private::RegisterSet *GetRegisterSet(uint32_t i) const;
 
   uint32_t GetRegisterSetIndexByName(lldb_private::ConstString _name,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 815c87f - [lldb/test] Disable TestScriptedProcess.py unless Darwin

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-08T15:19:54+02:00
New Revision: 815c87fbe20a66d28e4d09af89a39cb39270dcf0

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

LOG: [lldb/test] Disable TestScriptedProcess.py unless Darwin

This patch disables TestScriptedProcess.py on Linux and Windows while I
investigate the OS specific failure:

http://lab.llvm.org:8011/#/builders/68/builds/19793

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py 
b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
index b595d3dba32e..2341aa27e66b 100644
--- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -88,6 +88,7 @@ def test_scripted_process_and_scripted_thread(self):
 for idx, reg in enumerate(registers, start=1):
 self.assertEqual(idx, int(reg.value, 16))
 
+@skipUnlessDarwin
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""



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

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa758c9f7204c: [lldb/Plugins] Add memory region support in 
ScriptedProcess (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D108953?vs=377724=378189#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/bindings/interface/SBMemoryRegionInfoList.i
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/API/SBMemoryRegionInfoList.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -166,6 +166,10 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(void *data) {
+  return nullptr;
+}
+
 extern lldb::ValueObjectSP
 LLDBSWIGPython_GetValueObjectSPFromSBValue(void *data) {
   return nullptr;
Index: lldb/test/API/functionalities/scripted_process/main.c
===
--- lldb/test/API/functionalities/scripted_process/main.c
+++ lldb/test/API/functionalities/scripted_process/main.c
@@ -1,5 +1,8 @@
-#include 
-
-int main() {
-  return 0; // break here
+int bar(int i) {
+  int j = i * i;
+  return j; // break here
 }
+
+int foo(int i) { return bar(i); }
+
+int main() { return foo(42); }
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -1,4 +1,4 @@
-import os,struct,signal
+import os,struct, signal
 
 from typing import Any, Dict
 
@@ -6,12 +6,12 @@
 from lldb.plugins.scripted_process import ScriptedProcess
 from lldb.plugins.scripted_process import ScriptedThread
 
-class MyScriptedProcess(ScriptedProcess):
+class DummyScriptedProcess(ScriptedProcess):
 def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
 super().__init__(target, args)
 
 def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
-return self.memory_regions[0]
+return None
 
 def get_thread_with_id(self, tid: int):
 return {}
@@ -39,10 +39,10 @@
 return True
 
 def get_scripted_thread_plugin(self):
-return MyScriptedThread.__module__ + "." + MyScriptedThread.__name__
+return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
 
 
-class MyScriptedThread(ScriptedThread):
+class DummyScriptedThread(ScriptedThread):
 def __init__(self, target):
 super().__init__(target)
 
@@ -50,7 +50,10 @@
 return 0x19
 
 def get_name(self) -> str:
-return MyScriptedThread.__name__ + ".thread-1"
+return DummyScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateStopped
 
 def get_stop_reason(self) -> Dict[str, Any]:
 return { "type": lldb.eStopReasonSignal, "data": {
@@ -81,7 +84,7 @@
 if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
 debugger.HandleCommand(
 "process launch -C %s.%s" % (__name__,
- MyScriptedProcess.__name__))
+ DummyScriptedProcess.__name__))
 else:
 print("Name of the class that will manage the scripted process: '%s.%s'"
-% (__name__, MyScriptedProcess.__name__))
\ 

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG59d8dd79e1f9: [lldb/Plugins] Add support for ScriptedThread 
in ScriptedProcess (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D107585?vs=377324=378188#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -223,6 +223,12 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSwigPythonCreateScriptedThread(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::TargetSP _sp, std::string _string) {
+  return nullptr;
+}
+
 extern "C" void *
 LLDBSWIGPython_CreateFrameRecognizer(const char *python_class_name,
  const char *session_dictionary_name) {
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -72,6 +72,28 @@
 self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
 self.assertEqual(hello_world, memory_read)
 
+self.assertEqual(process.GetNumThreads(), 1)
+
+thread = process.GetSelectedThread()
+self.assertTrue(thread, "Invalid thread.")
+self.assertEqual(thread.GetThreadID(), 0x19)
+self.assertEqual(thread.GetName(), "MyScriptedThread.thread-1")
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonSignal)
+
+self.assertGreater(thread.GetNumFrames(), 0)
+
+frame = thread.GetFrameAtIndex(0)
+register_set = frame.registers # Returns an SBValueList.
+for regs in register_set:
+if 'GPR' in regs.name:
+registers  = regs
+break
+
+self.assertTrue(registers, "Invalid General Purpose Registers Set")
+self.assertEqual(registers.GetNumChildren(), 21)
+for idx, reg in enumerate(registers, start=1):
+self.assertEqual(idx, int(reg.value, 16))
+
 def test_launch_scripted_process_cli(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -0,0 +1,48 @@
+//===-- ScriptedThreadPythonInterface.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedProcessInterface.h"
+

[Lldb-commits] [lldb] a758c9f - [lldb/Plugins] Add memory region support in ScriptedProcess

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-08T14:54:07+02:00
New Revision: a758c9f7204c41d8791e76d24f9bc4791534f1b8

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

LOG: [lldb/Plugins] Add memory region support in ScriptedProcess

This patch adds support for memory regions in Scripted Processes.
This is necessary to read the stack memory region in order to
reconstruct each stackframe of the program.

In order to do so, this patch makes some changes to the SBAPI, namely:
- Add a new constructor for `SBMemoryRegionInfo` that takes arguments
  such as the memory region name, address range, permissions ...
  This is used when reading memory at some address to compute the offset
  in the binary blob provided by the user.
- Add a `GetMemoryRegionContainingAddress` method to `SBMemoryRegionInfoList`
  to simplify the access to a specific memory region.

With these changes, lldb is now able to unwind the stack and reconstruct
each frame. On top of that, reloading the target module at offset 0 allows
lldb to symbolicate the `ScriptedProcess` using debug info, similarly to an
ordinary Process.

To test this, I wrote a simple program with multiple function calls, ran it in
lldb, stopped at a leaf function and read the registers values and copied
the stack memory into a binary file. These are then used in the python script.

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/examples/python/scripted_process/main.stack-dump
lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

Modified: 
lldb/bindings/interface/SBMemoryRegionInfo.i
lldb/bindings/interface/SBMemoryRegionInfoList.i
lldb/bindings/python/python-wrapper.swig
lldb/examples/python/scripted_process/my_scripted_process.py
lldb/examples/python/scripted_process/scripted_process.py
lldb/include/lldb/API/SBMemoryRegionInfo.h
lldb/include/lldb/API/SBMemoryRegionInfoList.h
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
lldb/source/API/SBMemoryRegionInfo.cpp
lldb/source/API/SBMemoryRegionInfoList.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
lldb/test/API/functionalities/scripted_process/main.c
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBMemoryRegionInfo.i 
b/lldb/bindings/interface/SBMemoryRegionInfo.i
index 3460dc0d06e22..0316c5a5be1d8 100644
--- a/lldb/bindings/interface/SBMemoryRegionInfo.i
+++ b/lldb/bindings/interface/SBMemoryRegionInfo.i
@@ -20,6 +20,9 @@ public:
 
 SBMemoryRegionInfo (const lldb::SBMemoryRegionInfo );
 
+SBMemoryRegionInfo::SBMemoryRegionInfo(const char *name, lldb::addr_t 
begin,
+lldb::addr_t end, uint32_t permissions, bool mapped, bool stack_memory);
+
 ~SBMemoryRegionInfo ();
 
 void

diff  --git a/lldb/bindings/interface/SBMemoryRegionInfoList.i 
b/lldb/bindings/interface/SBMemoryRegionInfoList.i
index c2e74f1cd0dcd..0097512775421 100644
--- a/lldb/bindings/interface/SBMemoryRegionInfoList.i
+++ b/lldb/bindings/interface/SBMemoryRegionInfoList.i
@@ -24,6 +24,9 @@ public:
 uint32_t
 GetSize () const;
 
+bool
+GetMemoryRegionContainingAddress (lldb::addr_t addr, SBMemoryRegionInfo 
_info);
+
 bool
 GetMemoryRegionAtIndex (uint32_t idx, SBMemoryRegionInfo _info);
 

diff  --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index feb4e54240bc1..5a950e259e993 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -974,6 +974,22 @@ LLDBSWIGPython_CastPyObjectToSBValue
 return sb_ptr;
 }
 
+SWIGEXPORT void*
+LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo
+(
+PyObject* data
+)
+{
+lldb::SBMemoryRegionInfo* sb_ptr = NULL;
+
+int valid_cast = SWIG_ConvertPtr(data, (void**)_ptr, 
SWIGTYPE_p_lldb__SBMemoryRegionInfo, 0);
+
+if (valid_cast == -1)
+return NULL;
+
+return sb_ptr;
+}
+
 SWIGEXPORT bool
 LLDBSwigPythonCallCommand
 (

diff  --git a/lldb/examples/python/scripted_process/main.stack-dump 
b/lldb/examples/python/scripted_process/main.stack-dump
new file mode 100644
index 

[Lldb-commits] [lldb] 59d8dd7 - [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-10-08T14:54:07+02:00
New Revision: 59d8dd79e1f9dead2dc2756e139073083e487228

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

LOG: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

This patch introduces the `ScriptedThread` class with its python
interface.

When used with `ScriptedProcess`, `ScriptedThreaad` can provide various
information such as the thread state, stop reason or even its register
context.

This can be used to reconstruct the program stack frames using lldb's unwinder.

rdar://74503836

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.h

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h

Modified: 
lldb/bindings/python/python-wrapper.swig
lldb/examples/python/scripted_process/my_scripted_process.py
lldb/examples/python/scripted_process/scripted_process.py
lldb/include/lldb/Interpreter/ScriptedInterface.h
lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
lldb/include/lldb/lldb-forward.h
lldb/source/Plugins/Process/scripted/CMakeLists.txt
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index a148ec3544ab8..feb4e54240bc1 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -340,6 +340,63 @@ LLDBSwigPythonCreateScriptedProcess
 Py_RETURN_NONE;
 }
 
+SWIGEXPORT void*
+LLDBSwigPythonCreateScriptedThread
+(
+const char *python_class_name,
+const char *session_dictionary_name,
+const lldb::TargetSP& target_sp,
+std::string _string
+)
+{
+if (python_class_name == NULL || python_class_name[0] == '\0' || 
!session_dictionary_name)
+Py_RETURN_NONE;
+
+PyErr_Cleaner py_err_cleaner(true);
+
+auto dict = 
PythonModule::MainModule().ResolveName(session_dictionary_name);
+auto pfunc = 
PythonObject::ResolveNameWithDictionary(python_class_name, 
dict);
+
+if (!pfunc.IsAllocated()) {
+error_string.append("could not find script class: ");
+error_string.append(python_class_name);
+return nullptr;
+}
+
+// I do not want the SBTarget to be deallocated when going out of scope
+// because python has ownership of it and will manage memory for this
+// object by itself
+PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new 
lldb::SBTarget(target_sp)));
+
+if (!target_arg.IsAllocated())
+Py_RETURN_NONE;
+
+llvm::Expected arg_info = pfunc.GetArgInfo();
+if (!arg_info) {
+llvm::handleAllErrors(
+arg_info.takeError(),
+[&](PythonException ) {
+error_string.append(E.ReadBacktrace());
+},
+[&](const llvm::ErrorInfoBase ) {
+error_string.append(E.message());
+});
+Py_RETURN_NONE;
+}
+
+PythonObject result = {};
+if (arg_info.get().max_positional_args == 1) {
+result = pfunc(target_arg);
+} else {
+error_string.assign("wrong number of arguments in __init__, should be 
2 or 3 (not including self)");
+Py_RETURN_NONE;
+}
+
+if (result.IsAllocated())
+return result.release();
+Py_RETURN_NONE;
+}
+
 SWIGEXPORT void*
 LLDBSwigPythonCreateScriptedThreadPlan
 (

diff  --git a/lldb/examples/python/scripted_process/my_scripted_process.py 
b/lldb/examples/python/scripted_process/my_scripted_process.py
index 429531e80e1e0..9257cd9c85126 100644
--- a/lldb/examples/python/scripted_process/my_scripted_process.py
+++ b/lldb/examples/python/scripted_process/my_scripted_process.py
@@ -1,7 +1,10 @@
-import os
+import os,struct,signal
+
+from typing import Any, Dict
 
 import lldb
 from lldb.plugins.scripted_process import ScriptedProcess
+from 

[Lldb-commits] [PATCH] D111402: [lldb] [test] Use secondary pty end for testing Terminal

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71fd5a735ec2: [lldb] [test] Use secondary pty end for 
testing Terminal (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111402

Files:
  lldb/unittests/Host/posix/TerminalTest.cpp

Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -21,16 +21,22 @@
 class TerminalTest : public ::testing::Test {
 protected:
   PseudoTerminal m_pty;
+  int m_fd;
+  Terminal m_term;
 
   void SetUp() override {
-EXPECT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
+ASSERT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
   llvm::Succeeded());
+ASSERT_THAT_ERROR(m_pty.OpenSecondary(O_RDWR | O_NOCTTY),
+  llvm::Succeeded());
+m_fd = m_pty.GetSecondaryFileDescriptor();
+ASSERT_NE(m_fd, -1);
+m_term.SetFileDescriptor(m_fd);
   }
 };
 
 TEST_F(TerminalTest, PtyIsATerminal) {
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  EXPECT_EQ(term.IsATerminal(), true);
+  EXPECT_EQ(m_term.IsATerminal(), true);
 }
 
 TEST_F(TerminalTest, PipeIsNotATerminal) {
@@ -44,39 +50,35 @@
 
 TEST_F(TerminalTest, SetEcho) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetEcho(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
 
-  ASSERT_EQ(term.SetEcho(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
 }
 
 TEST_F(TerminalTest, SetCanonical) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetCanonical(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
 
-  ASSERT_EQ(term.SetCanonical(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
 
 TEST_F(TerminalTest, SaveRestoreRAII) {
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
-
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
   {
-TerminalState term_state{term};
+TerminalState term_state{m_term};
 terminfo = orig_terminfo;
 
 // make an arbitrary change
@@ -85,11 +87,11 @@
 cfsetospeed(,
 cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ),
+ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ),
   0);
   }
 
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }
 
@@ -98,19 +100,18 @@
 
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  term_state.Save(term, false);
+  term_state.Save(m_term, false);
   terminfo = orig_terminfo;
 
   // make an arbitrary change
   cfsetispeed(, cfgetispeed(_terminfo) == B9600 ? B4800 : B9600);
   cfsetospeed(, cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-  ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ), 0);
+  ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ), 0);
 
   term_state.Restore();
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 71fd5a7 - [lldb] [test] Use secondary pty end for testing Terminal

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-08T14:42:47+02:00
New Revision: 71fd5a735ec2f30105249dc367754a604ad00308

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

LOG: [lldb] [test] Use secondary pty end for testing Terminal

Open and use the secondary end of a pty for testing Terminal properties
in order to fix the tests on Darwin.  While at it, streamline getting
the fd and Terminal class.

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

Added: 


Modified: 
lldb/unittests/Host/posix/TerminalTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/posix/TerminalTest.cpp 
b/lldb/unittests/Host/posix/TerminalTest.cpp
index 788c2a6ea579e..dc5c6e9d0f37e 100644
--- a/lldb/unittests/Host/posix/TerminalTest.cpp
+++ b/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -21,16 +21,22 @@ using namespace lldb_private;
 class TerminalTest : public ::testing::Test {
 protected:
   PseudoTerminal m_pty;
+  int m_fd;
+  Terminal m_term;
 
   void SetUp() override {
-EXPECT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
+ASSERT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
   llvm::Succeeded());
+ASSERT_THAT_ERROR(m_pty.OpenSecondary(O_RDWR | O_NOCTTY),
+  llvm::Succeeded());
+m_fd = m_pty.GetSecondaryFileDescriptor();
+ASSERT_NE(m_fd, -1);
+m_term.SetFileDescriptor(m_fd);
   }
 };
 
 TEST_F(TerminalTest, PtyIsATerminal) {
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  EXPECT_EQ(term.IsATerminal(), true);
+  EXPECT_EQ(m_term.IsATerminal(), true);
 }
 
 TEST_F(TerminalTest, PipeIsNotATerminal) {
@@ -44,39 +50,35 @@ TEST_F(TerminalTest, PipeIsNotATerminal) {
 
 TEST_F(TerminalTest, SetEcho) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetEcho(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
 
-  ASSERT_EQ(term.SetEcho(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
 }
 
 TEST_F(TerminalTest, SetCanonical) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetCanonical(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
 
-  ASSERT_EQ(term.SetCanonical(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
 
 TEST_F(TerminalTest, SaveRestoreRAII) {
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
-
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
   {
-TerminalState term_state{term};
+TerminalState term_state{m_term};
 terminfo = orig_terminfo;
 
 // make an arbitrary change
@@ -85,11 +87,11 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
 cfsetospeed(,
 cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ),
+ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ),
   0);
   }
 
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }
 
@@ -98,19 +100,18 @@ TEST_F(TerminalTest, SaveRestore) {
 
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  term_state.Save(term, false);
+  term_state.Save(m_term, false);
   terminfo = orig_terminfo;
 
   // make an arbitrary change
   cfsetispeed(, cfgetispeed(_terminfo) == B9600 ? B4800 : B9600);
   cfsetospeed(, cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-  ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ), 
0);
+  ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ), 0);
 
   term_state.Restore();
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }



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

[Lldb-commits] [PATCH] D111402: [lldb] [test] Use secondary pty end for testing Terminal

[Raphael Isemann via Phabricator via lldb-commits]

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

If Green Dragon was still with us this would make it happy (and green), so LGTM.


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

https://reviews.llvm.org/D111402

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

[Lldb-commits] [PATCH] D111402: [lldb] [test] Use secondary pty end for testing Terminal

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: teemperor, labath, krytarowski, emaste.
mgorny requested review of this revision.

Open and use the secondary end of a pty for testing Terminal properties
in order to fix the tests on Darwin.  While at it, streamline getting
the fd and Terminal class.


https://reviews.llvm.org/D111402

Files:
  lldb/unittests/Host/posix/TerminalTest.cpp

Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -21,16 +21,22 @@
 class TerminalTest : public ::testing::Test {
 protected:
   PseudoTerminal m_pty;
+  int m_fd;
+  Terminal m_term;
 
   void SetUp() override {
-EXPECT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
+ASSERT_THAT_ERROR(m_pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY),
   llvm::Succeeded());
+ASSERT_THAT_ERROR(m_pty.OpenSecondary(O_RDWR | O_NOCTTY),
+  llvm::Succeeded());
+m_fd = m_pty.GetSecondaryFileDescriptor();
+ASSERT_NE(m_fd, -1);
+m_term.SetFileDescriptor(m_fd);
   }
 };
 
 TEST_F(TerminalTest, PtyIsATerminal) {
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  EXPECT_EQ(term.IsATerminal(), true);
+  EXPECT_EQ(m_term.IsATerminal(), true);
 }
 
 TEST_F(TerminalTest, PipeIsNotATerminal) {
@@ -44,39 +50,35 @@
 
 TEST_F(TerminalTest, SetEcho) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetEcho(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
 
-  ASSERT_EQ(term.SetEcho(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetEcho(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
 }
 
 TEST_F(TerminalTest, SetCanonical) {
   struct termios terminfo;
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
 
-  ASSERT_EQ(term.SetCanonical(true), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(true), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
 
-  ASSERT_EQ(term.SetCanonical(false), true);
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(m_term.SetCanonical(false), true);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
 
 TEST_F(TerminalTest, SaveRestoreRAII) {
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
-
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
   {
-TerminalState term_state{term};
+TerminalState term_state{m_term};
 terminfo = orig_terminfo;
 
 // make an arbitrary change
@@ -85,11 +87,11 @@
 cfsetospeed(,
 cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ),
+ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ),
   0);
   }
 
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }
 
@@ -98,19 +100,18 @@
 
   struct termios orig_terminfo;
   struct termios terminfo;
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), _terminfo), 0);
+  ASSERT_EQ(tcgetattr(m_fd, _terminfo), 0);
 
-  Terminal term{m_pty.GetPrimaryFileDescriptor()};
-  term_state.Save(term, false);
+  term_state.Save(m_term, false);
   terminfo = orig_terminfo;
 
   // make an arbitrary change
   cfsetispeed(, cfgetispeed(_terminfo) == B9600 ? B4800 : B9600);
   cfsetospeed(, cfgetospeed(_terminfo) == B9600 ? B4800 : B9600);
 
-  ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, ), 0);
+  ASSERT_EQ(tcsetattr(m_fd, TCSANOW, ), 0);
 
   term_state.Restore();
-  ASSERT_EQ(tcgetattr(m_pty.GetPrimaryFileDescriptor(), ), 0);
+  ASSERT_EQ(tcgetattr(m_fd, ), 0);
   ASSERT_EQ(memcmp(, _terminfo, sizeof(terminfo)), 0);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:42-43
+//% self.expect_var_path("a.data", summary='"FOOB"')
+// Should this be "FO\0B" instead?
+//% self.expect_var_path("b.data", summary='"FO"')
 }

This would be consistent with gdb as well, and it was the preferred result of 
last years irc straw poll, but it will require a different fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111399

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

[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: teemperor, jingham.
labath requested review of this revision.
Herald added a project: LLDB.

I believe this is a more natural behavior, and it also matches what gdb
does.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111399

Files:
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp


Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
 #include 
+#include 
+
+struct A {
+  char data[4];
+  char overflow[4];
+};
 
 int main (int argc, char const *argv[])
 {
+A a, b;
+// Deliberately write past the end of data to test that the formatter stops
+// at the end of array.
+memcpy(a.data, "FOOBAR", 7);
+memcpy(b.data, "FO\0BAR", 7);
 std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); 
//%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables 
true"))
 const char* constcharstar = stdstring.c_str();
 std::string longstring(
@@ -20,12 +31,15 @@
   );
 const char* longconstcharstar = longstring.c_str();
 return 0; //% if self.TraceOn(): self.runCmd('frame variable')
-  //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary() 
== '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% 
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() == 
'"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+  //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am 
here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI 
am here\\t\\tto say hello\\n"')
  //% self.runCmd("setting set escape-non-printables false")
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary() 
== '"Hello\t\tWorld\nI am here\t\tto say hello\n"')
- //% 
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() == 
'"Hello\t\tWorld\nI am here\t\tto say hello\n"')
+ //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
+ //% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
 //% 
self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
 //% 
self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
+//% self.expect_var_path("a.data", summary='"FOOB"')
+// Should this be "FO\0B" instead?
+//% self.expect_var_path("b.data", summary='"FO"')
 }
 
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -722,7 +722,7 @@
   new StringSummaryFormat(string_flags, "${var%s}"));
 
   lldb::TypeSummaryImplSP string_array_format(
-  new StringSummaryFormat(string_array_flags, "${var%s}"));
+  new StringSummaryFormat(string_array_flags, "${var%char[]}"));
 
   RegularExpression any_size_char_arr(llvm::StringRef("char \\[[0-9]+\\]"));
 


Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
 #include 
+#include 
+
+struct A {
+  char data[4];
+  char overflow[4];
+};
 
 int main (int argc, char const *argv[])
 {
+A a, b;
+// Deliberately write past the end of data to test that the formatter stops
+// at the end of array.
+memcpy(a.data, "FOOBAR", 7);
+memcpy(b.data, "FO\0BAR", 7);
 std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); //%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables true"))
 const char* constcharstar = stdstring.c_str();
 std::string longstring(
@@ -20,12 +31,15 @@
   );
 const char* longconstcharstar = longstring.c_str();
 return 0; //% if self.TraceOn(): self.runCmd('frame variable')
-  //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+  //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
  //% self.runCmd("setting set escape-non-printables false")

[Lldb-commits] [PATCH] D111396: [lldb] [ConnectionFileDescriptorPosix] Combine m_read_sp & m_write_sp

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste.
mgorny requested review of this revision.

Combine m_read_sp and m_write_sp into a single m_io_sp.  In all
currently existing code paths, they are pointing to the same object
anyway.


https://reviews.llvm.org/D111396

Files:
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -63,9 +63,8 @@
 ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
-  m_write_sp =
+  m_io_sp =
   std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
-  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -120,8 +119,7 @@
 }
 
 bool ConnectionFileDescriptor::IsConnected() const {
-  return (m_read_sp && m_read_sp->IsValid()) ||
- (m_write_sp && m_write_sp->IsValid());
+  return m_io_sp && m_io_sp->IsValid();
 }
 
 ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
@@ -191,9 +189,8 @@
 return eConnectionStatusSuccess;
   }
 
-  if (m_read_sp && m_read_sp->IsValid() &&
-  m_read_sp->GetFdType() == IOObject::eFDTypeSocket)
-static_cast(*m_read_sp).PreDisconnect();
+  if (m_io_sp->GetFdType() == IOObject::eFDTypeSocket)
+static_cast(*m_io_sp).PreDisconnect();
 
   // Try to get the ConnectionFileDescriptor's mutex.  If we fail, that is
   // quite likely because somebody is doing a blocking read on our file
@@ -222,12 +219,11 @@
   // Prevents reads and writes during shutdown.
   m_shutting_down = true;
 
-  Status error = m_read_sp->Close();
-  Status error2 = m_write_sp->Close();
-  if (error.Fail() || error2.Fail())
+  Status error = m_io_sp->Close();
+  if (error.Fail())
 status = eConnectionStatusError;
   if (error_ptr)
-*error_ptr = error.Fail() ? error : error2;
+*error_ptr = error;
 
   // Close any pipes we were using for async interrupts
   m_pipe.Close();
@@ -269,14 +265,14 @@
 
   Status error;
   size_t bytes_read = dst_len;
-  error = m_read_sp->Read(dst, bytes_read);
+  error = m_io_sp->Read(dst, bytes_read);
 
   if (log) {
 LLDB_LOGF(log,
   "%p ConnectionFileDescriptor::Read()  fd = %" PRIu64
   ", dst = %p, dst_len = %" PRIu64 ") => %" PRIu64 ", error = %s",
   static_cast(this),
-  static_cast(m_read_sp->GetWaitableHandle()),
+  static_cast(m_io_sp->GetWaitableHandle()),
   static_cast(dst), static_cast(dst_len),
   static_cast(bytes_read), error.AsCString());
   }
@@ -295,7 +291,7 @@
 switch (error_value) {
 case EAGAIN: // The file was marked for non-blocking I/O, and no data were
  // ready to be read.
-  if (m_read_sp->GetFdType() == IOObject::eFDTypeSocket)
+  if (m_io_sp->GetFdType() == IOObject::eFDTypeSocket)
 status = eConnectionStatusTimedOut;
   else
 status = eConnectionStatusSuccess;
@@ -373,14 +369,14 @@
   Status error;
 
   size_t bytes_sent = src_len;
-  error = m_write_sp->Write(src, bytes_sent);
+  error = m_io_sp->Write(src, bytes_sent);
 
   if (log) {
 LLDB_LOGF(log,
   "%p ConnectionFileDescriptor::Write(fd = %" PRIu64
   ", src = %p, src_len = %" PRIu64 ") => %" PRIu64 " (error = %s)",
   static_cast(this),
-  static_cast(m_write_sp->GetWaitableHandle()),
+  static_cast(m_io_sp->GetWaitableHandle()),
   static_cast(src), static_cast(src_len),
   static_cast(bytes_sent), error.AsCString());
   }
@@ -443,7 +439,7 @@
   // Make a copy of the file descriptors to make sure we don't have another
   // thread change these values out from under us and cause problems in the
   // loop below where like in FS_SET()
-  const IOObject::WaitableHandle handle = m_read_sp->GetWaitableHandle();
+  const IOObject::WaitableHandle handle = m_io_sp->GetWaitableHandle();
   const int pipe_fd = m_pipe.GetReadFileDescriptor();
 
   if (handle != IOObject::kInvalidHandleValue) {
@@ -464,7 +460,7 @@
 if (have_pipe_fd)
   select_helper.FDSetRead(pipe_fd);
 
-while (handle == m_read_sp->GetWaitableHandle()) {
+while (handle == m_io_sp->GetWaitableHandle()) {
 
   Status error = select_helper.Select();
 
@@ -533,11 +529,9 @@
   Socket::UnixDomainAccept(socket_name, m_child_processes_inherit, socket);
   if (error_ptr)
 *error_ptr = error;
-  m_write_sp.reset(socket);
-  m_read_sp = m_write_sp;
-  if 

[Lldb-commits] [PATCH] D111321: [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching

[Michał Górny via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b46a4134385: [lldb] [ConnectionFileDescriptorPosix] 
Refactor scheme matching (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D111321?vs=377948=378148#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111321

Files:
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp

Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -120,9 +120,9 @@
 m_device_id = std::string(host);
 
   m_socket_namespace.reset();
-  if (scheme == ConnectionFileDescriptor::UNIX_CONNECT_SCHEME)
+  if (scheme == "unix-connect")
 m_socket_namespace = AdbClient::UnixSocketNamespaceFileSystem;
-  else if (scheme == ConnectionFileDescriptor::UNIX_ABSTRACT_CONNECT_SCHEME)
+  else if (scheme == "unix-abstract-connect")
 m_socket_namespace = AdbClient::UnixSocketNamespaceAbstract;
 
   std::string connect_url;
Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -50,27 +50,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-const char *ConnectionFileDescriptor::LISTEN_SCHEME = "listen";
-const char *ConnectionFileDescriptor::ACCEPT_SCHEME = "accept";
-const char *ConnectionFileDescriptor::UNIX_ACCEPT_SCHEME = "unix-accept";
-const char *ConnectionFileDescriptor::CONNECT_SCHEME = "connect";
-const char *ConnectionFileDescriptor::TCP_CONNECT_SCHEME = "tcp-connect";
-const char *ConnectionFileDescriptor::UDP_SCHEME = "udp";
-const char *ConnectionFileDescriptor::UNIX_CONNECT_SCHEME = "unix-connect";
-const char *ConnectionFileDescriptor::UNIX_ABSTRACT_CONNECT_SCHEME =
-"unix-abstract-connect";
-const char *ConnectionFileDescriptor::FD_SCHEME = "fd";
-const char *ConnectionFileDescriptor::FILE_SCHEME = "file";
-
-static llvm::Optional GetURLAddress(llvm::StringRef url,
- llvm::StringRef scheme) {
-  if (!url.consume_front(scheme))
-return llvm::None;
-  if (!url.consume_front("://"))
-return llvm::None;
-  return url;
-}
-
 ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit)
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
 
@@ -154,133 +133,41 @@
 
   OpenCommandPipe();
 
-  if (!path.empty()) {
-llvm::Optional addr;
-if ((addr = GetURLAddress(path, LISTEN_SCHEME))) {
-  // listen://HOST:PORT
-  return SocketListenAndAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, ACCEPT_SCHEME))) {
-  // unix://SOCKNAME
-  return NamedSocketAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UNIX_ACCEPT_SCHEME))) {
-  // unix://SOCKNAME
-  return NamedSocketAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, CONNECT_SCHEME))) {
-  return ConnectTCP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, TCP_CONNECT_SCHEME))) {
-  return ConnectTCP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UDP_SCHEME))) {
-  return ConnectUDP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UNIX_CONNECT_SCHEME))) {
-  // unix-connect://SOCKNAME
-  return NamedSocketConnect(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UNIX_ABSTRACT_CONNECT_SCHEME))) {
-  // unix-abstract-connect://SOCKNAME
-  return UnixAbstractSocketConnect(*addr, error_ptr);
-}
-#if LLDB_ENABLE_POSIX
-else if ((addr = GetURLAddress(path, FD_SCHEME))) {
-  // Just passing a native file descriptor within this current process that
-  // is already opened (possibly from a service or other source).
-  int fd = -1;
-
-  if (!addr->getAsInteger(0, fd)) {
-// We have what looks to be a valid file descriptor, but we should make
-// sure it is. We currently are doing this by trying to get the flags
-// from the file descriptor and making sure it isn't a bad fd.
-errno = 0;
-int flags = ::fcntl(fd, F_GETFL, 0);
-if (flags == -1 || errno == EBADF) {
-  if (error_ptr)
-error_ptr->SetErrorStringWithFormat("stale file descriptor: %s",
-path.str().c_str());
-  m_read_sp.reset();
-  

[Lldb-commits] [lldb] 4b46a41 - [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-08T11:47:57+02:00
New Revision: 4b46a4134385eda6449e93ad124f9b2685b25bb2

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

LOG: [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching

Move the POSIX-specific fd:// and file:// scheme handling into
separate methods.  Replace the custom GetURLAddress() matching with
splitting into scheme and path, and matching scheme via
llvm::StringSwitch.  Use early returns.

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

Added: 


Modified: 
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 42be989dfa7b6..57bb01d42f232 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -28,17 +28,6 @@ class SocketAddress;
 
 class ConnectionFileDescriptor : public Connection {
 public:
-  static const char *LISTEN_SCHEME;
-  static const char *ACCEPT_SCHEME;
-  static const char *UNIX_ACCEPT_SCHEME;
-  static const char *CONNECT_SCHEME;
-  static const char *TCP_CONNECT_SCHEME;
-  static const char *UDP_SCHEME;
-  static const char *UNIX_CONNECT_SCHEME;
-  static const char *UNIX_ABSTRACT_CONNECT_SCHEME;
-  static const char *FD_SCHEME;
-  static const char *FILE_SCHEME;
-
   ConnectionFileDescriptor(bool child_processes_inherit = false);
 
   ConnectionFileDescriptor(int fd, bool owns_fd);
@@ -95,6 +84,10 @@ class ConnectionFileDescriptor : public Connection {
   lldb::ConnectionStatus UnixAbstractSocketConnect(llvm::StringRef socket_name,
Status *error_ptr);
 
+  lldb::ConnectionStatus ConnectFD(llvm::StringRef args, Status *error_ptr);
+
+  lldb::ConnectionStatus ConnectFile(llvm::StringRef args, Status *error_ptr);
+
   lldb::IOObjectSP m_read_sp;
   lldb::IOObjectSP m_write_sp;
 

diff  --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 9a6a5aa835f55..b192c65b619d9 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -50,27 +50,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-const char *ConnectionFileDescriptor::LISTEN_SCHEME = "listen";
-const char *ConnectionFileDescriptor::ACCEPT_SCHEME = "accept";
-const char *ConnectionFileDescriptor::UNIX_ACCEPT_SCHEME = "unix-accept";
-const char *ConnectionFileDescriptor::CONNECT_SCHEME = "connect";
-const char *ConnectionFileDescriptor::TCP_CONNECT_SCHEME = "tcp-connect";
-const char *ConnectionFileDescriptor::UDP_SCHEME = "udp";
-const char *ConnectionFileDescriptor::UNIX_CONNECT_SCHEME = "unix-connect";
-const char *ConnectionFileDescriptor::UNIX_ABSTRACT_CONNECT_SCHEME =
-"unix-abstract-connect";
-const char *ConnectionFileDescriptor::FD_SCHEME = "fd";
-const char *ConnectionFileDescriptor::FILE_SCHEME = "file";
-
-static llvm::Optional GetURLAddress(llvm::StringRef url,
- llvm::StringRef scheme) {
-  if (!url.consume_front(scheme))
-return llvm::None;
-  if (!url.consume_front("://"))
-return llvm::None;
-  return url;
-}
-
 ConnectionFileDescriptor::ConnectionFileDescriptor(bool 
child_processes_inherit)
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
 
@@ -154,133 +133,41 @@ ConnectionStatus 
ConnectionFileDescriptor::Connect(llvm::StringRef path,
 
   OpenCommandPipe();
 
-  if (!path.empty()) {
-llvm::Optional addr;
-if ((addr = GetURLAddress(path, LISTEN_SCHEME))) {
-  // listen://HOST:PORT
-  return SocketListenAndAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, ACCEPT_SCHEME))) {
-  // unix://SOCKNAME
-  return NamedSocketAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UNIX_ACCEPT_SCHEME))) {
-  // unix://SOCKNAME
-  return NamedSocketAccept(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, CONNECT_SCHEME))) {
-  return ConnectTCP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, TCP_CONNECT_SCHEME))) {
-  return ConnectTCP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UDP_SCHEME))) {
-  return ConnectUDP(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, UNIX_CONNECT_SCHEME))) {
-  // unix-connect://SOCKNAME
-  return NamedSocketConnect(*addr, error_ptr);
-} else if ((addr = GetURLAddress(path, 

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdcb0e687faeb: [lldb] [ConnectionFileDescriptorPosix] Use a 
single NativeFile (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111314

Files:
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -271,9 +269,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -271,9 +269,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] dcb0e68 - [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-08T11:26:29+02:00
New Revision: dcb0e687faebb8b8c31f61a0deb8c583125ffc72

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

LOG: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

Replace separate read and write NativeFile instances with a single
instance shared for reading and writing.  There is no clear indication
why two instances were used in the first place, and replacing them
with just one does not seem to cause any regressions in tests or manual
'process connect file://...'.

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

Added: 


Modified: 
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 2d93e457a6990..9a6a5aa835f55 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, 
bool owns_fd)
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@ ConnectionStatus 
ConnectionFileDescriptor::Connect(llvm::StringRef path,
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -271,9 +269,8 @@ ConnectionStatus 
ConnectionFileDescriptor::Connect(llvm::StringRef path,
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif



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

[Lldb-commits] [lldb] f4145c0 - [lldb/gdb-remote] Refactor ReadExtFeature

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-08T10:43:37+02:00
New Revision: f4145c074cb81441f2e4a965f618f9949fa4f071

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

LOG: [lldb/gdb-remote] Refactor ReadExtFeature

replace stl and lldb apis with standard llvm ones.

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index e49bf22512cf..c68fcc1789d8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1703,17 +1703,13 @@ Status 
GDBRemoteCommunicationClient::LoadQXferMemoryMap() {
 return error;
   }
 
-  std::string xml;
-  lldb_private::Status lldberr;
-  if (!ReadExtFeature(ConstString("memory-map"), ConstString(""), xml,
-  lldberr)) {
-error.SetErrorString("Failed to read memory map");
-return error;
-  }
+  llvm::Expected xml = ReadExtFeature("memory-map", "");
+  if (!xml)
+return Status(xml.takeError());
 
   XMLDocument xml_document;
 
-  if (!xml_document.ParseMemory(xml.c_str(), xml.size())) {
+  if (!xml_document.ParseMemory(xml->c_str(), xml->size())) {
 error.SetErrorString("Failed to parse memory map xml");
 return error;
   }
@@ -3883,15 +3879,14 @@ GDBRemoteCommunicationClient::GetModulesInfo(
 
 // query the target remote for extended information using the qXfer packet
 //
-// example: object='features', annex='target.xml', out= return:
-// 'true'  on success
-//  'false' on failure (err set)
-bool GDBRemoteCommunicationClient::ReadExtFeature(
-const lldb_private::ConstString object,
-const lldb_private::ConstString annex, std::string ,
-lldb_private::Status ) {
-
-  std::stringstream output;
+// example: object='features', annex='target.xml'
+// return:  or error
+llvm::Expected
+GDBRemoteCommunicationClient::ReadExtFeature(llvm::StringRef object,
+ llvm::StringRef annex) {
+
+  std::string output;
+  llvm::raw_string_ostream output_stream(output);
   StringExtractorGDBRemote chunk;
 
   uint64_t size = GetRemoteMaxPacketSize();
@@ -3905,28 +3900,22 @@ bool GDBRemoteCommunicationClient::ReadExtFeature(
   while (active) {
 
 // send query extended feature packet
-std::stringstream packet;
-packet << "qXfer:" << object.AsCString("")
-   << ":read:" << annex.AsCString("") << ":" << std::hex << offset
-   << "," << std::hex << size;
+std::string packet =
+("qXfer:" + object + ":read:" + annex + ":" +
+ llvm::Twine::utohexstr(offset) + "," + llvm::Twine::utohexstr(size))
+.str();
 
 GDBRemoteCommunication::PacketResult res =
-SendPacketAndWaitForResponse(packet.str(), chunk);
+SendPacketAndWaitForResponse(packet, chunk);
 
-if (res != GDBRemoteCommunication::PacketResult::Success) {
-  err.SetErrorString("Error sending $qXfer packet");
-  return false;
-}
-
-const std::string  = std::string(chunk.GetStringRef());
-if (str.length() == 0) {
-  // should have some data in chunk
-  err.SetErrorString("Empty response from $qXfer packet");
-  return false;
+if (res != GDBRemoteCommunication::PacketResult::Success ||
+chunk.GetStringRef().empty()) {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Error sending $qXfer packet");
 }
 
 // check packet code
-switch (str[0]) {
+switch (chunk.GetStringRef()[0]) {
 // last chunk
 case ('l'):
   active = false;
@@ -3934,20 +3923,19 @@ bool GDBRemoteCommunicationClient::ReadExtFeature(
 
 // more chunks
 case ('m'):
-  output << str.substr(1);
-  offset += str.length() - 1;
+  output_stream << chunk.GetStringRef().drop_front();
+  offset += chunk.GetStringRef().size() - 1;
   break;
 
 // unknown chunk
 default:
-  err.SetErrorString("Invalid continuation code from $qXfer packet");
-  return false;
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Invalid continuation code from $qXfer packet");
 }
   }
 
-  out = output.str();
-  err.Success();
-  return true;
+  return output_stream.str();
 }
 
 // Notify the target that gdb is prepared to serve symbol lookup requests.

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 

[Lldb-commits] [lldb] 3d7161e - [lldb] Remove shared_ptr from some global Properties objects

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-08T10:43:37+02:00
New Revision: 3d7161e3c14c25a60a3c03aa4c07a1dc4e35511b

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

LOG: [lldb] Remove shared_ptr from some global Properties objects

they're unnecessary, make the code longer, and their removal actually
ensures proper initialization in multithreaded scenarios.

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/lldb-forward.h
lldb/source/Core/Debugger.cpp
lldb/source/Interpreter/CommandInterpreter.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 878739c46fa4a..9aed70188097f 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -55,7 +55,6 @@ class PlatformProperties : public Properties {
   void SetDefaultModuleCacheDirectory(const FileSpec _spec);
 };
 
-typedef std::shared_ptr PlatformPropertiesSP;
 typedef llvm::SmallVector MmapArgList;
 
 /// \class Platform Platform.h "lldb/Target/Platform.h"
@@ -84,7 +83,7 @@ class Platform : public PluginInterface {
 
   static void Terminate();
 
-  static const PlatformPropertiesSP ();
+  static PlatformProperties ();
 
   /// Get the native host platform plug-in.
   ///

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 741c9e1021cc6..5f4c1272ef28d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -106,8 +106,6 @@ class ProcessProperties : public Properties {
   std::unique_ptr m_experimental_properties_up;
 };
 
-typedef std::shared_ptr ProcessPropertiesSP;
-
 // ProcessAttachInfo
 //
 // Describes any information that is required to attach to a process.
@@ -501,7 +499,7 @@ class Process : public 
std::enable_shared_from_this,
 
   static void SettingsTerminate();
 
-  static const ProcessPropertiesSP ();
+  static ProcessProperties ();
 
   /// Find a Process plug-in that can debug \a module using the currently
   /// selected architecture.

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 4ddaf3fe2fca6..c8fc66db7ef4a 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -562,7 +562,7 @@ class Target : public std::enable_shared_from_this,
 
   // Settings accessors
 
-  static const lldb::TargetPropertiesSP ();
+  static TargetProperties ();
 
   std::recursive_mutex ();
 

diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 0f6b5741573ee..a48be19005153 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -57,8 +57,6 @@ class ThreadProperties : public Properties {
   uint64_t GetMaxBacktraceDepth() const;
 };
 
-typedef std::shared_ptr ThreadPropertiesSP;
-
 class Thread : public std::enable_shared_from_this,
public ThreadProperties,
public UserID,
@@ -149,7 +147,7 @@ class Thread : public std::enable_shared_from_this,
 
   static void SettingsTerminate();
 
-  static const ThreadPropertiesSP ();
+  static ThreadProperties ();
 
   lldb::ProcessSP GetProcess() const { return m_process_wp.lock(); }
 

diff  --git a/lldb/include/lldb/lldb-forward.h 
b/lldb/include/lldb/lldb-forward.h
index bb6ab1fa0a843..b68e9048ce3b4 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -399,7 +399,6 @@ typedef 
std::shared_ptr
 SyntheticChildrenFrontEndSP;
 typedef std::shared_ptr TargetSP;
 typedef std::weak_ptr TargetWP;
-typedef std::shared_ptr TargetPropertiesSP;
 typedef std::shared_ptr ThreadSP;
 typedef std::weak_ptr ThreadWP;
 typedef std::shared_ptr ThreadCollectionSP;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index eb092cc705c4b..1ddd95726f0c9 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -723,10 +723,10 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   m_collection_sp->AppendProperty(
   

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D111314#3050392 , @mgorny wrote:

> In D111314#3050383 , @labath wrote:
>
>> At one point these didn't use to be shared pointers so we couldn't copy them 
>> like this, though I am still unclear as to why we need two of them in the 
>> first place.
>
> Actually, I've asked you on IRC yesterday if you think I should replace them 
> with one field, or if we should keep the separate semantics for some possible 
> use in the future.

Yeah, and I don't really know the answer to that.

But if you make a patch to merge the two,  I'll probably approve it. :)


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

https://reviews.llvm.org/D111314

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

[Lldb-commits] [PATCH] D111321: [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching

[Pavel Labath via Phabricator via lldb-commits]

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

thanks




Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:142-143
+  auto method =
+  llvm::StringSwitch>(scheme)
+  .Case("listen", ::SocketListenAndAccept)

Maybe just drop std::function?



Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:164-171
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat("unsupported connection URL: '%s'",
   path.str().c_str());
 return eConnectionStatusError;
   }
   if (error_ptr)
 error_ptr->SetErrorString("invalid connect arguments");

Could you also please convert this to early returns, while you're inside. I 
spent 10 minutes looking at this, and I'm still not sure whether the error 
strings are correctly related to the individual checks.


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

https://reviews.llvm.org/D111321

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

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D111314#3050383 , @labath wrote:

> At one point these didn't use to be shared pointers so we couldn't copy them 
> like this, though I am still unclear as to why we need two of them in the 
> first place.

Actually, I've asked you on IRC yesterday if you think I should replace them 
with one field, or if we should keep the separate semantics for some possible 
use in the future.


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

https://reviews.llvm.org/D111314

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

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Pavel Labath via Phabricator via lldb-commits]

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

At one point these didn't use to be shared pointers so we couldn't copy them 
like this, though I am still unclear as to why we need two of them in the first 
place.


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

https://reviews.llvm.org/D111314

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

[Lldb-commits] [PATCH] D111387: [NFC] [Clang] Remove pre-computed complex float types

[Qiu Chaofan via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00c0ce0655da: [NFC] [Clang] Remove pre-computed complex 
float types (authored by qiucf).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111387

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -60,11 +60,11 @@
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeDouble),
   context.DoubleTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeDoubleComplex),
-  context.DoubleComplexTy));
+  context.getComplexType(context.DoubleTy)));
   EXPECT_TRUE(
   context.hasSameType(GetBasicQualType(eBasicTypeFloat), context.FloatTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeFloatComplex),
-  context.FloatComplexTy));
+  context.getComplexType(context.FloatTy)));
   EXPECT_TRUE(
   context.hasSameType(GetBasicQualType(eBasicTypeHalf), context.HalfTy));
   EXPECT_TRUE(
@@ -75,8 +75,9 @@
   context.hasSameType(GetBasicQualType(eBasicTypeLong), context.LongTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongDouble),
   context.LongDoubleTy));
-  EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongDoubleComplex),
-  context.LongDoubleComplexTy));
+  EXPECT_TRUE(
+  context.hasSameType(GetBasicQualType(eBasicTypeLongDoubleComplex),
+  context.getComplexType(context.LongDoubleTy)));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongLong),
   context.LongLongTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeNullPtr),
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -981,21 +981,25 @@
 }
 break;
 
-  case DW_ATE_complex_float:
-if (QualTypeMatchesBitSize(bit_size, ast, ast.FloatComplexTy))
-  return GetType(ast.FloatComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.DoubleComplexTy))
-  return GetType(ast.DoubleComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.LongDoubleComplexTy))
-  return GetType(ast.LongDoubleComplexTy);
-else {
-  CompilerType complex_float_clang_type =
-  GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
-   bit_size / 2);
-  return GetType(
-  ast.getComplexType(ClangUtil::GetQualType(complex_float_clang_type)));
-}
-break;
+  case DW_ATE_complex_float: {
+CanQualType FloatComplexTy = ast.getComplexType(ast.FloatTy);
+if (QualTypeMatchesBitSize(bit_size, ast, FloatComplexTy))
+  return GetType(FloatComplexTy);
+
+CanQualType DoubleComplexTy = ast.getComplexType(ast.DoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, DoubleComplexTy))
+  return GetType(DoubleComplexTy);
+
+CanQualType LongDoubleComplexTy = ast.getComplexType(ast.LongDoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, LongDoubleComplexTy))
+  return GetType(LongDoubleComplexTy);
+
+CompilerType complex_float_clang_type =
+GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
+ bit_size / 2);
+return GetType(
+ast.getComplexType(ClangUtil::GetQualType(complex_float_clang_type)));
+  }
 
   case DW_ATE_float:
 if (type_name == "float" &&
@@ -2051,11 +2055,11 @@
   case eBasicTypeLongDouble:
 return ast->LongDoubleTy.getAsOpaquePtr();
   case eBasicTypeFloatComplex:
-return ast->FloatComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->FloatTy).getAsOpaquePtr();
   case eBasicTypeDoubleComplex:
-return ast->DoubleComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->DoubleTy).getAsOpaquePtr();
   case eBasicTypeLongDoubleComplex:
-return ast->LongDoubleComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->LongDoubleTy).getAsOpaquePtr();
   case eBasicTypeObjCID:
 return ast->getObjCIdType().getAsOpaquePtr();
   case eBasicTypeObjCClass:
Index: clang/lib/AST/ASTContext.cpp

[Lldb-commits] [lldb] 00c0ce0 - [NFC] [Clang] Remove pre-computed complex float types

[Qiu Chaofan via lldb-commits]

Author: Qiu Chaofan
Date: 2021-10-08T15:52:16+08:00
New Revision: 00c0ce0655da804c2ffb1a2a807052298032acc6

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

LOG: [NFC] [Clang] Remove pre-computed complex float types

As discussed in D109948, pre-computing all complex float types is not
necessary and brings extra overhead. This patch removes these defined
types, and construct them in-place when needed.

Reviewed By: teemperor

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 582134e586e0d..d4e813c5ed9d7 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1091,8 +1091,6 @@ class ASTContext : public RefCountedBase {
   CanQualType HalfTy; // [OpenCL 6.1.1.1], ARM NEON
   CanQualType BFloat16Ty;
   CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3
-  CanQualType FloatComplexTy, DoubleComplexTy, LongDoubleComplexTy;
-  CanQualType Float128ComplexTy;
   CanQualType VoidPtrTy, NullPtrTy;
   CanQualType DependentTy, OverloadTy, BoundMemberTy, UnknownAnyTy;
   CanQualType BuiltinFnTy;

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d1fd3ce061415..4085477bc45ad 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1411,12 +1411,6 @@ void ASTContext::InitBuiltinTypes(const TargetInfo 
,
   if (LangOpts.MatrixTypes)
 InitBuiltinType(IncompleteMatrixIdxTy, BuiltinType::IncompleteMatrixIdx);
 
-  // C99 6.2.5p11.
-  FloatComplexTy  = getComplexType(FloatTy);
-  DoubleComplexTy = getComplexType(DoubleTy);
-  LongDoubleComplexTy = getComplexType(LongDoubleTy);
-  Float128ComplexTy   = getComplexType(Float128Ty);
-
   // Builtin types for 'id', 'Class', and 'SEL'.
   InitBuiltinType(ObjCBuiltinIdTy, BuiltinType::ObjCId);
   InitBuiltinType(ObjCBuiltinClassTy, BuiltinType::ObjCClass);
@@ -6341,10 +6335,10 @@ QualType 
ASTContext::getFloatingTypeOfSizeWithinDomain(QualType Size,
 case Float16Rank:
 case HalfRank: llvm_unreachable("Complex half is not supported");
 case Ibm128Rank: llvm_unreachable("Complex __ibm128 is not supported");
-case FloatRank:  return FloatComplexTy;
-case DoubleRank: return DoubleComplexTy;
-case LongDoubleRank: return LongDoubleComplexTy;
-case Float128Rank:   return Float128ComplexTy;
+case FloatRank:  return getComplexType(FloatTy);
+case DoubleRank: return getComplexType(DoubleTy);
+case LongDoubleRank: return getComplexType(LongDoubleTy);
+case Float128Rank:   return getComplexType(Float128Ty);
 }
   }
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 99ed2a906635c..152b570a6f913 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -981,21 +981,25 @@ CompilerType 
TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 }
 break;
 
-  case DW_ATE_complex_float:
-if (QualTypeMatchesBitSize(bit_size, ast, ast.FloatComplexTy))
-  return GetType(ast.FloatComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.DoubleComplexTy))
-  return GetType(ast.DoubleComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.LongDoubleComplexTy))
-  return GetType(ast.LongDoubleComplexTy);
-else {
-  CompilerType complex_float_clang_type =
-  GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
-   bit_size / 2);
-  return GetType(
-  
ast.getComplexType(ClangUtil::GetQualType(complex_float_clang_type)));
-}
-break;
+  case DW_ATE_complex_float: {
+CanQualType FloatComplexTy = ast.getComplexType(ast.FloatTy);
+if (QualTypeMatchesBitSize(bit_size, ast, FloatComplexTy))
+  return GetType(FloatComplexTy);
+
+CanQualType DoubleComplexTy = ast.getComplexType(ast.DoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, DoubleComplexTy))
+  return GetType(DoubleComplexTy);
+
+CanQualType LongDoubleComplexTy = ast.getComplexType(ast.LongDoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, LongDoubleComplexTy))
+  return GetType(LongDoubleComplexTy);
+
+CompilerType complex_float_clang_type =
+GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
+ bit_size / 2);
+return 

[Lldb-commits] [PATCH] D111387: [NFC] [Clang] Remove pre-computed complex float types

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

Would be nice to have the motivation for this in the commit message (IIUC this 
is based on the suggestion from D109948  ). 
Patch itself LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111387

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

[Lldb-commits] [PATCH] D111387: [NFC] [Clang] Remove pre-computed complex float types

[Qiu Chaofan via Phabricator via lldb-commits]

qiucf created this revision.
qiucf added reviewers: rjmccall, teemperor.
qiucf requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111387

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -60,11 +60,11 @@
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeDouble),
   context.DoubleTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeDoubleComplex),
-  context.DoubleComplexTy));
+  context.getComplexType(context.DoubleTy)));
   EXPECT_TRUE(
   context.hasSameType(GetBasicQualType(eBasicTypeFloat), context.FloatTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeFloatComplex),
-  context.FloatComplexTy));
+  context.getComplexType(context.FloatTy)));
   EXPECT_TRUE(
   context.hasSameType(GetBasicQualType(eBasicTypeHalf), context.HalfTy));
   EXPECT_TRUE(
@@ -75,8 +75,9 @@
   context.hasSameType(GetBasicQualType(eBasicTypeLong), context.LongTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongDouble),
   context.LongDoubleTy));
-  EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongDoubleComplex),
-  context.LongDoubleComplexTy));
+  EXPECT_TRUE(
+  context.hasSameType(GetBasicQualType(eBasicTypeLongDoubleComplex),
+  context.getComplexType(context.LongDoubleTy)));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeLongLong),
   context.LongLongTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeNullPtr),
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -981,21 +981,25 @@
 }
 break;
 
-  case DW_ATE_complex_float:
-if (QualTypeMatchesBitSize(bit_size, ast, ast.FloatComplexTy))
-  return GetType(ast.FloatComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.DoubleComplexTy))
-  return GetType(ast.DoubleComplexTy);
-else if (QualTypeMatchesBitSize(bit_size, ast, ast.LongDoubleComplexTy))
-  return GetType(ast.LongDoubleComplexTy);
-else {
-  CompilerType complex_float_clang_type =
-  GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
-   bit_size / 2);
-  return GetType(
-  ast.getComplexType(ClangUtil::GetQualType(complex_float_clang_type)));
-}
-break;
+  case DW_ATE_complex_float: {
+CanQualType FloatComplexTy = ast.getComplexType(ast.FloatTy);
+if (QualTypeMatchesBitSize(bit_size, ast, FloatComplexTy))
+  return GetType(FloatComplexTy);
+
+CanQualType DoubleComplexTy = ast.getComplexType(ast.DoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, DoubleComplexTy))
+  return GetType(DoubleComplexTy);
+
+CanQualType LongDoubleComplexTy = ast.getComplexType(ast.LongDoubleTy);
+if (QualTypeMatchesBitSize(bit_size, ast, LongDoubleComplexTy))
+  return GetType(LongDoubleComplexTy);
+
+CompilerType complex_float_clang_type =
+GetBuiltinTypeForDWARFEncodingAndBitSize("float", DW_ATE_float,
+ bit_size / 2);
+return GetType(
+ast.getComplexType(ClangUtil::GetQualType(complex_float_clang_type)));
+  }
 
   case DW_ATE_float:
 if (type_name == "float" &&
@@ -2051,11 +2055,11 @@
   case eBasicTypeLongDouble:
 return ast->LongDoubleTy.getAsOpaquePtr();
   case eBasicTypeFloatComplex:
-return ast->FloatComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->FloatTy).getAsOpaquePtr();
   case eBasicTypeDoubleComplex:
-return ast->DoubleComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->DoubleTy).getAsOpaquePtr();
   case eBasicTypeLongDoubleComplex:
-return ast->LongDoubleComplexTy.getAsOpaquePtr();
+return ast->getComplexType(ast->LongDoubleTy).getAsOpaquePtr();
   case eBasicTypeObjCID:
 return ast->getObjCIdType().getAsOpaquePtr();
   case eBasicTypeObjCClass:
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1411,12 +1411,6