Hi Greg,
About the break commit:
commit 9a28ccf836c1b9f0eb5e1163964042eddc207697
Author: chao.an <anc...@xiaomi.com>
Date: Fri Feb 21 09:54:47 2020 +0800
nsh/parse: try the builtin configuration first
In the case of enable the BUILTIN_APPS/FILE_APPS at the same
time, try the builtin list first to ensure that the relevant
configuration(stacksize, priority) can be set normally.
Yes, the change did not consider the case where the file and builtin
application have the same name,
but I think it is too violent to directly revert it ...
The original intention of this change is to solve the problem that the builtin
attribute
(priority and stack) cannot take effect when both the file and builtin
application are enabled.
Think about what should we do if an file application needs a stack size larger
than spawn?
For example,
1. Suppose the program needs a large stack size:
diff --git a/examples/hello/hello_main.c b/examples/hello/hello_main.c
index 953597a..7b832b6 100644
--- a/examples/hello/hello_main.c
+++ b/examples/hello/hello_main.c
@@ -50,6 +50,10 @@
int main(int argc, FAR char *argv[])
{
+ char dummy_array[CONFIG_EXAMPLES_HELLO_STACKSIZE - 2048];
+
+ memset(dummy_array, 0, sizeof(dummy_array));
printf("Hello, World!!\n");
+ sleep(10000);
return 0;
}
2. Check the stack usage, since the default CONFIG_EXAMPLES_HELLO_STACKSIZE
follows CONFIG_TASK_SPAWN_DEFAULT_STACKSIZE are 8192, so there is no problem
nsh> Hello, World!!
ps
PID PPID PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED
FILLED COMMAND
0 0 0 FIFO Kthread N-- Ready 00000000 000000 000000
0.0% Idle Task
2 1 100 FIFO Task --- Running 00000000 008160 002236
27.4% /system/bin/nsh
3 2 100 FIFO Task --- Waiting Signal 00000000 008176 007240
88.5%! hello
nsh>
3. But if we change the stack size to 20480, see what happens:
+CONFIG_EXAMPLES_HELLO_STACKSIZE=20480
nsh> hello &
hello [0:100]
nsh> Hello, World!!
Segmentation fault (core dumped)
Size 8192 is still used in posix_spawn, which will cause a loadable application
stack overflow,
Of course you will say that in this case we can increase the stack size of
posix_spawn,
but in ths case, we will waste more stack space for small applications.
-----------------------------------------------------------------------------------
To resolve this issue I sent a pull request and made a simple fix:
https://github.com/apache/incubator-nuttx-apps/pull/258
1. Bring back the BUILTIN_APPS / FILE_APPS calling sequence,
2. Try FILE_APPS first in the case of builtin:
After patch:
nsh> hello &
hello [3:100]
nsh> Hello, World!!
ps
PID PPID PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED
FILLED COMMAND
0 0 0 FIFO Kthread N-- Ready 00000000 000000 000000
0.0% Idle Task
2 1 100 FIFO Task --- Running 00000000 008160 002300
28.1% /system/bin/nsh
3 2 100 FIFO Task --- Waiting Signal 00000000 020464 019528
95.4%! hello
It is make sense to use the builtin attribute for applications if we enabled
the BUILTIN_APPS / FILE_APPS at the same time.
BRs,
________________________________________
发件人: Gregory Nutt <spudan...@gmail.com>
发送时间: 2020年5月18日 6:59
收件人: dev@nuttx.apache.org
主题: [External Mail]Re: heap malloc when executing binary/elf file
Hi, Brennan,
>>> I just created incubator-nuttx-apps PR 255 that should restore the
>>> correct behavior:
>> https://github.com/apache/incubator-nuttx-apps/pull/255
>>
>> This is a major bug and, if we release do a bugfix release sometime,
>> this should be included.
>>
> I was thinking the same. I created a label backport/9.0 that we can attach
> to PRs that we think should be backported to a bugfix release. I suspect
> especially with the amount of changes that went into this release it won't
> be the only one.
> @btashton btashton added the backport/9.0 label 10 seconds ago
I agree that this should be backported. It is a major bug; a complete
loss of important functionality.
But let's let is simmer for a bit to make sure that it does the job and
doesn't introduce new issues.
Is this your preferred way of handling bugs? Adding the backport/9.0
tag? Optionally, we could just add the commits to the tip of the
releases/9.0 branch where they could be picked up at some later bugfix
release.
I have no preference. At some point, I think we also have to say that
9.0.x "is what it is". Or do you see us continuing to maintain the
release forever? In the dark, pre-Apache past, I had about a two week
period after the release after which it "is what it is". I distrubuted
patches up until that point. What policy would you recommend?
Greg
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from XIAOMI,
which is intended only for the person or entity whose address is listed above.
Any use of the information contained herein in any way (including, but not
limited to, total or partial disclosure, reproduction, or dissemination) by
persons other than the intended recipient(s) is prohibited. If you receive this
e-mail in error, please notify the sender by phone or email immediately and
delete it!******/#
