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

lldb-server process crashes when debugging on pcc64le #54520

Closed
tbaederr opened this issue Mar 24, 2022 · 8 comments
Closed

lldb-server process crashes when debugging on pcc64le #54520

tbaederr opened this issue Mar 24, 2022 · 8 comments
Labels
backend:PowerPC crash Prefer [crash-on-valid] or [crash-on-invalid] lldb

Comments

@tbaederr
Copy link
Contributor

Backtrace:

#0  0x00007fffed4d6efc in __pthread_kill_implementation () from /lib64/libc.so.6
#1  0x00007fffed47633c in raise () from /lib64/libc.so.6
#2  0x00007fffed45076c in abort () from /lib64/libc.so.6
#3  0x000000001052b628 in std::__replacement_assert (__file=0x1104bc40 "/usr/include/c++/11/array", __line=188,
    __function=0x1104bac8 "std::array<_Tp, _Nm>::value_type& std::array<_Tp, _Nm>::operator[](std::array<_Tp, _Nm>::size_type) [with _Tp = lldb_private::process_linux::NativeRegisterContextLinux_ppc64le::DREG; long unsigned int"..., __condition=0x1104bab0 "__n < this->size()") at /usr/include/c++/11/ppc64le-redhat-linux/bits/c++config.h:514
#4  0x00000000105dd13c in std::array<lldb_private::process_linux::NativeRegisterContextLinux_ppc64le::DREG, 4ul>::operator[] (this=0x115c7f78, __n=4) at /usr/include/c++/11/array:188
#5  0x00000000100a6f18 in lldb_private::process_linux::NativeRegisterContextLinux_ppc64le::GetWatchpointSize (this=0x115c76f0, wp_index=4)
    at ../source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:648
#6  0x00000000100a7210 in lldb_private::process_linux::NativeRegisterContextLinux_ppc64le::GetWatchpointHitIndex (this=0x115c76f0, wp_index=@0x7fffffffd060: 4, trap_addr=140737353910136)
    at ../source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:673
#7  0x0000000010093f00 in lldb_private::process_linux::NativeProcessLinux::MonitorSIGTRAP (this=0x115c70c0, info=..., thread=...) at ../source/Plugins/Process/Linux/NativeProcessLinux.cpp:690
#8  0x0000000010093098 in lldb_private::process_linux::NativeProcessLinux::MonitorCallback (this=0x115c70c0, pid=539214, exited=false, status=...) at ../source/Plugins/Process/Linux/NativeProcessLinux.cpp:488
#9  0x000000001009bbfc in lldb_private::process_linux::NativeProcessLinux::SigchldHandler (this=0x115c70c0) at ../source/Plugins/Process/Linux/NativeProcessLinux.cpp:1976
#10 0x000000001058b2a0 in lldb_private::process_linux::NativeProcessLinux::NativeProcessLinux(int, int, lldb_private::NativeProcessProtocol::NativeDelegate&, lldb_private::ArchSpec const&, lldb_private::MainLoop&, llvm::ArrayRef<int>)::{lambda(lldb_private::MainLoopBase&)#1}::operator()(lldb_private::MainLoopBase&) const () at ../source/Plugins/Process/Linux/NativeProcessLinux.cpp:323
#11 0x00000000105af330 in std::__invoke_impl<void, lldb_private::process_linux::NativeProcessLinux::NativeProcessLinux(pid_t, int, lldb_private::NativeProcessProtocol::NativeDelegate&, const lldb_private::ArchSpec&, lldb_private::MainLoop&, llvm::ArrayRef<int>)::<lambda(lldb_private::MainLoopBase&)>&, lldb_private::MainLoopBase&>(std::__invoke_other, struct {...} &, lldb_private::MainLoopBase &) (__f=...)
    at /usr/include/c++/11/bits/invoke.h:61
#12 0x00000000105a2bec in std::__invoke_r<void, lldb_private::process_linux::NativeProcessLinux::NativeProcessLinux(pid_t, int, lldb_private::NativeProcessProtocol::NativeDelegate&, const lldb_private::ArchSpec&, lldb_private::MainLoop&, llvm::ArrayRef<int>)::<lambda(lldb_private::MainLoopBase&)>&, lldb_private::MainLoopBase&>(struct {...} &, lldb_private::MainLoopBase &) (__fn=...)
    at /usr/include/c++/11/bits/invoke.h:154
#13 0x0000000010598afc in std::_Function_handler<void(lldb_private::MainLoopBase&), lldb_private::process_linux::NativeProcessLinux::NativeProcessLinux(pid_t, int, lldb_private::NativeProcessProtocol::NativeDelegate&, const lldb_private::ArchSpec&, lldb_private::MainLoop&, llvm::ArrayRef<int>)::<lambda(lldb_private::MainLoopBase&)> >::_M_invoke(const std::_Any_data &, lldb_private::MainLoopBase &) (__functor=...,
    __args#0=...) at /usr/include/c++/11/bits/std_function.h:290
#14 0x0000000010522760 in std::function<void (lldb_private::MainLoopBase&)>::operator()(lldb_private::MainLoopBase&) const (this=0x7fffffffd678, __args#0=...) at /usr/include/c++/11/bits/std_function.h:590
#15 0x000000001005aab8 in lldb_private::MainLoop::ProcessSignal (this=0x7fffffffd9e0, signo=17) at ../source/Host/common/MainLoop.cpp:416
#16 0x0000000010059fd8 in lldb_private::MainLoop::RunImpl::ProcessEvents (this=0x7fffffffd808) at ../source/Host/common/MainLoop.cpp:253
#17 0x000000001005a868 in lldb_private::MainLoop::Run (this=0x7fffffffd9e0) at ../source/Host/common/MainLoop.cpp:403
#18 0x00000000104e9fa0 in main_gdbserver (argc=7, argv=0x7fffffffeb58) at ../tools/lldb-server/lldb-gdbserver.cpp:519
#19 0x000000001050092c in main (argc=7, argv=0x7fffffffeb58) at ../tools/lldb-server/lldb-server.cpp:65

NativeRegisterContextLinux_ppc64le::m_hwp_regs is a std::array of size 4, but the m_max_hwp_supported variable is initialized to 16. Is this intentional?

I understand that m_max_hwp_supported can later be assigned a different value via ReadHardwareDebugInfo(), but that has either not happened or yields 16 anyway.

@tbaederr tbaederr added lldb crash Prefer [crash-on-valid] or [crash-on-invalid] labels Mar 24, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 24, 2022

@llvm/issue-subscribers-lldb

@labath
Copy link
Collaborator

labath commented Mar 24, 2022

PPC64 in lldb is not very well maintained. I don't have the hardware to try it out, but I suppose it's possible that some (newer) ppc64 machines have 16 watchpoints, and that we didn't anticipate that. Are you able to recompile lldb and change the array size to 16? It would be interesting to see if that fixes the crash, and whether you are actually able to set all 16 watchpoints.

If you have gdb around, you could also try setting 16 watchpoints there.

@tbaederr
Copy link
Contributor Author

Making m_max_hwp_supported size 16 fixes the crash. Setting watchpoints doesn't work so well however:

(lldb) watchpoint set variable argc
error: Watchpoint creation failed (addr=0x7fffffffe7c0, size=4, variable expression='argc').
error: Target supports (0) hardware watchpoint slots.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 24, 2022

@llvm/issue-subscribers-backend-powerpc

@tbaederr
Copy link
Contributor Author

@labath Any opinion on just increasing the array size? Or should I investigate why we're trying to access 16 watchpoints in the first place? I can provide patches as well.

@labath
Copy link
Collaborator

labath commented Mar 28, 2022

If watchpoints don't work (you could check with gdb whether they are /supposed to/ work on your machine), then I don't think increasing the array size is very helpful -- I'd be tempted to just yank out the ppc watchpoint support code completely.

Unless, of course, you want to investigate and fix watchpoint support -- that would definitely be better, but I have no idea how much work would that be.

@nikic
Copy link
Contributor

nikic commented Oct 18, 2022

I looked into this again, and I think there are a few things worth noting: The problematic case is the one where the user does not try to make use of hardware watchpoints. In this case ReadHardwareDebugInfo() never gets called and m_max_hwp_supported stays at the default value of 16. When GetWatchpointHitIndex() later gets called (I believe around the time the process exits?) we're still using that default and read outside the bounds of the array.

On the hardware I'm testing (which is likely the same that @tbaederr used) no hardware watchpoints are actually available, or at least that's what num_data_bps in the ptrace PPC_PTRACE_GETHWDBGINFO call reports.

So I think just making the array size and m_max_hwp_supported consistent is all that needs to be done here. Worth noting that the ARM implementation of watchpoints is pretty much exactly the same as on ppc64le, the only notable difference is that they don't have this size mismatch...

I guess the only question is whether the array size should increase to 16 or the default m_max_hwp_supported decrease. I'm not familiar with the hardware capabilities here, but increasing the array size should be the safer option (in case there is hardware with many watchpoints).

@nikic
Copy link
Contributor

nikic commented Oct 18, 2022

Put up https://reviews.llvm.org/D136144 with this change.

@nikic nikic closed this as completed in 6f59f30 Oct 18, 2022
sid8123 pushed a commit to sid8123/llvm-project that referenced this issue Oct 25, 2022
The size of the m_hwp_regs array should match the default value of
m_max_hwp_supported. This ensures that no out-of-bounds accesses
occur, even if the array is accessed prior to a call to
ReadHardwareDebugInfo().

Fixes llvm#54520, see also
there for additional background.

Differential Revision: https://reviews.llvm.org/D136144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC crash Prefer [crash-on-valid] or [crash-on-invalid] lldb
Projects
None yet
Development

No branches or pull requests

5 participants