Skip to content
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

[BUG] LLDB's bundled ncurses6 can't read terminal database on Linux/Darwin #1565

Closed
rprichard opened this issue Aug 20, 2021 · 24 comments
Closed
Assignees
Labels

Comments

@rprichard
Copy link
Collaborator

rprichard commented Aug 20, 2021

macOS

When I start LLDB on macOS (11.5.1 "Big Sur"), it fails with a terminal error:

$ export DYLD_LIBRARY_PATH=$HOME/android-ndk-r23/toolchains/llvm/prebuilt/darwin-x86_64/python3/lib 
$ ./android-ndk-r23/toolchains/llvm/prebuilt/darwin-x86_64/bin/lldb
terminals database is inaccessible
$

I don't think I'm supposed to need DYLD_LIBRARY_PATH, but if I remove it, I get a different failure (#1566).

I don't know if this affects Android Studio, but hopefully not? The current version of Android Studio I have installed on macOS doesn't have a bundled ncurses. IIRC we recently started bundling ncurses with Studio's LLDB. Instead, the binaries I have use the system ncurses:

/Applications/Android Studio.app/Contents/bin/lldb/lib64$ otool -L liblldb.11.0.4git.dylib 
liblldb.11.0.4git.dylib:
	@rpath/liblldb.11.0.4git.dylib (compatibility version 0.0.0, current version 11.0.4)
	@rpath/libpython3.8.dylib (compatibility version 3.8.0, current version 3.8.0)
	/usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
	/usr/lib/libform.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
	/usr/lib/libpanel.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
	@rpath/libxml2.2.9.10.dylib (compatibility version 0.0.0, current version 2.9.10)
	@rpath/libedit.0.dylib (compatibility version 1.0.0, current version 1.61.0)
        ...

(Studio's lib64 directory has a libedit.0.dylib and a libpython3.8.dylib in it.)

Linux

When I start LLDB on Linux, I see this prompt.

$ cd prebuilts/clang/host/linux-x86/clang-r428724
$ LD_LIBRARY_PATH=$PWD/python3/lib ./bin/lldb
Cannot read termcap database;
using dumb terminal settings.
(lldb) 

(I tested with CentOS 8 and Ubuntu 20.04, in addition to gLinux.)

termcap is obsolete in favor of terminfo, but I don't think that's the issue. I think we're using the AOSP libedit (which seems to be a tarball from Apple? corrected below), which depends on the ncurses we're shipping.

I assume the error is coming from here:
https://android.googlesource.com/platform/external/libedit/+/refs/heads/llvm-r416183/src/terminal.c#869

If I delete the DSOs, the error goes away:

$ rm lib64/libncurses.so.6 lib64/libpanel.so.6 lib64/libform.so.6 
$ LD_LIBRARY_PATH=$PWD/python3/lib ./bin/lldb
(lldb) 

Line editing largely works anyway. I noticed that long-line editing was broken. This line is suspicious:
https://android.googlesource.com/platform/external/libedit/+/refs/heads/llvm-r416183/src/terminal.c#875

Example:

  • Resize terminal to 120 columns.

  • Type Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est lacus, iaculis a diam eget, accumsan blandit arcu. and press Enter.

  • Press Up, Down, Up, Down, etc.

  • The output looks like:

    $ LD_LIBRARY_PATH=$PWD/python3/lib ./bin/lldb
    Cannot read termcap database;
    using dumb terminal settings.
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    lacus, iaculis a diam eget, accumsan blandit arcu.
    error: 'Lorem' is not a valid command.
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    (lldb)                                                                                                                 
    (lldb) Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean vestibulum purus sed vestibulum euismod. Sed est 
    lacus, iaculis a diam eget, accumsan blandit arcu.
    

This example works correctly if the DSOs are deleted.

@rprichard rprichard added the bug label Aug 20, 2021
@rprichard rprichard changed the title [BUG] LLDB's bundled ncurses.so.6 can't read terminal database on Linux [BUG] LLDB's bundled ncurses6 can't read terminal database on Linux/Darwin Aug 20, 2021
@rprichard
Copy link
Collaborator Author

The LLDB from the upstream clang+llvm-12.0.0-x86_64-linux-gnu-ubuntu-20.04 package uses the libncurses6 DSOs (libncurses.so.6, libform.so.6, libpanel.so.6, libtinfo.so.6), but there's no DT_NEEDED for *edit* or *readline*. It has no line editing. (I've been using it to test e.g. https://llvm.org/PR50935, so I use rlwrap lldb to get line editing.)

@rprichard rprichard self-assigned this Aug 20, 2021
@rprichard
Copy link
Collaborator Author

IIRC we recently started bundling ncurses with Studio's LLDB.

I'm thinking of http://b/172971586.

@enh-google
Copy link
Collaborator

I think we're using the AOSP libedit (which seems to be a tarball from Apple?), which depends on the ncurses we're shipping.

shouldn't be... METADATA says

  url {
    type: HOMEPAGE
    value: "https://www.thrysoee.dk/editline/"
  }
  url {
    type: ARCHIVE
    value: "https://www.thrysoee.dk/editline/libedit-20191025-3.1.tar.gz"
  }

that project in turn seems to be tracking the BSD upstream (http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit).

(so we are out of date. this is an interesting case, where even if hhb was still around to run the script, we wouldn't catch this project because it's not in the AOSP manifest. [i don't think we can handle arbitrary download URLs anyway, only github, so there are many problems here. TL;DR: if you do update libedit, please keep METADATA up to date :-) ])

This line is suspicious:
https://android.googlesource.com/platform/external/libedit/+/refs/heads/llvm-r416183/src/terminal.c#875

it looks like they already have the code they need in terminal_get_size() in the same file, they're just not calling it. i know it's not the long-term fix, but might be worth sending a patch for that? (even if it doesn't cover SIGWINCH, it's still going to be right in more cases than the code currently is :-) ) hmm... looks like they already do that at the end of the function you linked to with the = 80...

				/* get the correct window size */
	(void) terminal_get_size(el, &lins, &cols);

@rprichard
Copy link
Collaborator Author

I think we're using the AOSP libedit (which seems to be a tarball from Apple?), which depends on the ncurses we're shipping.

shouldn't be... METADATA says

Oh, it seems I searched for "libedit 20191025", found this macports link, then confused macports with Apple's own tarballs. I didn't realize there was a URL in the METADATA file. That's helpful.

@enh-google
Copy link
Collaborator

yeah, not all METADATA files have that information, but it's the goal! any time you do the detective work to find where some code came from, i'm happy to +2 a CL that adds the corresponding METADATA. especially if it's a github URL and SHA, because then hhb's update script can do its magic...

@enh-google
Copy link
Collaborator

(fyi, looks like libedit is no longer in the lldb-master-dev manifest. there is a newer version at https://www.thrysoee.dk/editline/ but since afaik no-one's using the version we have, i haven't updated it.)

@enh-google
Copy link
Collaborator

(fyi, looks like libedit is no longer in the lldb-master-dev manifest. there is a newer version at https://www.thrysoee.dk/editline/ but since afaik no-one's using the version we have, i haven't updated it.)

@pirama-arumuga-nainar says this isn't true, so someone probably should update that...

@pirama-arumuga-nainar
Copy link
Collaborator

FWIW, with llvm-toolchain built locally on an M1, lldb.sh works:

$ out/install/darwin-x86/clang-dev/bin/lldb.sh
(lldb) 
error: empty command
(lldb) 

IIUC, we want to be bundling our own ncurses because the build servers may not have them and also to avoid difference across different OS versions.

We should update external/libncurses and check if a prebuilt from a non-M1 mac works fine on an M1 Mac.

@pirama-arumuga-nainar
Copy link
Collaborator

lldb is still broken after the ncurses upgrade. (In build https://ci.android.com/builds/branches/aosp-llvm-toolchain/grid?head=9269573&tail=9269573).

This issue, and the dependent #1749, are part of r25c. @rprichard Do you have any other pointers for this?

@enh-google
Copy link
Collaborator

lldb is still broken after the ncurses upgrade.

define "broken"? i think there are several issues on this bug from "lldb won't start because it can't find a needed library" to "line editing doesn't work right". which end of that spectrum are you on? (and if the former, what does your otool output look like?)

@pirama-arumuga-nainar
Copy link
Collaborator

Aah sorry for being vague :). I meant the terminal database issue and it's more pressing on Darwin where it fails to start with the libncurses that we bundle in the prebuilts.

$ clang-9269573/bin/lldb.sh
terminals database is inaccessible
$ TERMINFO=/usr/share/terminfo clang-9269573/bin/lldb.sh
'xterm-256color': unknown terminal type.

@DanAlbert
Copy link
Member

I don't suppose that message is just a warning and it's exiting for an unrelated reason without an error message?

@enh-google
Copy link
Collaborator

/me wonders whether ncurses is configured correctly for the mac? i know from my terminal emulator writing days that darwin stores terminfo slightly differently to everyone else (a bug where they have %d instead of %c when constructing the path, iirc?), so maybe there's a configuration knob that needs tweaking? what does the equivalent of strace say between a working and a non-working lldb?

@pirama-arumuga-nainar
Copy link
Collaborator

pirama-arumuga-nainar commented Nov 11, 2022

After fighting with Darwin SIP for a day, dtruss confirmed enh's guess. ncurses was looking at /usr/share/terminfo/x instead of /usr/share/terminfo/78 (78 == ascii 'x'). However, I'm not sure if it's related to disabling SIP, now ncurses no longer complains 🤷‍♂️ . I'll enable SIP and try next week.

In the meantime, @enh-google , can you try if lldb.sh gets to the shell prompt in either of the two builds?

@enh-google
Copy link
Collaborator

despite having santa set to "minimal protection", i get the "“lldb” cannot be opened because the developer cannot be verified" dialog...

@enh-google
Copy link
Collaborator

actually, the usual xattr -r -d com.apple.quarantine . dance fixes that.

okay, on M1 running 12.6.1 i get "terminals database is inaccessible", whereas on x86-64 running 13.0.1 i get "xterm-256color: unknown terminal type".

@gsingh93
Copy link

gsingh93 commented Apr 8, 2023

Is there any known workaround for this issue? I'm seeing the same behavior described above:

$ ./lldb.sh
terminals database is inaccessible
$ TERMINFO=/usr/share/terminfo ./lldb.sh
'xterm-256color': unknown terminal type.

macOS 13.3, NDK version 25.2.9519653 installed through the SDK manager from Android Studio.

@YungRaj
Copy link

YungRaj commented Apr 18, 2023

I was experiencing this issue while debugging root daemons and system processes (e.g. surfaceflinger) on a device/emulator on macOS 13.x

I used gdbclient.py's --setup-forwarding argument as a workaround, so that (I presume) the python script doesn't have to take over the terminal's input and output, but it requires entering the commands yourself in lldb in order to attach to the gdb/lldbserver on the device/emulator

(the script provides the commands for you if you use --setup-forwarding)

@pirama-arumuga-nainar
Copy link
Collaborator

The 78 vs x difference is because Mac has a case-insensitive file system where the subdirs of /usr/share/terminfo are ascii values ('78') instead of letters ('x'). The ncurses configure script detects this based on the filesystem it's running on. Unfortunately, the android builds run on a separate volume with a case-sensitive file system, breaking this detection. I was able to fix/override this detection by setting the --disable-mixed-case configure flag.

The other issue with TERMINFO dir can be fixed by setting --with-default-terminfo-dir.

Fix is in https://android-review.googlesource.com/c/toolchain/llvm_android/+/2583755. I'll verify a post-submit build and then plan to fix this at the very least in r26 if not in an r25 point release.

@enh-google
Copy link
Collaborator

Mac has a case-insensitive file system

(oh, is that why they do that? i'd always assumed it was just a %d versus %c bug in their implementation that they couldn't change for backwards compatibility!)

@pirama-arumuga-nainar
Copy link
Collaborator

Mac has a case-insensitive file system
(oh, is that why they do that?

Yea, the actual rationale was sprinkled across a few different files. The help-text for the mixed-case option did not imply it relates to how TERMINFO is found 👎 .

Is there any known workaround for this issue?

I did stumble upon one yesterday.

$ mkdir -p ~/.terminfo
$ cd ~/.terminfo
$ ln -s /usr/share/terminfo/78 x

This should work for terminal names that start with an x. For other terminals, replace the last command with the starting letter and the corresponding ascii value (in hex).

@enh-google
Copy link
Collaborator

(yeah, i've been using the symlink workaround for about 20 years now :-) )

jrguzman-ms pushed a commit to msft-mirror-aosp/platform.prebuilts.clang.host.darwin-x86 that referenced this issue May 12, 2023
Bug: android/ndk#1565

This is from build 10104233 with aosp/2583755

TEst: manually run bin/lldb.sh
Change-Id: I918a650a8b150f966578cec28ab8a0f8385795a1
jrguzman-ms pushed a commit to msft-mirror-aosp/platform.prebuilts.clang.host.linux-x86 that referenced this issue May 12, 2023
Bug: android/ndk#1565

This is from build 10104233 with aosp/2583755

Test: manually run bin/lldb.sh
Change-Id: Ibadb51f8955ddef76298f4cdf730e0b30d54c650
@github-project-automation github-project-automation bot moved this from Triaged to Merged in NDK r26 May 12, 2023
@appujee
Copy link
Collaborator

appujee commented May 15, 2023

@stephenhines
Copy link
Collaborator

I think this is the right CL - https://android-review.googlesource.com/c/toolchain/llvm_android/+/2583755

kongy pushed a commit to kongy/llvm_android that referenced this issue Jun 2, 2023
Bug: android/ndk#1565

1. Mac has a case-insensitive file system where the subdirs of
   /usr/share/terminfo are ascii values ('78') instead of letters ('x').
   The ncurses configure script detects this based on the filesystem
   it's running on.  Unfortunately, the android build servers run on a
   separate volume with a case-sensitive file system, breaking this
   detection.  Override by setting --disable-mixed-case config flag.

2. Set --with-default-terminfo-dir to always read from
   /usr/share/terminfo for all platforms.

Test: lldb.sh opens successfully on a Mac
Change-Id: I1b9da0093be44331b80b6b21cbe3254de8f53408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Merged
Development

No branches or pull requests

8 participants