-
Notifications
You must be signed in to change notification settings - Fork 86
strace 6.12 + aarch64 support #131
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
base: master
Are you sure you want to change the base?
Conversation
|
with the presens of #130, you should probably remove the linux-headers commit in this pr. not that @michaelforney merges prs through github afaik, but nevertheless :p |
👍 done, just to be clear though this will not build without that commit |
|
Awesome. I think you were sufficently clear about this requirement in this PR, so looks good to me. 👍
|
michaelforney
left a comment
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 pushed the 6.12 commit (with config.h regenerated). I have a few minor comments on the aarch64 commit, but it mostly looks good to me.
Thanks for working on this!
pkg/strace/gen.lua
Outdated
| aarch64='aarch64', | ||
| x86_64='x86_64', | ||
| })[config.target.platform:match('[^-]*')] | ||
| if not arch then error('unsupported arch') end |
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.
If we error here, then setup.lua will fail on any other architecture, even if strace isn't selected in config.lua.
Instead, maybe we should just return here, as if strace doesn't exist on those targets?
pkg/strace/gen.lua
Outdated
| '-D IN_STRACE', | ||
| -- it is important that the arch-specific directory is searched first | ||
| '-I $srcdir/src/linux/x86_64', | ||
| '-I $srcdir/src/linux/' .. arch, |
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.
Style nitpick: the rest of the project doesn't use spaces around the .. operator
pkg/strace/gen.lua
Outdated
|
|
||
| local arch_h_count = {} | ||
| arch_h_count['aarch64'] = 1 | ||
| arch_h_count['x86_64'] = 2 |
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 think I prefer just putting these values in the table constructor instead of adding them later.
local arch_h_count = {
aarch64=1,
x86_64=2,
}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 realized I forgot the config.h changes during the update to 6.8, so I added those in a separate commit.
Compared to what I get with a configure, I see several differences here:
diff --git a/pkg/strace/config.h b/pkg/strace/config.h
index d23aaf92..2bbd1090 100644
--- a/pkg/strace/config.h
+++ b/pkg/strace/config.h
@@ -222,7 +222,7 @@
#define HAVE_STRUCT_IOCB_AIO_RW_FLAGS 1
#define HAVE_STRUCT_KBDIACRSUC 1
#define HAVE_STRUCT_KBDIACRUC 1
-/* #undef HAVE_STRUCT_KVM_CPUID2 */
+#define HAVE_STRUCT_KVM_CPUID2 1
#define HAVE_STRUCT_KVM_REGS 1
#define HAVE_STRUCT_KVM_SREGS 1
#define HAVE_STRUCT_KVM_USERSPACE_MEMORY_REGION 1
@@ -258,13 +258,13 @@
#define HAVE_STRUCT_TERMIOS2 1
/* #undef HAVE_STRUCT_TERMIOS_C_ISPEED */
/* #undef HAVE_STRUCT_TERMIOS_C_OSPEED */
-/* #undef HAVE_STRUCT_USER_DESC */
-/* #undef HAVE_STRUCT_USER_DESC_LM */
+#define HAVE_STRUCT_USER_DESC 1
+#define HAVE_STRUCT_USER_DESC_LM 1
#define HAVE_STRUCT_UTSNAME_DOMAINNAME 1
/* #undef HAVE_STRUCT___AIO_SIGSET */
#define HAVE_STRUCT___KERNEL_SOCK_TIMEVAL 1
#define HAVE_STRUCT___KERNEL_TIMESPEC 1
-/* #undef HAVE_STRUCT___OLD_KERNEL_STAT */
+#define HAVE_STRUCT___OLD_KERNEL_STAT 1
#define HAVE_STRUCT___PTRACE_SYSCALL_INFO 1
#define HAVE_SYNC_FILE_RANGE 1
#define HAVE_SYS_EVENTFD_H 1Is this due to running on aarch64 vs x86_64, or maybe the linux headers your system compiler sees are causing configure to detect things differently?
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.
Yes this is from running the script on aarch64 vs x86_64. I should have looked earlier and left a comment.
HAVE_STRUCT_KVM_CPUID2is specific tox86/x86_64HAVE_STRUCT_USER_DESCis specific tox86/x86_64HAVE_STRUCT_USER_DESC_LMis specific tox86_64HAVE_STRUCT___OLD_KERNEL_STATis forx86kernels orx86_64kernels running with 32-bit emulation enabled.
These are all provided by the architecture specific linux headers. I'm not really sure why strace is checking for any of them specifically since checking the architecture being targeted would be sufficient for all of these.
|
I have rebased and hopefully addressed all your comments, let me know if there are further issues! |
Hi, I've recently acquired an aarch64 laptop and wanted to use strace on it. While I was at it I bumped the version. This requires the linux-headers bump, #130, as well.
There are still a few packages that don't build on aarch64 - python, ffmpeg, and dav1d. If I have the time I will look into them as well.