[Bug tree-optimization/65724] New: __builtin_object_size difference for C and C++

[Bug tree-optimization/65724] New: __builtin_object_size difference for C and C++

[xur at google dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724

            Bug ID: 65724
           Summary: __builtin_object_size difference for C and C++
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: xur at google dot com

For the following code:
--------------
#include <stdio.h>
#include <stdlib.h>

typedef struct {
    int i;
    char data[];
} foo;

typedef struct {
    foo foo_var;
} bar;


int main() {
  bar *bar_var = (bar *) malloc(atoi("1000"));
  printf("size %zd\n", __builtin_object_size(bar_var->foo_var.data, 1));
  return 0;
}
------------------

"gcc -O2" prints out -1 while "g++ -O2" prints out a wrong value of 0.

There are some frontend difference that triggers different code path in
addr_object_size() in tree-object-size.c. (g++ sets the TYPE_SIZE_UNIT for the
array as 0.) But nevertheless, it's reasonable to expect the same output with
gcc or g++.


In addr_object_size(), it seems we should always return a unknown size for the
last field if it's an array (due to zero size of flexible size array).
The code works if the field access is one level, but it breaks for multi-level
field accesses like in the above example.

More specifically, the following is questionable:
 337                     while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
 338                       if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
 339                           != UNION_TYPE
 340                           && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
 341                           != QUAL_UNION_TYPE)
 342                         break;
 343                       else
 344                         v = TREE_OPERAND (v, 0);

It seems to me after we detect this is the last array field, we should always
peel the field,i.e removing code from line 338 to 343.

[Bug tree-optimization/65724] __builtin_object_size difference for C and C++

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, C has flexible array members, while C++ does not have those, so there is
a significant difference in between the two.  And when you embed a [0] array
into another structure, it is intentional that _FORTIFY_SOURCE=2 (i.e. the more
restrictive mode, beyond the standards) only allows to fill the fields and not
cross to outside of that.

[Bug tree-optimization/65724] __builtin_object_size difference for C and C++

[xur at google dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724

--- Comment #2 from xur at google dot com ---
Do you mean the result of 0 in g++ is intentional?

But I'm not quite understand the relation with _FORTIFY_SOURCE=2. This
macro does not seem to be check in tree-objsz pass.
In other words, if I unset _FORTIFY_SOURCE or set it 1, I still get the
same result.

Could you give me some pointer of the reference that I can check?

Thanks,

Rong

On Thu, Apr 9, 2015 at 2:05 PM, jakub at gcc dot gnu.org <
gcc-bugzilla@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724
>
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
>
>            What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
>
> --- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Well, C has flexible array members, while C++ does not have those, so
> there is
> a significant difference in between the two.  And when you embed a [0]
> array
> into another structure, it is intentional that _FORTIFY_SOURCE=2 (i.e. the
> more
> restrictive mode, beyond the standards) only allows to fill the fields and
> not
> cross to outside of that.
>
> --
> You are receiving this mail because:
> You reported the bug.
>

[Bug tree-optimization/65724] __builtin_object_size difference for C and C++

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to xur from comment #2)
> Do you mean the result of 0 in g++ is intentional?

Yes.

> But I'm not quite understand the relation with _FORTIFY_SOURCE=2. This
> macro does not seem to be check in tree-objsz pass.
> In other words, if I unset _FORTIFY_SOURCE or set it 1, I still get the
> same result.

With -D_FORTIFY_SOURCE=1, __builtin_object_size (ptr, 1) is never used in the C
library headers, only __builtin_object_size (ptr, 0) for everything.
In that case you are allowed to cross field boundaries not just with memcpy and
the like, but also with strcpy etc.
-D_FORTIFY_SOURCE=2 is a mode that rejects some valid C programs, simply
imposes additional security related restrictions that str* etc. functions can't
cross field boundaries, %n in *scanf family can't be used if the string literal
isn't read only etc.

[Bug tree-optimization/65724] __builtin_object_size difference for C and C++

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65724

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Works as designed.