bigbrett
commented
3 years ago Potentially related to #313 and Ceedling/issues/541
Open bigbrett opened 3 years ago
bigbrett
commented
3 years ago Potentially related to #313 and Ceedling/issues/541
bigbrett
commented
3 years ago @mvandervoord any chance you might be able to look at this? Thought upgrading to 2.5.2 (which has fix for #313) would fix, but seems it wasn't related. Let me know any other info I can provide to help.
Right now I'm running
Ceedling:: 0.30.0
Unity:: 2.5.1
CMock:: 2.5.2
CException:: 1.3.2
mvandervoord
commented
3 years ago Is it possible you can send me the header file you are attempting to mock? (If you're not comfortable posting it here, you are welcome to send it to me directly).
bigbrett
commented
3 years ago Is it possible you can send me the header file you are attempting to mock? (If you're not comfortable posting it here, you are welcome to send it to me directly).
Whoops sorry it wasn't clearer in my original post. It's nrfx_rtc.h
bigbrett
commented
3 years ago If you define 'SUPPRESS_STATIC_INLINE' the sdk will remove the definitions, however it still has the prototypes with static inline defined.
bigbrett
commented
3 years ago @mvandervoord any more info I can provide to be helpful?
bigbrett
commented
3 years ago @mvandervoord I've managed to replicate this issue on a completely different project with a completely different SDK. The treatment of static inlines functions seems to be completely broken. Do you have enough information to deal with this?
Letme
commented
3 years ago You should use :strippables: CMock configuration and add __STATIC_INLINE to it. For starters. The relative includes: "good luck with that" is the most probable answer, but otherwise manual adding of include paths to make that relative paths useful as you already did. The timekeeper_test has a segmentation fault - that is cause of the bad access, memory wise. What happens if you just run ./test_timekeeper.out ? Does gdb give you any more information?
Also post a content of test_timekeeper.c which produces segmentation fault?
I still don't think this has anything to do with the static inline since mocks are created (look at file: mock_nrfx_rtc.c).That file that you pasted also has no direct memory access (quick glance), so maybe you are doing that in your test file or something that is not mocked? So problem with mocks could be if there is something strange in mock_nrfx_rtc.c.
bigbrett
commented
3 years ago @Letme thats good info re: :strippables: I'll look into that. In the meantime, the other project that I replicated the issue on doesn't use any macro magic, just normal static inline.
Re: the segfault, yes that is when I manually edited the autogenerated mocks to try and massage it into working...I wouldn't put too much credence in that
bigbrett
commented
3 years ago the relative path issue is unfortunately at the core of the problem
bigbrett
commented
3 years ago @Letme why do you think the relative path issues are a "good luck" type of thing? Isn't that part of the core functionality of how all of this should work, and it's broken? The handling of static inline functions right now is wrong.
I'm trying to dig through the ruby code to figure out where and why, but I don't know ruby so am a bit lost. It looks like @laurensmiers wrote a lot of the code for the static inline functions...hopefully it isn't inappropriate to ping you on this.
As an example with a completely different SDK (an NXP chip this time):
given the following source layout:
common/hal/fsl_flexspi.h // contains static inline definition in the header
common/proto/proto_flash.{c,h} // includes fsl_flexspi.hand the following project include paths and inline configuration
:paths:
:source:
- common/hal/**
- common/proto/**
:treat_inlines: :includeUnit tests for proto_flash.c will fail to build because the generated file with inlines removed at build/test/fsl_flexspi.h contains the wrong include file path (#include "common/hal/fsl_flexspi.h") instead of the correct (#include "fsl_flexspi.h")
I can't fix this by adding the root of my project to the include path, as the include string to the compiler invocation is too long and ceedling crashes.
bigbrett
commented
3 years ago Here is a git repo containing a bare-minimum reproduction of the bug, with only three files and one test.
https://github.com/bigbrett/cmock-bug-repro
Clone the repo and run ./ceedling to see how the generated include paths are broken.
Letme
commented
3 years ago As an easy answer: The core of unit testing is to swap paths, so if you have /bla/sub/folder/unit.h you want to have it in build/bla/sub/folder/unit.h. Now, this can become a problem with relative paths and hence we should only #include "unit.h" and let the linker do the path swapping.
I went a bit through your example and I see the issue where gcc includes_preprocessor_tool returns common/hal/fsl_common.h as include path. Not sure if this is new or something, but it seems wrong:
$ ./vendor/ceedling/bin/ceedling verbosity[4] test
...
Generating include list for fsl_flexspi.h...
> Shell executed command:
'gcc -E -MM -MG -I"/home/bla/cmock-bug-repro/vendor/ceedling/vendor/unity/src" -I"/home/bla/cmock-bug-repro/vendor/ceedling/vendor/cmock/src" -I"build/test/mocks" -I"test" -I"test/common" -I"test/common/proto" -I"common" -I"common/hal" -I"common/proto" -DCPU_MIMXRT1051DVL6B -DTEST -DUNITY_INCLUDE_PRINT_FORMATTED -DCPU_MIMXRT1051DVL6B -DTEST -DGNU_COMPILER "build/temp/_fsl_flexspi.h"'
> Produced output:
_fsl_flexspi.o: build/temp/_fsl_flexspi.h common/hal/fsl_common.h \
@@@@fsl_common.h
...I also moved the test file around different subfolders and that makes no effect, so its purely something to do with preprocessor and adding includes. I think somehow that relative path is added to the file, while it is skipped for non-inline parser. Same as you, my Ruby knowledge is basically non-existent, so all you can do is wait...
laurensmiers
commented
3 years ago Hi @bigbrett,
Thanks for the example. What cmock currently does to generate that new header (in build/test/mocks) is simpler than you expect I think. We just replace the inline functions with regular declarations and that's it. That was my initial goal, to only touch the bare minimum to mock the header containing the inline functions since cmock doesn't know what else is in that header that could be important and shouldn't be changed. So there is no fancy stuff happening to convert include paths, etc.
This explains why the adding of the root-directory to the include paths in your example works. The include in the original header expects the root of the project to be in the include-path, so the generated header just has the same assumptions/requirements.
We don't replace includes in the generated file to be relative to the include directories specified in the project.yml.
This is not the behaviour I would expect from cmock and I wouldn't know how it could figure out the diff between the original include path and the relative-path to the include directories specified in the project.yml (in a consistent way that is).
bigbrett
commented
3 years ago @laurensmiers thanks for responding!
This explains why the adding of the root-directory to the include paths in your example works. The include in the original header expects the root of the project to be in the include-path, so the generated header just has the same assumptions/requirements.
Hmmm except that isn't quite true... the include in the original header is not a path relative from project root (e.g. it doesn't expect the root of the project to be in the include-path, as you claimed). That is why I think this is a bug. Unless I am misunderstanding you?
The include in the original header is #include "fsl_common.h" whereas the include in the generated header with the inline stuff stripped out is #include "common/hal/fsl_common.h"
laurensmiers
commented
3 years ago Oh damn, sorry completely missed that part.... Will read @Letme 's comment a bit more in detail cause the rabbit hole seems to be deeper than I thought.
Some quick observations: cmock seems to receive the header with already a full relative path iso #include "fsl_common.h" as source-to-be-parsed when it gets to mocking it (+transforming the inline functions), so it's happening before cmock gets to touch the file.
If I disable the test preprocessor (in project.yml: :use_test_preprocessor: FALSE iso TRUE) in your example code, I don't see the issue.
I know the testprocessor setting runs the test files and headers through the gcc preprocessor and hands these outputs to ceedling/cmock to be parsed (so cmock doesn't know about the original include here actually...), but I wouldn't expect him to change paths like this (but again, this may be due to my lack of knowledge) Will try to understand this better but can't promise a solution. To fix this (very crude solution), cmock would need to know the original file (or have access to its original non-preprocessed sources) and use that to generate the non-inline header. However, I guess some users want this behaviour, so you would need a way to flag a header as use_test_preprocessor or not, which quickly becomes a mess I fear.
EDIT: Using the test preprocessor together with the static-inline mocking is giving several issues ( I think #328 is also related to this). To be honest, I don't know what the best solution is to be able to use these together. I'd have to look more into what ceedling does with the headers and how it calls cmock for generating the mocks.
Letme
commented
3 years ago @laurensmiers ok, so we figured that preprocessor is the problem for the inlines as it adds something too much I assume. Maybe a solution would be to check the output of the preprocessor to see if the other non-inline parser skips something. Because I know you are dealing with includes and maybe when :use_test_preprocessor: is enabled, you should actually not deal with them, or deal differently? That might be the difference...
bigbrett
commented
3 years ago interesting....so this could be a ceedling pre-processor bug? @laurensmiers Should I file an issue in the ceedling repo?
Unfortunately, I'm not able to turn off :use_test_preprocessor: without breaking other tests.
CezaryGapinski
commented
3 years ago Hi!
I'm trying to fix it in ceedling when preprocessor is enabled. If anybody interested here is my branch for tests.
I think ceedling shouldn't take preprocessed header with expanded include paths to generate parsed header with non-inline functions. When :treat_inlines option is disabled then generated mock points to real unpreprocessed header, so when :treat_inlines is set to :include mock also should points to generated header which is not preprocessed and let compiler to preprocess and compile everything in the next steps.
At this moment I have to generate mock twice, once for unpreprocessed header to generate header without inlines, and next again to generate mock based on preprocessed header.
Letme
commented
3 years ago Why would you still want relative paths even in the preprocessed header? Tests include :source: includes as well as :test: includes and order determines what you get. So if :treat_inlines is set to :include then the original header is generated inside build folder, otherwise it stays where it is. Linker will find it in both ways right?
bigbrett
commented
3 years ago I agree, I don't think relative paths should ever be a thing. Explicit includes in the original source files should not be changed, period, right? It should be up to the developer to setup the correct include paths in project.yml-->:source: such that the source includes can be resolved by the linker.
CezaryGapinski
commented
3 years ago Why would you still want relative paths even in the preprocessed header? Tests include :source: includes as well as :test: includes and order determines what you get. So if :treat_inlines is set to :include then the original header is generated inside build folder, otherwise it stays where it is. Linker will find it in both ways right?
@Letme I think the linker has nothing to do with this. It is preprocessor job.
I have doubts if :treat_inlines :include can work when header file is placed in the same directory with source file. I think gcc always first search #include "file.h" relative to the directory of the current file, so always original header will be included for this case (https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html).
It has chances to work if the header is in the different directory. Include path with generated mocks should have priority, and the generated header should be found first. But if in one test file we are using mocked version and in the next one standard version, then this second test will fail in ceedling due to undefined reference to the function (the generated header funciton declaration will be included instead of original function definition).
It is not that easy as I thought.
CezaryGapinski
commented
3 years ago If anybody interested I updated my branch. Now I'm trying to cheat compiler to use generated header with the highest priority by the -include file option from gcc which can help to "Process file as if #include "file" appeared as the first line of the primary source file.".
bigbrett
commented
3 years ago @CezaryGapinski I'm interested! Let me know when you are ready and I can test
CezaryGapinski
commented
3 years ago @bigbrett Great! Please test as it is and we will see. I I don't have any more clever idea at this moment. I tested it on my more complex project and it works, but I'm curious if it help you.
bigbrett
commented
3 years ago @CezaryGapinski I'm not able to get it to succeed on the test repo I provided. Very possible I'm doing something wrong though. Do I need to tweak the project.yml file at all?
CezaryGapinski
commented
3 years ago @bigbrett Please go to vendor folder in your project and remove ceedling folder. Next clone my branch in this folder:
git clone --branch static-inline-headers --recursive https://github.com/CezaryGapinski/ceedling.gitNext try to run ceedling clobber test:all
You should have this output:
Clobbering all generated files...
(For large projects, this task may take a long time to complete)
Test 'test_proto_flash.c'
-------------------------
Generating include list for fsl_flexspi.h...
Creating mock for fsl_flexspi...
Creating mock for fsl_flexspi...
Generating include list for proto_flash.c...
Generating runner for test_proto_flash.c...
Compiling test_proto_flash_runner.c...
Compiling test_proto_flash.c...
Compiling mock_fsl_flexspi.c...
Compiling unity.c...
Compiling proto_flash.c...
common/proto/proto_flash.c: In function ‘proto_flash_init’:
common/proto/proto_flash.c:5:20: warning: passing argument 1 of ‘FLEXSPI_SoftwareReset’ makes pointer from integer without a cast [-Wint-conversion]
5 | #define FLASH_ADDR (0xFEEDBEEF)
| ^~~~~~~~~~~~
| |
| unsigned int
common/proto/proto_flash.c:9:27: note: in expansion of macro ‘FLASH_ADDR’
9 | FLEXSPI_SoftwareReset(FLASH_ADDR);
| ^~~~~~~~~~
In file included from <command-line>:
./build/test/mocks/fsl_flexspi.h:9:35: note: expected ‘void *’ but argument is of type ‘unsigned int’
9 | void FLEXSPI_SoftwareReset(void * addr);
| ~~~~~~~^~~~
Compiling cmock.c...
Linking test_proto_flash.out...
Running test_proto_flash.out...
--------------------
OVERALL TEST SUMMARY
--------------------
TESTED: 1
PASSED: 1
FAILED: 0
IGNORED: 0Of course there is a warning but I think you should change #define FLASH_ADDR (0xFEEDBEEF) to
#define FLASH_ADDR (void *)(0xFEEDBEEF)
harryofskyrim
commented
3 years ago
- When I try and mock one of the functions that happens to be static inline in nrfx_rtc.h by using :cmock: :treat_inlines: :include in project.yml, the output at build/test/mocks/nrf_rtc.h contains TWO different definitions for each original instance of static inline functions... one that is static inline and one that is not...
@bigbrett Did you ever solve your second issue?
M-Bab
commented
1 year ago Please check if this is the problem described here: https://github.com/ThrowTheSwitch/Ceedling/issues/706#issuecomment-1380244582
When I try to mock driver functions from a file in the NRF5 SDK that contain
static inlinefunctions the generated mocks seem broken.For example, lets have a module in my app called
timekeeper.cthat depends upon nrfx_rtc.h (which should be mocked).The two errors I can see in the generated mocks are:
project.yml. Therefore it results in the file not being able to be discovered?static inlineinnrfx_rtc.hby using:cmock: :treat_inlines: :includeinproject.yml, the output atbuild/test/mocks/nrf_rtc.hcontains TWO different definitions for each original instance ofstatic inlinefunctions... one that isstatic inlineand one that is not...e.g.That is about as deep as I am able to trace things down. When I manually edit the generated files to have the correct include path, it chokes on the two different definitions. When I try and manually remove one of the definitions just to see what would happen, the test never executes