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

An anonymous union can only have non-static data members #43

Closed
krjw opened this issue Dec 17, 2018 · 14 comments
Closed

An anonymous union can only have non-static data members #43

krjw opened this issue Dec 17, 2018 · 14 comments

Comments

@krjw
Copy link

krjw commented Dec 17, 2018

I am trying to compile a c++ program with the cryptoauthlib and I get this (title) error in the atca_iface.h. I think this is a c11 feature, which is not compatible with c++:

All named structs in the unnamed union throw the error.

typedef struct
{

    ATCAIfaceType  iface_type;      // active iface - how to interpret the union below
    ATCADeviceType devtype;         // explicit device type

    union                           // each instance of an iface cfg defines a single type of interface
    {
        struct ATCAI2C
        {
            uint8_t  slave_address; // 8-bit slave address
            uint8_t  bus;           // logical i2c bus number, 0-based - HAL will map this to a pin pair for SDA SCL
            uint32_t baud;          // typically 400000
        } atcai2c;

        struct ATCASWI
        {
            uint8_t bus;        // logical SWI bus - HAL will map this to a pin	or uart port
        } atcaswi;

        struct ATCAUART
        {
            int      port;      // logic port number
            uint32_t baud;      // typically 115200
            uint8_t  wordsize;  // usually 8
            uint8_t  parity;    // 0 == even, 1 == odd, 2 == none
            uint8_t  stopbits;  // 0,1,2
        } atcauart;

        struct ATCAHID
        {
            int      idx;           // HID enumeration index
            uint32_t vid;           // Vendor ID of kit (0x03EB for CK101)
            uint32_t pid;           // Product ID of kit (0x2312 for CK101)
            uint32_t packetsize;    // Size of the USB packet
            uint8_t  guid[16];      // The GUID for this HID device
        } atcahid;

        struct ATCACUSTOM
        {
            ATCA_STATUS (*halinit)(void *hal, void *cfg);
            ATCA_STATUS (*halpostinit)(void *iface);
            ATCA_STATUS (*halsend)(void *iface, uint8_t *txdata, int txlength);
            ATCA_STATUS (*halreceive)(void *iface, uint8_t* rxdata, uint16_t* rxlength);
            ATCA_STATUS (*halwake)(void *iface);
            ATCA_STATUS (*halidle)(void *iface);
            ATCA_STATUS (*halsleep)(void *iface);
            ATCA_STATUS (*halrelease)(void* hal_data);
        } atcacustom;

    };

    uint16_t wake_delay;    // microseconds of tWHI + tWLO which varies based on chip type
    int      rx_retries;    // the number of retries to attempt for receiving bytes
    void *   cfg_data;      // opaque data used by HAL in device discovery
} ATCAIfaceCfg;

I changed these structs to unnamed structs and it compiles, but I unfortunately get a segmentation fault when calling the init function ATCA_STATUS atinit(ATCAIface ca_iface);... I do not call this function directly. I call atcab_init(&cfg_ateccx08a_i2c_default);

Is there a way to use this without modifying the interface like I did and why do I get the segmentation fault?

My hardwaresetup is a CM3 with a ATECC608a connected to I2C 1. The interface is up and I can query it. Do I need to modify the default interface to the correct device type and the correct I2C interface? When I do so I get the same segmentation fault.

Thanks in advance! Greetings i7clock

@bryan-hunt
Copy link
Contributor

bryan-hunt commented Dec 17, 2018

The normal reason for this is that you also need to specify the HAL you’re using by defining ATCA_HAL_I2C globally (e.g. add '-DATCA_HAL_I2C' to your cflags)

@krjw
Copy link
Author

krjw commented Dec 17, 2018

Thanks! That gives me undefined references. I guess I missed something else? This is unrelated to the C11 issue right?

@bryan-hunt
Copy link
Contributor

Yes. I understand the c11 vs c++ issue. You have to include the HAL driver file(s) into your build as well. The convention is hal_platform_timer.c and hal_platform_i2c.c etc.

What is the platform you’re using?

@krjw
Copy link
Author

krjw commented Dec 17, 2018

what do you mean with platform?
armhf?
RPI3?
debian stretch?

@bryan-hunt
Copy link
Contributor

Ah okay. https://github.com/MicrochipTech/cryptoauthlib/blob/master/lib/CMakeLists.txt is a good reference for everything that is required for building the library. This file will properly build a .so for Linux (cmake does need to be invoked with -DATCA_HAL_I2C=on however)

@krjw
Copy link
Author

krjw commented Dec 17, 2018

I did set that in cmake :)
Oh now I understand what you meant with platform... Well I am using MacOS, running parallels for Windows 10, which crosscompiles for the rpi3 so I guess I need to set UNIX in cmake for the target??

GCC 7.3.1

@bryan-hunt
Copy link
Contributor

Oh boy. Okay so I haven’t attempted a pi cross compile. I’d have to look up some cmake cross compile stuff. But yes it does appear to be detecting the Windows environment rather than the target.

@krjw
Copy link
Author

krjw commented Dec 17, 2018

I unset WIN32 and APPLE and explicitly set UNIX but there i no change. I am using VisualGDB and I have running projects with that toolchain so in general this is not a problem.

@bryan-hunt
Copy link
Contributor

What are the undefined references? Are they things like hal_i2c_send or is it failing to link against libraries like udev? (That part can be removed from the CMakeLists.txt file. In our next release this will be taken care of automatically but for the moment it tries to link to that always)

@krjw
Copy link
Author

krjw commented Dec 17, 2018

It is things like hal_i2c_send. udev should not be the problem... here the command by cmake:
C:/SysGCC/gcc-linaro-7.3.1-2018.05-i686-mingw32_arm-linux-gnueabihf/bin/arm-linux-gnueabihf-g++.exe -std=c++17 -g "CMakeFiles/cryptoloader.dir/cryptoloader.cpp.o" -o cryptoloader -Wl,-rpath,C:/Users/User/source/repos/cryptoloader/cryptoloader/VisualGDB/Debug/cryptoauthlib/lib cryptoauthlib/lib/libcryptoauth.so -ludev -lrt

cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_sleep
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_send
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_discover_buses
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_wake
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_release
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_idle
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_receive
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_init
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_post_init
cryptoauthlib/lib/libcryptoauth.so(0): error : undefined reference to hal_i2c_discover_devices

@krjw
Copy link
Author

krjw commented Dec 18, 2018

Sry to ask, but I cannot find these functions anywhere.. Do you know what I am missing here?

EDIT1:

Found that I had to do set(ATCA_HAL_I2C TRUE) explicitly ... as an option it did not work for me, unfortunately.

Now I get a segmentation fault again here:

ATCA_STATUS atcab_random(uint8_t *rand_out)
{
    ...
    ATCACommand ca_cmd = _gDevice->mCommands;

    ...
}

@krjw
Copy link
Author

krjw commented Dec 18, 2018

I found the error. The Hardware Abstraction Layer could not connect because the default setting of the bus for the i2c bus .atcai2c.bus was set to 2 and it needs to be 1 otherwise it won't find the i2c device.

ATCAIfaceCfg cfg_ateccx08a_i2c_default = {
    .iface_type             = ATCA_I2C_IFACE,
    .devtype                = ATECC608A,
    .atcai2c.slave_address  = 0xC0,
    .atcai2c.bus            = 1,
    .atcai2c.baud           = 400000,
    //.atcai2c.baud = 100000,
    .wake_delay             = 1500,
    .rx_retries             = 20
};

I can now get the revision, the serial and random numbers from the chip with the following:

#include <iostream>
#include "cryptoauthlib.h"


int main(int argc, char *argv[])
{

	ATCAIfaceCfg *atca_cfg;
	atca_cfg = &cfg_ateccx08a_i2c_default;

	ATCA_STATUS status = atcab_init(atca_cfg);

	std::cout << "status: " << (int)status << std::endl;

	uint8_t revision[4];
	status = atcab_info(revision);
	if (status != ATCA_SUCCESS)
	{
		return -1;
	}
	std::cout << "revision: " << (int)revision << std::endl;

	uint8_t serial[ATCA_SERIAL_NUM_SIZE];
	status = atcab_read_serial_number(serial);
	if (status != ATCA_SUCCESS)
	{
		return - 1;
	}
	std::cout << "serial: " << (int)serial << std::endl;

	uint8_t num[32];
	status = atcab_random(num);
	if (status != ATCA_SUCCESS)
	{
		return - 1;
	}
	std::cout << "random: " << (int)num << std::endl;
	return 0;
}

Should this be documented somewhere else?

@krjw krjw closed this as completed Dec 18, 2018
@bryan-hunt
Copy link
Contributor

This is an area that I think falls less into documentation and more in the area of needing to fail better both at compile (when no HAL is specified) and runtime (when invalid bus/drivers are used). We are working on some improvements to the HAL layers that should be able to pass back better error codes instead of failing in this way.

We've done quite a bit of this with the PKCS11 implementation and documentation (as it's primarily driven by Raspberry Pi and Yocto builds) but we have not made that logic generic yet.

@Petezah
Copy link

Petezah commented May 6, 2019

The answer to this (which closed the issue) does not actually address the actual issue (anonymous structs/unions error). This is still an issue. I had to work around this by overriding the atca_iface.h header file with a solution like the OP mentioned. (I did this because of wanting to leave the upstream module intact, since it is for a contribution/integration for another project. It consumes it as a git submodule.)

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