Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

2016-09-20 Thread Walter via lldb-commits
I rebased it

2016-09-20 15:44 GMT-07:00 walter erquinigo :

> wallace updated this revision to Diff 71995.
> wallace added a comment.
>
> rebase
>
>
> https://reviews.llvm.org/D24284
>
> Files:
>   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
>   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
>
>


-- 
Walter Erquínigo Pezo

Every problem has a simple, fast and wrong solution.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

2016-09-21 Thread Walter via lldb-commits
Oh, I was doing it from linux, I will do it from Windows then...

2016-09-21 8:42 GMT-07:00 Zachary Turner :

> If it helps, I can tell you that on line 623 of ObjectFilePECOFF.cpp, the
> export_table has number of names == 64, but address of names = 0.  So that
> seems wrong.
>
> On Wed, Sep 21, 2016 at 8:38 AM Zachary Turner  wrote:
>
>> Unfortunately this is still crashing for me in every single test.  Can I
>> ask how you are running the test suite?
>>
>> I am doing the following from a Windows 10 machine using Visual Studio 15:
>>
>> cmake -G Ninja -DLLDB_TEST_DEBUG_TEST_CRASHES=1
>> -DPYTHON_HOME=C:\Python35 -DLLDB_TEST_COMPILER=d:\src\
>> llvmbuild\ninja_release\bin\clang.exe ..\..\llvm
>> ninja check-lldb
>>
>> Can you try this same set of steps to see if you can reproduce?
>>
>> On Tue, Sep 20, 2016 at 3:44 PM Walter  wrote:
>>
>>> I rebased it
>>>
>>> 2016-09-20 15:44 GMT-07:00 walter erquinigo :
>>>
 wallace updated this revision to Diff 71995.
 wallace added a comment.

 rebase


 https://reviews.llvm.org/D24284

 Files:
   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h


>>>
>>>
>>> --
>>> Walter Erquínigo Pezo
>>>
>>> Every problem has a simple, fast and wrong solution.
>>>
>>


-- 
Walter Erquínigo Pezo

Every problem has a simple, fast and wrong solution.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D110269: Fix LLDB build on old Linux kernels

2021-09-23 Thread Walter via lldb-commits
I will! Thanks!

Il Gio 23 Set 2021, 12:35 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> labath added a comment.
>
> I'm pretty sure Caleb does not have commit access. Walter, would you do
> the honors?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D110269/new/
>
> https://reviews.llvm.org/D110269
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Walter via lldb-commits
Cool. I hope that vscode catches up.

Do you have push permissions or should I land this for you?

Il Mar 9 Nov 2021, 2:16 AM Andy Yankovsky via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> werat added a comment.
>
> In D113400#3117500 , @wallace
> wrote:
>
> > could you share a screenshot of how this looks like?
>
> Sure! Here are the screenshots of before/after in Visual Studio 2022.
>
> Before everything is in one place (in Locals Window):
> F20179455: visual-studio-before.png 
> After, Registers Window (marked in red -- changed since previous step):
> F20179456: registers-after.png 
> After, Locals Window (locals are displayed directly, without "Locals"
> expand block):
> F20179454: locals-after.png 
>
> ---
>
> There are no visible changes in Visual Studio Code.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D113400/new/
>
> https://reviews.llvm.org/D113400
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D101128: [lldb-vscode] only report long running progress events

2021-06-21 Thread Walter via lldb-commits
I'm reverting it asap

Il giorno lun 21 giu 2021 alle ore 10:48 Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> stella.stamenova added a comment.
>
> @wallace : Your most recent change to comment out
> ProgressEventThreadFunction is causing warnings to be generated when
> building with clang. Please have a look:
>
>   /usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Itools/lldb/tools/lldb-vscode -I/mnt/vss/_work/1/s/lldb/tools/lldb-vscode
> -I/mnt/vss/_work/1/s/lldb/include -Itools/lldb/include -Iinclude
> -I/mnt/vss/_work/1/s/llvm/include
> -I/mnt/vss/_work/1/s/llvm/../clang/include -Itools/lldb/../clang/include
> -I/usr/include/libxml2 -fPIC -fvisibility-inlines-hidden -Werror
> -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default
> -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> -Wsuggest-override -Wstring-conversion -Wmisleading-indentation
> -fdiagnostics-color -ffunction-sections -fdata-sections
> -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing
> -Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG  -fno-exceptions
> -fno-rtti -UNDEBUG -std=c++14 -MD -MT
> tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o
> -MF
> tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o.d
> -o
> tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o
> -c /mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp
>   /mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp:370:6: error:
> unused function 'ProgressEventThreadFunction' [-Werror,-Wunused-function]
>   void ProgressEventThreadFunction() {
>^
>   1 error generated.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D101128/new/
>
> https://reviews.llvm.org/D101128
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Walter via lldb-commits
> BTW, I thought when a command returned the command string for the next
command, the command interpreter prepended it with the chain of parent
commands containing the command that was presenting the “next command
string”.

That is correct. The GetRepeatCommand function gets the full command with
the type Args. I could look for the current subcommand name within the Args
and use the tokens up to that position as the repeat command, however, it
would fail if there's token with the same value, like "memory read read".
It might be a very degenerate case and I'm okay with implementing this and
support enums if you agree with it. If you have a better idea, it'd be
great.

On Mon, Apr 6, 2020 at 12:23 PM Jim Ingham  wrote:

> Then only time that I can find where I customized the string based on the
> incoming full command was for “source list” because I needed to know
> whether the listing was going forward or backward, so I needed to write
> that into the “keep doing what you were doing” command.  That could have
> also been implemented by remembering the direction as well as the “current
> source location” internally, and then have “source list” mean “keep going
> the direction you were going.”  So I don’t think dynamically constructing
> the next command string is necessary to cover the needed cases.
>
> BTW, I thought when a command returned the command string for the next
> command, the command interpreter prepended it with the chain of parent
> commands containing the command that was presenting the “next command
> string”.  If it doesn’t do that, it probably should.  A command shouldn’t
> really know where it is sitting in the command hierarchy, so somebody has
> to do that for it.
>
> I don’t know of a case where a command would want its repeat command to be
> a wholly different command.  That seems odd, but if we do find we need that
> internally, we could add some way of saying “this command is not a variant
> of me…”  But in any case, for these purposes, it seems like the three
> useful cases are really “no repeat”, “repeat the command I was given” and
> “invoke my continue-from-where-I-left-off” command - which by convention is
> the command with no arguments.  If we make sure that when the repeat
> command is actually used the command interpreter adds the command’s
> parents, then I think we could do this with an enum.
>
> Jim
>
>
> > On Apr 6, 2020, at 11:57 AM, walter erquinigo via Phabricator <
> revi...@reviews.llvm.org> wrote:
> >
> > wallace updated this revision to Diff 255426.
> > wallace added a comment.
> > Herald added a subscriber: mgorny.
> >
> > - Moved the test to gtest. It's much better this way and I learned gtest
> > - Changed the API. Some notes:
> >
> > Within the scope of a subcommand, the subcommand doesn't know the
> parent's
> > command name. It only knows its own name and it doesn't have any
> referrence to
> > its parent. That makes it very difficult to implement an enum option for
> > eCommandNameRepeat, as @jingham suggested. The GetRepeatCommand
> signature also
> > doesn't provide useful info about the parsed arguments.
> >
> > I took a look at the existing implementations for GetRepeatCommand, and
> it seems
> > that most of them just return nullptr, "", or the same command without
> flags.
> >
> > I don't want to change any existing core command interpreter function,
> so I
> > think that a simple API that covers most of the cases is just providing
> nullptr
> > for repeating the same command, "" for not repeating, or a custom string
> for
> > any other case. If in the future anyone needs something very customized,
> a new
> > override could be created, but I don't think this will happen anytime
> soon.
> >
> > Another option is to provide a callback function instead of a string,
> but I
> > don't know if it's too much.
> >
> >
> > Repository:
> >  rG LLVM Github Monorepo
> >
> > CHANGES SINCE LAST ACTION
> >  https://reviews.llvm.org/D77444/new/
> >
> > https://reviews.llvm.org/D77444
> >
> > Files:
> >  lldb/include/lldb/API/SBCommandInterpreter.h
> >  lldb/source/API/SBCommandInterpreter.cpp
> >  lldb/unittests/Interpreter/CMakeLists.txt
> >  lldb/unittests/Interpreter/TestAutoRepeat.cpp
> >
> > 
>
>

-- 
Walter Erquínigo Pezo

Every problem has a simple, fast and wrong solution.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-10-01 Thread Walter via lldb-commits
> - I am surprised that it was not necessary to create a special process
plugin for this purpose. I have a feeling one will be necessary sooner or
later because of the need to customize the step/continue/etc. flows.
Currently, this will probably produce very bizarre if one tries to execute
those commands. The reason I'm bringing this up is because of the
`Target::GetTrace` method being added in the next patch. If there is a
trace-specific process class, then it might make sense for this class to
hold the trace object instead of adding the GetTrace method on every Target
object out there even though most targets will have that null. I don't know
if that will really be the case, but I think it's something worth keeping
in mind as you work on the subsequent patches.

Very good remark. Probably we'll end up doing that when we start
implementing reverse debugging. The tricky thing we'll need to solve in the
future is seamlessly transition between a trace and a live process. For
example, when reverse debugging a live process, you might be stopped at a
breakpoint in the trace, then you do "continue", it reaches the end of the
trace, and then it resumes the live process. I still haven't decided what
we'll propose for this, but probably we'll have to change a lot of the
current code to make that happen nicely.

> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The
two classes are very tightly coupled, so it's not clear to me what is the
advantage of separating it out this way (one could just move all the
relevant methods directly into the Trace class. What's the reason for this
design?

Well, there's definitely coupling, but there are two reasons. One is that
the Trace of a live process won't need any parsing. Parsing is only used
when loading a trace from a json file. That makes parsing one of the two
main ways to create a Trace. And besides, I think that the single
responsibility principle is good to follow. The Parser class does the
parsing, and the Trace class holds an actual trace.

Il giorno gio 1 ott 2020 alle ore 07:12 Pavel Labath via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> labath added a comment.
>
> I have two high-level remarks/questions about this:
>
> - I am surprised that it was not necessary to create a special process
> plugin for this purpose. I have a feeling one will be necessary sooner or
> later because of the need to customize the step/continue/etc. flows.
> Currently, this will probably produce very bizarre if one tries to execute
> those commands. The reason I'm bringing this up is because of the
> `Target::GetTrace` method being added in the next patch. If there is a
> trace-specific process class, then it might make sense for this class to
> hold the trace object instead of adding the GetTrace method on every Target
> object out there even though most targets will have that null. I don't know
> if that will really be the case, but I think it's something worth keeping
> in mind as you work on the subsequent patches.
> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The two
> classes are very tightly coupled, so it's not clear to me what is the
> advantage of separating it out this way (one could just move all the
> relevant methods directly into the Trace class. What's the reason for this
> design?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D85705/new/
>
> https://reviews.llvm.org/D85705
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-10-02 Thread Walter via lldb-commits
I totally agree with you, I didn't think about creating custom callbacks
=P. I'll refactor the code accordingly. Thanks, man

Il giorno ven 2 ott 2020 alle ore 01:51 Pavel Labath  ha
scritto:

> On 01/10/2020 20:57, Walter wrote:
> >> - I am surprised that it was not necessary to create a special process
> > plugin for this purpose. I have a feeling one will be necessary sooner
> > or later because of the need to customize the step/continue/etc. flows.
> > Currently, this will probably produce very bizarre if one tries to
> > execute those commands. The reason I'm bringing this up is because of
> > the `Target::GetTrace` method being added in the next patch. If there is
> > a trace-specific process class, then it might make sense for this class
> > to hold the trace object instead of adding the GetTrace method on every
> > Target object out there even though most targets will have that null. I
> > don't know if that will really be the case, but I think it's something
> > worth keeping in mind as you work on the subsequent patches.
> >
> > Very good remark. Probably we'll end up doing that when we start
> > implementing reverse debugging. The tricky thing we'll need to solve in
> > the future is seamlessly transition between a trace and a live process.
> > For example, when reverse debugging a live process, you might be stopped
> > at a breakpoint in the trace, then you do "continue", it reaches the end
> > of the trace, and then it resumes the live process. I still haven't
> > decided what we'll propose for this, but probably we'll have to change a
> > lot of the current code to make that happen nicely.
>
> Yes, that will be interesting. Even more interesting will be the
> question of how to communicate to the user the fact that even though we
> have "unwound" the PC to the previous location, variables and memory
> still contain the "live" values. Particularly, once we support rr-style
> reverse debugging which will "unwind" memory as well..
>
> >
> >> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The
> > two classes are very tightly coupled, so it's not clear to me what is
> > the advantage of separating it out this way (one could just move all the
> > relevant methods directly into the Trace class. What's the reason for
> > this design?
> >
> > Well, there's definitely coupling, but there are two reasons. One is
> > that the Trace of a live process won't need any parsing. Parsing is only
> > used when loading a trace from a json file. That makes parsing one of
> > the two main ways to create a Trace. And besides, I think that the
> > single responsibility principle is good to follow. The Parser class does
> > the parsing, and the Trace class holds an actual trace.
>
> Yeah, I'm all for the single responsibility principle, but the way it is
> implemented gives me pause. Like, if the Parser is supposed to "create"
> the Trace, then why does the code do it the other way around
> (Trace::CreateParser)? For one, that means that it will be possible to
> "parse" (by calling CreateParser()->...) any trace that you can get your
> hands on, even that of a live process. Ideally, the Parser would be the
> one creating a Trace object (in it's Parse function, or something), and
> after the Trace is created, it should no longer be possible to obtain
> the parser. All of this could be encapsulated in the "create_callback"
> that the plugin registers, and so the fact that there is a parser class
> involved would be completely unknown to the rest of the code. And then
> there could be a different create_callback which would create a Trace
> object suitable for tracing a live process...
>
> pl
>


-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

2021-01-21 Thread Walter via lldb-commits
I've tried to find a way to move the calls the way you mentioned, but it
doesn't seem trivial.

Some more information:
- The invocation to the thread plan is done by Thread::ShouldStop, where it
does

```
// We're starting from the base plan, so just let it decide;
if (current_plan->IsBasePlan()) {
  should_stop = current_plan->ShouldStop(event_ptr);
```

The interesting part of this call-stack starts
at Process::HandlePrivateEvent, which seems to be handling the Stop Event.
Where I think there's some inconsistent code is in
ThreadPlanBase::ShouldStop, because it tries to get the stop reason, which
fails for Linux, and then it processes eStopReasonExec a few lines below.
And see what it does:

```
  case eStopReasonExec:
  // If we crashed, discard thread plans and stop.  Don't force the
  // discard, however, since on rerun the target may clean up this
  // exception and continue normally from there.
  LLDB_LOGF(
  log,
  "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64
  " (exec.)",
  m_tid);
  GetThread().DiscardThreadPlans(false);
  return true;
```

It does discard the thread plans for that thread! This makes me believe
that it should be fine to delete the thread plans in the first place. I
wasn't able to figure out more, but I can dig deeper if you give me a few
pointers. In any case, this last code of block makes me believe that
deleting the thread plans or reseting the thread pointer in the base thread
plan might be fine.


Btw, this is a bunch of logs I got, which start at a breakpoint, then
there's a "continue", which triggers what I'm saying. The backtrace is below

(lldb) bt
* thread #1, name = 'runner', stop reason = instruction step into
  * frame #0: 0x7f1fe674838d libc.so.6`raise + 61
frame #1: 0x00400a3c runner`main(argc=2,
argv=0x7fff4b78fc08) at runner.cpp:20
frame #2: 0x7f1fe6734555 libc.so.6`__libc_start_main + 245
frame #3: 0x00400919 runner`_start + 41
(lldb) thread plan list /// See that
only the base plan is there
thread #1: tid = 0x2b72f1:
  Active plan stack:
Element 0: Base thread plan.
  Completed plan stack:
Element 0: Stepping one instruction past 0x7f1fe6748387 stepping
into calls
Element 1: Stepping one instruction past 0x7f1fe6748387 stepping
into calls
(lldb) c
lldb Process::Resume -- locking run lock
lldb Process::PrivateResume() m_stop_id = 3, public state:
stopped private state: stopped
lldb WillResume Thread #1 (0x0x7fcd3da0): tid = 0x2b72f1,
pc = 0x7f1fe674838d, sp = 0x7fff4b78fad8, fp = 0x7fff4b78fb20, plan = 'base
plan', state = running, stop others = 0
lldb Resuming thread: 2b72f1 with state: running.
lldb 0x4e7020
Listener::Listener('gdb-remote.resume-packet-sent')
lldb 0x4e7020 Listener::StartListeningForEvents (broadcaster =
0x4787a8, mask = 0x0001) acquired_mask = 0x0001 for
gdb-remote.resume-packet-sent
lldb 0x4e7020 Listener::StartListeningForEvents (broadcaster =
0x478ff8, mask = 0x0004) acquired_mask = 0x0004 for
gdb-remote.resume-packet-sent
lldb 0x494ab0
Broadcaster("lldb.process.gdb-remote.async-broadcaster")::BroadcastEvent
(event_sp = {0x7fcd40007cd0 Event: broadcaster = 0x478ff8
(lldb.process.gdb-remote.async-broadcaster), type = 0x0001 (async
thread continue), data = {"c"}}, unique =0) hijack = (nil)
lldb 0x494bf0
Listener('lldb.process.gdb-remote.async-listener')::AddEvent (event_sp =
{0x7fcd40007cd0})
lldb this = 0x004E7020, timeout = 500 us for
gdb-remote.resume-packet-sent
b-remote.async>  0x494bf0 'lldb.process.gdb-remote.async-listener'
Listener::FindNextEventInternal(broadcaster=(nil),
broadcaster_names=(nil)[0], event_type_mask=0x, remove=1) event
0x7fcd40007cd0
b-remote.async>  Process::SetPrivateState (running)
b-remote.async>  0x483f30
Broadcaster("lldb.process.internal_state_broadcaster")::BroadcastEvent
(event_sp = {0x7fcd40005380 Event: broadcaster = 0x477dd0
(lldb.process.internal_state_broadcaster), type = 0x0001, data = {
process = 0x477ce0 (pid = 2847473), state = running}}, unique =0) hijack =
(nil)
b-remote.async>  0x4841b0
Listener('lldb.process.internal_state_listener')::AddEvent (event_sp =
{0x7fcd40005380})
b-remote.async>  0x485cf0 Broadcaster("gdb-remote.client")::BroadcastEvent
(event_sp = {0x7fcd40005500 Event: broadcaster = 0x4787a8
(gdb-remote.client), type = 0x0001, data = }, unique =0) hijack =
(nil)
b-remote.async>  0x4e7020
Listener('gdb-remote.resume-packet-sent')::AddEvent (event_sp =
{0x7fcd40005500})
intern-state 0x4841b0 'lldb.process.internal_state_listener'
Listener::FindNextEventInternal(broadcaster=(nil),
broadcaster_names=(nil)[0], event_type_mask=0x, remove=1) event
0x7fcd40005380
lldb 0x4e7020 'gdb-remote.resume-packet-sent'
Listener::Fi

Re: [Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-25 Thread Walter via lldb-commits
I've already submitted a fix, let's see if the buildbot gets fixed

Il giorno lun 25 gen 2021 alle ore 14:18 Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> stella.stamenova added a comment.
>
> It looks like this broke the windows lldb bot:
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D93951/new/
>
> https://reviews.llvm.org/D93951
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

2021-01-27 Thread Walter via lldb-commits
Good to know. I'll work on that right now. Thanks!

Il giorno mer 27 gen 2021 alle ore 11:41 Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> stella.stamenova added inline comments.
>
>
> 
> Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9
> +
> +#if !defined(WIN32)
> +#include 
> 
> Also, this is no good. It works if you are targeting windows on windows,
> but not if you are targeting something else and building on windows. There
> are a few different ways this is done in LLVM/clang and they are generally
> not based on defined(WIN32). Here are a couple of examples:
>
> From llvm\lib\Support\Path.cpp:
>
> ```
> #if !defined(_MSC_VER) && !defined(__MINGW32__)
> ```
>
> From clang\lib\Driver\Driver.cpp:
>
> ```
> #if LLVM_ON_UNIX
> ```
>
> From llvm\lib\Support\ErrorHandling.cpp:
>
> ```
> #if defined(HAVE_UNISTD_H)
> ```
>
> I suggest browsing through the code and finding the most appropriate way
> to manage your includes. In the mean time, this is breaking our internal
> builds that run on Windows but do not target Windows.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D93951/new/
>
> https://reviews.llvm.org/D93951
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 0bca9a7 - Fix lldb-vscode builds on Windows targeting POSIX

2021-02-04 Thread Walter via lldb-commits
This is great to know. Til. I'll change all of those macros to _WIN32 for
consistency then

Il Gio 4 Feb 2021, 3:25 AM Martin Storsjö  ha scritto:

> On Thu, 28 Jan 2021, Walter Erquinigo via lldb-commits wrote:
>
> >
> > Author: Walter Erquinigo
> > Date: 2021-01-28T09:36:13-08:00
> > New Revision: 0bca9a7ce2eeaa9f1d732ffbc17769560a2b236e
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/0bca9a7ce2eeaa9f1d732ffbc17769560a2b236e
> > DIFF:
> https://github.com/llvm/llvm-project/commit/0bca9a7ce2eeaa9f1d732ffbc17769560a2b236e.diff
> >
> > LOG: Fix lldb-vscode builds on Windows targeting POSIX
> >
> > @stella.stamenova found out that lldb-vscode's Win32 macros were failing
> > when building on windows targetings POSIX platforms.
> >
> > I'm changing these macros for LLVM_ON_UNIX, which should be more
> > accurate.
>
> Just for the record, the reason for things failing wasn't so much that
> LLVM_ON_UNIX is more accurate than the macros that the code tried to use
> before. Before, the code tried to check the define "WIN32" in some files,
> and "_WIN32" in others (and one case even used the macro "WIN_32").
>
> "_WIN32" is the correct one to check, it's predefined by the compilers
> when targeting windows. Most build tools don't normally define "WIN32"
> without a leading underscore (although e.g. some visual studio project
> file templates include it in the list of user-defined macros). So that's
> why things broke before.
>
> Btw, the checks like "#if LLVM_ON_UNIX" are a bit incorrect too, they
> should be "#ifdef LLVM_ON_UNIX", as the macro is entirely undefined for
> non-unix platforms.
>
> These commits should be backported to the 12.x release branch too, because
> building lldb for windows (or in particular, lldb-vscode) is broken there
> right now.
>
> // Martin
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D96060: [lldb-vscode] correctly use Windows macros

2021-02-04 Thread Walter via lldb-commits
I don't feel knowledgeable enough to backport the commit, so if you can do
it, it would be great :)

Il giorno gio 4 feb 2021 alle ore 13:04 Martin Storsjö via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> mstorsjo added a comment.
>
> In D96060#2542890 , @mstorsjo
> wrote:
>
> > I can test build it in a couple hours
>
> It still built fine for me now - thanks!
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D96060/new/
>
> https://reviews.llvm.org/D96060
>
>

-- 
- Walter Erquínigo Pezo
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D81978: Redo of Add terminateCommands to lldb-vscode protocol

2020-06-24 Thread Walter via lldb-commits
I'll revert this patch. This seems to be the reason

Il Mer 24 Giu 2020, 6:27 AM Florian Hahn via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> fhahn added a comment.
>
> I noticed that it appears that `  lldb-api ::
> tools/lldb-vscode/attach/TestVSCode_attach.py` is flaky and I noticed it
> failing spuriously relatively frequently
>
> For example
> http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5519 failed
> with
>
>   FAIL: lldb-api :: tools/lldb-vscode/attach/TestVSCode_attach.py (825 of
> 2068)
>    TEST 'lldb-api ::
> tools/lldb-vscode/attach/TestVSCode_attach.py' FAILED 
>   Script:
>   --
>   /usr/bin/python3.6
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/dotest.py
> --arch aarch64 -s
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-traces -S
> nm -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env
> OBJCOPY=/usr/bin/objcopy --env
> LLVM_LIBS_DIR=/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./lib
> --build-dir
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex
> --lldb-module-cache-dir
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
> --clang-module-cache-dir
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-build.noindex/module-cache-clang/lldb-api
> --executable
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/lldb --compiler
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/clang
> --dsymutil
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/dsymutil
> --filecheck
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./bin/FileCheck
> --lldb-libs-dir /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./lib
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/tools/lldb-vscode/attach
> -p TestVSCode_attach.py
>   --
>   Exit Code: 1
>
>   Command Output (stdout):
>   --
>   lldb version 11.0.0
> clang revision 72131423cc952ccbd6d8e021ff7c04fa22297fe3
> llvm revision 72131423cc952ccbd6d8e021ff7c04fa22297fe3
>   LLDB library dir:
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin
>   LLDB import library dir:
> /home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/./lib
>   Libc++ tests will not be run because: Compiling with -stdlib=libc++
> fails with the error: :1:10: fatal error: 'algorithm' file not found
>   #include 
>^~~
>   1 error generated.
>
>   Skipping following debug info categories: ['dsym', 'gmodules']
>   description: breakpoint 1.1
>   description: breakpoint 1.1
>
>   --
>   Command Output (stderr):
>   --
>
>   Session logs for test failures/errors/unexpected successes will go into
> directory
> '/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/lldb-test-traces'
>   PASS: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_by_name (TestVSCode_attach.TestVSCode_attach)
>   UNSUPPORTED: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_by_name_waitFor (TestVSCode_attach.TestVSCode_attach) (requires one
> of darwin, macosx, ios, watchos, tvos, bridgeos)
>   PASS: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_by_pid (TestVSCode_attach.TestVSCode_attach)
>   FAIL: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_commands (TestVSCode_attach.TestVSCode_attach)
>   CLEANUP ERROR: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_commands (TestVSCode_attach.TestVSCode_attach)
>   PASS: LLDB
> (/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11-aarch64)
> :: test_terminate_commands (TestVSCode_attach.TestVSCode_attach)
>   ==
>   FAIL: test_commands (TestVSCode_attach.TestVSCode_attach)
>   --
>   Traceback (most recent call last):
> File
> "/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py",
> line 174, in test_commands
>   self.continue_to_breakpoints(breakpoint_ids)
> File
> "/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py",
> line 226, in continue_to_breakpoints
>   self.verify_breakpoint_hit(breakpoint_ids)
> File
> "/home/tcwg-buildslave/worker/lldb-cmake-aarch64/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py",
> line 85, in verify_breakpoint_hit
>   self.assertTrue(False, "breakpoint not hit")
>   AssertionError: False is not True : breakpoint not hit
>
> Config=aarch64-/home/tcwg-buildslave/worker/lldb-cmake-aarch64/build/bin/clang-11
>   -