Re: [lldb-dev] LLDB tests getting stuck on GDBRemoteCommunicationClientTest.GetMemoryRegionInfo ?

[Pavel Labath via lldb-dev]

On Thu, 26 Apr 2018 at 22:58, Leonard Mosescu via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> I just did a clean build (debug) on Linux, and I noticed that the LLDB
tests seem to consistently get stuck:

>   -- Testing:
1002 tests, 12 threads --

>   99%
[==-]
ETA: 00:00:01
> lldb-Suite :: types/TestIntegerTypes.py


> At this point there are a bunch of llvm-lit processes waiting and two
suspicious LLDB unit tests:


> ProcessGdbRemoteTests
--gtest_filter=GDBRemoteCommunicationClientTest.GetMemoryRegionInfo
> ProcessGdbRemoteTests
--gtest_filter=GDBRemoteCommunicationClientTest.GetMemoryRegionInfoInvalidResponse


> I took a quick look and they both seem to blocked on communicating with
the remote:

> thread #2, name = 'ProcessGdbRemot', stop reason = signal SIGSTOP

These tests should have two threads communicating with each other. Can you
check what the other thread is doing?

My bet would be that fact that we are now running dotest tests concurrently
with the unittests is putting more load on the system (particularly in
debug builds), and the communication times out. You can try increasing the
timeout in GDBRemoteTestUtils.cpp:GetPacket to see if that helps.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [RFC] Improving import failure checking strategy inside ASTImporter

[Adrian Prantl via lldb-dev]

Adding lldb-dev to the thread, since LLDB is a primary user of ASTImporter.

> On Apr 27, 2018, at 8:54 AM, Aleksei Sidorin via cfe-dev 
>  wrote:
> 
> Hello Gabor,
> 
> Thank you for the reply!
> 
> 26.04.2018 12:39, Gábor Márton пишет:
>> Hi Aleksei,
>> 
>> It is a great and welcoming initiative to identify the weaknesses in
>> the ASTImporter.
>> 
>> About, handling the error case after a call:
>> I couldn't agree more that this is a burden to always have a check at
>> the call site, but this is the price we pay for not having exceptions.
>> By using macros I have a feeling that the code would be even less
>> readable because we would hide control flow modifications inside the
>> macro, also because finally we'd need to create so many different
>> kinds of macros.
>> As far as I know, there are cases where an `Import` can return a
>> `nullptr` or a default even in case of non-error situations, please
>> correct me if I am wrong. Thus, introducing an optional like return
>> type would require to discover all these cases. Still, I think this
>> would worth it, because the enforcement of error checking (as you
>> mentioned), plus it would make it more explicit which situation is
>> really an error.
> Yes, this is what I described as problem (2).
> 
>> Actually, I think this should be done with a few large commits, e.g.
>> once we change the return type of `ASTImporter::Import(Decl*)` to
>> `Optional` then we have to change all call sites.
>> By introducing an `ImportOrError` and then doing the changes gradually
>> would result the same final state as if we had done it in one larger
>> step where we changed all call sites, but until all call sites are
>> changed we have to maintain both `Import` and `ImportOrError`.
>> 
>> 
>> Beside the problem of handling the error case after an `Import` call
>> we have observed several other weaknesses, and would like to hear your
>> opinion about them as well.
>> So, the list of problems so far:
>> 
>> - Do the same things after and before a new AST node is created (::Create)
>> The original import logic should be really simple, we create the node,
>> then we mark that as imported, then we recursively import the parts
>> for that node and then set them on that node.
> That sounds pretty good, and this is what I was thinking about some time ago. 
> You mean something like CreateDeserialized?
> The problem here are Types: their creation requires all dependencies to be 
> imported before the Type is created. I don't know how to deal with it.
> 
>> However, the AST is actually a graph, so we have to handle circles.
>> If we mark something as imported (Imported()) then we return with the
>> corresponding To whenever we want to import that node again, this way
>> circles are handled.
>> In order to make this algorithm work we must ensure things, which are
>> not enforced in the current ASTImporter:
>> * There are no Import() calls in between any node creation (::Create)
>> and the Imported() call.
>> * There are no VisitXXX() calls in any other VisitYYY() function, only
>> call of Import() allowed.
>> * Before actually creating an AST node (::Create), we must check if
>> the Node had been imported already, if yes then return with that one.
>> One very important case for this is connected to templates: we may
>> start an import both from the templated decl of a template and from
>> the template itself.
>> There are good news, the first and third points can be enforced by
>> providing a variadic forwarding template for the creation, we are
>> currently preparing to upstream this change from our fork
>> (https://github.com/Ericsson/clang/pull/354/files 
>> ) but this is going
>> to be a huge change.
>> Still, I have no idea how to enforce the second point. (Maybe a checker?)
> If I remember correctly, this pattern is not widely used and such code can be 
> refactored easily.
> 
>> - Decl chains are not imported
>> Currently we import declarations only if there is no available definition.
>> If in the "From" TU there are both a declaration and a definition we
>> import only the definition.
>> Thus we do not import all information from the original AST.
>> One example:
>> struct B { virtual void f(); };
>> void B::f() {}
>> Here, when we import the definition, then the virtual flag is not
>> getting imported, so finally we import a function as a non-virtual
>> function.
>> An other problem is with recursive functions:
>> void f(); void f() { f(); }
>> When we import the prototype then we end up having two identical
>> definitions for f(), and no prototype.
>> We again preparing a patch for this, but this solves the problem only
>> in case of functions.
> Yes, I have met this problem before. I tried to import all redeclaration 
> chain but haven't completed the work. One of problems here is how to stick 
> the redeclaration chain to an existing declaration.
> 
>> By not importing the whole decl chain we loose all attribu