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

Controller/BUILD: Cannot add controller tests for certain platforms (Darwin, clang) #9173

Closed
sharadb-amazon opened this issue Aug 20, 2021 · 8 comments · Fixed by #9638
Closed
Assignees

Comments

@sharadb-amazon
Copy link
Contributor

sharadb-amazon commented Aug 20, 2021

Problem

This PR (#8824) attempts to add tests for src/controller: https://github.com/project-chip/connectedhomeip/pull/8824/files#diff-8b9a0a48a2fb362c82aef9724b90a5476c4b80358995f69c83324546c0c96648

This builds with gcc, but fails for clang, Darwin with the following errors:

Undefined symbols for architecture x86_64:
"chip::app::ReadSingleClusterData(chip::app::ClusterInfo&, chip::TLV::TLVWriter*, bool*)", referenced from:
chip::app::reporting::Engine::RetrieveClusterData(chip::app::AttributeDataElement::Builder&, chip::app::ClusterInfo&) in libCHIPDataModel.a(libCHIPDataModel.Engine.cpp.o)
"chip::app::ServerClusterCommandExists(unsigned int, unsigned int, unsigned short)", referenced from:
chip::app::CommandHandler::ProcessCommandDataElement(chip::app::CommandDataElement::Parser&) in libCHIPDataModel.a(libCHIPDataModel.CommandHandler.cpp.o)
"chip::app::DispatchSingleClusterCommand(unsigned int, unsigned int, unsigned short, chip::TLV::TLVReader&, chip::app::CommandHandler*)", referenced from:
chip::app::CommandHandler::ProcessCommandDataElement(chip::app::CommandDataElement::Parser&) in libCHIPDataModel.a(libCHIPDataModel.CommandHandler.cpp.o)
"chip::app::DispatchSingleClusterResponseCommand(unsigned int, unsigned int, unsigned short, chip::TLV::TLVReader&, chip::app::CommandSender*)", referenced from:
chip::app::CommandSender::ProcessCommandDataElement(chip::app::CommandDataElement::Parser&) in libCHIPDataModel.a(libCHIPDataModel.CommandSender.cpp.o)
"chip::app::WriteSingleClusterData(chip::app::ClusterInfo&, chip::TLV::TLVReader&, chip::app::WriteHandler*)", referenced from:
chip::app::WriteHandler::ProcessAttributeDataList(chip::TLV::TLVReader&) in libCHIPDataModel.a(libCHIPDataModel.WriteHandler.cpp.o)
ld: symbol(s) not found for architecture x86_64

Expectation is that src/controller/tests can be built and run for all platforms.

Proposed Solution

TBD

@mspang
Copy link
Contributor

mspang commented Aug 20, 2021

@yunhanw-google Why do we have these undefined symbols?

@erjiaqing
Copy link
Contributor

You need to add these files to XCode project.

@erjiaqing erjiaqing removed their assignment Aug 24, 2021
@sharadb-amazon
Copy link
Contributor Author

Not sure if I understood. The build failure happens for clang as well (if I include the controller tests for all platforms), not just Darwin. Which files/libs need to be added to the XCode project? Can you explain why that would fix this problem?

@erjiaqing
Copy link
Contributor

Not sure if I understood. The build failure happens for clang as well (if I include the controller tests for all platforms), not just Darwin. Which files/libs need to be added to the XCode project? Can you explain why that would fix this problem?

Seems I misunderstood your need, can you try adding ${chip_root}/src/controller/data_model to src/controller/tests/BUILD.gn

@sharadb-amazon
Copy link
Contributor Author

sharadb-amazon commented Aug 24, 2021

Tried that on a branch with this change. Build on Darwin succeeds, but then all-clusters-app build starts to fail on esp32, nrfconenct, qemu with the following type of error. Full logs: https://github.com/sharadb-amazon/connectedhomeip/runs/3413744436 This happens only when I make the above change.

../../../../../../config/esp32/third_party/connectedhomeip/src/controller/data_model/zap-generated/CHIPClientCallbacks.cpp: In function 'void ApplicationLauncherClusterApplicationLauncherListListAttributeFilter(chip::TLV::TLVReader*, chip::Callback::Cancelable*, chip::Callback::Cancelable*)':
../../../../../../config/esp32/third_party/connectedhomeip/src/controller/data_model/zap-generated/CHIPClientCallbacks.cpp:376:6: error: stack usage might be unbounded [-Werror=stack-usage=]
void ApplicationLauncherClusterApplicationLauncherListListAttributeFilter(TLV::TLVReader * tlvData,
...
...

@mspang
Copy link
Contributor

mspang commented Aug 24, 2021

Hold on, why would we need to add src/controller/data_model ?

@bzbarsky-apple
Copy link
Contributor

It looks like #8824 merged. @sharadb-amazon is there still a problem here? If so, what are the steps to reproduce?

@bzbarsky-apple
Copy link
Contributor

It looks like #8824 merged by just not building these tests on various OSes, and #9374 disabled them on still others where they were failing.

Fixing the build on Darwin here is pretty simple: just use the right spelling of "darwin" in the chip_device_platform tests in src/controller/tests/BUILD.gn (and why are there platform-specific bits here to start with???). But then the test crashes:

    frame #0: 0x00007fff67383143 libdispatch.dylib`dispatch_source_create + 167
  * frame #1: 0x00000001000c6c9c TestDevice`chip::Inet::UDPEndPoint::Bind(this=0x000000010012fbd8, addrType=kIPAddressType_IPv4, addr=0x000000010012e6c0, port=5540, intfId=0) at UDPEndPoint.cpp:254:27
   ...
frame #1: 0x00000001000c6c9c TestDevice`chip::Inet::UDPEndPoint::Bind(this=0x000000010012fbd8, addrType=kIPAddressType_IPv4, addr=0x000000010012e6c0, port=5540, intfId=0) at UDPEndPoint.cpp:254:27
   251 	    {
   252 	        unsigned long fd = static_cast<unsigned long>(mSocket);
   253 	
-> 254 	        mReadableSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_READ, fd, 0, dispatchQueue);
   255 	        ReturnErrorCodeIf(mReadableSource == nullptr, CHIP_ERROR_NO_MEMORY);
   256 	
   257 	        dispatch_source_set_event_handler(mReadableSource, ^{

where dispatchQueue is a garbage value because this test never initialized the platform manager.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 13, 2021
1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes project-chip#9173
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 13, 2021
1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes project-chip#9173
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 13, 2021
1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes project-chip#9173
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 13, 2021
1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes project-chip#9173
bzbarsky-apple added a commit that referenced this issue Sep 13, 2021
…9638)

1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes #9173
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

Successfully merging a pull request may close this issue.

6 participants