date:20190220

[Lldb-commits] [PATCH] D58410: [Reproducers] Initialize reproducers before initializing the debugger.

2019-02-20 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I think this looks mostly fine. See my comment about not using SB classes in 
the reproducer api. I still kind of like the idea of naming the reproducer 
class in some special way, to make it more obvious that it is not "just 
another" SB class, but I'm not sure if that would be just more confusing.

Comment at: lldb/include/lldb/API/SBReproducer.h:18-20
   static bool Replay();
+  static lldb::SBError InitializeCapture(const char *path);
+  static lldb::SBError InitializeReplay(const char *path);

So, in this new world, how are we expected to perform the replay? 
`InitializeReplay()`, followed by `Replay()` ? Could we fold those two calls 
into one? I don't see what meaningful work could the user do between those two 
calls, as all sb calls (including SBDebugger::Initialize) will now be recorded 
(right?).

Comment at: lldb/source/API/SBDebugger.cpp:127
 void SBDebugger::Initialize() {
-  SBInitializerOptions options;
-  SBDebugger::Initialize(options);
+  SBError ignored = SBDebugger::InitializeWithErrorHandling();
 }

I think the usual way to do that is to just cast the result to void.

Comment at: lldb/source/API/SBReproducer.cpp:49
+SBError SBReproducer::InitializeCapture(const char *path) {
+  SBError error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Capture, FileSpec(path)))

JDevlieghere wrote:
> The SBError is a problem. We can create it after the initialization, but then 
> we’d need a bogus repro::Record to make sure the api boundary is correctly 
> registered. Let me know if you can think of an alternative.
Maybe this function should not return an SBError at all (just like it does not 
accept an SBFileSpec due to bootstrapping problems). You could just return a 
std::string, and have an empty string denote success. This would be consistent 
with the idea that the repro API is not a part of SB API, but rather something 
sitting slightly above (or at least, besides) it.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58410

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

[Lldb-commits] [PATCH] D54670: 03/03: .debug_types: Update of D32167 (.debug_types) on top of D51578 (section concatenation)

2019-02-20 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I am still worried about the divergence from llvm's dwarf reader here. We now 
have a relatively nice Object API, which presents a view of the object file 
that everybody is used to (i.e., as a bunch of sections). This includes the 
llvm object and dwarf readers, and the DWARF spec (!). But now we are drilling 
a hole in that API, which basically reverts the sections and gives us back the 
a view of the object file as a stream of bytes. And if the object file happens 
to be compressed, then we have to mock up a new uncompressed buffer with all of 
the data, even though the compression is supposed to be transparent and 
unobtrusive (and it mostly is, if you stick to viewing the object file as a 
collection of sections). All because our dwarf reader cannot handle the fact 
that the debug info can come from multiple sections? How come nobody else has 
this problem?

Treating dwarf data as a single buffer with various chunks starting at random 
offsets should essentially be isomorphous with treating it as a collection of 
independent buffers. We are here introducing a function called 
`get_debug_types_offset`. Why not have `get_debug_types_data_extractor()` 
instead? That may mean some functions need to be changed so that they take an 
additional buffer argument, instead of just assuming everything refers to the 
single grand buffer, but I think that's a good thing, because it brings us 
closer to reality instead of continuing to fabricate ever more complicated 
fiction. BTW, I don't see a `get_debug_info_offset()`. AFAIK, there's no 
requirement that .debug_info has to come first in an object file. What would 
happen if the order of the sections in the object file is reversed?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D54670



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

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

2019-02-20 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thank you for jumping onto this. I've tried the patch out on linux, and the
stop-hook-threads.test now passes. However, the basic test now fails for me
because the stop hook isn't fired in the "run" command. it is caused by the "is
hijacked" check in `ProcessEventData::DoOnRemoval`, as removing that condition
causes to hook to fire, but that's probably not the right thing to do here. The
difference here is probably caused by PlatformLinux hijacking the process
events during the launch sequence
https://github.com/llvm-mirror/lldb/blob/76c0545e7f24cd6636df61292472df64cfa2900e/source/Plugins/Platform/Linux/PlatformLinux.cpp#L337,
but I am not sure where is the error here. I definitely know that there is
something fishy going on in PlatformLinux, as e.g. we don't get a "Process XXX
stopped" message when launching with the --stop-at-entry flag, so it's quite
possible the error is there. If you think that the general behavior is right
there, then we can just XFAIL this test for linux, and one of us will get to
fixing PlatformLinux eventually.

Comment at: lit/ExecControl/StopHook/stop-hook-threads.test:36
# When we don't filter, we expect to count 12 stopped threads in the thread
list output
-# CHECK-NO-FILTER-COUNT-12: at stop-hook-threads.cpp{{.*}} stop reason =
breakpoint
\ No newline at end of file
+# CHECK-NO-FILTER-COUNT-12: , stop reason = breakpoint 3.1

BTW, this check is fairly dodgy, because in the case of two threads hitting a
breakpoint simultaneously, we will get 4 stop reasons instead of two ("thread
list" will be executed twice, and each instance will print the stop reason for
both threads). It might be better to replace "thread list" with a different
command, or add synchronization to the test to prevent two threads from hitting
the breakpoint at once, depending on what we want to test.

39 matches

Mail list logo