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

LTE RRC SystemInformation-r8-IEs ASN1 compilation issue (and work around). #95

Open
mdrenwick opened this issue May 27, 2016 · 12 comments

Comments

@mdrenwick
Copy link

'C' code output by asn1c from compilation of one specific section of the the published LTE ASN1 RRC (36.331 v990) spec, produced gcc compilation issues.
[gcc version is (Ubuntu 5.2.1-22ubuntu2) 5.2.1 20151010)]

SystemInformation-r8-IEs ::=        SEQUENCE {

    sib-TypeAndInfo                     SEQUENCE (SIZE (1..maxSIB)) OF CHOICE {
        sib2                                SystemInformationBlockType2, 
                  -- snipped for brevity
        ...,
        sib12-v920                          SystemInformationBlockType12-r9,
        sib13-v920                          SystemInformationBlockType13-r9
    },
    nonCriticalExtension                SystemInformation-v8a0-IEs                          OPTIONAL
 }

when asn1c compiled, and then gcc compiled, produced the following gcc warnings:

Sib-TypeAndInfo.h:57:9: warning: ‘struct Sib_TypeAndInfo__Member’ declared inside parameter list
   union Sib_TypeAndInfo__Member_u {
         ^
./asn_SET_OF.h:17:16: note: in definition of macro ‘A_SET_OF’
   void (*free)(type *);   \
                ^
Sib-TypeAndInfo.h:55:2: note: in expansion of macro ‘A_SEQUENCE_OF’
  A_SEQUENCE_OF(struct Sib_TypeAndInfo__Member {
  ^

As a work around that got rid of the gcc compilation warnings, I decomposed the ASN1 slightly to:

Sib-TypeAndInfo ::=                      CHOICE {
    sib2                                SystemInformationBlockType2,
                  -- snipped for brevity
    ...,
    sib12-v920                          SystemInformationBlockType12-r9,
    sib13-v920                          SystemInformationBlockType13-r9
}

SystemInformation-r8-IEs ::=        SEQUENCE {
    sib-TypeAndInfo     SEQUENCE (SIZE (1..maxSIB)) OF Sib-TypeAndInfo,
    nonCriticalExtension                SystemInformation-v8a0-IEs                          OPTIONAL
} 
@brchiu
Copy link
Contributor

brchiu commented May 29, 2016

Hi, @mdrenwick ,

Because this rrc-7.1.0.asn1 file comes from 3GPP's official document which evolve to newer version regularly and there are many places inside this ASN.1 file has similar construction, I think hand-modification of content of this file might not be a feasible way.

I spent this weekend tweaking several files in asn1c and manage to eliminate this warning when compiling RRC example of asn1c. I sent out this modification as pull request #96. But I am not sure whether it works for 36.331 v990's ASN.1 or not. Just for your reference.

I hope @vlm and you can have a look at this pull request and give me advice about how to improve it. If you have any question, please let me know.

Thanks.
regards,
brchiu.

@mdrenwick
Copy link
Author

Hi @brchiu

I tried again, after applying a patch with your changes. But no change :(

Thanks for your help, nonetheless.

There's a later RRC ASN.1 spec here that you could download to test against, if you have the time.

mdrenwick

@brchiu
Copy link
Contributor

brchiu commented May 31, 2016

@mdrenwick , Many thanks for your feedback.

For R12's EUTRA-RRC-Definitions.asn you provided, I found that I have to modify function asn1c_lang_C_type_CHOICE() function with the same trick which I used on asn1c_lang_C_type_SEQUENCE(). Now I did not see this A_SET() warning when compiling the files generated by this R12 RRC ASN.1 file on Linux platform.

I have already made this change in my pull request.

For your reference.

@mdrenwick
Copy link
Author

mdrenwick commented Jun 1, 2016

@brchiu, I tried your lastest changes without success.

I think the approach of forward declaring extra struct's to fix this problem is insufficient.

The crux of the problem is within the macro definition "A_SET_OF":

#define A_SET_OF(type) /*snip*/ void (*free)(type *);}

When certain nested ASN.1 constructs (example: original bug report) get compiled by asn1c, the resultant generated code places a type-declaration within the last line of the expanded macro - which is a parameter of the free function pointer.

Whereas typically an invocation of the macro A_SET_OF is not nested like:
A_SEQUENCE_OF(struct InterFreqCarrierFreqInfo) list;
that expands to
/*snip*/ void (*free)(struct InterFreqCarrierFreqInfo * );}

the nested case is more like
A_SEQUENCE_OF(struct System_snip__Member { /*struct-members*/ }) list;
that expands to
/*snip*/ void (*free)(struct System_snip__Member{ /* struct-members*/ } * );}

And its the inclusion of the { /* struct-members */ } within the function parameter list that happens during the macro expansion of the nested structure that's the problem.

No amount of additional forward declaring will solve it. I believe "C" considers the inclusion of { /*struct members*/ } a declaration of a new type for the function parameter - regardless of any matching prior declared type.

A general fix would need to (for example) create an top level typedef for the nested A_SEQUENCE_OF type and then invoke the nested macro A_SET_OF with the argument type of typedef'd struct:
A_SEQUENCE_OF(struct System_snip__Member) list;
To be a complete fix, this would need to be done for any (up to a max?) depth of nested SEQUENCEs.

@brchiu
Copy link
Contributor

brchiu commented Jun 1, 2016

Hi, @mdrenwick,

Look strange, since I don't see warning message using my yesterday's updated commit.
(I squashed two commits into one, so you might see only one commit in my pull request. please re-get the files, especially asn1c_C.c under libasn1compiler directory)

I put my generated SystemInformation-r8-IEs.c in gist.
(I rename your R12 ASN.1 file to rrc-7.1.0.asn1 in order not to change build environment.)

The type definition of struct SystemInformation_r8_IEs__sib_TypeAndInfo__Member is moved forward declaration section and only SystemInformation_r8_IEs__sib_TypeAndInfo__Member inside A_SEQUENCE_OF() macro.

/* SystemInformation-r8-IEs */
typedef struct SystemInformation_r8_IEs {
    struct SystemInformation_r8_IEs__sib_TypeAndInfo {
        A_SEQUENCE_OF(SystemInformation_r8_IEs__sib_TypeAndInfo__Member) list;

        /* Context for parsing across buffer boundaries */
        asn_struct_ctx_t _asn_ctx;
    } sib_TypeAndInfo;
    struct SystemInformation_v8a0_IEs   *nonCriticalExtension   /* OPTIONAL */;

    /* Context for parsing across buffer boundaries */
    asn_struct_ctx_t _asn_ctx;
} SystemInformation_r8_IEs_t;

So the pre-processed output by gcc's -E flag is :

typedef struct SystemInformation_r8_IEs {
 struct SystemInformation_r8_IEs__sib_TypeAndInfo {
  struct {
  SystemInformation_r8_IEs__sib_TypeAndInfo__Member **array; 
  int count; int size; 
  void (*free)(SystemInformation_r8_IEs__sib_TypeAndInfo__Member *); 
 } list;

  asn_struct_ctx_t _asn_ctx;
 } sib_TypeAndInfo;
 struct SystemInformation_v8a0_IEs *nonCriticalExtension ;

 asn_struct_ctx_t _asn_ctx;
} SystemInformation_r8_IEs_t;

Or is there any difference between the R12 RRC ASN.1 file you uploaded and Release 9 (v990) ASN.1 you mentioned in your issue ?

@brchiu
Copy link
Contributor

brchiu commented Jun 1, 2016

Hi, @mdrenwick

By the way, if you just execute make under examples/sample.source.RRC directory, it will not re-generate *.c and *.h files, you have to execute make regen to do this. The safest way to ensure code re-generation is deleting all *.c and *.h then execute make again.

@mdrenwick
Copy link
Author

Hi @brchiu,

Aha! Yes, I needed to regenerate the asn1c compiler! My error.

Having done that, the RRC v9.9.0 ASN.1 (including the nested SEQUENCE) is parsed faultlessly with your changes.

Thank you!

@brchiu
Copy link
Contributor

brchiu commented Jun 1, 2016

Hi, @mdrenwick,

Great to know this !!
Many thanks for your feedback which help me improving my original pull request.

@mdrenwick
Copy link
Author

Hi @brchiu,

Newb question: Should I close this issue now, or should I close it once merged, or will you (or vlm) close it?

Cheers

@brchiu
Copy link
Contributor

brchiu commented Jun 1, 2016

Hi, @mdrenwick,

I don't know neither, Ha !! ^_^

I am just an ordinary user who would like to use this @vlm great project in my future work. I never communicate with him. Moreover, I am not sure whether @vlm will accept my pull request or he will revise another better one.

IMHO, we can leave this issue opened as this moment, then anyone who is interested in this issue can see our conversation here and get a temporary solution.

@vlm
Copy link
Owner

vlm commented Jul 3, 2016

@brchiu, @mdrenwick, thank you for your investigation and the patch. I'll merge it in shortly!

@kevinchychen
Copy link

Hi @vlm ,

May I know if it has be merged already?

Best regards,
Kevin Chen

mouse07410 referenced this issue in mouse07410/asn1c Jan 30, 2017
mouse07410 referenced this issue in mouse07410/asn1c Jan 31, 2017
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

4 participants