Skip to content

Conversation

@W-M-R
Copy link
Contributor

@W-M-R W-M-R commented Oct 15, 2024

Summary

  1. clang: Support compiling rt.profile library
      1. Since the implementation of gcov has changed since clang17, versions before clang17 need to use the libunwind.a file
  1. llvm gcov supports interrupt code coverage detection, as shown below
    for the code coverage analysis of armv8-m/arm_doirq.c:
 -:    0:Source:armv8-m/arm_doirq.c
            -:    0:Graph:./arm_doirq.gcno
            -:    0:Data:./arm_doirq.gcda
            -:    0:Runs:1
            -:    0:Programs:1
            -:    1:/****************************************************************************
            -:    2: * arch/arm/src/armv8-m/arm_doirq.c
            -:    3: *
            -:    4: * Licensed to the Apache Software Foundation (ASF) under one or more
            -:    5: * contributor license agreements.  See the NOTICE file distributed with
            -:    6: * this work for additional information regarding copyright ownership.  The
            -:    7: * ASF licenses this file to you under the Apache License, Version 2.0 (the
            -:    8: * "License"); you may not use this file except in compliance with the
            -:    9: * License.  You may obtain a copy of the License at
            -:   10: *
            -:   11: *   http://www.apache.org/licenses/LICENSE-2.0
            -:   12: *
            -:   13: * Unless required by applicable law or agreed to in writing, software
            -:   14: * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
            -:   15: * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
            -:   16: * License for the specific language governing permissions and limitations
            -:   17: * under the License.
            -:   18: *
            -:   19: ****************************************************************************/
            -:   20:
            -:   21:/****************************************************************************
            -:   22: * Included Files
            -:   23: ****************************************************************************/
            -:   24:
            -:   25:#include <nuttx/config.h>
            -:   26:
            -:   27:#include <stdint.h>
            -:   28:#include <assert.h>
            -:   29:
            -:   30:#include <nuttx/irq.h>
            -:   31:#include <nuttx/arch.h>
            -:   32:#include <nuttx/board.h>
            -:   33:#include <arch/board/board.h>
            -:   34:#include <sched/sched.h>
            -:   35:
            -:   36:#include "arm_internal.h"
            -:   37:#include "exc_return.h"
            -:   38:#include "nvic.h"
            -:   39:#include "psr.h"
            -:   40:
            -:   41:/****************************************************************************
            -:   42: * Public Functions
            -:   43: ****************************************************************************/
            -:   44:
         4395:   45:void exception_direct(void)
            -:   46:{
         4395:   47:  int irq = getipsr();
            -:   48:
            -:   49:#ifdef CONFIG_ARCH_FPU
         4395:   50:  __asm__ __volatile__
            -:   51:    (
            -:   52:      "mov r0, %0\n"
            -:   53:      "vmsr fpscr, r0\n"
            -:   54:      :
            -:   55:      : "i" (ARMV8M_FPSCR_LTPSIZE_NONE)
            -:   56:    );
            -:   57:#endif
            -:   58:
         4395:   59:  arm_ack_irq(irq);
         4395:   60:  irq_dispatch(irq, NULL);
            -:   61:
         4395:   62:  if (g_running_tasks[this_cpu()] != this_task())
            -:   63:    {
           36:   64:      up_trigger_irq(NVIC_IRQ_PENDSV, 0);
           36:   65:    }
         4395:   66:}
            -:   67:
           37:   68:uint32_t *arm_doirq(int irq, uint32_t *regs)
            -:   69:{
           37:   70:  struct tcb_s *tcb = this_task();
            -:   71:
            -:   72:  board_autoled_on(LED_INIRQ);
            -:   73:#ifdef CONFIG_SUPPRESS_INTERRUPTS
            -:   74:  PANIC();
            -:   75:#else
            -:   76:
            -:   77:  /* Acknowledge the interrupt */
            -:   78:
           37:   79:  arm_ack_irq(irq);
            -:   80:
            -:   81:  /* Set current regs for crash dump */
            -:   82:
           37:   83:  up_set_current_regs(regs);
            -:   84:
           37:   85:  if (irq == NVIC_IRQ_PENDSV)
            -:   86:    {
            -:   87:#ifdef CONFIG_ARCH_HIPRI_INTERRUPT
            -:   88:      /* Dispatch the PendSV interrupt */
            -:   89:
            -:   90:      irq_dispatch(irq, regs);
            -:   91:#endif
            -:   92:
           17:   93:      up_irq_save();
           17:   94:      g_running_tasks[this_cpu()]->xcp.regs = regs;
           17:   95:    }
            -:   96:  else
            -:   97:    {
            -:   98:      /* Dispatch irq */
            -:   99:
           20:  100:      tcb->xcp.regs = regs;
           20:  101:      irq_dispatch(irq, regs);
            -:  102:    }
            -:  103:
            -:  104:  /* If a context switch occurred while processing the interrupt then
            -:  105:   * current_regs may have change value.  If we return any value different
            -:  106:   * from the input regs, then the lower level will know that a context
            -:  107:   * switch occurred during interrupt processing.
            -:  108:   */
            -:  109:
           37:  110:  tcb = this_task();
            -:  111:
            -:  112:  /* Update scheduler parameters */
            -:  113:
            -:  114:  nxsched_suspend_scheduler(g_running_tasks[this_cpu()]);
            -:  115:  nxsched_resume_scheduler(tcb);
            -:  116:
            -:  117:  /* Record the new "running" task when context switch occurred.
            -:  118:   * g_running_tasks[] is only used by assertion logic for reporting
            -:  119:   * crashes.
            -:  120:   */
            -:  121:
           37:  122:  g_running_tasks[this_cpu()] = tcb;
           37:  123:  regs = tcb->xcp.regs;
            -:  124:#endif
            -:  125:
            -:  126:  /* Clear current regs */
            -:  127:
           37:  128:  up_set_current_regs(NULL);
            -:  129:
            -:  130:  board_autoled_off(LED_INIRQ);
            -:  131:
            -:  132:#ifdef CONFIG_ARMV8M_TRUSTZONE_HYBRID
            -:  133:  if (((1 << this_cpu()) & CONFIG_ARMV8M_TRUSTZONE_CPU_BITMASK) == 0)
            -:  134:    {
            -:  135:      regs[REG_EXC_RETURN] &=
            -:  136:        ~(EXC_RETURN_EXC_SECURE | EXC_RETURN_SECURE_STACK);
            -:  137:    }
            -:  138:  else
            -:  139:    {
            -:  140:      regs[REG_EXC_RETURN] |=
            -:  141:        (EXC_RETURN_EXC_SECURE | EXC_RETURN_SECURE_STACK);
            -:  142:    }
            -:  143:#endif
            -:  144:
           37:  145:  return regs;
            -:  146:}

Impact

Testing

Enable CONFIG_SYSTEM_GCOV CONFIG_SCHED_GCOV and add compilation options to -fprofile-generate --coverage to the module that needs to analyze code coverage, and execute gcov -d /data/xxxx to get the analysis data

@github-actions github-actions bot added Area: Tooling Area: Build system Area: OS Components OS Components issues Size: M The size of the change in this PR is medium labels Oct 15, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 15, 2024

[Experimental Bot, please feedback here]

This PR does not meet the NuttX requirements. Here's why:

  • Insufficient Summary: The summary provides a high-level overview but lacks crucial details:
    • Why is this change necessary? Clearly state the problem the PR solves. Is code coverage broken for LLVM/Clang? Is this a new feature to enable better tooling?
    • How does the change work? Be more specific than "porting the library." Explain how the library integrates, what modifications were made for NuttX, etc.
  • Impact Section is Unrealistic: It's highly unlikely a change of this nature has no impact. Address each point specifically:
    • Build Impact: Adding a new library and integrating it into the kernel will change the build process. How?
    • Hardware Impact: Which architectures are affected by the LLVM compiler runtime?
    • Documentation: Does this require documentation updates?
    • Compatibility: Will this affect applications built against older NuttX versions?
  • Testing is Insufficient: "No" is not an acceptable answer. You must provide:
    • Build Host Details: List the OS, CPU architecture, compiler version used for testing.
    • Target Details: Specify the architectures and boards used for validation.
    • Testing Logs: Include relevant logs demonstrating the issue before the change and the improvement after the change.

In short, the PR needs significantly more detail and evidence of testing to be considered.

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. and removed Size: M The size of the change in this PR is medium labels Oct 28, 2024
@github-actions
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@W-M-R
Copy link
Contributor Author

W-M-R commented Oct 28, 2024

I will update this patch until #14472 is merged

@xiaoxiang781216
Copy link
Contributor

please rebas the patch to.fix.the.conflict.

@github-actions github-actions bot added Board: arm Size: M The size of the change in this PR is medium Area: Tooling and removed Area: Tooling Area: Build system Arch: arm Issues related to ARM (32-bit) architecture Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Oct 29, 2024
1. Since the implementation of gcov has changed since clang17, versions before clang17 need to use the libunwind.a file

Signed-off-by: wangmingrong1 <wangmingrong1@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components issues Area: Tooling Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants