You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/01 08:21:51 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #933: sim: Suppress "has no symbols" warnings for macOS
yamt opened a new pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933
## Summary
https://github.com/apache/incubator-nuttx/issues/920
## Impact
## Testing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418483568
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
We hit the similar issue with ceva toolchain before, and find a general solution:
# cofflib -r will core dump if the library contain an object file
# without any public symbol, so let's always generate a dummy global
# function to overcome this issue.
ARCHCPUFLAGS += -DCKSUM_SELF=$(if $<,$(word 1,$(shell cksum $<)))
ARCHCPUFLAGS += -include dummy.h
Here is two related files:
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/ceva/src/tl4/Toolchain.defs
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/ceva/src/tl4/dummy.h
Without a general solution, we have to fix the same issue again and again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418498617
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
Anyway, we need a general(no ad-hoc) solution regardless it's a toolchian or nuttx problem.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418623493
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
The check result show that #933 work as expect:
https://github.com/apache/incubator-nuttx/pull/936/checks?check_run_id=636779093
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418545299
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
@yamt and @xiaoxiang781216 what is the consensus? Should we merge it or wait for a more general solution?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418491029
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
my impression is that it's a problem in nuttx, not toolchain.
(well, it should not core dump though)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418483568
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
We hit the similar issue with ceva toolchain before, and find a general solution:
```
# cofflib -r will core dump if the library contain an object file
# without any public symbol, so let's always generate a dummy global
# function to overcome this issue.
ARCHCPUFLAGS += -DCKSUM_SELF=$(if $<,$(word 1,$(shell cksum $<)))
ARCHCPUFLAGS += -include dummy.h
```
Here is two related files:
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/ceva/src/tl4/Toolchain.defs
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/ceva/src/tl4/dummy.h
Without a general solution, we have to fix the same issue again and again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418863914
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
Yes, it's too quick. How about you reopen PR and revert my patch with more better fix?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418568136
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
@yamt and @acassis here is the PR I suggest: https://github.com/apache/incubator-nuttx/pull/933.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418824716
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
i feel we merge patches too quickly. can we wait for at least one day when a patch is controversial?
the merged patch is IMO too much hack. it even pollutes the end binary.
if you don't like my approach in this patch, you can just remove the warnings in the wrapper script instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418641648
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
No, we just need merge one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #933: sim: Suppress "has no symbols" warnings for macOS
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #933:
URL: https://github.com/apache/incubator-nuttx/pull/933#discussion_r418628827
##########
File path: binfmt/Makefile
##########
@@ -47,6 +47,7 @@ BINFMT_ASRCS =
BINFMT_CSRCS = binfmt_globals.c binfmt_initialize.c binfmt_register.c binfmt_unregister.c
BINFMT_CSRCS += binfmt_loadmodule.c binfmt_unloadmodule.c binfmt_execmodule.c
BINFMT_CSRCS += binfmt_exec.c binfmt_copyargv.c binfmt_dumpmodule.c
+BINFMT_CSRCS += dummy.c
Review comment:
@xiaoxiang781216 Does PR #936 replace this one? I merged that one. Or should this one be merged too?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org