{bp-15258} gcc/gcov: fix problems with fork

[jerpelea]

Summary

gcov: Fix gcov fork() issue:

1. After code coverage is enabled, fork will be replaced by __gcov_fork
   gcov: Prevent pile insertion recursion
   gcov: Disable stack checking

    When enable CONFIG_STACK_CANARIES, in general, the stack check in the __gcov_fork function is:
   

   " return fork();
   18: e59f3020 ldr r3, [pc, Fix stm32l4_otgfshost.c: error: 'ret' undeclared #32] @ 40 <__gcov_fork+0x40>
   1c: e5932000 ldr r2, [r3]
   20: e59d3004 ldr r3, [sp, devif_poll_tcp_timer shouldn't be skipped in the multiple card case #4]
   24: e0332002 eors r2, r3, r2
   28: e3a03000 mov r3, #0
   2c: 1a000002 bne 3c <__gcov_fork+0x3c>"
   r3 is obtained by taking the value of sp offset. But after opening thumb, the second comparison value in
   "8c6: 4a06 ldr r2, [pc, Fix wait loop and void cast #24] @ (8e0 <__gcov_fork+0x30>)
   8c8: 6811 ldr r1, [r2, #0]
   8ca: 687a ldr r2, [r7, devif_poll_tcp_timer shouldn't be skipped in the multiple card case #4]
   8cc: 4051 eors r1, r2"
   is obtained through r7. Since r7 stores the stack address at this time, which stores the address of the parent process, the stack out of bounds will occur in the child process

Impact

RELEASE

Testing

CI

[nuttxpr]

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although some sections could be more detailed.

Here's a breakdown of why and suggestions for improvement:

Strengths:

• Clear Summary: The summary explains the "why," "what," and "how" of the changes. The mention of __gcov_fork and stack checking provides technical context.
• Impact Section: Marking it as a RELEASE impact highlights its significance.
• Testing Section: Mentioning CI implies that Continuous Integration tests were run and passed. This is good, but could be strengthened (see below).

Weaknesses & Suggestions:

• Missing Issue References: Even if there isn't a directly related issue, it's good practice to briefly state that (e.g., "No related NuttX issue").

• Impact Section Too Brief: While RELEASE indicates a significant impact, the other fields should still be explicitly addressed. Even if the answer is "NO," state it explicitly. For example:

  • "Impact on user: NO"
  • "Impact on build: NO (unless CONFIG_GCOV is enabled)" (This is an assumption, but illustrates the level of detail desired)
  • "Impact on hardware: NO"
  • "Impact on documentation: YES (Documentation should be updated to reflect the changes to fork() behavior when gcov is enabled)"
  • "Impact on security: YES (The fix for the stack canary issue addresses a potential security vulnerability)"
  • "Impact on compatibility: NO (Expected to be backward compatible)"

• Testing Section Lacks Detail: "CI" is a start, but insufficient. Specify:

  • Build Host(s): The operating system, compiler, and architecture of the CI environment. (e.g., "GitHub Actions Ubuntu Latest, GCC 11.x, x86_64")
  • Target(s): The architectures and boards tested on CI. (e.g., "sim:qemu-x86_64, stm32f4discovery:nsh")
  • Missing Logs: The "Testing logs before change" and "Testing logs after change" sections are empty. While full logs might be excessive, include snippets that demonstrate the issue being fixed and the successful resolution. Even a simple "before: segmentation fault," "after: program completes successfully" would be better than nothing. Ideally, show how the code coverage data is now correctly collected after the fix.

Example of improved Testing Section:

Testing

I confirm that changes are verified on local setup and works as intended:
* Build Host(s): GitHub Actions: Ubuntu Latest, GCC 11.x, x86_64
* Target(s): sim:qemu-x86_64, stm32f4discovery:nsh

Testing logs before change:

qemu-system-x86_64: terminating on signal SIGSEGV

Testing logs after change:

nsh> gcov_example # Example command demonstrating the application works after the change
... output showing program completion and potential code coverage statistics ...

By adding this level of detail, the PR becomes much stronger and easier for reviewers to assess and approve confidently.