Bug 4097 - Typecodes for UNIONs with multiple labels per individual case are generated incorrectly.
Summary: Typecodes for UNIONs with multiple labels per individual case are generated i...
Status: NEW
Alias: None
Product: TAO
Classification: Unclassified
Component: IDL Compiler (show other bugs)
Version: 2.1.7
Hardware: All Windows XP
: P3 normal
Assignee: Simon McQueen
URL:
Depends on:
Blocks:
 
Reported: 2013-02-08 04:18 CST by Simon Massey
Modified: 2013-08-12 07:10 CDT (History)
0 users

See Also:


Attachments
A test case for this problem. (2.91 KB, application/octet-stream)
2013-02-08 10:19 CST, Simon Massey
Details
Fix for a resource leak under error condition that the test hilights (732 bytes, patch)
2013-02-11 03:25 CST, Simon Massey
Details
Fix for problem: TAO_IDL/be/be_visitor_typecode/union_typecode.cpp (4.31 KB, patch)
2013-02-11 06:19 CST, Simon Massey
Details
Enhanced Fix for problem: TAO_IDL/be/be_visitor_typecode/union_typecode.cpp (5.32 KB, patch)
2013-02-12 05:05 CST, Simon Massey
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Massey 2013-02-08 04:18:15 CST
For example the following legal IDL union declarion:

union MultiLabelUnion {
 case 0:
 case 1:
   char mlu_char;
 case 2:
   double mlu_double;
};

Currently TAO (and a few other orbs including JacOrb and eOrb'C' eORB'C++') do not handle the typecode generation correctly for this. Only one of the above cases for the mlu_char member are dealt with in the typecode information, even though the generated (de)marshaling code correctly handles all case labels. This means that such unions sent inside anys are impossiable to decode at the receiving end if the missing case label is being used for the union it holds.

When using the typecode interface functions for the above it incorrectly gives:
tc->member_count () returns 2 (for the two types)
tc->member_label(0), tc->member_name(0) and tc->member_type(0)
  returns 0, "mlu_char", and char
tc->member_label(1), tc->member_name(1) and tc->member_type(1)
  returns 2, "mlu_double", and double

The exact handling for the above is not actually dealt with in the CORBA spec
as far as I can see excepting that multiple case labels for a type are perfectly valid for unions. BUT I've found a little passage detailing this in the Henning and Vinoski “Advanced CORBA Programming with C++” bilble. See page 700. (of the 16.3.2 Chapter “Type Code Parameters” page 698-700 covering “Type Code Parameters for Unions”). This states that the member_count for unions should actually be the number of case labels NOT the number of types. This would make the typecode information that needs to be generated for the above type produce:

tc->member_count () returns 3 (for the the number of labels)
tc->member_label(0), tc->member_name(0) and tc->member_type(0)
  returns 0, "mlu_char", and char
tc->member_label(1), tc->member_name(1) and tc->member_type(1)
  returns 1, "mlu_char", and char
tc->member_label(2), tc->member_name(2) and tc->member_type(2)
  returns 2, "mlu_double", and double

This was found by a Prismtech customer who noted that inter-op with TIDorb wasn't working correctly when extracting such a union from an any as generated by TAO. As a work-a-round, if they modified the IDL for the type to:

union MultiLabelUnion {
 case 0:
   char mlu_char;
 case 1:
   char mlu_char;
 case 2:
   double mlu_double;
};

everything started working again, but that the normal IDL (that doesn't work) should also work.
Comment 1 Simon Massey 2013-02-08 04:18:47 CST
Prismtech reference Jira TAO-69.
Comment 2 Simon Massey 2013-02-08 04:33:55 CST
With the original "normal" union IDL defintion:

CORBA::ORB_var orb (CORBA::ORB_init (argc, argv));

CORBA::Object_var codecFactory_obj (orb->resolve_initial_references ("CodecFactory"));
IOP::CodecFactory_var codecFactory (IOP::CodecFactory::_narrow (codecFactory_obj));

IOP::Encoding cdr_encoding;
cdr_encoding.format = IOP::ENCODING_CDR_ENCAPS;
cdr_encoding.major_version = 1;
cdr_encoding.minor_version = 2;
IOP::Codec_var codec (codecFactory->create_codec (cdr_encoding));

MultiLabelUnion mlu;
mlu.mlu_char ('4');
mlu._d (1);
CORBA::Any any;
any <<= mlu;
CORBA::OctetSeq_var messageInCDR = codec->encode (any);
CORBA::Any_ptr result = codec->decode (messageInCDR);

The above "result" is unusable as the any is corrupt as if the union had used an unknown case label and the data for the '4' is not copied into it. This is due to the $TAO_ROOT/tao/AnyTypeCode/skip.cpp in the TAO_Marshal_Union::skip (tc, InputCDR) function (lines 316-545) doesn't have the correct information from the typecode to decode the correct case.  (It would also effect the append.cpp in the same way).
Comment 3 Simon Massey 2013-02-08 04:53:47 CST
I think this is generated from:
// TAO_IDL - Generated from
// be/be_visitor_typecode/union_typecode.cpp:66
Comment 4 Simon Massey 2013-02-08 06:42:17 CST
Sorry obviosuly the idl unions above should be:

union MultiLabelUnion switch (long) {
 case 0:
 case 1:
   char mlu_char;
 case 2:
   double mlu_double;
};

and

union MultiLabelUnion switch (long) {
 case 0:
   char mlu_char;
 case 1:
   char mlu_char;
 case 2:
   double mlu_double;
};
Comment 5 Simon Massey 2013-02-08 07:27:51 CST
Looks like JacOrb is actually generating the typecode correctly with duplicate members for each label as per Henning and Vinoski's statement. It's just TAO and eORB which isn't.
Comment 6 Simon Massey 2013-02-08 10:19:48 CST
Created attachment 1453 [details]
A test case for this problem.

Added a test case for this problem untar with tar -xzf Bug_4097_Regression.gztar
Comment 7 Simon Massey 2013-02-11 03:25:24 CST
Created attachment 1454 [details]
Fix for a resource leak under error condition that the test hilights

Line 160 "empty_value" is not actually deleted by the destructor of the "replacement" so the "replacement_safety" auto_ptr is not enough to protect against memory leak of the "empty_value" under error conditions. This diff adds another auto_ptr for the "empty_value".
Comment 8 Simon Massey 2013-02-11 03:28:14 CST
Problem seems to be from:
be/be_visitor_typecode/union_typecode.cpp:174

TAO::be_visitor_union_typecode::visit_cases (be_union * node) see comment at line 214-216.
The typecode is not getting generated with all the correct information.
Comment 9 Simon Massey 2013-02-11 06:19:48 CST
Created attachment 1455 [details]
Fix for problem: TAO_IDL/be/be_visitor_typecode/union_typecode.cpp

Test now passes. Typecode for multi-case labels for union type now created correctly by tao_idl compiler.
Comment 10 Simon Massey 2013-02-12 05:05:36 CST
Created attachment 1456 [details]
 Enhanced Fix for problem: TAO_IDL/be/be_visitor_typecode/union_typecode.cpp 

Replacing the original as it had a flaw for one of the existing IDL_Tests, this enhanced version corrects that and reduces the amount of static memory required if the default type is aliased.
Comment 11 Johnny Willemsen 2013-08-12 07:10:23 CDT
Reassign because Simon Massey left PT. Simon, what is the status of this?