- User Since
- Apr 27 2019, 10:34 PM (43 w, 5 d)
Jan 23 2020
Yeah, I'm not sure why the LoadModules function is calling target.SetExecutableModule. It is true that the libraries-svr4 will not include the main executable in its list.
This code was added in the context of providing qXfer:libraries support here: https://reviews.llvm.org/D9471. I don't see any mention of including the executable on that packet though: https://sourceware.org/gdb/current/onlinedocs/gdb/Library-List-Format.html. @clayborg was the main reviewer there (although this was 5 years ago or so) and he does mention multiple times in the comments this exact issue with calling target.SetExecutableModule. Maybe he can still remember and provide some light here :).
Dec 5 2019
Fair enough, I haven't seen evidence of this (haven't searched for it) but I imagine IDEs need to ignore this as well otherwise they just barf if they're expecting Content-Length and a wild print appears. The notion of stdout of SBDebugger is the SBCommandReturnObject which doesn't print right away (only when the command finishes executing) and from my previous tests .flush() doesn't help with this. I believe this is the reason why people opt to use print instead in their custom commands. But I don't think it's a reasonable assumption (or api contract) to tell people they can't use print or it breaks lldb-vscode.
Dec 3 2019
Maybe you could do something similar to LocalLLDBInit.test ?
That test uses the lldb -S (and others) flags that lldb-vscode doesn't support :(. These flags should really be added to the initialize packet but they're very specific to lldb and the DAP doesn't support it.. We could implement something like what Driver::ProcessArgs does and pass options through envs but, the logic in ProcessArgs itself is sketchy (just like the comment mentions) and I l also find it odd to pass options through env vars...
Dec 2 2019
Put the logic into a function
Use an env var instead
Solve this at the failing test level. We can't be sure of the filename will end up in the debug info so we just add both keys to the mapping.
Yes. This happens when lldb-vscode is run in stdin/stdout mode (which is what happens in tests and I've also seen some IDEs doing the same) and installed commands are doing "print". In this case the client gets data that does not start with Content-Length.
This fixes the tests but overall is still a problem for IDEs (but that's another discussion).
Yeah, that's what I do in D70882. I still think tests can be more robust by using explicit lldb command names but don't really care much :). I'm happy to abandon it.
Yes, I'll change this to env it makes more sense.
The way phabricator shows the diff is tricky, this change has nothing to do with D70882 and stands by itself. The important part here is making the g_vsc.debugger.SetOutputFileHandle(out, true); g_vsc.debugger.SetErrorFileHandle(out, false); happen before the debugger creation. Not sure how to create a test for this though since there's no mechanism to give lldb-vscode an initial file to load...
Dec 1 2019
Oops, wrnog default
Nov 15 2019
Nov 14 2019
Thank you for writing this up! I was thinking how I would find this.. I don't see myself clicking on the "Caveats" link (could also be a me problem though). Could we also add a link to it in this section? https://lldb.llvm.org/resources/build.html#id22. Something like "NOTE: You need to use the python binary you built with, check caveats to learn more"?
Nov 11 2019
Clarify the point of the install_path
Nov 10 2019
The proposed path in this patch, -rpath "@loader_path/../../../", uses the @loader_path expansion which is the directory containing the binary that the load command is in (in this case liblldb's directory). Popping 3 directories up from that is likely not sane in most build configurations, but if it works for you meh...
Updating to only add an rpath that points to the directory where LLDB.frameworks gets installed.
Nov 8 2019
That should work fine yeah. It should be a matter of adding "@loader_path/../../../" to the rpath.
Nov 6 2019
Yeah, weird. I wonder if this is something cmake does implicitly when the target is defined as FRAMEWORK: https://cmake.org/cmake/help/latest/prop_tgt/FRAMEWORK.html
Nov 5 2019
@hhb I just did, and get the same error as well :(
ninja: error: 'bin/LLDB.framework/LLDB', needed by 'bin/LLDB.framework/Resources/Python/lldb/_lldb.so', missing and no known rule to make it
Nov 4 2019
This is great! it's so much better. Thank so much for looking into this and fixing it.
Nov 3 2019
I'm sorry to reject but this diff breaks install-distribution on Darwin. Here's how to repro:
Oct 9 2019
@shafik I just pushed a fix, the issue was because on mac there are more unnamed symbols so the number didn't match, I changed it to match any number since this can easily change.
@shafik thanks, checking.
Oct 8 2019
Oct 7 2019
Don't destroy the Breakpad test purpose.
Oct 4 2019
Ok. Initially I thought that with the entry point we were making big assumptions there but after reading the parse unwind symbols I guess it's really no big difference.
I guess my main concern is that the user can no longer create symbols within the span of the entry point symbol, even if they happen to know better (it's not even possible to manually remove symbols). But like you said, the same is true with the other synthetic symbols..
Oct 3 2019
I ended reverting this because of SymbolFile/Breakpad/symtab.test. It seems that in this test we add symbols after the synthetic entry point symbol is added creating confusion.
I think I'll need to change the code that adds symbols in Symtab to explicitly check if we're overlapping the synthetic entry point and reduce the entry point or remove it in the case it overlaps at the beginning.
Rename lldb-scripts to lldb-python-scripts
Oct 2 2019
Remove .global _Start
Update comment and test
I actually just completely missed the first comment on the .s test and forgot about the comment 😇.
Oct 1 2019
Update tests and fix logic to correctly add the symtab entry.
Sep 30 2019
Add lit test to check dissassembly
Sep 27 2019
(substituting llvm-mc for as and ld.lld for linker).
Not everyone has both of these things enabled (particularly having lld is less common), but they are things that one _can_ enable no matter what is his host or target architecture, so I am not worried by that. And as tests like these become more common, I expect more people will start having these enabled by default.
Sep 26 2019
Use GetNextSyntheticSymbolName() to generate the symbol name
For the test, what would you say to writing that as a lit test instead (testing the address class deduction via the disassemble command you mentioned)?
Sep 25 2019
Aug 8 2019
I can see the appeal of having the contents next to the logic that is testing it, but I'm somewhat concerned for the cases where it includes +1000 lines of YAML in the test file. I think for those cases it might make sense to consider these fixtures and be in their own file?
Jul 29 2019
Jul 25 2019
@clayborg, yes, I made sure of that, we always first check the Xcode path.
Jul 24 2019
Jul 23 2019
I've just done these 2 steps:
@Hui what do you mean? the history is still there:
@jankratochvil interesting, we're also not able to run the lldb-vscode tests and it was on my todo list to take a look. Although I didn't realize it was a common think I thought it was specific to the environment where I was trying to run them. /cc @xiaobai. Thanks for looking into this!
Jul 22 2019
Handle error returned by GetLoadedModuleList
@teemperor This diff deleted all lldb-server and lldb-vscode tests :(