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

[Feature request] Declare named bit string positions #196

Open
hmpcabral opened this issue Jan 30, 2020 · 8 comments
Open

[Feature request] Declare named bit string positions #196

hmpcabral opened this issue Jan 30, 2020 · 8 comments
Assignees

Comments

@hmpcabral
Copy link

It would be helpful to generate #defines for named bit string positions, as in:

FruitSalad ::= BIT STRING { apple(0), banana(1), pear(2) } (SIZE (8))

Which could become something like:

#define FruitSalad_APPLE (1 << 0)
#define FruitSalad_BANANA (1 << 1)
#define FruitSalad_PEAR (1 << 2)

(I tried to understand how to add this feature myself, but unfortunately I have no F# experience and I couldn't quite figure out how to implement it.)

@maxime-esa
Copy link
Owner

@usr3-1415 can you please evaluate the complexity? If you can reuse the mechanisms used for named integers or enumerant with values, I'm in favour of supporting this ASN.1 syntax, which proves useful in some situations.

usr3-1415 added a commit that referenced this issue Feb 2, 2020
@usr3-1415
Copy link
Collaborator

feature implemented
please see the test case v4Tests/test-cases/acn/08-BIT-STRING/007.asn1

@maxime-esa
Copy link
Owner

Thank you! I did find a few issues, given this grammar:

Test DEFINITIONS ::= BEGIN
  SomeBits ::= BIT STRING { read(1), write(2), execute(99) } (SIZE(100))
  MoreBits ::= BIT STRING { foo(0), bar(1), hello(2), world(3) }(SIZE (2))
END
  1. The first type does not generate any constant for the execute bit position
  2. The second type compiles without warning, yet hello and world are impossible values given the size of the bit string

and then there are questions related to the Ada version:

This is not compatible with the type you generate:

 SomeBits_READ : CONSTANT adaasn1rtl.Asn1UInt:= 16#2#; 

you generate an unsigned_64 constant, which cannot be used with the bit string defined like this:

SUBTYPE SomeBits_array IS adaasn1rtl.BitArray(SomeBits_index);
type SomeBits is  record
    Data  : SomeBits_array;
end record;
type BitArray is array (Natural range <>) of BIT;
for BitArray'Component_Size use 1;

I think you should rather generate the constants like this:

 read  : constant someBits_array := (2 => 1, others => 0);
 write : constant someBits_array := (3 => 1, others => 0);
 execute : constant someBits_array := (100 => 1, others => 0);

In order to be able to use them:

 foo : somebits := (data => read);

However, the current representation of the bit string causes an another issue: because of the custom BIT type, you cannot use bitwise operators:

foo: somebits := (data => read or write);

If you had a definition of BitArray like this:

type BitArray is array (Natural range <>) of Boolean;

....then the bitwise operators would work.

The drawback is that existing projects using values 1 and 0 because of the current BIT type would need to be updated to use True and False instead. I am however not sure that so many projects would be impacted as BIT STRINGs are not such a commonly used type.

@maxime-esa maxime-esa reopened this Feb 3, 2020
@maxime-esa
Copy link
Owner

The C version cannot work either with the pattern suggested by @hmpcabral
Since the bit string is translated into an array of bytes, the constants could only work with the first byte (the first 8 bits)... Or in general the user would need to know which byte to apply it to.

@maxime-esa
Copy link
Owner

One more thing, quoting the book from Mr. Dubuisson:

Concerning the ordering of the bits, the following convention was
adopted: the first bit of the string is the one on the left-hand side (po-
sition 0) and the last bit is the one on the right-hand side.

This seems to comply to the Ada representation (arrary of bits/boolean) but not to the current constants generated in C

@usr3-1415
Copy link
Collaborator

  1. The first type does not generate any constant for the execute bit position
    Yes, this is generated for bit positions less than 64 (since the largest asn1 int type is 64 bits)
  2. The second type compiles without warning, yet hello and world are impossible values given the size of the bit string
    As mentioned in Mr. Dubuisson book "no size constraint is implicit ...". So, no error is generated.
    However, if you try to use this bit position by declaring bit string value such as
    myBitStrVal MoreBits ::= {hello}
    then an error is generated.
  1. you cannot use bitwise operators: (foo: somebits := (data => read or write);)
    You should do the ASN.1 way :
    foo SomeBits ::= {read, write}
    this will generate an initialized bit string value that can be used in both C and Ada.
    (see test case v4Tests/test-cases/acn/08-BIT-STRING/007.asn1). This notation supports any bit string positions (i.e. even the ones which are larger than 64).

hakanurhan pushed a commit to hakanurhan/asn1scc that referenced this issue Jan 9, 2021
@maxime-esa
Copy link
Owner

@usr3-1415 More insight on this issue:

  1. I do not understand what you mean: in Ada, the type is not a 64 bits unsigned integer, it is an array of BIT; as I said in the point 2, these constants cannot be used anywhere since they do not comply with the BIT STRING representation in Ada.

To illustrate concretely with the generated code:

SomeBits_READ : constant adaasn1rtl.Asn1UInt:= 16#2#;  --(1 << 1)
SomeBits_WRITE : constant adaasn1rtl.Asn1UInt:= 16#4#;  --(1 << 2)

subtype SomeBits_index is Integer range 1..100;
subtype SomeBits_array is adaasn1rtl.BitArray(SomeBits_index);
type SomeBits is  record
    Data  : SomeBits_array;
end record;

How can SomeBits_READ (64 bits uint) be used to set the bit 1 of the field SomeBits.Data?
That works in C because the BIT STRING is represented with integers, but not in Ada. This is why I suggested to generate the constants like this:

SomeBits_READ : constant someBits_array := (2 => 1, others => 0);

This is the form used for the generation of ASN.1 constants if you write in the ASN.1 grammar:

blah SomeBits ::= { read }
  1. "The second type compiles without warning, yet hello and world are impossible values given the size of the bit string"

However the generated code fails with an error when you try to compile it:

procedure MoreBits_set_hello(val : in out MoreBits) --  COVERAGE_IGNORE
is
begin
    val.Data(val.Data'First + 2) := 1;
end MoreBits_set_hello;

procedure MoreBits_set_world(val : in out MoreBits)
is
begin
    val.Data(val.Data'First + 3) := 1;
end MoreBits_set_world;

These two functions try to access the array out of range and gnat detects it:

foo.adb:75:29: value not in range of type "MoreBits_index" defined at foo.ads:63
foo.adb:75:29: "Constraint_Error" would have been raised at run time
foo.adb:81:29: value not in range of type "MoreBits_index" defined at foo.ads:63
foo.adb:81:29: "Constraint_Error" would have been raised at run time
  1. "you cannot use bitwise operators: (foo: somebits := (data => read or write);)"
    That is still true ; the generated constants cannot be used to make bitwise operations on the bit strings at code level, making the constants pretty much useless in practice.
    Why not using the built-in BOOLEAN type rather than the BIT type as it is done for SEQUENCE OF BOOLEAN ?
    Let me illustrate with a complete example:

I create two "equivalent" types, one with a BIT STRING, the other with a SEQUENCE OF BOOLEAN:

Foo DEFINITIONS ::= 
BEGIN
  Test-Bitwise ::= BIT STRING { foo(0), bar(1)}(SIZE (2))

  SeqOfBool ::= SEQUENCE (SIZE (2)) OF BOOLEAN

  myfoo Test-Bitwise ::= { foo }   -- needed because "foo" is an uint
  mybar Test-Bitwise ::= { bar }

  read SeqOfBool ::= { TRUE, FALSE }
  write SeqOfBool ::= { FALSE, TRUE }

END

In principle because of the "named bits", BIT STRINGS should be a better option, since for the read and write constants I need to specify the values of ALL the bits of the array.

However at code level we have two different representations (while memory wise they are probably identical):

subtype Test_Bitwise_index is Integer range 1..2;
subtype Test_Bitwise_array is adaasn1rtl.BitArray(Test_Bitwise_index);
type Test_Bitwise is  record
    Data  : Test_Bitwise_array;
end record;

vs

subtype SeqOfBool_index is Integer range 1..2;
type SeqOfBool_array is array (SeqOfBool_index) of adaasn1rtl.Asn1Boolean;
type SeqOfBool is  record
    Data  : SeqOfBool_array;
end record;

This little difference however allows SEQUENCE OF BOOLEAN to use Ada built-in bitwise operators on the Data field.
This is the program illustrating this:

with foo;
use foo;
with text_io; use text_io;

procedure blah is
   bitstr : Test_Bitwise;
   seqof : SeqOfBool;
begin
    Test_Bitwise_set_foo (bitstr);   -- only option to set individual bits (generated constants are unusable)
    Test_Bitwise_set_bar (bitstr);

    seqof.Data := read.data or write.data;  -- Allowed because BOOLEAN type is used
    -- bitstr.Data := myfoo.data or mybar.data; -- Not allowed because of the BIT type
end;

I hope this clarifies the point - basically it would be resolved by using the BOOLEAN type instead of the BIT type for BIT STRINGs.

@maxime-esa
Copy link
Owner

Update: unfortunately even if the proposed replacement of BIT with BOOLEAN would improve the usability of the type from user perspective, a bigger problem has been found related to a mis-alignment between C and Ada memory layout of the BIT STRING construct. See #290

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants