Issue when combining use_test_preprocessor and CMock's treat_inlines

i-adamov commented 1 year ago

I am working on a big firmware project where Ceedling is used to run unittests and mock driver headers. The issue I am having is that if I enable the test preprocessor feature and have CMock configured to treat header files with inline functions (of which we have some) the compilation of the unittest fails.

Additional info specific to our project is that the unittests (and project.yml file) are located in a separate directory so I am doing CEEDLING_MAIN_PROJECT_FILE=./unittests/project.yml before calling Ceedling.

I was able to recreate the issue using a simple example project that contains only a few files - https://github.com/i-adamov/ceedling-issue-example I have a module which I need to test (./src/example_file.c and ./inc/example_file.h) which includes a driver header (./driverv/drv_bbb.h) and in turn the driver header includes a HAL header (./driver/hal/hal_aaa.h). This simulates how our project is structured. There is another header (./inc/other_header.h) which also includes the driver header and it is also included by the example_file module. It simulates the header of another module that may interact with the module I am testing.

When running Ceedling without the preprocessing it works fine: export CEEDLING_MAIN_PROJECT_FILE=./unittests/project.yml ; ceedling clobber test:all

Clobbering all generated files...
(For large projects, this task may take a long time to complete)

Test 'test_unittest.c'
----------------------
Creating mock for drv_aaa...
Generating runner for test_unittest.c...
Compiling test_unittest_runner.c...
Compiling test_unittest.c...
Compiling mock_drv_aaa.c...
Compiling unity.c...
Compiling cmock.c...
Linking test_unittest.out...
Running test_unittest.out...

[==========] Running 2 tests from 1 test cases.
[----------] Global test environment set-up.   
[----------] 2 tests from test_unittest.c      
[ RUN      ] test_unittest.c.test_func1
[       OK ] test_unittest.c.test_func1 (0 ms)
[ RUN      ] test_unittest.c.test_static_func2
[       OK ] test_unittest.c.test_static_func2 (0 ms)
[----------] 2 tests from test_unittest.c (0 ms total)

[----------] Global test environment tear-down.
[==========] 2 tests from 0 test cases ran.
[  PASSED  ] 2 tests.
[  FAILED  ] 0 tests.

 0 FAILED TESTS

 0 FAILED TESTS

However if I set :use_test_preprocessor: TRUE the build fails:

Clobbering all generated files...
(For large projects, this task may take a long time to complete)

Test 'test_unittest.c'
----------------------
Generating include list for drv_aaa.h...
Creating mock for drv_aaa...
In file included from unittests/build/test/mocks/mock_drv_aaa.h:6:0,
                 from unittests/test/test_unittest.c:3:
unittests/build/test/mocks/drv_aaa.h:1:10: fatal error: driver/hal/hal_bbb.h: No such file or directory
 #include "driver/hal/hal_bbb.h"
          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
ERROR: Shell command failed.
> Shell executed command:
'gcc -E -I"/var/lib/gems/2.5.0/gems/ceedling-0.31.1/vendor/unity/src" -I"/var/lib/gems/2.5.0/gems/ceedling-0.31.1/vendor/cmock/src" -I"unittests/build/test/mocks" -I"unittests/test" -I"unittests/test/support" -I"src" -I"inc" -I"driver/inc" -I"driver/hal" -D__STATIC_INLINE="static inline" -DTEST -D__STATIC_INLINE="static inline" -DTEST -DGNU_COMPILER "unittests/test/test_unittest.c" -o "unittests/build/test/preprocess/files/test_unittest.c"'
> And exited with status: [1].

rake aborted!
ShellExecutionException: ShellExecutionException
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/tool_executor.rb:88:in `exec'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/preprocessinator_file_handler.rb:12:in `preprocess_file'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/preprocessinator.rb:48:in `preprocess_file'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/preprocessinator.rb:12:in `block in setup'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/preprocessinator_helper.rb:37:in `preprocess_test_file'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/preprocessinator.rb:33:in `preprocess_test_and_invoke_test_mocks'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/test_invoker.rb:84:in `block in setup_and_invoke'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/test_invoker.rb:51:in `setup_and_invoke'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/tasks_tests.rake:13:in `block (2 levels) in <top (required)>'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/bin/ceedling:345:in `block in <top (required)>'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/bin/ceedling:332:in `<top (required)>'
/usr/local/bin/ceedling:23:in `load'
/usr/local/bin/ceedling:23:in `<main>'
Tasks: TOP => test:all
(See full trace by running task with --trace)
ERROR: Ceedling Failed

What I see as a difference is that the driver header file (unittests/build/test/mocks/drv_aaa.h) which is processed by CMock to make the inline functions testable has changes to the include macros in the top of the file. It is also missing its include guard. When running without preprocessor:

#ifndef DRV_AAA_H
#define DRV_AAA_H

#include "hal_bbb.h"

#define AAA 10

int get_aaa(void);

#endif

When running with preprocessor (empty lines truncated):

#include "driver/hal/hal_bbb.h"

int get_aaa(void);

The use of this driver/hal/hal_bbb.h filepath makes it impossible for the compiler to locate the file as the root directory is not used as an include path. However if I add it to the :paths: :include: section of the project.yml file, I get another issue with undefined macros:

Clobbering all generated files...
(For large projects, this task may take a long time to complete)

Test 'test_unittest.c'
----------------------
Generating include list for drv_aaa.h...
Creating mock for drv_aaa...
Generating runner for test_unittest.c...
Compiling test_unittest_runner.c...
Compiling test_unittest.c...
In file included from unittests/test/test_unittest.c:4:0:
src/example_file.c: In function 'func1':
inc/other_header.h:6:20: error: 'AAA' undeclared (first use in this function)
 #define SOMETHING (AAA + 5)
                    ^
src/example_file.c:13:24: note: in expansion of macro 'SOMETHING'
     return a + b + x + SOMETHING;
                        ^~~~~~~~~
inc/other_header.h:6:20: note: each undeclared identifier is reported only once for each function it appears in
 #define SOMETHING (AAA + 5)
                    ^
src/example_file.c:13:24: note: in expansion of macro 'SOMETHING'
     return a + b + x + SOMETHING;
                        ^~~~~~~~~
unittests/test/test_unittest.c: In function 'test_func1':
inc/other_header.h:6:20: error: 'AAA' undeclared (first use in this function)
 #define SOMETHING (AAA + 5)
                    ^
unittests/test/test_unittest.c:25:36: note: in expansion of macro 'SOMETHING'
     int expected = a + x * x + x + SOMETHING;
                                    ^~~~~~~~~
ERROR: Shell command failed.
> Shell executed command:
'gcc -I"/var/lib/gems/2.5.0/gems/ceedling-0.31.1/vendor/unity/src" -I"/var/lib/gems/2.5.0/gems/ceedling-0.31.1/vendor/cmock/src" -I"unittests/build/test/mocks" -I"unittests/test" -I"unittests/test/support" -I"src" -I"." -I"inc" -I"driver/inc" -I"driver/hal" -D__STATIC_INLINE="static inline" -DTEST -DGNU_COMPILER -g -c "unittests/test/test_unittest.c" -o "unittests/build/test/out/c/test_unittest.o" -MMD -MF "unittests/build/test/dependencies/test_unittest.d"'
> And exited with status: [1].

#<Thread:0x0000556d90ed87c8@/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/par_map.rb:7 run> terminated with exception (report_on_exception is true):
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/tool_executor.rb:88:in `exec': ShellExecutionException (ShellExecutionException)
        from /var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/generator.rb:99:in `generate_object_file'
        from /var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/rules_tests.rake:17:in `block in <top (required)>'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:271:in `block in execute'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:271:in `each'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:271:in `execute'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:213:in `block in invoke_with_call_chain'
        from /usr/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:193:in `invoke_with_call_chain'
        from /usr/lib/ruby/vendor_ruby/rake/task.rb:182:in `invoke'
        from /var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/task_invoker.rb:97:in `block in invoke_test_objects'
        from /var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/par_map.rb:10:in `block (2 levels) in par_map'
rake aborted!
ShellExecutionException: ShellExecutionException
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/tool_executor.rb:88:in `exec'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/generator.rb:99:in `generate_object_file'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/rules_tests.rake:17:in `block in <top (required)>'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/task_invoker.rb:97:in `block in invoke_test_objects'
/var/lib/gems/2.5.0/gems/ceedling-0.31.1/lib/ceedling/par_map.rb:10:in `block (2 levels) in par_map'
Tasks: TOP => unittests/build/test/out/c/test_unittest.o
(See full trace by running task with --trace)
ERROR: Ceedling Failed

What am I doing wrong? Do I need to enable some of the other Ceedling setting? I need to be able to preprocess macros and to mock static functions in header files.

M-Bab commented 1 year ago

First of all thanks for writing this issue as it helped me to figure out my problem (which is exactly the same). I can confirm the issue and also provide info why this is the case and propose a solution.

Here is the problem step-by-step:

If :treat_inlines: :include is enabled, a modified header file needs to be generated (obviously because static/inline is removed from the header code as well). The modified header file can be found in test/mocks and is prioritized over the original header. The generation of this modified header happens in this line: https://github.com/ThrowTheSwitch/CMock/pull/261/files#diff-04520bc1e2e09e1dd300c2060876f6341f8b76093d60f845c52b8021bf36c5f1R66 as part of CMock.
If :use_test_preprocessor: TRUE is also enabled in Ceedling any header code provided to CMock is "filtered" through the actual GCC preprocessor. That preprocessor does a lot of stuff and among them is the removal of all macros - obviously because these macros are usually applied by the GCC preprocessor.
As consequence of 1+2 you end up with a modified header file that is not only stripped of the keywords static/inline but also of all macros. When the GCC compiler finally tries to compile all the sources and any piece of code tries to use a macro of the original file it fails.

Here are two solutions that came to my mind:

Let the preprocessor do its job but prevent the removal of macros. The gcc preprocessor has extensive options (https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html) where it might support anything like this, but I did not further dive into this. Also the ceedling option use_preprocessor_directives sounds pretty much like that, but did not resolve the problem described above.
Avoid feeding the preprocessor filtered file to the part of CMock which creates the modified header (without static/inline). Instead ensure this part gets the original header file where it might strip out all static/inline keywords. Then the modified but not preprocessed file is stored under test/mocks. This is completely legitimate because once it actually compiles the stuff with gcc the preprocessor will run over the header again and is able to apply all the macros which are still stored. I will create a pull request where @mvandervoord can check if this is a good/okay solution.

@laurensmiers as the creator of the original feature (https://github.com/ThrowTheSwitch/CMock/pull/261) you might also be interested in this issue.

Letme commented 1 year ago

The 2. solution is not what you want. The point of the use_test_preprocessor is to remove (well more like expand) the macros by preprocessor, so that CMock and other tools can correctly parse and generate only C valid (used) C code. In that part the 3. is actually what you would expect.

The top issue more points out that the path of the file included should be dependant on the preprocessed file and/or built file instead of original, or that inclusion of the file should actually be compiler argument include path dependant (not include directive dependant). It should also have include guard (which I agree is most probably the bug), but that will not solve the problem of removing the macro (which is what preprocessor does - not cmock, as you found out). So why do you want to use preprocessor if you do not want macros from header files to be removed?

M-Bab commented 1 year ago

The 2. solution is not what you want. The point of the use_test_preprocessor is to remove (well more like expand) the macros by preprocessor, so that CMock and other tools can correctly parse and generate only C valid (used) C code. In that part the 3. is actually what you would expect.

I fear you didnt fully grasp my description. CMock will still get the correctly preprocessed header to operate normally. Only the very specific part of generating a copy of the original header without the static/inline keywords operates on the original file and not on the preprocessed file.

But I have to admit this issue is pretty much brainfuck.

Letme commented 1 year ago

So you do not forward the preprocessed file to the CMock for stripping static inline functions, but the original file is passed to the function?

M-Bab commented 1 year ago

My solution forwards both parts to CMock: The preprocessed header file and the original file. All usual operations run as always in CMock only for the specific part of creating the copy with the static/inline keywords stripped the original file is fed.

Here are the decision tree options:

:treat_inlines: :exclude (default) - No copy of the original header file is created. The original header file is used for compilation.
:treat_inlines: :include and :use_test_preprocessor: FALSE - A copy of the original header with the static/include keywords stripped is created. This file works fine and still contains all the macros. It is obviously also what you get when you call CMock directly (without Ceedling there is no preprocessor option).
:treat_inlines: :include and :use_test_preprocessor: TRUE - This is the error case. A copy of the original header is created where all the macros are removed. If the tests try to build and need any macros from the original header this build will fail.

Letme commented 1 year ago

And (check if I am correct):

:treat_inlines: :exclude (default) and use_test_preprocessor: TRUE - Creates preprocessed copy of a header file where all macros are removed

M-Bab commented 1 year ago

Yes and no. There is a copy created which is preprocessed but not what I consider as a copy in the description above. So not a copy which is placed in the include path under test/mocks.

Yes: There is a cache copy somewhere and the preprocessed data is passed to CMock.

No: This preprocessed header is not used when compiling the header later. When compiling the unittests the original header is used again and therefore naturally preprocessed by GCC and all macros are available. If this would be different the problem would be much bigger.

So to be more specific:

:treat_inlines: :exclude (default) - No copy of the original header file is created in the include path. The original header file is used for compilation of the unittests. This statement is valid independent of the use_test_preprocessor setting.

informatimago commented 1 year ago

You may add -I. so that #include "driver/hal/hal_bbb.h" be successful. You can do that in project.yml:

:paths:
  :source:
    - .
    - hal # etc

But it's strange and this shouldn't be needed.

stemschmidt commented 7 months ago

I am very surprised that treat_inlines creates a new header file even if the original header does not have any inline functions!?!

I understand the need to create a header file if the inline implementation has to be mocked. But why is this a global setting and not depending on the content of the header?

M-Bab commented 7 months ago

Probably because its easier. Just do a batch processing of all files instead of actually looking into them and create a copy only if required. But even the "smarter" behavior would still be fatal: Chances that important macros and inline functions are in the same header are pretty high.

stemschmidt commented 7 months ago

Adding to your solution 2 from January 12th 2023: Wouldn't it be good to have the strippables also be applied to the modified header (if your compiler does not understand some options)? As far as I reverse engineered it this is only applied to the mocked files?

M-Bab commented 7 months ago

Phew ... the modified headers only exist for the sake of treat_inlines. I am not sure if my solution meddles with strippables in any way. I hope the behavior of strippables just stays exactly the same as without treat_inlines.

As also mentioned in my PRs: I am not very happy with the solution I created there. It was just the best shot I had in a few tries with very limited knowledge of ruby & rake.

mkarlesky commented 2 months ago

@i-adamov I believe the latest prerelease of Ceedling 1.0.0 (formerly 0.32) fixes the problems documented in this issue. Ceedling's much improved preprocessing and CMock's :treat_inlines now work together as they should. I am going to leave this issue open to collect any followup. For anyone following this thread, please let us know if this problem has been corrected.

M-Bab commented 2 months ago

This is great news and will make Ceedling 1.0 the framework we are absolutely looking forward to. I will try this out as soon as I can (but this might need till begin of August) and provide feedback, if this works well in our setup.

ThrowTheSwitch / Ceedling

Issue when combining use_test_preprocessor and CMock's treat_inlines #706