Skip to content

lldb: print formatters format inconsistently #65076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidtwco opened this issue Aug 29, 2023 · 16 comments
Open

lldb: print formatters format inconsistently #65076

davidtwco opened this issue Aug 29, 2023 · 16 comments
Labels

Comments

@davidtwco
Copy link

davidtwco commented Aug 29, 2023

One of rustc's debuginfo tests is failing with newer versions of lldb. I'm testing locally with a build of d43f324.

When printing an instance of std::num::NonZeroI8 (which has a trivial pretty printer in our test suite, included below):

(lldb) print/d nz_i8
(core::num::nonzero::NonZeroI8) $0 = '\v' { __0 = 11 }

The /d suffix seems to only apply to the printing of 11 but not the '\v. I can confirm this by requesting a different formatting and see it only change the 11.

As far as I can tell, it used to apply to both in previous versions (as our test expects $0 = 11 and passes with earlier versions of lldb). Using print or frame variable or any other command I've found doesn't seem to make a difference.

You should be able to reproduce it using the following:

// numeric-types.rs
use std::num::*;

fn main() {
    let nz_i8 = NonZeroI8::new(11).unwrap();
    zzz(); // #break
}

fn zzz() { }

(original)

# lldb_lookup.py
import re

class RustType(object):
    OTHER = "Other"
    STRUCT = "Struct"
    STD_NONZERO_NUMBER = "StdNonZeroNumber"

STD_NONZERO_NUMBER_REGEX = re.compile(r"^core::num::([a-z_]+::)*NonZero.+$")

STD_TYPE_TO_REGEX = {
    RustType.STD_NONZERO_NUMBER: STD_NONZERO_NUMBER_REGEX,
}

def classify_rust_type(type):
    type_class = type.GetTypeClass()
    if type_class == lldb.eTypeClassStruct:
        return classify_struct(type.name, type.fields)
    return RustType.OTHER

def classify_struct(name, fields):
    for ty, regex in STD_TYPE_TO_REGEX.items():
        if regex.match(name):
            return ty

    return RustType.STRUCT

def summary_lookup(valobj, dict):
    # type: (SBValue, dict) -> str
    """Returns the summary provider for the given value"""
    rust_type = classify_rust_type(valobj.GetType())

    if rust_type == RustType.STD_NONZERO_NUMBER:
        return StdNonZeroNumberSummaryProvider(valobj, dict)

    return ""

def StdNonZeroNumberSummaryProvider(valobj, _dict):
    # type: (SBValue, dict) -> str
    objtype = valobj.GetType()
    field = objtype.GetFieldAtIndex(0)
    element = valobj.GetChildMemberWithName(field.name)
    return element.GetValue()

(original providers and original lookup and original types)

$ rustc -g numeric-types.rs
(lldb) type summary add -F -F lldb_lookup.summary_lookup  -e -x -h '^core::num::([a-z_]+::)*NonZero.+$' --category Rust
(lldb) type category enable Rust
(lldb) breakpoint set --file 'numeric-types.rs' --line 5
(lldb) print/d nz_i8

(original)

You can also just clone the rust-lang/rust repository and run ./x.py test tests/debuginfo --test-args numeric-types.

@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2023

@llvm/issue-subscribers-lldb

@DavidSpickett
Copy link
Collaborator

As far as I can tell, it used to apply to both in previous versions (as our test expects $0 = 11 and passes with earlier versions of lldb).

I assume this was released versions, do you happen to know which ones?

@davidtwco
Copy link
Author

As far as I can tell, it used to apply to both in previous versions (as our test expects $0 = 11 and passes with earlier versions of lldb).

I assume this was released versions, do you happen to know which ones?

I noticed it on macOS (Xcode 15.0 Beta 7):

lldb-1500.0.22.5
Apple Swift version 5.9 (swiftlang-5.9.0.128.106 clang-1500.0.40.1)

I built locally to confirm it wasn't a change from Apple's fork, and it doesn't appear to be. Our CI is still on an older version of lldb, so it hasn't affected us yet in that capacity, I'm just getting ahead of the change to make sure everything continues to pass as more users start using newer versions. I presume there is a release version but I don't have time to check at the moment, will update this when I can.

@DavidSpickett
Copy link
Collaborator

Thanks, I'll try a few versions of upstream and bisect it.

@DavidSpickett
Copy link
Collaborator

Reproduced it from main e6260ec49c5d16dc93ebe8b53e645200a0cfc0cd. There are some line breaks but the content is the same.

Had to add import lldb to the formatter and break on line 6 just after the variable was declared.

$ ./bin/lldb /tmp/test.o -o "script import lldb_lookup" -o "type summary add -F lldb_lookup.summary_lookup  -e -x -h '^core::num::([a-z_]+::)*NonZero.+$' --category Rust" -o "type category enable Rust" -o "b test.rs:6" -o "run" -o "print/d nz_i8"
(lldb) target create "/tmp/test.o"
Current executable set to '/tmp/test.o' (aarch64).
(lldb) script import lldb_lookup
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h '^core::num::([a-z_]+::)*NonZero.+$' --category Rust
(lldb) type category enable Rust
(lldb) b test.rs:6
Breakpoint 1: where = test.o`test::main::hb4d3c5bcd986131c + 32 at test.rs:6:5, address = 0x0000000000006de4
(lldb) run
Process 3545387 stopped
* thread #1, name = 'test.o', stop reason = breakpoint 1.1
    frame #0: 0x0000aaaaaaab0de4 test.o`test::main::hb4d3c5bcd986131c at test.rs:6:5
   3
   4    fn main() {
   5        let nz_i8 = NonZeroI8::new(11).unwrap();
-> 6        zzz(); // #break
   7    }
   8
   9    fn zzz() { }
Process 3545387 launched: '/tmp/test.o' (aarch64)
(lldb) print/d nz_i8
(core::num::nonzero::NonZeroI8) '\v' {
  __0 = 11
}

lldb 15 branch:

(lldb) print/d nz_i8
(core::num::nonzero::NonZeroI8) $0 = 11 {
  __0 = 11
}

So something has changed for sure.

@jimingham
Copy link
Collaborator

jimingham commented Aug 29, 2023 via email

@DavidSpickett
Copy link
Collaborator

Spot on.

$ git bisect good
9584ef1b02c0c2f77454c15008cb4a047a91a348 is the first bad commit
commit 9584ef1b02c0c2f77454c15008cb4a047a91a348
Author: Dave Lee <davelee.com@gmail.com>
Date:   Fri Feb 17 19:53:16 2023 -0800

    Recommit "[lldb] Redefine p alias to dwim-print command"

9584ef1

Back on main:

(lldb) print/d nz_i8
(core::num::nonzero::NonZeroI8) '\v' {
  __0 = 11
}
(lldb) expr/d nz_i8
(core::num::nonzero::NonZeroI8) $0 = 11 {
  __0 = 11
}

@kastiglione
Copy link
Contributor

I can see one possible difference between expression and dwim-print. Is there a way to test this with C or C++?

@kastiglione
Copy link
Contributor

kastiglione commented Aug 29, 2023

Update: I have made myself a C++ based test case.

@kastiglione
Copy link
Contributor

Here's what I know so far:

  • expression defaults to printing a persistent result variable (ie $0, $1)
  • conversely, dwim-print defaults to not printing a persistent result
  • When print/d nz_i8 is run with the previous versions of lldb, the d format is being applied to the persistent result
  • When print/d nz_i8 is run with the latest version of lldb, the d format is not being applied to the summary string

The last item seems to make some sense: a summary string can be arbitrary, so how could lldb apply a formatting to an unknown string?

However lldb seems to do exactly that (formatting the summary string) when producing a persistent result. I haven't looked into why the formatting would happen for persistent results but not otherwise.

Thoughts @jimingham?

@jimingham
Copy link
Collaborator

jimingham commented Aug 29, 2023 via email

@kastiglione
Copy link
Contributor

lldb is setting the formatting on the ValueObject that is being returned, and then when the code in the summary formatter does GetValue() for one of the child VO's, the GetValue code checks up the VO hierarchy to see if any parent has a formatting options set, and finds the one set on the top level VO that holds the result.

so the bug is that this logic is happening only for persistent results?

If I run v/d instead of print/d I get the non-formatted output too. I don't think I made any changes to how v works. However this contradicts what @davidtwco said:

Using print or frame variable or any other command I've found doesn't seem to make a difference.

@kastiglione
Copy link
Contributor

@davidtwco

what's the output of?

(lldb) v/d nz_i8

@jimingham
Copy link
Collaborator

jimingham commented Aug 29, 2023 via email

@davidtwco
Copy link
Author

@davidtwco

what's the output of?

(lldb) v/d nz_i8
(lldb) v/d nz_i8
(core::num::nonzero::NonZeroI8) nz_i8 = '\v' { __0 = 11 }

print has changed from being expr -- to the dwim-print by default.

Perhaps unrelated to this bug, but something I forgot to mention (apologies!), I've been adding the following to our test runner prelude to re-enable persistent results since all our tests expected those in the output:

command unalias print
command alias print dwim-print --persistent-result on --
command unalias p
command alias p dwim-print --persistent-result on --

@jimingham
Copy link
Collaborator

jimingham commented Aug 30, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants