riebl / vanetza

Open-source implementation of the ETSI C-ITS protocol stack
Other
204 stars 156 forks source link

Timestamp Its sub-type Problem on 32 bit #78

Closed om3g4zell closed 5 years ago

om3g4zell commented 5 years ago

Hi, it's me again,

I have implemented DENM, it works between two 64 bits devices but it doesn't work on 32 bit devices. I have this error at the message sending

Exit: Can't determine size for unaligned PER encoding of type DENM because of TimestampIts sub-type

At this lines

auto confirm = Application::request(request, std::move(packet));
if (!confirm.accepted()) {
    throw std::runtime_error("DENM application data request failed");
}

I set the timestampIts sub types here :

const auto time_now = duration_cast<milliseconds>(runtime_.now().time_since_epoch());
int ret1 = asn_long2INTEGER(&message->denm.management.detectionTime, time_now.count());
int ret2 = asn_long2INTEGER(&message->denm.management.referenceTime, TimestampIts_utcStartOf2004);
std::cout << "ret 1 : " << ret1 << "ret 2 : " << ret2 << std::endl;
assert(ret1 + ret2 == 0);

We have ret1 + ret2 = 0, then the convertion works. TimeStampIts is an Integer_t May the issue be caused by the fact that the system used to generate asn1 files was 64 bits ?

riebl commented 5 years ago

As far as I know, the long type is just 32-bit wide on 32-bit systems. You may try the asn_int642INTEGER function from asn1c instead of asn1_long2INTEGER.

om3g4zell commented 5 years ago

Thanks for your answer, but i have the same message

valt2017 commented 5 years ago

The only working solution on 32bit systems I found (idea comes from OpenC2X and Wireshark ITS plugin): Change Timestamp from Integer to Bitstring in ASN and recompile. TimestampIts ::= BIT STRING(SIZE(42)) -- units of milliseconds, 7 byte --TimestampIts ::= INTEGER {utcStartOf2004(0), oneMillisecAfterUTCStartOf2004(1)} (0..4398046511103) Unfortunatelly you have to handle it in your code as Bitstring and order bytes accordingly. If you find something better please let me know.

om3g4zell commented 5 years ago

Thanks for your answer, i compiled ASN files with the patch but i donc know how to handle bit string here

&message->denm.management.detectionTime
&message->denm.management.referenceTime

Can you give me some advice ? Is there any other changes to do ?

om3g4zell commented 5 years ago

I tested without set anything in

&message->denm.management.detectionTime
&message->denm.management.referenceTime

And it works !! Thanks for the trick 👍

om3g4zell commented 5 years ago

For those which have the same problem, just replace TimeStampIts.h and .c in vanetza/asn1/its/

TimeStampIts.h

/*
 * Generated by asn1c-0.9.29 (http://lionet.info/asn1c)
 * From ASN.1 module "ITS-Container"
 *  found in "ITS_ASN1-CDD_TS102894-2-ITS-Container.asn"
 *  `asn1c -fcompound-names -fincludes-quoted -no-gen-example -D its/`
 */

#ifndef _TimestampIts_H_
#define _TimestampIts_H_

#include "asn_application.h"

/* Including external dependencies */
#include "BIT_STRING.h"

#ifdef __cplusplus
extern "C" {
#endif

/* TimestampIts */
typedef BIT_STRING_t     TimestampIts_t;

/* Implementation */
extern asn_per_constraints_t asn_PER_type_TimestampIts_constr_1;
extern asn_TYPE_descriptor_t asn_DEF_TimestampIts;
asn_struct_free_f TimestampIts_free;
asn_struct_print_f TimestampIts_print;
asn_constr_check_f TimestampIts_constraint;
ber_type_decoder_f TimestampIts_decode_ber;
der_type_encoder_f TimestampIts_encode_der;
xer_type_decoder_f TimestampIts_decode_xer;
xer_type_encoder_f TimestampIts_encode_xer;
oer_type_decoder_f TimestampIts_decode_oer;
oer_type_encoder_f TimestampIts_encode_oer;
per_type_decoder_f TimestampIts_decode_uper;
per_type_encoder_f TimestampIts_encode_uper;

#ifdef __cplusplus
}
#endif

#endif  /* _TimestampIts_H_ */
#include "asn_internal.h"

TimeStampIts.c

/*
 * Generated by asn1c-0.9.29 (http://lionet.info/asn1c)
 * From ASN.1 module "ITS-Container"
 *  found in "ITS_ASN1-CDD_TS102894-2-ITS-Container.asn"
 *  `asn1c -fcompound-names -fincludes-quoted -no-gen-example -D its/`
 */

#include "TimestampIts.h"

int
TimestampIts_constraint(const asn_TYPE_descriptor_t *td, const void *sptr,
            asn_app_constraint_failed_f *ctfailcb, void *app_key) {
    const BIT_STRING_t *st = (const BIT_STRING_t *)sptr;
    size_t size;

    if(!sptr) {
        ASN__CTFAIL(app_key, td, sptr,
            "%s: value not given (%s:%d)",
            td->name, __FILE__, __LINE__);
        return -1;
    }

    if(st->size > 0) {
        /* Size in bits */
        size = 8 * st->size - (st->bits_unused & 0x07);
    } else {
        size = 0;
    }

    if((size == 42)) {
        /* Constraint check succeeded */
        return 0;
    } else {
        ASN__CTFAIL(app_key, td, sptr,
            "%s: constraint failed (%s:%d)",
            td->name, __FILE__, __LINE__);
        return -1;
    }
}

/*
 * This type is implemented using BIT_STRING,
 * so here we adjust the DEF accordingly.
 */
static asn_oer_constraints_t asn_OER_type_TimestampIts_constr_1 CC_NOTUSED = {
    { 0, 0 },
    42  /* (SIZE(42..42)) */};
asn_per_constraints_t asn_PER_type_TimestampIts_constr_1 CC_NOTUSED = {
    { APC_UNCONSTRAINED,    -1, -1,  0,  0 },
    { APC_CONSTRAINED,   0,  0,  42,  42 }  /* (SIZE(42..42)) */,
    0, 0    /* No PER value map */
};
static const ber_tlv_tag_t asn_DEF_TimestampIts_tags_1[] = {
    (ASN_TAG_CLASS_UNIVERSAL | (3 << 2))
};
asn_TYPE_descriptor_t asn_DEF_TimestampIts = {
    "TimestampIts",
    "TimestampIts",
    &asn_OP_BIT_STRING,
    asn_DEF_TimestampIts_tags_1,
    sizeof(asn_DEF_TimestampIts_tags_1)
        /sizeof(asn_DEF_TimestampIts_tags_1[0]), /* 1 */
    asn_DEF_TimestampIts_tags_1,    /* Same as above */
    sizeof(asn_DEF_TimestampIts_tags_1)
        /sizeof(asn_DEF_TimestampIts_tags_1[0]), /* 1 */
    { &asn_OER_type_TimestampIts_constr_1, &asn_PER_type_TimestampIts_constr_1, TimestampIts_constraint },
    0, 0,   /* No members */
    &asn_SPC_BIT_STRING_specs   /* Additional specs */
};
riebl commented 5 years ago

I would prefer to get rid of any dirty workraounds and solve the issue properly. Is the value of time_now.count() negative on 32-bit systems? TimestampIts allows only positive values so it may be the type constraint firing here. vanetza::Clock uses int64_t (so it can represent negative durations) but I guess this can cause problems when vanetza::Clock::time_point overflows.

om3g4zell commented 5 years ago

I tried with dummy values like 10 for both members, and i had the same problem

riebl commented 5 years ago

I have compiled Vanetza with -m32 flag and can confirm that even setting timestamps to TimestampIts_utcStartOf2004 fails while it is working fine in 64-bit mode, i.e. without -m32 flag. Out of curiosity, I have generated the code of asn1/its again with a 32-bit version of asn1c but without any difference.

Unfortunately, I am not that familiar with the internals of asn1c. However, stepping with the debugger into INTEGER_encode_uper revealed that it fails after the range check in INTEGER.c line 757 (value = 0 is greater than ct->upper_bound = -1).

riebl commented 5 years ago

I have found a fix and ask you test it on your systems as well, see my asn1c_per_intmax branch. Basically, I have changed a few long and unsigned long to intmax_t and uintmax_t, respectively. I guess this slows down encoding on 32bit systems slightly and thus I am not sure if this modification is appropriate for upstream asn1c.

om3g4zell commented 5 years ago

I just tested it and it works ! But i don't know wich patch is the best, maybe yours because we don't need to change the code between 32 bits and 64 bits. Please let me know if you will merge it to master to know wether I'll have to adapt my code or not

Thanks,

valt2017 commented 5 years ago

Although the official version of ASN1C recommends a 64-bit compiler (https://github.com/vlm/asn1c/blob/master/REQUIREMENTS.md) some fix of this issue is already mentioned together with pull request: https://github.com/vlm/asn1c/issues/303

riebl commented 5 years ago

@valt2017 Thanks for pointing to this issue ticket, I have not been aware of it. Unfortunately, the previous pull request has not been merged and the submitter closed it. I will try to maintain my pull request as long as it takes to get it merged upstream (hopefully).

riebl commented 5 years ago

@om3g4zell master branch includes fixed code now. I have used a patched version of asn1c: https://github.com/riebl/asn1c/commit/2a9e2a3cdc494fe7c587f952d50c9dc7b8ab0e8e

om3g4zell commented 5 years ago

Ok Thanks !