-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[LLDB][MIPS] Fix signal SIGBUS number mismatch error on mips target #132688
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
Conversation
@llvm/pr-subscribers-lldb Author: None (yingopq) ChangesMIPS changed the SIGBUS signal number to be the same as other architectures. So we would fail to compile llvm on mips. The relevant modification link is: Full diff: https://github.com/llvm/llvm-project/pull/132688.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp b/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
index eaecc84df15d4..aaada3b1e4c8f 100644
--- a/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
+++ b/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
@@ -61,10 +61,17 @@ void LinuxSignals::Reset() {
AddSignal(5, "SIGTRAP", true, true, true, "trace trap (not reset when caught)");
AddSignal(6, "SIGABRT", false, true, true, "abort()/IOT trap", "SIGIOT");
- AddSignal(7, "SIGBUS", false, true, true, "bus error");
- ADD_SIGCODE(SIGBUS, 7, BUS_ADRALN, 1, "illegal alignment");
- ADD_SIGCODE(SIGBUS, 7, BUS_ADRERR, 2, "illegal address");
- ADD_SIGCODE(SIGBUS, 7, BUS_OBJERR, 3, "hardware error");
+#if defined(mips) || defined(__mips__) || defined(__mips)
+ AddSignal(10, "SIGBUS", false, true, true, "bus error");
+ ADD_SIGCODE(SIGBUS, 10, BUS_ADRALN, 1, "illegal alignment");
+ ADD_SIGCODE(SIGBUS, 10, BUS_ADRERR, 2, "illegal address");
+ ADD_SIGCODE(SIGBUS, 10, BUS_OBJERR, 3, "hardware error");
+#else
+ AddSignal(7, "SIGBUS", false, true, true, "bus error");
+ ADD_SIGCODE(SIGBUS, 7, BUS_ADRALN, 1, "illegal alignment");
+ ADD_SIGCODE(SIGBUS, 7, BUS_ADRERR, 2, "illegal address");
+ ADD_SIGCODE(SIGBUS, 7, BUS_OBJERR, 3, "hardware error");
+#endif
AddSignal(8, "SIGFPE", false, true, true, "floating point exception");
ADD_SIGCODE(SIGFPE, 8, FPE_INTDIV, 1, "integer divide by zero");
@@ -77,7 +84,11 @@ void LinuxSignals::Reset() {
ADD_SIGCODE(SIGFPE, 8, FPE_FLTSUB, 8, "subscript out of range");
AddSignal(9, "SIGKILL", false, true, true, "kill");
- AddSignal(10, "SIGUSR1", false, true, true, "user defined signal 1");
+#if defined(mips) || defined(__mips__) || defined(__mips)
+ AddSignal(7, "SIGUSR1", false, true, true, "user defined signal 1");
+#else
+ AddSignal(10, "SIGUSR1", false, true, true, "user defined signal 1");
+#endif
AddSignal(11, "SIGSEGV", false, true, true, "segmentation violation");
ADD_SIGCODE(SIGSEGV, 11, SEGV_MAPERR, 1, "address not mapped to object", SignalCodePrintOption::Address);
|
But your change appears to use different numbers for MIPS, which looks like the opposite. Also please cite in the PR description the changes in the Linux Kernel that did this. Also I'm pretty sure @labath removed a bunch of Linux MIPS support code at some point due to lack of use. So if you are about to add it all back, it would be good to know what your use case here is. Are you trying to get LLDB on MIPS Linux working again? (we still support MIPS FreeBSD, though the latest FreeBSD release has dropped it) |
ADD_SIGCODE(SIGBUS, 7, BUS_ADRALN, 1, "illegal alignment"); | ||
ADD_SIGCODE(SIGBUS, 7, BUS_ADRERR, 2, "illegal address"); | ||
ADD_SIGCODE(SIGBUS, 7, BUS_OBJERR, 3, "hardware error"); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that this code is also used in the lldb
client, which may be built on a platform other than the target platform. For example, I can debug MIPS Linux remotely from a Mac OS host. It will need to be a runtime check in this case, and the compile time checks relaxed.
If this code only ever runs in lldb-server, then it could be fine.
There has a issue, when we compile LLVM of rust on mips, target we will face an error And now I did not has a clear test case, just found this problem when compiling. |
This needs to be a runtime instead of a build-time choice, because we support debugging foreign architectures, so I guess that what you need is more closer to reverting 93a4553. However, your last comment makes me suspect you don't actually care about lldb+mips working, and just want to get rust building. Is that the case? If it is, then we may just want to make this build in that configuration, but not actually claim to support mips debugging (in theory, you should still be able to debug e.g. x86 core files with a mips build of lldb) |
One way to do that would be to change |
e8cf384
to
1b1285a
Compare
1b1285a
to
6b95040
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I just tweaked the comment a bit. Please also update the endif comment on line 39.
// Now, because we do not support mips debugging, if we compile LLVM on mips | ||
// target, would report error `static assertion failed:Value mismatch for signal | ||
// number SIGBUS`, so add this condition to avoid error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Now, because we do not support mips debugging, if we compile LLVM on mips | |
// target, would report error `static assertion failed:Value mismatch for signal | |
// number SIGBUS`, so add this condition to avoid error. | |
// mips-linux debugging is not supported and mips uses different numbers for some signals (e.g. SIGBUS) on linux, so we skip the static checks below. The definitions here can be used for debugging non-mips targets on a mips-hosted lldb. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@labath please help review again, thanks!
Now, because we do not support mips debugging, if we compile LLVM on mips target, would report error `static assertion failed:Value mismatch for signal number SIGBUS`, so add this condition to avoid error.
The patch (still) looks good. I think what you need is for someone to push the submit button, so I'll do that now. |
…lvm#132688) Now, because we do not support mips debugging, if we compile LLVM on mips target, would report error `static assertion failed:Value mismatch for signal number SIGBUS`, so add this condition to avoid error.
…lvm#132688) Now, because we do not support mips debugging, if we compile LLVM on mips target, would report error `static assertion failed:Value mismatch for signal number SIGBUS`, so add this condition to avoid error.
Now, because we do not support mips debugging, if we compile LLVM on mips target, would report error
static assertion failed:Value mismatch for signal number SIGBUS
, so add this condition to avoid error.