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

Timestamp Its sub-type Problem on 32 bit #78

Closed
om3g4zell opened this issue Jul 17, 2019 · 15 comments

Comments

@om3g4zell
Copy link

commented Jul 17, 2019

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

This comment has been minimized.

Copy link
Owner

commented Jul 17, 2019

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

This comment has been minimized.

Copy link
Author

commented Jul 17, 2019

Thanks for your answer,
but i have the same message

@valt2017

This comment has been minimized.

Copy link

commented Jul 17, 2019

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

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

I tested without set anything in

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

And it works !! Thanks for the trick 👍

@om3g4zell

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

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

This comment has been minimized.

Copy link
Owner

commented Jul 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

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

@riebl

This comment has been minimized.

Copy link
Owner

commented Jul 19, 2019

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

This comment has been minimized.

Copy link
Owner

commented Jul 19, 2019

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

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

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,

riebl added a commit to riebl/asn1c that referenced this issue Jul 19, 2019

per_support: use intmax_t for integer constraints
long causes trouble on 32bit systems, see riebl/vanetza#78
@valt2017

This comment has been minimized.

Copy link

commented Jul 21, 2019

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:
vlm/asn1c#303

@riebl

This comment has been minimized.

Copy link
Owner

commented Jul 22, 2019

@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 riebl closed this in 38ab63c Aug 2, 2019

@riebl

This comment has been minimized.

Copy link
Owner

commented Aug 2, 2019

@om3g4zell master branch includes fixed code now.
I have used a patched version of asn1c: riebl/asn1c@2a9e2a3

@om3g4zell

This comment has been minimized.

Copy link
Author

commented Aug 2, 2019

Ok Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.