asn1c icon indicating copy to clipboard operation
asn1c copied to clipboard

Support of Aligned PER and Information Object Class

Open AuthenticEshkinKot opened this issue 8 years ago • 60 comments

With this changes I can parse asn1 from 3GPP TS 36.413 and decode incoming S1 messages. This patchset consists of two parts:

  1. Patches from https://github.com/vlm/asn1c/pull/99, which brings support of Information Object Class.
  2. Patches from https://github.com/osmocom/asn1c/tree/aper-prefix, which brings support of Aligned PER. Unfortunately, I was unable to apply them "as is", so I made necessary fixes and that's why they are signed off by me. But I've saved original messages so it's easy to find corresponding commits.

AuthenticEshkinKot avatar Oct 05 '16 09:10 AuthenticEshkinKot

How does this PR measure up to #125 ? Considering that #125 not only adds APER support, but also (unlike this one) passes the CI tests - wouldn't #125 be a better option?

mouse07410 avatar Jan 27 '17 03:01 mouse07410

@mouse07410 This PR adds support of Information Object Class

AuthenticEshkinKot avatar Jan 27 '17 12:01 AuthenticEshkinKot

This PR adds support of Information Object Class

I did not look into #125 beyond "does it pass the tests". If it does not support Information Object Class, perhaps it would be better for you to add that support on top of #125? Because two competing PRs doing (mostly) the same thing isn't good, and my fork already merged #125 (and in addition - #129 so larger constraints are supported). If #125 lacks Info Object support, and you add that support to it - I'll merge your code. Otherwise, you'd have to wait for @vlm to decide which one of the two to merge, and in the end it might not be yours anyway.

mouse07410 avatar Jan 27 '17 16:01 mouse07410

OK, I'll see what I can do

AuthenticEshkinKot avatar Jan 27 '17 18:01 AuthenticEshkinKot

Thank you! Please feel free to take a closer look at my fork, to see what enhancements are already there, so your amount of work could be less. ;-)

mouse07410 avatar Jan 27 '17 19:01 mouse07410

@AuthenticEshkinKot also please take a look at #99 - the main reason I didn't merge it is that it fails CI. I didn't look for the cause (yet). If it's simple enough to fix, and if it addresses your needs - perhaps you could propose a change, and I'd integrate #99 with your fix?

I just don't know what's the simplest/fastest/easiest way to add the capability you need - whether it's for you to create a subset of #115 that's compatible with #125 and #129, or to fix #99 , or...

mouse07410 avatar Jan 30 '17 03:01 mouse07410

@mouse07410 I've got two items of news - good one and bad one. The good news is that I've merged #99 with #125 and it can successfully parse asn1 from 3GPP TS 36.413. The bad news is that it can't decode S1 message correctly, while #115 can do it. Using asn1c debug mode, I've found some differences between #115 and #125, which lead to this situation. I've fixed some of them, but there are many left, so I think that it would be better, if you can look at them by yourself. I've prepared some test environment which can help you to understand the reason of problem.

  1. Source asn1 - https://yadi.sk/d/dSl028jW3BqzsJ. These sources can be found in 3GPP TS 36.413. I parse them with 'asn1c -gen-PER -S ~/asn1c/skeletons/ ~/path/to/S1AP-Constants.asn ... the rest of files'
  2. Example of decoder - https://yadi.sk/d/DdCxUGWr3Br4nr. It can be compiled by gcc with -DEMIT_ASN_DEBUG (for debug output).
  3. Input data for decoder - https://yadi.sk/d/B99Uc1d23BqYoL. This is APER coded data. To see correct contents of it you should use asn1c from #115 and change all 'ANY_to_type' calls to 'ANY_to_type_aper' in main_test.c

Hope, this will help.

AuthenticEshkinKot avatar Jan 30 '17 13:01 AuthenticEshkinKot

And one more thing, that I forgot. To be able to parse asn1, you should merge #99 with #125. There will be no conflicts on merge, so it will be easy to do.

AuthenticEshkinKot avatar Jan 30 '17 14:01 AuthenticEshkinKot

First, thank you for the work you're doing.

Merging #99 and #125 - my main concern is that #99 fails the CI tests. I'm trying to avoid introducing code that does not pass the CI. By the way, this was the main reason I initially merged #125 (that passes the CI) rather than #115 (that fails the CI).

Since having support for wider constraints in PER is desirable - perhaps I need to re-consider my approach and merge #115 instead... For that to happen, I need for #115 to (a) pass the CI tests, and (b) support wider constraints (what #129 accomplishes).

Also, we now know that #125 did not address some issues resulting in failure to properly support Information Object Class. It would also be good to know if there are ASN.1 constructs supported in #125 that #115 does not deal with. I.e., I am trying to figure - are these two PRs each cover things that the other one doesn't, or does one of them (#115) support everything that the other one (#125) does?

mouse07410 avatar Jan 30 '17 16:01 mouse07410

Update

Here are some details regarding why I'm not merging #99 until it is fixed:

$ make check
. . . . .
Checking ../tests/58-param-OK.asn1 against ../tests/58-param-OK.asn1.-EF
WARNING: Parameterized type ub-name expected for ub-name at line 19 in ../tests/58-param-OK.asn1
WARNING: Parameterized type ub-name expected for ub-name at line 19 in ../tests/58-param-OK.asn1
. . . . .
Checking ../tests/97-type-identifier-SW.asn1 against ../tests/97-type-identifier-SW.asn1.-EF
WARNING: ASN.1 expression "TYPE-IDENTIFIER" at line 17 of module ASN1C-UsefulInformationObjectClasses
clashes with expression "TYPE-IDENTIFIER" at line 23 of module ModuleTypeIdentifier1 (../tests/97-type-identifier-SW.asn1).
Rename or remove either instance to resolve the conflict in ../skeletons/standard-modules/ASN1C-UsefulInformationObjectClasses.asn1
. . . . .
Checking ../tests/99-class-sample-OK.asn1 against ../tests/99-class-sample-OK.asn1.-EFprint-class-matrix
--- .tmp.check-parsing.59808.old	2017-01-30 12:21:23.000000000 -0500
+++ .tmp.check-parsing.59808.new	2017-01-30 12:21:23.000000000 -0500
@@ -13,12 +13,16 @@
     &Type	 OPTIONAL
 } WITH SYNTAX { [TYPE &Type] [WITH CODE &code] IDENTIFIED BY &id }
 
--- Class matrix has 4 entries:
+-- Class matrix has 8 entries:
 --    [             &id][           &code][           &Type]
 -- [1] request-whatever        <no entry>        <no entry> 
 -- [2]   response-stuff                 1        <no entry> 
 -- [3]       request-id                 2        SampleType 
 -- [4]     request-salt        <no entry>              Salt 
+-- [5] request-whatever        <no entry>        <no entry> 
+-- [6]   response-stuff                 1        <no entry> 
+-- [7]       request-id                 2        SampleType 
+-- [8]     request-salt        <no entry>              Salt 
 
 
 SampleClassObjectSet SAMPLE-CLASS ::= {{ IDENTIFIED BY request-whatever } | { WITH CODE 1 IDENTIFIED BY response-stuff } | { TYPE SampleType WITH CODE 2 IDENTIFIED BY request-id } | { TYPE Salt IDENTIFIED BY request-salt }}
Error while processing ../tests/99-class-sample-OK.asn1.-EFprint-class-matrix (from ../tests/99-class-sample-OK.asn1)
FAIL check-parsing.sh (exit status: 1)

Take a good look at the last error cluster - it looks like that code generates two copies of the same thing!

Until and unless all of the above are fixed - that code is going nowhere. (As I said, it was the reason I did not consider #115 for inclusion, and chose to settle on #125 as it passed all the CI checks.)

mouse07410 avatar Jan 30 '17 17:01 mouse07410

@mouse07410 Thank you for detailed answer! I already have version that passes CI tests, but it's code is not good enough, so some time is needed to finish it. Then I'll take a look on #129.

AuthenticEshkinKot avatar Jan 30 '17 18:01 AuthenticEshkinKot

My pleasure - and we both are trying to enhance and improve asn1c, which is great!

Looking forward for your PR.

P.S. I'm sure you noticed that asn1c currently has no working SET_OF_encode_aper() (or ...uper()). Do you see it as a problem?

mouse07410 avatar Jan 30 '17 19:01 mouse07410

One thing more. Since #125 and #129 have been successfully merged into my fork, it might make sense for you to base your work on it (my fork passes all th CI, and has no compiler awnings). Basing on my fork would likely result in less work for me, and less code to write for you.

You certainly can base it on the main repo - but then it would be unclear when it would be merged.

mouse07410 avatar Jan 31 '17 05:01 mouse07410

Fixed #125 and add a repo with @AuthenticEshkinKot example code https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot

ikarso avatar Jan 31 '17 12:01 ikarso

@ikarso thank you! Your fix has just been tested and merged into https://github.com/mouse07410/asn1c fork.

@AuthenticEshkinKot can you confirm that it works for you? Is anything else needed to parse (and encode) your messages?

To both of you: #99 fails CI checks, and appears to be buggy (as described here https://github.com/vlm/asn1c/pull/115#issuecomment-276128487). Can you provide a fixed version of #99 so I can integrate it cleanly?

mouse07410 avatar Jan 31 '17 18:01 mouse07410

@mouse07410 I've tested it today and it failed to parse my data. I suppose, that did something wrong, so I'm going to test it more carefully tomorrow.

AuthenticEshkinKot avatar Jan 31 '17 18:01 AuthenticEshkinKot

@AuthenticEshkinKot I suggest picking my fork (it has the entire #125 merged, including the latest changes), and merging #99 on top of it. Then try to parse your data. If it works - just help getting #99 into merge-able shape. If it doesn't - I guess you and @ikarso need to talk. :-)

mouse07410 avatar Jan 31 '17 18:01 mouse07410

@mouse07410 I've tried version from your repo with #99 applied. Besides that there were some build errors with 'missed separator' in Makefile (on @CODE_COVERAGE_RULES@), it's still unable to parse data, which I linked earlier. To be precise, it tries to parse inner DownlinkNASTransport with ber_decoder and fails.

AuthenticEshkinKot avatar Feb 01 '17 08:02 AuthenticEshkinKot

there were some build errors with 'missed separator' in Makefile (on @CODE_COVERAGE_RULES@)

This is related to absence of file ax_code_coverage.m4 that is included in autoconf-archive package (installed on some of my machines, where the build went smoothly, not installed on a new machine, where the build failed like yours - until I installed that package). The file in question is http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_code_coverage.m4 and you can copy it to either the m4/ subdirectory, or (like on my Mac machines) to /opt/local/share/aclocal/.

it's still unable to parse data, which I linked earlier. To be precise, it tries to parse inner DownlinkNASTransport with ber_decoder and fails.

Now that is interesting. So it does not stick to PER...? @ikarso can you comment?

Also, @ikarso seemed to successfully parse your DownlinkNASTransport, according to https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot. He says that you need to (assuming you used my fork and merged #99 on top of it):

$ /path/to/asn1c -fcompound-names -gen-PER -pdu=DownlinkNASTransport ../ASN/*.asn
$ make -f Makefile.am.sample clean
$ make -f Makefile.am.sample
$ ./progname -iaper ../data/DownlinkNASTransport

That was supposed to produce

<DownlinkNASTransport>
    <protocolIEs>
        <ProtocolIE-Field>
            <id>0</id>
            <criticality><reject/></criticality>
            <value>C0 01 00 70 57</value>
        </ProtocolIE-Field>
        <ProtocolIE-Field>
            <id>8</id>
            <criticality><reject/></criticality>
            <value>80 1B 46 FD</value>
        </ProtocolIE-Field>
        <ProtocolIE-Field>
            <id>26</id>
            <criticality><reject/></criticality>
            <value>
                31 27 84 78 93 EF 01 7C 54 88 B1 D4 52 15 C8 1D 
                DA 18 22 04 45 49 63 8A EB 67 85 24 E8 07 9D 60 
                01 32 A5 97 BE 07 7A 22 7B 98 DD D4 7D 97 E6 A0 
                1C 3D
            </value>
        </ProtocolIE-Field>
    </protocolIEs>
</DownlinkNASTransport>

Did the above fail for you?

P.S. If somebody provided a fixed version of #99, this all would be easier, I think - and the main repo would have that Info Object support. My own attempt to merge it leads to failed CI, and to a crash of asn1c executable on attempt to compile your ASN.1 stuff:

$ ../../asn1c/asn1c/asn1c -fcompound-names -gen-PER -pdu=DownlinkNASTransport ../ASN/*.asn
WARNING: Cannot find standard modules in /opt/local/share/asn1c/standard-modules
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
. . . . .
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
. . . . .
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 130 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 130 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 131 in ../ASN/S1AP-Containers.asn
. . . . .
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolExtensions expected for maxProtocolExtensions at line 173 in ../ASN/S1AP-Containers.asn
. . . . .
WARNING: Parameterized type S1AP-PRIVATE-IES expected for S1AP-PRIVATE-IES at line 195 in ../ASN/S1AP-Containers.asn
Compiled Criticality.c
Compiled Criticality.h
Compiled Presence.c
Compiled Presence.h
Compiled PrivateIE-ID.c
Compiled PrivateIE-ID.h
Compiled ProcedureCode.c
Compiled ProcedureCode.h
Compiled ProtocolExtensionID.c
Compiled ProtocolExtensionID.h
Compiled ProtocolIE-ID.c
Compiled ProtocolIE-ID.h
. . . . .
Compiled S1AP-PDU.c
Compiled S1AP-PDU.h
Compiled InitiatingMessage.c
Compiled InitiatingMessage.h
Compiled SuccessfulOutcome.c
Compiled SuccessfulOutcome.h
Compiled UnsuccessfulOutcome.c
Compiled UnsuccessfulOutcome.h
Copied /opt/local/share/asn1c/ANY.h	-> ANY.h
Copied /opt/local/share/asn1c/ANY.c	-> ANY.c
Copied /opt/local/share/asn1c/BOOLEAN.h	-> BOOLEAN.h
Copied /opt/local/share/asn1c/BOOLEAN.c	-> BOOLEAN.c
Copied /opt/local/share/asn1c/INTEGER.h	-> INTEGER.h
Copied /opt/local/share/asn1c/NativeEnumerated.h	-> NativeEnumerated.h
Copied /opt/local/share/asn1c/INTEGER.c	-> INTEGER.c
Copied /opt/local/share/asn1c/NULL.h	-> NULL.h
Copied /opt/local/share/asn1c/NULL.c	-> NULL.c
Copied /opt/local/share/asn1c/NativeEnumerated.c	-> NativeEnumerated.c
Copied /opt/local/share/asn1c/NativeInteger.h	-> NativeInteger.h
Copied /opt/local/share/asn1c/NativeInteger.c	-> NativeInteger.c
Copied /opt/local/share/asn1c/OBJECT_IDENTIFIER.h	-> OBJECT_IDENTIFIER.h
Copied /opt/local/share/asn1c/OBJECT_IDENTIFIER.c	-> OBJECT_IDENTIFIER.c
Copied /opt/local/share/asn1c/PrintableString.h	-> PrintableString.h
Copied /opt/local/share/asn1c/PrintableString.c	-> PrintableString.c
Copied /opt/local/share/asn1c/asn_SEQUENCE_OF.h	-> asn_SEQUENCE_OF.h
Copied /opt/local/share/asn1c/asn_SEQUENCE_OF.c	-> asn_SEQUENCE_OF.c
Copied /opt/local/share/asn1c/asn_SET_OF.h	-> asn_SET_OF.h
Copied /opt/local/share/asn1c/asn_SET_OF.c	-> asn_SET_OF.c
Copied /opt/local/share/asn1c/constr_CHOICE.h	-> constr_CHOICE.h
Copied /opt/local/share/asn1c/constr_CHOICE.c	-> constr_CHOICE.c
Copied /opt/local/share/asn1c/constr_SEQUENCE.h	-> constr_SEQUENCE.h
Copied /opt/local/share/asn1c/constr_SEQUENCE.c	-> constr_SEQUENCE.c
Copied /opt/local/share/asn1c/constr_SEQUENCE_OF.h	-> constr_SEQUENCE_OF.h
Copied /opt/local/share/asn1c/constr_SEQUENCE_OF.c	-> constr_SEQUENCE_OF.c
Copied /opt/local/share/asn1c/constr_SET_OF.h	-> constr_SET_OF.h
Copied /opt/local/share/asn1c/constr_SET_OF.c	-> constr_SET_OF.c
Copied /opt/local/share/asn1c/asn_application.h	-> asn_application.h
Copied /opt/local/share/asn1c/asn_system.h	-> asn_system.h
Copied /opt/local/share/asn1c/asn_codecs.h	-> asn_codecs.h
Copied /opt/local/share/asn1c/asn_internal.h	-> asn_internal.h
Copied /opt/local/share/asn1c/OCTET_STRING.h	-> OCTET_STRING.h
Copied /opt/local/share/asn1c/OCTET_STRING.c	-> OCTET_STRING.c
Copied /opt/local/share/asn1c/BIT_STRING.h	-> BIT_STRING.h
Copied /opt/local/share/asn1c/BIT_STRING.c	-> BIT_STRING.c
Copied /opt/local/share/asn1c/asn_codecs_prim.c	-> asn_codecs_prim.c
Copied /opt/local/share/asn1c/asn_codecs_prim.h	-> asn_codecs_prim.h
Copied /opt/local/share/asn1c/ber_tlv_length.h	-> ber_tlv_length.h
Copied /opt/local/share/asn1c/ber_tlv_length.c	-> ber_tlv_length.c
Copied /opt/local/share/asn1c/ber_tlv_tag.h	-> ber_tlv_tag.h
Copied /opt/local/share/asn1c/ber_tlv_tag.c	-> ber_tlv_tag.c
Copied /opt/local/share/asn1c/ber_decoder.h	-> ber_decoder.h
Copied /opt/local/share/asn1c/ber_decoder.c	-> ber_decoder.c
Copied /opt/local/share/asn1c/der_encoder.h	-> der_encoder.h
Copied /opt/local/share/asn1c/der_encoder.c	-> der_encoder.c
Copied /opt/local/share/asn1c/constr_TYPE.h	-> constr_TYPE.h
Copied /opt/local/share/asn1c/constr_TYPE.c	-> constr_TYPE.c
Copied /opt/local/share/asn1c/constraints.h	-> constraints.h
Copied /opt/local/share/asn1c/constraints.c	-> constraints.c
Copied /opt/local/share/asn1c/xer_support.h	-> xer_support.h
Copied /opt/local/share/asn1c/xer_support.c	-> xer_support.c
Copied /opt/local/share/asn1c/xer_decoder.h	-> xer_decoder.h
Copied /opt/local/share/asn1c/xer_decoder.c	-> xer_decoder.c
Copied /opt/local/share/asn1c/xer_encoder.h	-> xer_encoder.h
Copied /opt/local/share/asn1c/xer_encoder.c	-> xer_encoder.c
Copied /opt/local/share/asn1c/per_support.h	-> per_support.h
Copied /opt/local/share/asn1c/per_support.c	-> per_support.c
Copied /opt/local/share/asn1c/per_decoder.h	-> per_decoder.h
Copied /opt/local/share/asn1c/per_decoder.c	-> per_decoder.c
Copied /opt/local/share/asn1c/per_encoder.h	-> per_encoder.h
Copied /opt/local/share/asn1c/per_encoder.c	-> per_encoder.c
Copied /opt/local/share/asn1c/per_opentype.h	-> per_opentype.h
Copied /opt/local/share/asn1c/per_opentype.c	-> per_opentype.c
Copied /opt/local/share/asn1c/converter-sample.c	-> converter-sample.c
Generated Makefile.am.sample
asn1c(20579,0x7fffc63d93c0) malloc: *** error for object 0x7ff3fa603020: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

P.P.S. Your current PR still fails CI horribly. Here's the log asn1c/test-suite.log: 115-test-suite.log.txt

mouse07410 avatar Feb 01 '17 19:02 mouse07410

Ok, here's my view. #125 passes CI, and has been merged in my fork. #115 that is supposed to be an alternative to it, does not pass CI. Furthermore, the main reason to use #115 appears to be its better ability to work with #99 to parse 3GPP data. Since it's all pointless without #99, and #99 is in no shape to be merged - I'm pausing this effort until somebody makes #99 usable (which necessarily includes passing CI).

mouse07410 avatar Feb 02 '17 04:02 mouse07410

@mouse07410 I've already made #99 usable by this commit - https://github.com/vlm/asn1c/pull/115/commits/461f3fa3eb603841ea4c90f8c01308af00816b64

AuthenticEshkinKot avatar Feb 02 '17 07:02 AuthenticEshkinKot

I've built @ikarso asn1c with applied #99 and successfully parsed .asn1. But my test application still fails to parse example APER, that I've posted previously. Anyway, it looks strange that user should specify such an option like "-pdu=DownlinkNASTransport" for asn1c. What does it mean? In my PR it works correctly without it.

AuthenticEshkinKot avatar Feb 02 '17 09:02 AuthenticEshkinKot

@AuthenticEshkinKot do you refer to linker error? for example main_test.o: In functionprocessTAI': main_test.c:(.text+0x1c): undefined reference to ANY_to_type_aper' I belive ANY are deprecated. If you copy skeletons/ANY.* from #115 it will work with your testapplication.

For converter-sample.c provided with asn1c you need to specify the pdu, but if you write your own application just compile without specify the PDUs. ( http://lionet.info/asn1c/faq.html )

ikarso avatar Feb 02 '17 10:02 ikarso

@ikarso Of course, I've replaced all 'ANY_to_type_aper' by 'ANY_to_type', so there are no compiler or linker errors. Maybe ANY is deprecated, but for me it is the only way to check the type of incoming data.

AuthenticEshkinKot avatar Feb 02 '17 10:02 AuthenticEshkinKot

I have add an modified version of main_test.c and add the function ANY_to_type_aper from #115 https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot check description under "PR125 with main_test_125.c".

ikarso avatar Feb 02 '17 12:02 ikarso

@mouse07410 From now it passes all tests and has support of larger constraints.

AuthenticEshkinKot avatar Feb 02 '17 15:02 AuthenticEshkinKot

After applying the current #115:

. . . . .
  CC       libasn1cskeletons_la-OCTET_STRING.lo
OCTET_STRING.c:1495:6: error: use of undeclared identifier 'asn_DEF_OCTET_STRING_specs'; did
      you mean 'asn_SPC_OCTET_STRING_specs'?
                : &asn_DEF_OCTET_STRING_specs;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
                   asn_SPC_OCTET_STRING_specs
OCTET_STRING.c:17:43: note: 'asn_SPC_OCTET_STRING_specs' declared here
static const asn_OCTET_STRING_specifics_t asn_SPC_OCTET_STRING_specs = {
                                          ^
OCTET_STRING.c:1842:5: error: use of undeclared identifier 'asn_DEF_OCTET_STRING_specs'; did
      you mean 'asn_SPC_OCTET_STRING_specs'?
        : &asn_DEF_OCTET_STRING_specs;
           ^~~~~~~~~~~~~~~~~~~~~~~~~~
           asn_SPC_OCTET_STRING_specs
OCTET_STRING.c:17:43: note: 'asn_SPC_OCTET_STRING_specs' declared here
static const asn_OCTET_STRING_specifics_t asn_SPC_OCTET_STRING_specs = {
                                          ^
2 errors generated.

Probably caused by #89. Fixing in my branch.

Update Fix was trivial, it passes the CI now. Does one need to apply #99 on top of this?

mouse07410 avatar Feb 02 '17 15:02 mouse07410

CI passed (good!) but asn1c crashes on your 3GPP data:

$ lldb ../../asn1c/asn1c/asn1c
(lldb) target create "../../asn1c/asn1c/asn1c"
Current executable set to '../../asn1c/asn1c/asn1c' (x86_64).
(lldb) run -fcompound-names -gen-PER ../ASN/*.asn
Process 91032 launched: '../../asn1c/asn1c/asn1c' (x86_64)
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type maxProtocolIEs expected for maxProtocolIEs at line 122 in ../ASN/S1AP-Containers.asn
. . . . .
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 130 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 130 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 131 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 131 in ../ASN/S1AP-Containers.asn
WARNING: Parameterized type S1AP-PROTOCOL-IES expected for S1AP-PROTOCOL-IES at line 131 in ../ASN/S1AP-Containers.asn
. . . . .
Copied /opt/local/share/asn1c/converter-sample.c	-> converter-sample.c
Generated Makefile.am.sample
asn1c(18802,0x7fff75fec000) malloc: *** error for object 0x1006dcaf0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Process 18802 stopped
* thread #1: tid = 0x128b863, 0x00007fff85863f06 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00007fff85863f06 libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`__pthread_kill:
->  0x7fff85863f06 <+10>: jae    0x7fff85863f10            ; <+20>
    0x7fff85863f08 <+12>: movq   %rax, %rdi
    0x7fff85863f0b <+15>: jmp    0x7fff8585e7cd            ; cerror_nocancel
    0x7fff85863f10 <+20>: retq   
(lldb) bt
* thread #1: tid = 0x128b863, 0x00007fff85863f06 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007fff85863f06 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff8b7df4ec libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fff97bde6df libsystem_c.dylib`abort + 129
    frame #3: 0x00007fff92844041 libsystem_malloc.dylib`free + 425
    frame #4: 0x00000001000109ab asn1c`asn1p_value_free(v=0x00000001006dcaf0) + 379 at asn1p_value.c:263
    frame #5: 0x0000000100011559 asn1c`asn1p_expr_free(expr=0x00000001005304b0) + 537 at asn1p_expr.c:267
    frame #6: 0x000000010001149a asn1c`asn1p_expr_free(expr=0x000000010052fd30) + 346 at asn1p_expr.c:252
    frame #7: 0x000000010000f948 asn1c`asn1p_module_free(mod=0x0000000100527040) + 184 at asn1p_module.c:36
    frame #8: 0x000000010000fa46 asn1c`asn1p_delete(asn=0x0000000100201da0) + 134 at asn1p_module.c:58
    frame #9: 0x00000001000018bc asn1c`main(ac=6, av=0x00007fff5fbfee78) + 3724 at asn1c.c:333
    frame #10: 0x00007fff935715ad libdyld.dylib`start + 1
(lldb) 

The cause is somewhere in #99, ~~or (less likely) in 461f3fa.~~ I see this problem with #125 + #99, and with #115 (with #99).

Update The actual crash happens in libasn1parser/asn1p_value.c line 263, which is the final free(v); at the end of asn1p_value_free() function. Invoked by libasn1parser/asn1p_expr.c line 267 asn1p_value_free(expr->marker.default_value);

mouse07410 avatar Feb 02 '17 16:02 mouse07410

@mouse07410 #99 is already applied to this PR, so no need to apply it again. Crash looks rather strange, because I've tested parsing of 3GPP asn1 before pulling to repo and everything was good. Surely, I'll check it again tomorrow.

AuthenticEshkinKot avatar Feb 02 '17 19:02 AuthenticEshkinKot

Crash looks rather strange, because I've tested parsing...

Different platforms - that's the only thing that comes to mind. And I'm using https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot data for testing.

The crash is caused by the attempt to free expr->marker.default_value in libasn1parser/asn1p_expr.c line 267. When I comment lines 266-267 out (allowing memory leak), the problem disappears. So something does not properly fills the marker.default_value for your 3GPP data, and asn1c instead of catching it, crashes.

mouse07410 avatar Feb 02 '17 19:02 mouse07410

@mouse07410 I've tried full build cycle on different PC five minutes ago (never mind, why =) ) and everything is OK, if not to mention dozens of warnings on asn1 parsing and test program compilation. I used data, that I posted earlier. Maybe or maybe not, your crash is connected with this commit https://github.com/AuthenticEshkinKot/asn1c/commit/5da982f48ad0e00115b8698179655bc3fff0023c. Anyway, I'm going to explore it deeper and do something with asn warnings.

AuthenticEshkinKot avatar Feb 02 '17 19:02 AuthenticEshkinKot

Maybe or maybe not, your crash is connected with this commit AuthenticEshkinKot@5da982f.

Rather unlikely, because this commit does not apply - check the relevant file in my fork.

One possible issue is that I'm merging all the relevant PRs, not just this. But that (merging every improvement that makes sense) is unavoidable - otherwise there's no point in maintaining a separate fork.

Anyway, I'm going to explore it deeper and do something with asn warnings.

Yes please!

P.S. As you see, not freeing marker.default_value is a reliable (so far :) workaround for this problem. I suspect that the memory leak is not huge, and in any case it can hurt only the compilation stage - there is no impact on the generated code. Still, it would be nice if we could find the cause and eradicate it.

I've added two branches: 99 (my master with @ikarso 's updates to #125, and #99 ) and 115 (re-formulated master without #125 , but with #115). Both require the workaround described above, otherwise both exhibit the crash described earlier.

It would be great if you could (a) try to find what causes this problem (common for both branches), and (b) try branch 99 with main_test_125.c from https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot . It worked for me, but I'd like your confirmation.

mouse07410 avatar Feb 02 '17 20:02 mouse07410

@mouse07410 Could you get backtrace for your crash?

AuthenticEshkinKot avatar Feb 03 '17 12:02 AuthenticEshkinKot

Could you get backtrace for your crash?

Already posted it here: https://github.com/vlm/asn1c/pull/115#issuecomment-277011343

If you'd like something different - please let me know.

mouse07410 avatar Feb 03 '17 13:02 mouse07410

@mouse07410 Sorry, I've just forgot to scroll up :( As far as I can see, the reason of crash is a freeing of unallocated memory. But I suppose that the real reason is 'double free', which also suits this description. I've reread #99 diff and it seems to me, that the crash may be connected with this https://github.com/vlm/asn1c/pull/99/commits/2b1bb9e7e4bca97de45252c2a9306b05b822b2f4, because this commit adds support of some references and this may cause a 'double free', when we have multiple refs on one place in memory. Maybe it would be reasonable to create your own simple .asn with such references and check, will it crash or not. I would like to explore it myself, but I haven't got such a crash on my PCs.

AuthenticEshkinKot avatar Feb 03 '17 15:02 AuthenticEshkinKot

the reason of crash is a freeing of unallocated memory. But I suppose that the real reason is 'double free', which also suits this description

I suspect that either there's a double-free (without zeroing the pointer, because the value passed to asn1p_value_free() is not NULL), or the marker.default_value does not get properly initalized or zeroized, so it contains some random value that later (as being non-zero) gets passed to asn1p_value_free().

Maybe it would be reasonable to create your own simple .asn with such references and check, will it crash or not.

Absolutely. Unfortunately I've no clue about this part of ASN.1, so cannot create a reasonable (even small) ASN.1 that would exercise this. Would you be able to help here? I'll run all the tests.

mouse07410 avatar Feb 03 '17 16:02 mouse07410

@mouse07410 OK, I'll write it.

AuthenticEshkinKot avatar Feb 03 '17 18:02 AuthenticEshkinKot

OK, I'll write it.

Thank you!! And I'll test it and post the results/traces.

mouse07410 avatar Feb 03 '17 20:02 mouse07410

Here's what I think is happening. During cloning, the part marker.default_value is not clone-constructed, but instead just the pointer is copied along with the rest of the content of the original object. As a result, when the original object is deleted (and its memory freed), this pointer becomes invalid.

A fix would be figuring out where this cloning/copying is happening (maybe it is in https://github.com/vlm/asn1c/pull/99/commits/2b1bb9e7e4bca97de45252c2a9306b05b822b2f4), and add code there that clones marker.default_value if the original has it set no non-NULL, or explicitly setting it to NULL otherwise.

Comments?

mouse07410 avatar Feb 05 '17 01:02 mouse07410

@mouse07410

the part marker.default_value is not clone-constructed, but instead just the pointer is copied along with the rest of the content of the original object

Exactly my thoughts. So, I think that it will be very useful to see references values before and after clone. In theory, they should be different, but in your case some of them will be equal.

AuthenticEshkinKot avatar Feb 05 '17 19:02 AuthenticEshkinKot

...it will be very useful to see references values before and after clone...

There likely to be a lot of values, so we either need to figure how to trace very selectively (to avoid being drowned in debugging data), or to get a good small test that would exercise precisely that feature.

Where in the code would you like to see the trace output added? And can you provide a minimalistic test (ASN.1 example) that would exercise this?

mouse07410 avatar Feb 06 '17 02:02 mouse07410

I think I've fixed this crash. Please feel free to:

  1. Check whether branch 115 of my fork works for you. It worked for me on your data.
  2. Check whether branch 99 of my fork works for you. It worked for me on your data.
  3. Provide me a small test-case (ASN.1 file) that I can validate the code on.

mouse07410 avatar Feb 06 '17 19:02 mouse07410

It works correctly for #115. Could you make a PR to my repo?

AuthenticEshkinKot avatar Feb 07 '17 09:02 AuthenticEshkinKot

It works correctly for #115.

Can you please check also my #99 or master branch?

Could you make a PR to my repo?

Certainly (at least will do my best) - if you post the URL of your repo here.

mouse07410 avatar Feb 07 '17 16:02 mouse07410

Your master and #99 got same problem, as @ikarso's version - they fail to decode my example of APER. My repo (just a fork) - https://github.com/AuthenticEshkinKot/asn1c.

AuthenticEshkinKot avatar Feb 08 '17 08:02 AuthenticEshkinKot

First of all - thank you for trying #99 and master for me.

Your master and #99 got same problem, as @ikarso's version - they fail to decode my example of APER.

This is sad, and strange. I've tried both of these branches with the data in https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot, and it all parsed/decoded beautifully.

What data were you using that failed to decode? What test-program did you use? I followed what @ikarso wrote in the README of his repo - and all worked well.

Can you please add the data that #99 fails to decode to your fork (with some description, hopefully :)? So I can pull it and try myself?

My repo (just a fork) - https://github.com/AuthenticEshkinKot/asn1c.

PR has been created. There are two conflicts.

One - in skeletons/OCTET_STRING.c caused by the change in the naming (#89, I believe). I think it's both safe and OK to choose the #115's version over your current master.

The other one is in libasn1compiler/asn1c_misc.c. Here I'm not sure which one is more correct. Can you take a look and comment? Depending on the answer, I may change my master as well.

mouse07410 avatar Feb 08 '17 16:02 mouse07410

@mouse07410 Finally, I've found that @ikarso's main_test.c differs from mine and has its own special ANY_to_type_aper. So, I've tested your master with it and everything is OK, it works with my APER. When I wrote about PR to my repo I meant only one commit https://github.com/mouse07410/asn1c/commit/ec0ade4f87c807e763e3f35fc5466adb6dda3473, not the whole branch. Sorry, it is my fault - I didn't consider this. So, I would like to reject the whole PR and get the commit. Anyway, many thanks for your effort and patience! :)

AuthenticEshkinKot avatar Feb 08 '17 21:02 AuthenticEshkinKot

I've tested your master with it and everything is OK, it works with my APER.

Perfect! Thanks!!

When I wrote about PR to my repo I meant only one commit mouse07410@ec0ade4

Ha! :-)

So, I would like to reject the whole PR and get the commit.

Sure, whatever's the most convenient way.

Anyway, many thanks for your effort and patience! :)

My pleasure - and thank you for fixing #115, and helping with the rest of this thing. Without your help Info Object support wouldn't have been merged.

mouse07410 avatar Feb 08 '17 21:02 mouse07410

Hi, does one of you know if the APER support will be integrated into the master branch?

At the OpenairInterface project we need to use S1AP release 14 and we need the APER encoding/decoding functionality. Our current fork does not compile the version 14 of S1AP, probably because of the information object class, which you implemented after our fork.

It would actually be good for us to switch back to the main (this one here) asn1c instead of using our own version.

From what I understand, the only problem for us to do so is the support of APER (but I didn't check deeper) which is not present.

I can provide some help if you need.

Thanks and regards.

cedric-roux avatar Jan 03 '18 16:01 cedric-roux

Hi, @cedric-roux My fork can compile S1AP release 14. Feel free to try it.

AuthenticEshkinKot avatar Jan 03 '18 17:01 AuthenticEshkinKot

@cedric-roux , you can also try @vlm's master repository and adding #226, #234 and #238.

By the way, I am an individual member of openairinterface5g and not affiliated to any organization, can I still contribute to it ?

brchiu avatar Jan 04 '18 00:01 brchiu

@AuthenticEshkinKot @brchiu thanks. To be honest I would prefer to see APER put into this repository. I don't want to juggle with various repositories and patch sets. I will get lost. :)

But for some particular research projects we have at OpenairInterface, that may make it. Thanks.

cedric-roux avatar Jan 04 '18 09:01 cedric-roux

ITNOA

Hi, @vlm did you any plane to merge this PR?

soroshsabz avatar Jan 10 '19 08:01 soroshsabz

Hi, I used this 115 Pull request code, But still I am facing issue like this : badhri85@node:~/asn1c/asn1c$ ./asn1c/asn1c -EF -P -gen-PER asnfiles/*.asn ASN.1 grammar parse error near line 13 (token "BEGIN"): syntax error, unexpected TOK_BEGIN, expecting TOK_DEFINITIONS Cannot parse "asnfiles/S1AP-PDU-Contents.asn"

badhrinathpa avatar Oct 08 '19 22:10 badhrinathpa

Hi, @badhrinathpa ! Maybe some semicolon or comma is missed.

AuthenticEshkinKot avatar Oct 09 '19 12:10 AuthenticEshkinKot

Hi @AuthenticEshkinKot Updated the ASN file. But facing the following issue now : FATAL: Terminal value for ProtocolIE-ContainerList->maxnoofE-RABs not found in asnfiles/specasn/S1AP-Containers.asn

badhrinathpa avatar Oct 17 '19 23:10 badhrinathpa

@badhrinathpa Add "maxnoofE-RABs" to "FROM S1AP-Constants;" section in S1AP-Containers.asn

AuthenticEshkinKot avatar Oct 18 '19 12:10 AuthenticEshkinKot

@AuthenticEshkinKot Thanks for the help. That solved the max ERAB problem. But there were other issues too. I needed the APER encode/decode. So, Used the code from master branch of @mouse07410 and used the command : asn1c -fcompound-names -fno-include-deps -gen-PER -findirect-choice ../*.asn The files got generated now.

badhrinathpa avatar Oct 18 '19 23:10 badhrinathpa

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: AuthenticEshkinKot
:white_check_mark: KrzysztofW
:x: zhanglei002
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar May 15 '20 10:05 CLAassistant

@AuthenticEshkinKot I see you've made updates and applied some fixes to this PR - could you please check it against my fork https://github.com/mouse07410/asn1c.git (which merged the original #115), branch vlm_master - and rebase those fixes that aren't there yet?

Thanks!

mouse07410 avatar Jun 23 '20 19:06 mouse07410