Samsung / TizenRT

TizenRT is a lightweight RTOS-based platform to support low-end IoT devices
Apache License 2.0
566 stars 571 forks source link

os/syscall: Fix link error when sched info logs enabled RTL8730e #6383

Closed agnel-samsung closed 3 weeks ago

agnel-samsung commented 1 month ago

Building loadable apps or loadable ext with sched info logs enabled for RTL8730e results in error for undefined symbol g_funcnames in os/syscall/syscall_names.c. This commit fixes the error by removing enclosing #if defined __KERNEL__ directive and unused headers.

neel-samsung commented 1 month ago

Please add a folder location (where the change is) to the start of commit title. Commit description is mandatory. Please add it.

agnel-samsung commented 1 month ago

Please add a folder location (where the change is) to the start of commit title. Commit description is mandatory. Please add it.

Done

kishore-sn commented 4 weeks ago

Please add a folder location (where the change is) to the start of commit title. Commit description is mandatory. Please add it.

@agnel-samsung This has not yet reflected in the commit. You need to make change to the commit and force push again.

Also, please change : os/syscall Fix link error when sched info logs enabled RTL8730e to os/syscall: Fix link error when sched info logs enabled RTL8730e

Also, please add commit description. In the description you need to write what the issue was. Pls paste the error msg also. And how you have resolved it.

kishore-sn commented 4 weeks ago

Please refer to https://github.com/Samsung/TizenRT/wiki/Documentations#coding-rules and https://github.com/Samsung/TizenRT/wiki/Documentations#commit-rules

sunghan-chang commented 3 weeks ago

I don't get why removing kernel resolves build error with sched info logs. Could you explain morw?

agnel-samsung commented 3 weeks ago

I don't get why removing kernel resolves build error with sched info logs. Could you explain morw?

The g_funcnames symbol definition was inside a #if defined(__KERNEL__) block so was excluded by pre-processor.

#if defined(CONFIG_LIB_SYSCALL) && defined(__KERNEL__)
char *g_funcnames[]..
#endif

whereas it was being used in a place where __KERNEL__ is not defined (os/arch/arm/src/armv7-a/arm_syscall.c)

 81 static void dump_syscall(const char *tag, uint32_t cmd, const uint32_t *regs)
 82 {
...
 87 #ifdef CONFIG_LIB_SYSCALL
 88     if (cmd >= CONFIG_SYS_RESERVED) {
 89         svcinfo("SYSCALL %s: regs: %p cmd: %d name: %s\n", tag, regs, cmd, g_funcnames[cmd - CONFIG_SYS_RESERVED]);
 90     } else
 91 #endif
...
 99 }

So the the #if surrounding the definition was updated to exclude the defined(__KERNE__) condition.

sunghan-chang commented 3 weeks ago

I don't get why removing kernel resolves build error with sched info logs. Could you explain morw?

The g_funcnames symbol definition was inside a #if defined(__KERNEL__) block so was excluded by pre-processor.

#if defined(CONFIG_LIB_SYSCALL) && defined(__KERNEL__)
char *g_funcnames[]..
#endif

whereas it was being used in a place where __KERNEL__ is not defined (os/arch/arm/src/armv7-a/arm_syscall.c)

 81 static void dump_syscall(const char *tag, uint32_t cmd, const uint32_t *regs)
 82 {
...
 87 #ifdef CONFIG_LIB_SYSCALL
 88     if (cmd >= CONFIG_SYS_RESERVED) {
 89         svcinfo("SYSCALL %s: regs: %p cmd: %d name: %s\n", tag, regs, cmd, g_funcnames[cmd - CONFIG_SYS_RESERVED]);
 90     } else
 91 #endif
...
 99 }

So the the #if surrounding the definition was updated to exclude the defined(__KERNE__) condition.

Another way is adding __kernel__ at dump_syscall instead of removal. I think that conditional was added to use the global variable in the kernel context only. If we remove it, no issue in user context?

sunghan-chang commented 3 weeks ago

@agnel-samsung @kishore-sn We should check what is proper change, addition or removal regardless of resolving build break itself.

kishore-sn commented 3 weeks ago

@sunghan-chang The problem is happening here becase there is a single Makefile in syscall folder. It is used to build both the stubs and proxies. But as you know proxies will go into user binary. So we cannot define KERNEL for syscall folder.

On the other hand, arm_syscall.c is only built for kernel. So if we check for KERNEL in dump_syscall, then we can never print the syscall name.

So, as a work around, we decided to remove the KERNEL check for the proxynames array. It is just a list of char strings and it will not cause any problem if it is included in the app binary. Also, since it is not used anywhere in the app binary, it will get removed by the linker. So, I think this fix is ok.