Skip to content
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

build(bugfix):pac sim elf ONLY in Linux platform #14800

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

xuxin930
Copy link
Contributor

Summary

avoid SIM compilation post build issues on other platforms

this should fix issue #14773 (comment)

Impact

bugfix

Testing

./tools/configure.sh -l sim:nsh
make -j12
LD:  nuttx
Pac SIM with dynamic libs..
'/lib/x86_64-linux-gnu/libpthread.so.0' -> 'sim-pac/libs/libpthread.so.0'
'/lib/x86_64-linux-gnu/librt.so.1' -> 'sim-pac/libs/librt.so.1'
'/lib/x86_64-linux-gnu/libz.so.1' -> 'sim-pac/libs/libz.so.1'
'/lib/x86_64-linux-gnu/libc.so.6' -> 'sim-pac/libs/libc.so.6'
'/lib64/ld-linux-x86-64.so.2' -> 'sim-pac/ld-linux-x86-64.so.2'
SIM elf with dynamic libs archive in nuttx.tgz

avoid SIM compilation post build issues on other platforms

Signed-off-by: xuxin19 <[email protected]>
@github-actions github-actions bot added Board: simulator Size: S The size of the change in this PR is small labels Nov 15, 2024
Copy link
Member

@lupyuen lupyuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it doesn't compile on macOS Arm64:

$ git clone https://github.com/xuxin930/nuttx --branch bugfix-1115
$ cd nuttx ; tools/configure.sh sim/nsh
$ make
LD:  nuttx
Undefined symbols for architecture arm64:
  "_accept4", referenced from:
      _main in nuttx.rel
      _main in nuttx.rel
  "_dns_add_nameserver", referenced from:
      _main in nuttx.rel
  "_inet_ntoa_r", referenced from:
      _main in nuttx.rel
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [nuttx] Error 1
make: *** [nuttx] Error 2

https://gist.github.com/lupyuen/4a635b0b88c9bfd2715a280f5551ac10

Update: sim:nsh was working OK on macOS Arm64 some days ago: https://gist.github.com/lupyuen/ae40bef4147eabcc4c8f7d5eab9eb6f3

Now it's broken on master branch. Strange...

$ git clone https://github.com/apache/nuttx
...
LD:  nuttx
Undefined symbols for architecture arm64:
  "_accept4", referenced from:
      _main in nuttx.rel
      _main in nuttx.rel
  "_dns_add_nameserver", referenced from:
      _main in nuttx.rel
  "_inet_ntoa_r", referenced from:
      _main in nuttx.rel
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@nuttxpr
Copy link

nuttxpr commented Nov 15, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it addresses a specific issue and provides some testing information, it lacks crucial details.

Here's a breakdown of what's missing:

  • Summary: While it mentions the problem and the related issue, it doesn't explain how the change fixes the issue. What code was modified and what is the logic behind the fix?
  • Impact: The impact section is extremely vague. It just says "bugfix." While true, it needs to be far more specific:
    • Impact on user: Does this change user-facing behavior in any way? Probably not, so explicitly state "NO."
    • Impact on build: Does the build process itself change (beyond fixing the error)? If not, state "NO."
    • Impact on hardware: Does this affect any specific architectures or boards? Likely "NO," but state it explicitly.
    • Impact on documentation: Does this require documentation updates? If not, state "NO."
    • Impact on security, compatibility: Address these explicitly, likely with "NO" in both cases.
  • Testing: While it shows the commands used, it lacks before and after logs demonstrating the problem and the fix. The provided logs only show the after scenario. Crucially, it only shows a successful build, not how the previous build failed. Show the error message encountered before the fix. Also, testing on only one configuration (sim:nsh) is insufficient. Consider testing on at least one other architecture to ensure the change doesn't introduce regressions. Specify the build host details as requested.

Example of Improved Information:

Summary:

This PR fixes a post-build issue with the SIM target that prevented compilation on other platforms. The problem stemmed from [explain the root cause of the issue, e.g., incorrect dependency ordering, missing library path, etc.]. This change addresses the issue by [explain the solution, e.g., modifying the Makefile to include the correct dependency, adding the missing library path to the linker script, etc.]. This resolves issue #14773.

Impact:

  • Impact on user: NO
  • Impact on build: NO (The build process itself is unchanged, only the error is resolved).
  • Impact on hardware: NO
  • Impact on documentation: NO
  • Impact on security: NO
  • Impact on compatibility: NO

Testing:

Build Host: Linux (Ubuntu 22.04), x86_64, GCC 11.3.0

Target 1: sim:nsh

Before:

./tools/configure.sh -l sim:nsh
make -j12
[... error message demonstrating the build failure ...]

After:

./tools/configure.sh -l sim:nsh
make -j12
LD:  nuttx
Pac SIM with dynamic libs..
[...]
SIM elf with dynamic libs archive in nuttx.tgz

Target 2: [e.g., stm32f4discovery:nsh]

Before:

[... build commands and error log, if applicable ...]

After:

[... build commands and success log ...]

By providing this level of detail, reviewers can easily understand the change, its impact, and verify its effectiveness. This greatly increases the chances of the PR being accepted.

@xuxin930
Copy link
Contributor Author

xuxin930 commented Nov 15, 2024

https://gist.github.com/lupyuen/4a635b0b88c9bfd2715a280f5551ac10

Yes @lupyuen I only limit the actions of PAC.
this doesn't help with your error.
however, this looks like it's caused by Mac linking libraries differently between mac x86 arch and mac arm arch?

Copy link
Member

@lupyuen lupyuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my apps folder was messed up, sim:nsh runs OK on macOS Arm64 now. Thanks!
https://gist.github.com/lupyuen/3ac496a3bf95c29c94dbbab932b424a1

@lupyuen
Copy link
Member

lupyuen commented Nov 15, 2024

sim:lua builds OK now, no LDD errors. But it's still giving seg fault:
https://gist.github.com/lupyuen/df3b10994f571ee39245fd256387a655

$ git clone https://github.com/xuxin930/nuttx --branch bugfix-1115
$ git clone https://github.com/apache/nuttx-apps apps    
$ cd nuttx ; tools/configure.sh sim:lua
$ kconfig-tweak --enable CONFIG_PIPES
$ make olddefconfig
$ make
$ ./nuttx
[1]    44365 segmentation fault  ./nuttx

@xiaoxiang781216
Copy link
Contributor

sim:lua builds OK now, no LDD errors. But it's still giving seg fault: https://gist.github.com/lupyuen/df3b10994f571ee39245fd256387a655

$ git clone https://github.com/xuxin930/nuttx --branch bugfix-1115
$ git clone https://github.com/apache/nuttx-apps apps    
$ cd nuttx ; tools/configure.sh sim:lua
$ kconfig-tweak --enable CONFIG_PIPES
$ make olddefconfig
$ make
$ ./nuttx
[1]    44365 segmentation fault  ./nuttx

could you attach gdb? so we can see the panic callstack.

@xiaoxiang781216 xiaoxiang781216 linked an issue Nov 15, 2024 that may be closed by this pull request
1 task
@lupyuen
Copy link
Member

lupyuen commented Nov 15, 2024

Lua seems to be crashing due to a stack problem?

➜  nuttx git:(bugfix-1115) $ lldb ./nuttx
(lldb) target create "./nuttx"
Current executable set to '/Users/luppy/riscv/nuttx/nuttx' (arm64).
(lldb) r
Process 30597 launched: '/Users/luppy/riscv/nuttx/nuttx' (arm64)
Process 30597 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xa8)
    frame #0: 0x000000010000b05c nuttx`nxsched_get_stackinfo(pid=0, stackinfo=0x000000016fde9670) at sched_get_stackinfo.c:103:38
   100          }
   101      }
   102 
-> 103    stackinfo->adj_stack_size  = qtcb->adj_stack_size;
   104    stackinfo->stack_alloc_ptr = qtcb->stack_alloc_ptr;
   105    stackinfo->stack_base_ptr  = qtcb->stack_base_ptr;
   106 
Target 0: (nuttx) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xa8)
  * frame #0: 0x000000010000b05c nuttx`nxsched_get_stackinfo(pid=0, stackinfo=0x000000016fde9670) at sched_get_stackinfo.c:103:38
    frame #1: 0x000000010002fab8 nuttx`tls_get_info at tls_getinfo.c:64:9
    frame #2: 0x000000010002fa7c nuttx`task_get_info at task_getinfo.c:52:33
    frame #3: 0x000000010002d2dc nuttx`atexit_register(type=4, func=(nuttx`uv_library_shutdown at uv-common.c:941), arg=0x0000000000000000, dso=0x0000000100130368) at lib_atexit.c:70:36
    frame #4: 0x000000010002d674 nuttx`__cxa_atexit(func=(nuttx`uv_library_shutdown at uv-common.c:941), arg=0x0000000000000000, dso_handle=0x0000000100130368) at lib_atexit.c:270:10
    frame #5: 0x00000001000fe4c8 nuttx`__GLOBAL_init_65535 + 36
    frame #6: 0x000000019842fe90 dyld`invocation function for block in dyld4::Loader::findAndRunAllInitializers(dyld4::RuntimeState&) const + 592
    frame #7: 0x000000019846e4d0 dyld`invocation function for block in dyld3::MachOAnalyzer::forEachInitializer(Diagnostics&, dyld3::MachOAnalyzer::VMAddrConverter const&, void (unsigned int) block_pointer, void const*) const + 340
    frame #8: 0x0000000198461c38 dyld`invocation function for block in dyld3::MachOFile::forEachSection(void (dyld3::MachOFile::SectionInfo const&, bool, bool&) block_pointer) const + 496
    frame #9: 0x00000001984142dc dyld`dyld3::MachOFile::forEachLoadCommand(Diagnostics&, void (load_command const*, bool&) block_pointer) const + 300
    frame #10: 0x0000000198460bcc dyld`dyld3::MachOFile::forEachSection(void (dyld3::MachOFile::SectionInfo const&, bool, bool&) block_pointer) const + 192
    frame #11: 0x000000019846dfe4 dyld`dyld3::MachOAnalyzer::forEachInitializer(Diagnostics&, dyld3::MachOAnalyzer::VMAddrConverter const&, void (unsigned int) block_pointer, void const*) const + 516
    frame #12: 0x000000019842fbb4 dyld`dyld4::Loader::findAndRunAllInitializers(dyld4::RuntimeState&) const + 176
    frame #13: 0x0000000198437190 dyld`dyld4::JustInTimeLoader::runInitializers(dyld4::RuntimeState&) const + 36
    frame #14: 0x0000000198430270 dyld`dyld4::Loader::runInitializersBottomUp(dyld4::RuntimeState&, dyld3::Array<dyld4::Loader const*>&, dyld3::Array<dyld4::Loader const*>&) const + 312
    frame #15: 0x0000000198434560 dyld`dyld4::Loader::runInitializersBottomUpPlusUpwardLinks(dyld4::RuntimeState&) const::$_0::operator()() const + 180
    frame #16: 0x0000000198430460 dyld`dyld4::Loader::runInitializersBottomUpPlusUpwardLinks(dyld4::RuntimeState&) const + 412
    frame #17: 0x000000019844fa0c dyld`dyld4::APIs::runAllInitializersForMain() + 424
    frame #18: 0x00000001984198c8 dyld`dyld4::prepare(dyld4::APIs&, dyld3::MachOAnalyzer const*) + 3268
    frame #19: 0x0000000198418bc0 dyld`dyld4::start(dyld4::KernelArgs*, void*, void*)::$_0::operator()() const + 544
    frame #20: 0x000000019841805c dyld`start + 2304
(lldb) 

@xiaoxiang781216
Copy link
Contributor

look like the postpone c++ initialization stop work? arch/sim/src/sim/posix/sim_macho_init.c.

@xiaoxiang781216
Copy link
Contributor

Let's ignore ci error which is fixed by: #14802

@xiaoxiang781216 xiaoxiang781216 merged commit 71b169e into apache:master Nov 15, 2024
11 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Board: simulator Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants