Thanks for the thorough explanation. LGTM.

I don't mind keeping it around, but I also haven't used the manual steps since the script's inception, so LGTM. We can always reinstate it if someone complains.

I meant to accept this, we can land this and deal with my suggestion as a follow up. LGTM.

Good catch, thank you! I wanted to suggest making the SBAddress constructor take the Address by const-reference but wanted to see how much work that'd be which resulted in D88249.

In D88048#2286776, @Higuoxing wrote:

In D88048#2286589, @dblaikie wrote:

maybe slightly long test case - could do this with the CU DIE itself? (perhaps a case where hand-crafted/substantially hand-modified assembly is OK, because it can be made /so/ simple - or maybe yaml2obj dwarf support is adequate for this use case?)

Hi, currently, obj2yaml doesn't work properly when dumping the .debug_abbrev/__debug_abbrev section. We might not be able to get the correct output when there are multiple DIEs in one abbrev table. D87179 has fixed it but hasn't been landed yet. I've created one test case for this patch. Hope it works :-)

Based on the signature of the call below this LGTM.

I'm very excited about this feature. Great job on the documentation, both in the help output as for the website. Do you have a potential use case in mind that we could add to the examples?

This is the same change as we did on Github, right? If so this LGTM.

In D62732#2254440, @luismarques wrote:

@labath @jrtc27 @clayborg Now that we have at least 3 open-source debug servers that we can use to test this with (OpenOCD, QEMU gdbstub, gdbserver) perhaps this can be merged? I had very good results using this patch with OpenOCD. This patch doesn't include automated tests, but I'm not sure what tests would be required for this patch, or that it makes sense to require them at this point. I'll be doing more work for LLDB RISC-V support, and I'll provide tests for specific fixes going forward.

LGTM

In D88048#2286251, @dblaikie wrote:

does this classify as a warning? Is there a distinction between ill-formed DWARF and "this is valid, but unlikely to be great"?

The change to use the configuration looks good, but I think Pavel raised a good point. Which tests are affected by this? How much work would it be to convert them as suggested?

What's the scenario where you run the DWARF linker with already-linked binaries?

I think this will be very useful. LGTM!

In D86996#2273445, @labath wrote:

This looks fine (sorry about the delay, I've been OOO).

Nice catch!

Remove spurious change

The reproducer instrumentation part LGTM.

LGTM. This must've been broken for a while. I'll re-enable running the API tests on http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/ so we can catch this earlier.

Thanks! I'll land it for you.

Thanks!

In D87333#2262260, @rupprecht wrote:

In D87333#2262241, @clayborg wrote:

In D87333#2262236, @rupprecht wrote:

In D87333#2262229, @clayborg wrote:

FYI: we switched away from "localhost" a long time ago due to issues with people having a "localhost" entry in their /etc/hosts folder.

I assume you mean people *not* having a "localhost" entry?

I believe the issue was with people having a modified version that replaces localhost with some other IP address.

I didn't even realize that was an option. Yes, this patch would break that. I'll have to take another approach, then.

You might check the "git log" on any files that have "127.0.0.1" in them for details.

Do you happen to have any references (bugs etc.) for those kinds of issues?

Looks like rLLDB202424 is the patch that did most of this. There may be more context in rdar://problem/16154630 but I don't have access to that.

LGTM

In D87243#2261687, @kastiglione wrote:

If an LLVM install disabled LLVM_ENABLE_WARNINGS, should other builds inherit that? I would think no, but is there a precedent for that that to be the case?

Sounds reasonable to me. I'll split it up in two patches. I'll update this one to use the double dashes so we can land the functionality and keep things consistent with the rest of LLDB. I'll create another patch to do the handling of raw commands to use a heuristic.

You need to add LLVM_ENABLE_WARNINGS to LLVMConfig.cmake.in so that the standalone builds know what value was set in the LLVM build. I think with the current patch the other projects won't inherit the value and just default to ON?

Thanks, should be fixed by 426fa35b655ffb8647d9d69580a69627c0d19024

In D86996#2252520, @JDevlieghere wrote:

In D86996#2252246, @labath wrote:

I agree that silly, but maybe the same fix should then be applied to the expr command (and any other command with similar behavior).

Sure, we can use the same trick in other places that use OptionsWithRaw. I'll do that in a separate patch.

In D86996#2252246, @labath wrote:

I agree that silly, but maybe the same fix should then be applied to the expr command (and any other command with similar behavior).

In D86996#2251485, @teemperor wrote:

So CommandObjectRaw does support arguments, but they way it works is that you need to have a delimiter for the 'raw' part which is --. If you have that, then you can just use invoke the normal option parsing like CommandObjectExpression does. The handcrafted implementation here adds a completely new lldb command syntax which has a 'raw' part without the -- which seems inconsistent (not saying that the syntax here is worse or better than script --language foo --, but it's just inconsistent with the way the rest of LLDB works). It's obviously also missing the other quirky things that are part of the command syntax (e.g., quoting arguments isn't supported and so on).

• Remove custom parsing and use the command options insofar possible.
• Require -- as a delimiter when language and code are specified together.

This is great, thanks for taking the time to fix all this.

Other than a small style nit this LGTM

In D86497#2248934, @davezarzycki wrote:

Hello. I have an auto-bisecting multi-stage bot that has identified this change as breaking release (without assertions) testing on Fedora 33 x86-64. Can we get a quick fix or revert this change for now?

FAIL: lldb-shell :: Reproducer/Functionalities/TestImageList.test (68782 of 69683)
******************** TEST 'lldb-shell :: Reproducer/Functionalities/TestImageList.test' FAILED ********************
Script:
--
: 'RUN: at line 6';   /tmp/_update_lc/r/bin/clang --target=specify-a-target-or-use-a-_host-substitution --target=x86_64-unknown-linux-gnu -pthread -fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell /home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/Inputs/stepping.c -g -o /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
: 'RUN: at line 8';   rm -rf /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
: 'RUN: at line 10';   echo "CAPTURE" >> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
: 'RUN: at line 11';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S /tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b  --capture --capture-path /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro     -o 'run'     -o 'image list'     -o 'reproducer generate'     /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out >> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt 2>&1
: 'RUN: at line 17';   echo "REPLAY" >> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
: 'RUN: at line 18';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S /tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b --replay /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro >> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt 2>&1
: 'RUN: at line 20';   cat /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
--
Exit Code: 1

Command Output (stderr):
--
clang-12: warning: argument unused during compilation: '-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell' [-Wunused-command-line-argument]

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  lldb-shell :: Reproducer/Functionalities/TestImageList.test


Testing Time: 125.54s
  Unsupported      : 10774
  Passed           : 58806
  Expectedly Failed:   102
  Failed           :     1
FAILED: CMakeFiles/check-all

• Fix LLDB.framework path
• Don't try to guess the LLDB.framework location