Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Fix compilation issues with ARM and IAR compilers #1526

Merged
merged 1 commit into from
Nov 20, 2019

Conversation

cypress-neil
Copy link
Contributor

Fix compilation issues with ARM and IAR compilers

Description

  • Add CMake toolchain file for ARM Compiler 6
    • arm-armclang.cmake is based on arm-keil.cmake
    • CMake 3.15 or newer is required to detect ARMClang
    • CMake requires the CMAKE_SYSTEM_PROCESSOR variable to be configured
      in order to detect ARMClang
  • LwIP: ARM Compiler does not provide sys/time.h
  • LwIP: Allow overriding LwIP's errno declaration
    • cc.h no longer defines LWIP_PROVIDE_ERRNO when
      LWIP_ERRNO_STDINCLUDE or LWIP_ERRNO_INCLUDE is defined
    • Remove libraries/3rdparty/lwip/src/include/lwip from the include
      path because LwIP's errno.h conflicts with the standard library's
      errno.h
    • Update source files that contained unqualified paths to LwIP
      header files
  • LwIP: Fix incorrect struct packing with IAR EWARM
    • When __packed appears before the struct keyword, it affects the
      alignment of each member of the structure. When __packed appears
      after the struct keyword, it affects the alignment of the struct
      type itself.

errno Notes

There are several possible definitions of errno:

  • C/C++ standard library. The C specification only requires EDOM, EILSEQ, and ERANGE.
  • LwIP (lwip/errno.h). LwIP uses some custom and/or POSIX-defined errno values which it can optionally define if the system headers do not.
  • FreeRTOS+POSIX: Also provides some POSIX-defined errno values.

However, there are some issues with using alternate errno implementations:

  1. When you use errno, it's hard to tell which one you are referring to.
  2. In the C specification, errno is a macro. Attempting to define it as a variable could cause a confusing compiler error message due to macro substitution.
  3. The C library is free to assign any values to the ERRNO constants. These may or may not be equivalent to LwIP's or FreeRTOS+POSIX's values which could cause macro redefinition errors and/or confusion about the meaning of the numeric values.
  4. Implementing errno as a (non-thread-local) variable is not thread-safe. FreeRTOS+POSIX and most multithread-aware C libraries have a thread-local implementation of errno but LwIP does not.

The Cypress projects are using the C library's errno with a custom header (via LWIP_ERRNO_INCLUDE) to define the additional values expected by LwIP.

Compatibility Notes

After this change, directives that refer to LwIP headers must use the directory name (e.g. lwip/socket.h). This change only affects CMake builds. I have searched for common LwIP header names in the in-tree targets but I have not verified whether all targets still compile.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • My code is Linted.

As far as I can tell none of the in-tree projects use cmake with armclang or IAR so I have only been able to compile with unpublished Cypress-specific targets. The Cypress targets seem to boot with these changes but I have not run a full test suite yet.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

gkwicker
gkwicker previously approved these changes Nov 18, 2019
@gkwicker gkwicker dismissed their stale review November 18, 2019 23:46

Changes to third party code

libraries/3rdparty/lwip/src/portable/arch/cc.h Outdated Show resolved Hide resolved
@@ -53,8 +53,7 @@
/* TCP/IP includes */
#include "lwip/tcpip.h"
#include "lwip/sockets.h"

int errno;
#include "lwip/errno.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

The same comment about lwip above applies here as well; I have emailed percepio support to let them know about this change. If they accept it, we will incorporate their change here upon receiving the updated version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've heard back from Percepio – they will be incorporating this change into their next release of the tracealyzer recorder library. When it becomes available, we'll integrate it into this repository.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the comment from Percepio - "The fix has been checked in on our side and will be included in Tracealyzer v4.3.6....make the change in your version and change the version number of that specific file to 'v4.3.5.1'"

- Add CMake toolchain file for ARM Compiler 6
  - arm-armclang.cmake is based on arm-keil.cmake
  - CMake 3.15 or newer is required to detect ARMClang
  - CMake requires the CMAKE_SYSTEM_PROCESSOR variable to be configured
    in order to detect ARMClang
- LwIP: ARM Compiler does not provide sys/time.h
- LwIP: Allow overriding LwIP's errno declaration
  - cc.h no longer defines LWIP_PROVIDE_ERRNO when
    LWIP_ERRNO_STDINCLUDE or LWIP_ERRNO_INCLUDE is defined
  - Remove libraries/3rdparty/lwip/src/include/lwip from the include
    path because LwIP's errno.h conflicts with the standard library's
    errno.h
  - Update source files that contained unqualified paths to LwIP
    header files
- LwIP: Fix incorrect struct packing with IAR EWARM
  - When __packed appears before the struct keyword, it affects the
    alignment of each member of the structure. When __packed appears
    after the struct keyword, it affects the alignment of the struct
    type itself.
@cypress-neil
Copy link
Contributor Author

Rebased to incorporate 1518. Updated version number in trcStreamingPort.c.

@dan4thewin
Copy link
Contributor

/bot run checks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants