-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
modlib: preprocess gnu-elf.ld for executable ELF #15444
Conversation
[Experimental Bot, please feedback here] Yes, this PR appears to meet the basic NuttX requirements, although it could be more thorough. While it addresses the key points, it lacks specific details in several areas which would make review easier. Here's a breakdown: Strengths:
Weaknesses:
Recommendation: The PR author should add the missing details to strengthen it. Specifically:
|
Seems that CI is blocked by issue similar to #15441? |
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.
Please include a Documentation/ at qemu page showing a usage example
This allows the header file to be useful from non-C sources such as assembly code or linker scripts. Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
@acassis, the usage is rather simple: just enable BINFMT_ELF_EXECUTABLE config for a kernel build. Actually this isn't new or specific to QEMU, I used it before for k230 devices, all canmv230 kernel mode have it enable by default, see pull/11513 for the old story. |
Thank you for explanation, I thought it was specific for QEMU. BTW, but you should create a board config demo and include it in the sim page |
@acassis I am unsure if |
Hi yf13,
ELF works in flat mode as well:
https://www.youtube.com/watch?v=oL6KAgkTb8M
Unless some recent modification has changed it.
BR,
Alan
…On Thu, Jan 9, 2025 at 8:33 PM yf13 ***@***.***> wrote:
BTW, but you should create a board config demo and include it in the sim
page
@acassis <https://github.com/acassis> I am unsure if sim device has
KERNEL build because existing defconfigs there are all for FLAT build. My
current understanding is that executable ELFs depend on SYSCALL thus not
proper for FLAT build?
—
Reply to this email directly, view it on GitHub
<#15444 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAJBCA3XJFJ236WFWXC2D32J4BM5AVCNFSM6AAAAABUXA6KQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBRGQ2DQNJYGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@acassis, thanks. Here I tried I then tried to blindly enable BINFMT_ELF_EXECUTABLE after selecting So if there is a way to make executable ELFs for sim, please teach. |
This generates gnu-elf.ld via preprocessing of gnu-elf.ld.in so that to reduce board specific app linker scripts in kernel mode when BINFMT_ELF_EXECUTABLE is enabled. Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
This avoids using `-r` option when linking executable programs. Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
This allows using BINFMT_ELF_EXECUTABLE for qemu-armv7a target. Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
Hi YF, I never tried it on SIM, only did it on ARM (STM32) some years ago. |
Sorry @yf13: This PR is causing "Instruction page fault" for
|
Lup,
Here I saw `apps/bin/init` is of relocatable type but the BINFMT_ELF_EXECTUABLE is selected in `rv-virt:knsh64`? This mismatch will cause trouble. I guess this issue might be there even before this PR.
I haven't used this old PC for months so need fix the github access before I can send a fix for rv-virt.
Regards,
yf
|
Thank you so much YF! I think we need a NuttX Mirror to make things easier for you :-) |
Fixed by YF: Thank you so much YF :-)
|
Sorry again @yf13: Static Variables don't seem to work correctly for Here's the code that prints a Static Var: hello_main.c static char test_static[] = "Testing Static Var";
int main(int argc, FAR char *argv[]) {
printf("test_static=%s\n", test_static);
printf("Address of test_static=%p\n", test_static);
This might explain why
|
@lupyuen, thanks, but I got different test result here: $ git diff
--- a/examples/hello/hello_main.c
+++ b/examples/hello/hello_main.c
@@ -28,6 +28,7 @@
/****************************************************************************
* Public Functions
****************************************************************************/
+static const char *NAME = "NuttX";
/****************************************************************************
* hello_main
@@ -35,6 +36,6 @@
int main(int argc, FAR char *argv[])
{
- printf("Hello, World!!\n");
+ printf("Hello, World of %s!!\n", NAME);
return 0;
} Check with rv-virt:knsh: $ qemu-system-riscv32 -M virt,aclint=on -semihosting -nographic -kernel nuttx
nsh> hello
Hello, World of NuttX!! The commit string in uname might be a different story. |
@yf13 What about knsh64?
|
@yf13 Can you also try static char test_static[] = "Testing Static Var";
static const char test_static_const[] = "Testing Static Const Var";
int main(int argc, FAR char *argv[]) {
printf("test_static=%s\n", test_static);
printf("Address of test_static=%p\n", test_static);
printf("test_static_const=%s\n", test_static_const);
printf("Address of test_static_const=%p\n", test_static_const);
test_static=
Address of test_static=0xc0100200
test_static_const=Testing Static Const Var
Address of test_static_const=0xc0001ee0 |
I removed the $ git diff examples/hello
diff --git a/examples/hello/hello_main.c b/examples/hello/hello_main.c
index 2cd6cb400..270f3f453 100644
--- a/examples/hello/hello_main.c
+++ b/examples/hello/hello_main.c
@@ -28,6 +28,7 @@
/****************************************************************************
* Public Functions
****************************************************************************/
+static char *NAME = "NuttX";
/****************************************************************************
* hello_main
@@ -35,6 +36,6 @@
int main(int argc, FAR char *argv[])
{
- printf("Hello, World!!\n");
+ printf("Hello, World of %s!!\n", NAME);
return 0;
}
$ cd ../nuttx
$ qemu-system-riscv64 -M virt,aclint=on -semihosting -nographic -kernel nuttx
nsh> hello
Hello, World of NuttX!! |
Hmmm which commit for NuttX and Apps are you using? Lemme try your branch. Thanks :-) |
Erm @yf13 could you try static char test_static[] = "Testing Static Var"; //// TODO: Added this
static const char test_static_const[] = "Testing Static Const Var"; //// TODO: Added this
static char *NAME = "NuttX";
test_static=
Address of test_static=0xc0100200
test_static_const=Testing Static Const Var
Address of test_static_const=0xc0001f10
Hello, World of NuttX!!
Address of NAME=0xc0001e10 The reason why I chose nuttx/libs/libc/misc/lib_utsname.c Lines 53 to 60 in a2d4d74
|
I finally reproduced it here with The issue looks like somehow initialized data section is not properly populated. I don't see how it relates to this PR as current rv-virt kernel build is still using board specific linker script. I will spend time later on this and update if I have findings. Do you know if this PR is the first broken commit? |
The problem doesn't happen for the commit just before this PR. Thanks for looking into this :-) https://gist.github.com/lupyuen/21167b5ce4f2c2e8b23e2057dcefbe64
Update: Milk-V Duo S / SG2000 still works OK today for
|
It seems that Criteria I used as a good commit:
|
Sorry @yf13 I can't reproduce the bug with
I compiled NuttX for 5076b0c:
|
How about the The ET_REL support seems working since then, but we need find out when ET_EXEC type starts having the issue. |
Sorry @yf13 I don't quite understand, what are the exact steps I need to run to reproduce the Static Vars bug? I'm not sure if the file type of Do we agree that 5076b0c and #14064 are not faulty, since they tested OK with |
Hi @yf13: About ET_EXEC vs ET_REL, could you post this as a NuttX Issue for the NuttX Community to decide? Perhaps also link the issue to the Mailing List? Meanwhile I think it's important to fix the Static Vars, since it will break a lot of code. Would you consider rolling back your PR? Thanks :-) |
I am hesitate to hide the issue, please allow more investigation, we'd better find the root cause. |
But shall we revert the PR for now? I'm hoping we can clear the bug before the Lunar New Year holidays, so everyone can be a little less stressed thanks :-) |
Fixed here yay! 🎉 |
Summary
This allows apps for kernel mode NuttX (e.g. on arm/qemu-armv7a) can be built as executable type ELF files. Benefits of executable programs are smaller in size and easier GDB debugging.
Initial scope was only for qemu-armv7a but it used an almost duplicated
gnu-elf.ld
which differs only at.text
and.data
addresses from thegnu-elf.ld
in modlib.To avoid such duplications, a preprocessed
gnu-elf.ld
in modlib is added so that to adapt to target config. This requires minor tweaks foraddrenv.h
so that some macros can be included ingnu-elf.ld.in
.Impacts
Hardware: qemu-armv7a or using kernel build.
Testing
qemu-armv7a:knsh
,bl602evb:elf
.