Re: [PATCH] Read corrrect auxiliary entry in AIX.

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Sangamesh Mallayya wrote:

> Attaching the modified patch which i sent earlier for reading correct
> auxiliary entry in AIX.
> Description is almost same, with the output of test case results added.
> 
> 
> When we use -qfuncsect xlc or -ffunction-sections gcc option,
> it places instructions for each function in a separate control section or
> CSECT.
> 
> One more extra symbol entry with one or more auxiliary entries will be
> created as shown below.
> So in below output, symbol 104 is having two auxiliary entries to indicate
> a separate csect.
> 
> We were missing to read SD/PR and information about a each separate
> functions csect.
> If no csect entry is present, we would just be going through and looking
> for LD/PR and reading function start address
> by doing jump to function_entry_point: label.
> 
> 
> [104]   m   0x10000500     .text     2  unamex                    .baz
> [105]   a1           0         0    92       0       0
> [106]   a4  0x0000005c       0    0     SD       PR    0    0
> [107]   m   0x10000500     .text     2  extern                    .baz
> [108]   a2           0              92    3808     119
> [109]   a4  0x00000068       0    0     LD       PR    0    0
> [110]   m   0x00000000     debug     0     fun                    baz:F-1
> [111]   m   0x10000518     .text     1     fcn                    .bf
> [112]   a1           0         4     0       0       0
> [113]   m   0x00000068     debug     0    psym                    a:p-1
> [114]   m   0x000000a4     debug     0   bstat                    .bs
> [115]   m   0x00000008     debug     0   stsym __func__:V13
> [116]   m   0x00000000     debug     0   estat                    .es
> [117]   m   0x10000540     .text     1     fcn                    .ef
> 
> And in case of gcc compiled binaries we were having symbol table entries
> as.
> 
> [149]   m   0x1000045c     .text     1  unamex                    .baz
> [150]   a4  0x0000005c       0    0     SD       PR    0    0
> [151]   m   0x1000045c     .text     2  extern                    .baz
> [152]   a2           0              92   29522     160
> [153]   a4  0x00000095       0    0     LD       PR    0    0
> [154]   m   0x00000000     debug     0     fun                    baz:F-1
> [155]   m   0x100003d8     .text     1     fcn                    .bf
> 
> So the below changes to read correct auxiliary entry works for both gcc &
> xlc compiled binaries even if we have 1 entry.
> 
> As the offical xcoff documentation say about csect auxilirary entry.
> 
> And we always need to read last auxiliary entry as symbols having a
> storage class of C_EXT, C_WEAKEXT or C_HIDEXT
> and more than one auxiliary entry must have the csect auxiliary entry as
> the last auxiliary entry.
> 
> So we need to read the last auxiliary entry which is having the SD/PR in
> case of qfuncsect or LD/PR is case of no csects.


The current code for handling this is already confusing to me.  Code
at the function_entry_point label copies the current main_aux to the
fcn_aux_saved variable, which is later accessed to retrieve the function
size (from fcn_aux_saved.x_sym.x_misc.x_fsize).

Now in current code, there are two paths to this label.  The first is
if we have a C_EXT / C_HIDEXT symbol with a single aux entry.  According
to the XCOFF spec, this must then be a csext aux entry, which contains
the SCLAS / SMTYP fields.  If those are LD/PR, then we end up at the
function_entry_point label, and *this* aux entry is copied into the
fcn_aux_saved variable.  This seems problematic, since this is at this
point a *csect* aux entry, but fcn_aux_saved is later assumed to be
a function aux entry that holds a fsize field -- but for csect aux
entries this is not true, right?

On the other hand, if I understand the XCOFF standard correctly, actual
functions will always have at least *two* aux entries, and therefore in
current code we could never actually run into that bad case?

Still in the current code base, C_EXT / C_HIDEXT symbols with more than
one aux entry (which supposedly includes all function symbols) will
therefore always fall through to the next if statement:

     /* If explicitly specified as a function, treat is as one.  This check
         evaluates to true for @FIX* bigtoc CSECT symbols, so it must occur
         after the above CSECT check.  */
      if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
        {
          bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
                                0, cs->c_naux, &main_aux);
          goto function_entry_point;
        }

Now this ISFCN check is somewhat unclear to me.  It checks the c_type
field in the symbol entry itself.  At least according to the XCOFF
standard here:
http://www.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.files/XCOFF.htm
this field is only ever supposed to be nonzero on C_EXT / C_HIDEXT /
C_WEAKEXT symbols, so I don't quite understand the C_TPDEF check.
Also, it is not quite clear whether ISFCN is set on the SD/PR symbol,
on the LD/PR symbol, or on both.

In any case, current code then simply assumes that the symbol must have
a first aux entry that contains the function size.  (If ISFCN were true
on SD/PR, this assumption would already be wrong, I guess.)

Even more confusing, the code that copies an aux entry from the external
storage format into the internal representation (_bfd_xcoff_swap_aux_in)
only even initializes the fsize field if the ISFCN check is true:

  if (ISFCN (type))
    {
      in->x_sym.x_misc.x_fsize = H_GET_32 (abfd, ext->x_sym.x_misc.x_fsize);
    }

so it is unclear what would happen if the LD/PR check and the ISFCN
check would ever yield different results ...   Do you understand
exactly when the c_type is set to indicate a function?


Given this, I do have a couple of questions about your proposed patch:

     /* XCOFF can have more than 1 auxent. */
     if ((cs->c_naux > 1) &&
         (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF))
       {
         /* We need to read the size if we have LD/PR, and with more than
            one auxiliary entry.  */

- The c_sclass check here is redundant since we're already in a
  if (cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
  block.

- B.t.w. do we need to handle C_WEAKEXT?

- Given the check above, can it *ever* happen that this check is false
  and we still reach the LD/PR case?  If so, we'd still have the problem
  that the record is the wrong one and doesn't actually contain a size,
  right?  But if not, shouldn't we just have the function_entry_point
  handling in one place and avoid the goto?

- If there is just one place, should it be where you added the check
  above, or rather in the LD/PR switch case?

- Finally, given the check added above, can it still ever happen that
  the original ISFCN check after the if C_EXT block is true?  If so,
  are we sure that the symbol will always have a function size in the
  first aux entry?  (I understand that if a symbol has just one aux
  entry, it must be a csect one which doesn't have the function size.)
  If not, this block should be removed as well.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com