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 2022/05/11 09:13:22 UTC
[GitHub] [incubator-nuttx] pussuw opened a new pull request, #6235: RISC-V: add C++ support to crt0
pussuw opened a new pull request, #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235
## Summary
Adds C++ support to CRT
## Impact
## Testing
icicle:knsh
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871865559
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
yes, so maybe we can remove
```
extern "C"
{
```
wrapper?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r874596359
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,53 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* Linker defined symbols to .ctors and .dtors */
+
+extern void (*_sctors)(void);
+extern void (*_ectors)(void);
+extern void (*_sdtors)(void);
+extern void (*_edtors)(void);
Review Comment:
Yes, I am sure. I tested it and the file is a .c file, so it is compiled with gcc always. This file could be pre-compiled and it would still work for C++ just by linking the crt0.o object. This is how many toolchains in fact do it.
Declaring these IMO don't belong to any public header, these are part of the CRT. I think this is the correct place, or optionally we can add another crtX.c file.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871860412
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
The `EXTERN` is undefined at the end of each header file... So maybe we can add
```
#if defined(__cplusplus)
#define EXTERN extern "C"
#else
#define EXTERN extern
#endif
```
on top of this file
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#issuecomment-1128523304
I think this file is not even compiled by CI so the build error is not my doing.
(Reason I think this file is not compiled: it is compiled by make import which I don't think is part of CI?)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871823170
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
Yes, C++ applications use the same CRT
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871859487
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
Ok. I didn't know that. So maybe we can just use `EXTERN` instead of `extern`?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870733040
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
We can always do the big unification later. This does patch not break anything, and adds only a few lines of code that makes C++ work for build kernel.
I'm already doing one massive unification task, I'd prefer not to take another.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#issuecomment-1129272306
Please rebase on top of the latest mainline to fix the CI issues
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870673875
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
The right fix is similar how you are doing for atexit:
1. Move ctors/dtors from binfmt_s to task_info_s
2. And then call it from the userspace entry point before and after main
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871861034
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
Or move those externs to `riscv_internal.h` and place somewhere near
```
EXTERN uint32_t _stext; /* Start of .text */
EXTERN uint32_t _etext; /* End_1 of .text + .rodata */
EXTERN const uint32_t _eronly; /* End+1 of read only section (.text + .rodata) */
EXTERN uint32_t _sdata; /* Start of .data */
EXTERN uint32_t _edata; /* End+1 of .data */
EXTERN uint32_t _sbss; /* Start of .bss */
EXTERN uint32_t _ebss; /* End+1 of .bss */
EXTERN uint32_t _stdata; /* Start of .tdata */
EXTERN uint32_t _etdata; /* End+1 of .tdata */
EXTERN uint32_t _stbss; /* Start of .tbss */
EXTERN uint32_t _etbss; /* End+1 of .tbss */
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871858319
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
I did it this way because the code style check complains if they are not indented. Other similar (c++) files do it this way 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870065738
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -124,9 +189,11 @@ void _start(int argc, char *argv[])
/* Call C++ constructors */
+ exec_ctors();
+
/* Setup so that C++ destructors called on task exit */
- /* REVISIT: Missing logic */
+ atexit(exec_dtors);
Review Comment:
This depends on: https://github.com/apache/incubator-nuttx/pull/6197
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r874433128
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
Done. Seems to work / the name mangling does not seem to break anything.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870729027
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
You need keep consider the flat and protected build when you develop kernel build feature.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870449898
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
I did, and that is a dead end. They run the code in kernel space with kernel privileges, ctors and dtors are user code, so they must be in the user CRT.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871841569
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
It is the process entry point, binfmt loads an .elf file and calls the entry point. Every elf file is compiled either with gcc or g++.
It does not matter if the name of start is name mangled, as it is not really "called", a jump to the address of _start is performed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871816120
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
do we expect to compile this file with CXX?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870686163
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -124,9 +189,11 @@ void _start(int argc, char *argv[])
/* Call C++ constructors */
+ exec_ctors();
+
/* Setup so that C++ destructors called on task exit */
- /* REVISIT: Missing logic */
+ atexit(exec_dtors);
Review Comment:
Maybe do the moving in a different patch
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871862552
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
I guess this will work as well, yes. I can do it like this but it goes to next week. For the ctors/dtors I don't think it will help, they will still have the funny indent. Unless we add
```
extern "C"
{
```
Which might still cause the code style check to produce an error
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871861299
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
does nxstyle complains here as well?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870673875
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
The right fix is similar how you are doing for atexit:
1. Move ctors/dtors from binfmt_s to task_info_s
2. And then call it from the userspace entry point before main
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#issuecomment-1129750093
@xiaoxiang781216 I do not have any comments. Pleas merge if your questions are addressed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870733040
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
We can always do the big unification later. This patch does not break anything, and adds only a few lines of code that makes C++ work for build kernel.
I'm already doing one massive unification task, I'd prefer not to take another.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870729027
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
You need keep consider the flat and protected build when you develop any kernel build feature, and find a solution to support three build with a reasonable and consistent way.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r874592483
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,53 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* Linker defined symbols to .ctors and .dtors */
+
+extern void (*_sctors)(void);
+extern void (*_ectors)(void);
+extern void (*_sdtors)(void);
+extern void (*_edtors)(void);
Review Comment:
@pussuw are you sure that name mangling does not affect this? I still think that we need to move it to a header and use `EXTERN`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r874599734
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,53 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* Linker defined symbols to .ctors and .dtors */
+
+extern void (*_sctors)(void);
+extern void (*_ectors)(void);
+extern void (*_sdtors)(void);
+extern void (*_edtors)(void);
Review Comment:
Great. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#issuecomment-1128639039
> I think this file is not even compiled by CI so the build error is not my doing.
>
> (Reason I think this file is not compiled: it is compiled by make import which I don't think is part of CI?)
It's related to the latest version of esptool (#6281). Let's ignore it for this PR.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871867012
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
I can try that next week, I think it should work.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871839614
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
But do we need to wrap `void _start(int argc, char *argv[])` with `extern "C"`? I can't find where `_start` is called.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870449898
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
I did, and that is a dead end. They run the code in kernel space with kernel privileges, ctors and dtors are user code, so they must be in the user CRT, just like the exit functions.
crt0 is used by CONFIG_BUILD_KERNEL, BUILD_FLAT / BUILD_PROTECTED don't need this file.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870691838
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
This way is also fine, and a quite widely used solution for static C++ ctors/dtors is to just put them into the crt files. There is no defined / required way to do this, so this way should be enough. For the kernel build using the linker defined symbols is the simplest way to do this.
Binfmt manually relocates ctors/dtors from the elf file to the binfmt structure. However, there is absolutely no reason to relocate ctors and dtors separately like binfmt does now. They are just initialized data, and data relocation does the work for us... I don't know why ctor/dtor handling was implemented in binfmt, as IMO it does not belong there. Maybe it is something needed by flat/protected ?
TLS ctors/dtors is another approach, but the kernel mode does not need this. I agree it would unify the implementations for FLAT/PROTECTED, but the scope of this PR is the kernel build.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#issuecomment-1130295783
> @xiaoxiang781216 I do not have any comments. Pleas merge if your questions are addressed
No, let's merge this patch first. I am preparing patch to unify c++ constructor/destructor, will submit when it's ready.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870438164
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
it is already done by binfmt, please search CONFIG_BINFMT_CONSTRUCTORS.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870682696
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -124,9 +189,11 @@ void _start(int argc, char *argv[])
/* Call C++ constructors */
+ exec_ctors();
+
/* Setup so that C++ destructors called on task exit */
- /* REVISIT: Missing logic */
+ atexit(exec_dtors);
Review Comment:
BTW, we can move all crt0.c from arch near libs/libc/sched/task_startup.c and implement as this:
```
void _start(int argc, char *argv[])
{
nxtask_startup(main, argc, argv);
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871841569
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
It is the process entry point, binfmt loads an .elf file and jumps to the entry point. Every elf file is compiled either with gcc or g++.
It does not matter if the name of start is name mangled, as it is not really "called", a jump to the address of _start is performed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871856950
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
Review Comment:
```suggestion
extern void (*_sctors)(void);
extern void (*_ectors)(void);
extern void (*_sdtors)(void);
extern void (*_edtors)(void);
```
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
```suggestion
static void exec_ctors(void)
{
for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
{
(*ctor)();
}
}
```
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
+ {
+ for (void (**dtor)(void) = &_sdtors; dtor != &_edtors; dtor++)
+ {
+ (*dtor)();
+ }
+ }
Review Comment:
```suggestion
static void exec_dtors(void)
{
for (void (**dtor)(void) = &_sdtors; dtor != &_edtors; dtor++)
{
(*dtor)();
}
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871818554
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
Sorry for dummy question, but do we expect to compile this file with CXX?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871864768
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
Yes it complains.
I guess we don't need to avoid the name mangling, the end result for C++ should still work as these are never called across C/C++ and since they are local functions called locally the linker must keep them.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871861765
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
Review Comment:
BTW, why do we need to avoid name mangling for local functions?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r870449898
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
I did, and that is a dead end. They run the code in kernel space with kernel privileges, ctors and dtors are user code, so they must be in the user CRT.
crt0 is used by CONFIG_BUILD_KERNEL, BUILD_FLAT / BUILD_PROTECTED don't need this file.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871142508
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/* Linker defined symbols to .ctors and .dtors */
+
+ extern void (*_sctors)(void);
+ extern void (*_ectors)(void);
+ extern void (*_sdtors)(void);
+ extern void (*_edtors)(void);
+
+#if defined(__cplusplus)
+}
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(__cplusplus)
+extern "C"
+{
+#endif
+
+/****************************************************************************
+ * Name: exec_ctors
+ *
+ * Description:
+ * Call static constructors
+ *
+ ****************************************************************************/
+
+ static void exec_ctors(void)
+ {
+ for (void (**ctor)(void) = &_sctors; ctor != &_ectors; ctor++)
+ {
+ (*ctor)();
+ }
+ }
+
+/****************************************************************************
+ * Name: exec_dtors
+ *
+ * Description:
+ * Call static destructors
+ *
+ ****************************************************************************/
+
+ static void exec_dtors(void)
Review Comment:
Ok, I will do it in this week.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6235: RISC-V: add C++ support to crt0
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6235:
URL: https://github.com/apache/incubator-nuttx/pull/6235#discussion_r871842266
##########
arch/risc-v/src/common/crt0.c:
##########
@@ -88,6 +88,71 @@ static void sig_trampoline(void)
);
}
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+#if defined(__cplusplus)
Review Comment:
I see. Thank you for explanation
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org