[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

Tue Mar 15 05:02:46 PDT 2022

labath added a comment.

I agree with everything Jim said.

One of the interesting 'strategy' questions would be the interaction between this feature and the `target.preload-symbols` feature. I don't think the interaction between the two is obvious or intuitive, and I'm even sure that supporting all possible combinations is useful. We could e.g. consider replacing the two settings with one ternary setting with values like "load everything upfront", "load everything the first time it's needed" and "load everything when the module is accessed in a specific way".

Also, I don't think that "run everything with the setting turned on" is a good testing strategy. Nearly all of our tests consist of a single binary and the very first action they do is set a file/line or a function breakpoint. That will immediately trigger a 'hydration' and the rest of the test will just check that the forwarders really forward all calls. Rerunning all tests to catch the few testcases that do something interesting is not a good tradeoff.

We should identify the interesting/important scenarios and write targeted tests for that. The tests in this patch are a pretty good start. My only complaint is that the shared library tests are linux-only. We currently don't have infrastructure in place to write shell tests with shared libraries portably. It may be worth rewriting those into api tests to make use of the existing shared library facilities.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631