Bug 48269 - [4.6 Regression] Incorrect fortify warning for a packed struct member
[4.6 Regression] Incorrect fortify warning for a packed struct member
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: middle-end
4.6.0
: P3 normal
: 4.6.1
Assigned To: Richard Biener
: diagnostic
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-24 09:54 UTC by İsmail "cartman" Dönmez
Modified: 2011-03-28 10:20 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.6.1, 4.7.0
Known to fail: 4.6.0
Last reconfirmed: 2011-03-24 11:14:23


Attachments
Preprocessed source (147.60 KB, application/x-gzip)
2011-03-24 09:55 UTC, İsmail "cartman" Dönmez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description İsmail "cartman" Dönmez 2011-03-24 09:54:02 UTC
[~]> g++-4.6 -m32 -O2 -fstack-protector -c kis_droneframe.ii 
In file included from packet.h:37:0,
                 from kis_droneframe.cc:28:
packet_ieee80211.h:248:40: warning: ‘packed’ attribute ignored [-Wattributes]
packet_ieee80211.h:253:43: warning: ‘packed’ attribute ignored [-Wattributes]
packet_ieee80211.h:273:43: warning: ‘packed’ attribute ignored [-Wattributes]
In file included from /usr/include/stdio.h:912:0,
                 from util.h:24,
                 from kis_droneframe.cc:25:
In function ‘int snprintf(char*, size_t, const char*, ...)’,
    inlined from ‘virtual int KisDroneFramework::SendSource(int, pst_packetsource*, int)’ at kis_droneframe.cc:507:49:
/usr/include/bits/stdio2.h:66:75: warning: call to int __builtin___snprintf_chk(char*, unsigned int, int, unsigned int, const char*, ...) will always overflow destination buffer [enabled by default]

Now the said line is;

        snprintf((char *) spkt->type_str, 16, "%s",
                 in_int->strong_source->FetchType().c_str());


where spkt is defined as;

struct drone_source_packet {
    uint16_t source_hdr_len;
    uint32_t source_content_bitmap;
    drone_trans_uuid uuid;
    // Kill this source, the rest of the data is empty
    uint16_t invalidate;
    // Null-terminated strings
    uint8_t name_str[16];
    uint8_t interface_str[16];
    uint8_t type_str[16];
    uint8_t channel_hop;
    uint16_t channel_dwell;
    uint16_t channel_rate;
} __attribute__((__packed__));

Using gcc version 4.6.0 20110314 [gcc-4_6-branch revision 170941] (SUSE Linux)
Comment 1 İsmail "cartman" Dönmez 2011-03-24 09:55:23 UTC
Created attachment 23766 [details]
Preprocessed source
Comment 2 marcus 2011-03-24 09:58:48 UTC
there is a malloc(sizeof(struct1)+sizeof(struct2))
and the struct 1 has 

uint8_t data[0] 
at the end, where struct2* = struct1*->data;

the malloc() seems to allocate insufficient memory, spo the overflow checker triggers.
Comment 3 Richard Biener 2011-03-24 10:43:29 UTC
  D.83064_56 = in_int_3(D)->strong_source;
  D.83065_57 = D.83064_56->_vptr.KisPacketSource;
  D.83086_59 = MEM[(int (*__vtbl_ptr_type) (void) *)D.83065_57 + 92B];
  D.73136 = OBJ_TYPE_REF(D.83086_59;D.83064_56->23) (D.83064_56); [return slot optimization]
  D.83087_61 = std::basic_string<char>::c_str (&D.73136);
  D.83088_63 = &MEM[(struct drone_source_packet *)dpkt_1 + 12B].interface_str;
  __s_143 = (char * restrict) D.83088_63;
  D.85546_145 = 16;
  __builtin___snprintf_chk (__s_143, 16, 1, D.85546_145, "%s", D.83087_61);
  std::basic_string<char>::~basic_string (&D.73136);
  D.83064_65 = in_int_3(D)->strong_source;
  D.83065_66 = D.83064_65->_vptr.KisPacketSource;
  D.83090_68 = MEM[(int (*__vtbl_ptr_type) (void) *)D.83065_66 + 96B];
  D.73137 = OBJ_TYPE_REF(D.83090_68;D.83064_65->24) (D.83064_65); [return slot optimization]
  D.83091_70 = std::basic_string<char>::c_str (&D.73137);
  D.83092_72 = &MEM[(struct drone_source_packet *)dpkt_1 + 12B].type_str;
  __s_147 = (char * restrict) D.83092_72;
  D.85552_149 = 9;
  __builtin___snprintf_chk (__s_147, 16, 1, D.85552_149, "%s", D.83091_70);


so for some reason we compute the object size of *D.38092_72 as 9.  We allocated
dpkt_1 as

  dpkt_1 = malloc (89);

which looks like enough.

C testcase:

typedef struct {
    unsigned int sentinel;
    char data[0];
} drone_packet;
typedef struct {
    char type_str[16];
    char channel_hop;
} drone_source_packet;
drone_packet *
foo(char *x)
{
  drone_packet *dpkt = __builtin_malloc(sizeof(drone_packet)
                                        + sizeof(drone_source_packet));
  drone_source_packet *spkt = (drone_source_packet *) dpkt->data;
  __builtin___snprintf_chk (spkt->type_str, 16, 
                            1, __builtin_object_size (spkt->type_str, 1), 
                            "%s", x);
  return dpkt;
}
Comment 4 Richard Biener 2011-03-24 11:14:23 UTC
I have a patch.
Comment 5 Richard Biener 2011-03-24 12:45:02 UTC
Author: rguenth
Date: Thu Mar 24 12:44:58 2011
New Revision: 171388

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171388
Log:
2011-03-24  Richard Guenther  <rguenther@suse.de>

	PR middle-end/48269
	* tree-object-size.c (addr_object_size): Do not double-account
	for MEM_REF offsets.

	* gcc.dg/builtin-object-size-10.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/builtin-object-size-10.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-object-size.c
Comment 6 Richard Biener 2011-03-24 15:01:48 UTC
Fixed for 4.7 sofar.
Comment 7 Richard Biener 2011-03-28 10:14:40 UTC
Author: rguenth
Date: Mon Mar 28 10:14:34 2011
New Revision: 171595

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171595
Log:
2011-03-28  Richard Guenther  <rguenther@suse.de>

	Backport from mainline
	2011-03-24  Richard Guenther  <rguenther@suse.de>

	PR middle-end/48269
	* tree-object-size.c (addr_object_size): Do not double-account
	for MEM_REF offsets.

	* gcc.dg/builtin-object-size-10.c: New testcase.

	2011-03-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/48228
	* tree-vrp.c (vrp_visit_phi_node): Do not stop propagating
	for single-arg PHIs.

	* gcc.dg/Wstrict-overflow-23.c: New testcase.

	2011-03-17  Richard Guenther  <rguenther@suse.de>

	PR middle-end/48134
	* tree-ssa.c (insert_debug_temp_for_var_def): If we propagated
	a value make sure to fold the statement.

	* gcc.dg/pr48134.c: New testcase.

	2011-03-15  Richard Guenther  <rguenther@suse.de>

	PR middle-end/48031
	* fold-const.c (fold_indirect_ref_1): Do not create new variable-sized
	or variable-indexed array accesses when in gimple form.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/Wstrict-overflow-23.c
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/builtin-object-size-10.c
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/pr48134.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/fold-const.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/gcc/tree-object-size.c
    branches/gcc-4_6-branch/gcc/tree-ssa.c
    branches/gcc-4_6-branch/gcc/tree-vrp.c
Comment 8 Richard Biener 2011-03-28 10:20:54 UTC
Fixed.