Project

General

Profile

Actions

Feature #13698

closed

CTF could handle C99 VLAs in function arguments

Added by Andy Fiddaman about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
tools - gate/build tools
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

ctfconvert cannot convert data for function definitions which contain some types of C99 variable length array (VLA) definitions.

For example:

void t(int n, int arr[n][n]) { }

build% /opt/onbld/bin/i386/ctfconvert -k VLA.o
ctfconvert: CTF conversion failed: failed to get DW_FORM_block1 (10) value for DW_AT_upper_bound: DW_DLE_ATTR_FORM_BAD: In function formudata (internal function) on seeing form  0xa  (DW_FORM_block1)

I've seen real world examples where the DWARF data form is DW_FORM_block1 and DW_FORM_ref4.

The DWARF data for this example is:

< 2><0x0000004e>      DW_TAG_formal_parameter
                        DW_AT_name                  n
                        DW_AT_decl_file             0x00000001 /data/omnios-build/ctf/VLA.c
                        DW_AT_decl_line             0x00000001
                        DW_AT_decl_column           0x0000000c
                        DW_AT_type                  <0x0000006b>
                        DW_AT_location              len 0x0002: 0x914c:
                            DW_OP_fbreg -52
< 2><0x0000005b>      DW_TAG_formal_parameter
                        DW_AT_name                  arr
                        DW_AT_decl_file             0x00000001 /data/omnios-build/ctf/VLA.c
                        DW_AT_decl_line             0x00000001
                        DW_AT_decl_column           0x00000013
                        DW_AT_type                  <0x0000008c>
                        DW_AT_location              len 0x0002: 0x9140:
                            DW_OP_fbreg -64
< 1><0x0000006b>    DW_TAG_base_type
                      DW_AT_byte_size             0x00000004
                      DW_AT_encoding              DW_ATE_signed
                      DW_AT_name                  int
< 1><0x00000072>    DW_TAG_array_type
                      DW_AT_type                  <0x0000006b>
                      DW_AT_sibling               <0x00000085>
< 2><0x0000007b>      DW_TAG_subrange_type
                        DW_AT_type                  <0x00000085>
                        DW_AT_upper_bound
                            DW_OP_fbreg -40
                            DW_OP_deref< 1><0x00000085>    DW_TAG_base_type
                      DW_AT_byte_size             0x00000008
                      DW_AT_encoding              DW_ATE_unsigned
                      DW_AT_name                  long unsigned int
< 1><0x0000008c>    DW_TAG_pointer_type
                      DW_AT_byte_size             0x00000008
                      DW_AT_type                  <0x00000072>

It seems that we could just set the upper bound to 0 here rather than failing the entire conversion and leaving the object with no CTF at all.

Actions #1

Updated by Robert Mustacchi about 2 years ago

Treating this as basically a zero length array like we do the arrays at the end of functions seems reasonable for CTF at this time.

Actions #2

Updated by Andy Fiddaman about 2 years ago

  • Status changed from New to In Progress
  • Assignee set to Andy Fiddaman
Actions #3

Updated by Electric Monk about 2 years ago

  • Gerrit CR set to 1404
Actions #4

Updated by Andy Fiddaman about 2 years ago

Investigating this further since the proposed fix makes these multi-dimensional arrays look like function pointers in the C-style output.

Here is how the DWARF looks for a couple of examples:

int vla1(int n1, int arr1[n1])

< 2><0x0000010d>      DW_TAG_formal_parameter
                        DW_AT_name                  arr1
                        DW_AT_decl_file             0x00000001 .../util-tests/tests/ctf/af.c
                        DW_AT_decl_line             0x0000001a
                        DW_AT_decl_column           0x00000012
                        DW_AT_type                  <0x0000011d>
                        DW_AT_location              len 0x0002: 0x9160:
                            DW_OP_fbreg -32

< 1><0x0000011d>    DW_TAG_pointer_type
                      DW_AT_byte_size             0x00000008
                      DW_AT_type                  <0x00000050>

< 1><0x00000050>    DW_TAG_base_type
                      DW_AT_byte_size             0x00000004
                      DW_AT_encoding              DW_ATE_signed
                      DW_AT_name                  int

int vla2(int n2, int arr2[n2][n2])

< 2><0x000000af>      DW_TAG_formal_parameter
                        DW_AT_name                  arr2
                        DW_AT_decl_file             0x00000001 .../util-tests/tests/ctf/af.c
                        DW_AT_decl_line             0x00000020
                        DW_AT_decl_column           0x00000012
                        DW_AT_type                  <0x000000d2>
                        DW_AT_location              len 0x0002: 0x9140:
                            DW_OP_fbreg -64

< 1><0x000000bf>    DW_TAG_array_type
                      DW_AT_type                  <0x00000050>
                      DW_AT_sibling               <0x000000d2>

< 2><0x000000c8>      DW_TAG_subrange_type
                        DW_AT_type                  <0x0000002d>
                        DW_AT_upper_bound

                            DW_OP_fbreg -40
                            DW_OP_deref< 1><0x000000d2>    DW_TAG_pointer_type
                      DW_AT_byte_size             0x00000008
                      DW_AT_type                  <0x000000bf>

with the following generated CTF data

- Functions ---------------------------------------------------------------------

  [0] vla1 (8) returns: 1 args: (1, 5)
  [1] vla2 (9) returns: 1 args: (1, 4)

- Types -------------------------------------------------------------------------

  <1> int encoding=SIGNED offset=0 bits=32
  [2] long encoding=SIGNED offset=0 bits=64
  [3] int [0] contents: 1, index: 2
  <4> int (*)[0] refers to 3
  <5> int * refers to 1
Actions #5

Updated by Robert Mustacchi about 2 years ago

It looks like the secret here is this block comment in ctf_type_qlname:

        /*   
         * If the type graph's order conflicts with lexical precedence order
         * for pointers or arrays, then we need to surround the declarations at
         * the corresponding lexical precedence with parentheses.  This can
         * result in either a parenthesized pointer (*) as in int (*)() or
         * int (*)[], or in a parenthesized pointer and array as in int (*[])().
         */

So that's where this is coming in from and while it looks like a function pointer, it is in fact not.

Actions #6

Updated by Andy Fiddaman about 2 years ago

I've tested this by running the CTF testsuite, including the new tests which failed before.
This has also been used to build software which failed CTF conversion previously, such as the OmniOS Extra stress-ng package. The output of ctfdump -c looks reasonable (versus none before):

Here's a sample of the functions that use VLAs

extern void stress_matrix_3d_zyx_sub(const size_t, stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0]);
extern void stress_matrix_3d_zyx_trans(const size_t, stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0]);
extern void stress_matrix_3d_zyx_zero(const size_t, stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0], stress_matrix_3d_type_t (*)[0][0]);
extern void stress_matrix_method_error(void);
extern void stress_matrix_set_default(void);
extern void stress_matrix_xy_add(const size_t, stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0]);
extern void stress_matrix_xy_all(const size_t, stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0]);
extern void stress_matrix_xy_copy(const size_t, stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0], stress_matrix_type_t (*)[0]);

Actions #7

Updated by Electric Monk about 2 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit e0cbdd5af707390adb289995fdf2dd8a3869dcca

commit  e0cbdd5af707390adb289995fdf2dd8a3869dcca
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2021-04-25T18:06:07.000Z

    13698 CTF could handle C99 VLAs in function arguments
    Reviewed by: Jason King <jason.king@joyent.com>
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Actions

Also available in: Atom PDF