[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D121967#3403886 <https://reviews.llvm.org/D121967#3403886>, @zequanwu wrote:

>> Can you say (in english) what are the properties that you are trying to check there? Maybe we can find a better way to do that...
>
> I'm trying to check for local variables values in inline functions by printing them (see inline comment on `PdbAstBuilder::ParseBlockChildren`). Since this test only runs on x64, I think I will change the live debugging test to a cpp file.

Sorry about the delay. This dropped off my radar.

If the goal is checking for local variables, then I'd say the test is way too strict for that. In particular, moving through code (even unoptimized code) with step instructions is very sensitive to codegen changes. And the hardcoded line numbers everywhere will make life hard for whoever needs to update this tests.

Please see inline comments on how I'd approach a local variable test. I use the `always_inline` attribute, which allows you to create inline functions in unoptimized builds. This greatly reduces our exposure to codegen cleverness. If building with -O0, then the optnone attribute is not necessary, but I've left it there to demonstrate how that can be used to keep a variable live in optimized builds. (It also provides a good anchor to place a breakpoint). I am also using breakpoints for moving through the code. Breakpoints are more reliable as they're usually not affected by instruction reordering (mainly important for optimized code, but a good practice in any case). I set the breakpoint's via markers left in the source code, which allows me to avoid using any line numbers. I am deliberately not matching the entire block that lldb prints when the process stops, as that is not relevant for this test.

I wasn't sure what was the exact setup you wanted to check (one problem with overly strict tests), so I've created something simple but still fairly similar to your setup. It should be fairly easy to tweak the test to do check other things as well.



================
Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:7
+// RUN:     %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+inline int bar(int bar_param) {
----------------
```
void __attribute__((optnone)) use(int) {}

void __attribute__((always_inline)) bar(int param) {
  use(param); // BP_bar
}

void __attribute__((always_inline)) foo(int param) {
  int local = param+1;
  bar(local);
  use(param); // BP_foo
  use(local);
}

int main(int argc) {
  foo(argc);
}
```



================
Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:24
+
+// CHECK:      (lldb) b main
+// CHECK-NEXT: Breakpoint 1: where = {{.*}}`main + 4 [inlined] foo at {{.*}}:14
----------------
```
br set -p BP_bar
br set -p BP_foo
run
(CHECK: stop reason = breakpoint 1)
(CHECK: frame #0: {{.*}}`main [inlined] bar)
p param
continue
(CHECK: stop reason = breakpoint 2)
(CHECK: frame #0: {{.*}}`main [inlined] foo)
p param
p local
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967