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

Espressif CMake set CMAKE_C_FLAGS unreliable results project-wide #5727

Closed
gojimmypi opened this issue Oct 23, 2022 · 5 comments
Closed

Espressif CMake set CMAKE_C_FLAGS unreliable results project-wide #5727

gojimmypi opened this issue Oct 23, 2022 · 5 comments
Assignees

Comments

@gojimmypi
Copy link
Contributor

Version

primarily affecting Espressif esp-wolfssl

Description

TL;DR: CMake add_compile_definitions() is more reliable than set(CMAKE_C_FLAGS ... and other options when needing project-wide defines.

idf.py reconfigure is also a possible solution, but should not be needed (or should happen automatically) to build.

Environment

I'm currently using an older ESP-IDF v4.2.2 with an ESP32-WROOM.

I've observed this problem using both command-line idf.py as well as using the VisualGDB extension in Visual Studio.

The project I am working on is a mix of C and C++. Most of the main project is C++.

# project-of-interest is a C & C++ ESP-IDF project that uses esp-tls component:
git clone https://github.com/project-of-interest global-define-demo
cd global-define-demo
git submodule update --init --recursive

# checkout a branch, if needed
git checkout 77d96c5c

# see below for setting up the "not defined" test in esp_tls.h

# observe "WOLFSSL_USER_SETTINGS not defined" problem here:
rm -rf ./build
{ idf.py build; } 2>&1 | grep "WOLFSSL_USER_SETTINGS not defined"


# prior build directory exists.
# observe "not defined" problem here:
idf.py clean
{ idf.py build; } 2>&1 | grep "WOLFSSL_USER_SETTINGS not defined"


# prior build directory exists from above build command
# observe NO instance of "not defined" here:
idf.py clean
idf.py reconfigure
{ idf.py build; } 2>&1 | grep "WOLFSSL_USER_SETTINGS not defined"

Background

I had been using the CMake set command to adjust project-wide compiler settings like this:

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DWOLFSSL_USER_SETTINGS")

For my specific example, I need the WOLFSSL_USER_SETTINGS setting in my local project with a local wolfssl component to be seen by the ESP-IDF esp-tls component.

Although not explicitly stated to be supported in the ESP-IDF Build Systems Docs, I had assumed all of the CMake capabilities would be available in an ESP-IDF project.

There's a submodule pointing to an old, stale wolfSSL in the Espressif esp-wolfssl component.

A code segment needed to be added to the case ESP_TLS_HANDSHAKE in the esp-tls like this to support SNI for a problematic CDN:

    case ESP_TLS_HANDSHAKE:
    #if defined(HAVE_SNI)
        ESP_LOGI(TAG, "\n\n Calling wolfSSL_CTX_UseSNI \n\n... %s", hostname);
        wolfSSL_CTX_UseSNI((WOLFSSL_CTX *)tls->priv_ctx, 0, hostname, (word16) XSTRLEN (hostname));
    #else
        ESP_LOGI(TAG, "\n\n Skipping wolfSSL_CTX_UseSNI \n\n...");
    #endif
        return esp_tls_handshake(tls, cfg);
        break;

It seems simple enough: look for the HAVE_SNI that is defined in the wolfSSL user_settings.h file.

How is the user_settings.h included? Typically the compiler looks for a define for WOLFSSL_USER_SETTINGS.

To actually see the symptom, I added this to the top of the esp_tls.h:

#ifndef WOLFSSL_USER_SETTINGS
#warning "WOLFSSL_USER_SETTINGS not defined in esp_tls.h"
#endif

The later includes in that file should then bring in the wolfSSL user_settings.h when CONFIG_ESP_TLS_USING_WOLFSSL is turned on in the sdkconfig).

Problem

The problem I've observed, is that depending on the prior state of the build, different results will occur when building an ESP-IDF project. Sometimes the define for WOLFSSL_USER_SETTINGS will be ok, other times it will be missing, even with the exact same code and makefiles.

This is in contrast to the comment by @igrr in #8021. Although yes, adding to EXTRA_CPPFLAGS usually works, it does not work 100% reliably,

For example: deleting the local ./build directory and building fresh with idf.py build results in
WOLFSSL_USER_SETTINGS NOT being visible to the esp-tls component.

However, upon subsequent builds the CMAKE_C_FLAGS with WOLFSSL_USER_SETTINGS is seen and properly
processed when compiling the eps-tls.

I believe the root cause of this may be related to the circular dependency mentioned by @projectgus in #6913, CMake cache, etc.

Alternatives

I tried putting the set(CMAKE_C_FLAGS.. in all sorts of places, the local components, project, main makefiles, etc.

I also tried various other set(EXTRA_CPPFLAGS.., set(CMAKE_CXX_FLAGS... etc. All with the same results.

I did not want the solution to be hard coded in the ESP-IDF components, as they are of course many all out in the wild already. I want a solution that works for everyone with existing ESP-IDF build system and just edit the project.

I also tried using this syntax as noted in the v4.2.4 build docs:

idf.py build -DWOLFSSL_USER_SETTINGS=y

It was essentially the same result as above. Sometimes it failed to have the global define visible to the ESP-IDF components. It was also less desirable to have the operator needing to remember the parameter anyhow.

The KConfig.projbuild is useful for project-wide defines and seems to work well. The problem for me is that all of the configuration parameters are prefixed with CONFIG_[param] and I need exactly the WOLFSSL_USER_SETTINGS defined.

The solution that works for me:

Using the CMake add_compile_definitions will add values to COMPILE_DEFINITIONS as noted in
the idf-build-properties
has been by far the most reliable.

add_compile_definitions("WOLFSSL_USER_SETTINGS")
add_compile_definitions("WOLFMQTT_USER_SETTINGS")

I put this in the main project CMakeFiles.txt:

if (WOLFSSL_USER_SETTINGS)
    message("Main project: Found predefined WOLFSSL_USER_SETTINGS")
else()
    message("Main project: Setting WOLFSSL_USER_SETTINGS")
    add_compile_definitions("WOLFSSL_USER_SETTINGS")
endif()

Also, "just to be safe" I include this in the CMakeFiles.txt of my wolfSSL components and other
local components using wolfSSL:

if (WOLFSSL_USER_SETTINGS)
    message("Component wolfssl: Found predefined WOLFSSL_USER_SETTINGS")
else()
    message("Component wolfmqtt: Setting WOLFSSL_USER_SETTINGS")
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DWOLFSSL_USER_SETTINGS")
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DWOLFSSL_USER_SETTINGS")
endif()

Next steps

I plan to test with other versions of the ESP-IDF. I've been building for the ESP32 for years, and only this week discovered this occasional and subtle anomaly.

In the coming days I plan to create a simple stand-alone project to demonstrate this odd issue.

Resolution for this issue? It is probable not a CMake bug, rather an issue for the intricacies of the ESP-IDF.

I'll be going back to visit all of the wolfSSL examples.

Perhaps just update the docs to indicate the safest way to apply project-wide defines.

@ESP-Marius I'm interested in your feedback here.

Thank you.

@ESP-Marius
Copy link

@gojimmypi Interesting issue, and a very detailed write up!

Could it be simply that your flag is overwritten by https://github.com/espressif/esp-idf/blob/release/v4.2/tools/cmake/toolchain-esp32.cmake? On the second run the cache entry is already set, so it wont be set again, hence your define being preserved.

@igrr
Copy link

igrr commented Oct 24, 2022

This does look similar to espressif/esp-idf#7507, which was fixed in release/v4.4 and later.

@gojimmypi
Copy link
Contributor Author

Thank you both for the prompt reply! I haven't had a chance to test with ESP-IDF 4.4, but the 7507 does sound like the root cause & solution.

I'm certainly glad someone else has seen this, as the problem is just too weird.

@gojimmypi
Copy link
Contributor Author

The best way to deal with the wolfssl user_settings.h file is to reference wolfssl/wolfcrypt/settings.h after first including the sdkconfig.h file and not explicity reference the user_settings.h, like this at the top of each source file:

#include "sdkconfig.h"

/* ESP specific */
#include <nvs_flash.h>

/* wolfSSL */
#include <wolfssl/wolfcrypt/settings.h> /* references user_settings.h */
/* Do not explicitly include wolfSSL user_settings.h */

#include <wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h>
#ifndef WOLFSSL_ESPIDF
    #warning "Problem with wolfSSL user_settings."
    #warning "Check components/wolfssl/include"
#endif

And set the compiler option in the main CMakeLists.txt

I'm leaving this issue open, pending updates to the local ESP32 examples

@gojimmypi
Copy link
Contributor Author

I'm fairly certain all examples in all repos have since been updated. If this problem is encountered again, we can open a new issue..

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants