Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays
2014-06-13 19:40 GMT+04:00 Jeff Law l...@redhat.com: On 06/12/14 17:38, Ilya Enkovich wrote: It looks ok to me - did you test with all languages? In particular did you test Ada? I configure compiler with no language disabling and then run bootstrap and make check. Does it mean all languages are covered? Will make more testing if required. In addition to Richi's comment able --enable-languages= ... You also need an Ada compiler installed to build Ada. Most Linux distributions provide Ada packages of some sort. It may be called ada or gnat. Thanks for tips! I added --enable-languages=all,ada,obj-c++,go and got no new failures with this patch. Ilya Jeff
Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays
On Fri, Jun 13, 2014 at 1:38 AM, Ilya Enkovich enkovich@gmail.com wrote:
2014-06-12 11:55 GMT+04:00 Richard Biener richard.guent...@gmail.com:
On Wed, Jun 11, 2014 at 6:08 PM, Ilya Enkovich enkovich@gmail.com
wrote:
Hi,
This patch fixes problem with size emitted for static structures with
flexible array. I found a couple of trackers in guzilla for this problem
but all of them are marked as fixed and problem still exists.
For a simple testcase
struct S { int a; int b[0]; } s = { 1, { 0, 0} };
current trunk produces (no flags):
.globl s
.data
.align 4
.type s, @object
.size s, 4
s:
.long 1
.long 0
.long 0
which has wrong size for object s.
This problem is important for checker because wrong size leads to wrong
bounds and false bounds violations. Following patch uses DECL_SIZE_UNIT
instead of type size and works well for me. Does it look OK?
There is a bug about this in bugzilla somewhere.
I looked through bugzilla and found two trackers with similar problem.
The first one is 57180 which is still open but with comment that
problem is fixed on the trunk. I checked it and it really passes for
the trunk (should tracker be closed then?).
Another one is 28865 (and a set of its duplicates). Interesting thing
here is that original testcase uses array of integers but testcases
which were added with commit use arrays of chars. Original test is
still compiled wrongly. I also see that a patch very similar to what I
posted was proposed as a solution but it was reported to cause a
problem with glibc/nss/nss_files/files-init.c. There is a
corresponding testcase in the tracker which results wrong padding when
patch is applied but it seems to be another problem because I do not
see any problem when use mpx compiler branch for this testcase.
It looks ok to me - did you test with all languages? In particular did
you test Ada?
I configure compiler with no language disabling and then run bootstrap
and make check. Does it mean all languages are covered? Will make more
testing if required.
That only enables default languages which excludes Objective-C++, Ada
and Go. To enable really all languages you have to specify
--enable-languages=all,ada,obj-c++,go
Richard.
Thanks,
Ilya
Thanks,
Richard.
Bootstrapped and tested on linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-06-11 Ilya Enkovich ilya.enkov...@intel.com
* config/elfos.h (ASM_DECLARE_OBJECT_NAME): Use decl size
instead of type size.
(ASM_FINISH_DECLARE_OBJECT): Likewise.
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index c1d5553..7929708 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -313,7 +313,7 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
(DECL) DECL_SIZE (DECL))\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, NAME, size); \
} \
\
@@ -341,7 +341,7 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
!size_directive_output)\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, name, size); \
} \
} \
Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays
On 06/12/14 17:38, Ilya Enkovich wrote: It looks ok to me - did you test with all languages? In particular did you test Ada? I configure compiler with no language disabling and then run bootstrap and make check. Does it mean all languages are covered? Will make more testing if required. In addition to Richi's comment able --enable-languages= ... You also need an Ada compiler installed to build Ada. Most Linux distributions provide Ada packages of some sort. It may be called ada or gnat. Jeff
Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays
On Wed, Jun 11, 2014 at 6:08 PM, Ilya Enkovich enkovich@gmail.com wrote:
Hi,
This patch fixes problem with size emitted for static structures with
flexible array. I found a couple of trackers in guzilla for this problem but
all of them are marked as fixed and problem still exists.
For a simple testcase
struct S { int a; int b[0]; } s = { 1, { 0, 0} };
current trunk produces (no flags):
.globl s
.data
.align 4
.type s, @object
.size s, 4
s:
.long 1
.long 0
.long 0
which has wrong size for object s.
This problem is important for checker because wrong size leads to wrong
bounds and false bounds violations. Following patch uses DECL_SIZE_UNIT
instead of type size and works well for me. Does it look OK?
There is a bug about this in bugzilla somewhere.
It looks ok to me - did you test with all languages? In particular did
you test Ada?
Thanks,
Richard.
Bootstrapped and tested on linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-06-11 Ilya Enkovich ilya.enkov...@intel.com
* config/elfos.h (ASM_DECLARE_OBJECT_NAME): Use decl size
instead of type size.
(ASM_FINISH_DECLARE_OBJECT): Likewise.
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index c1d5553..7929708 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -313,7 +313,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
(DECL) DECL_SIZE (DECL))\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, NAME, size); \
} \
\
@@ -341,7 +341,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
!size_directive_output)\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, name, size); \
} \
} \
Re: [PATCH, Pointer Bounds Checker 35/x] Fix object size emitted for structures with flexible arrays
2014-06-12 11:55 GMT+04:00 Richard Biener richard.guent...@gmail.com:
On Wed, Jun 11, 2014 at 6:08 PM, Ilya Enkovich enkovich@gmail.com wrote:
Hi,
This patch fixes problem with size emitted for static structures with
flexible array. I found a couple of trackers in guzilla for this problem
but all of them are marked as fixed and problem still exists.
For a simple testcase
struct S { int a; int b[0]; } s = { 1, { 0, 0} };
current trunk produces (no flags):
.globl s
.data
.align 4
.type s, @object
.size s, 4
s:
.long 1
.long 0
.long 0
which has wrong size for object s.
This problem is important for checker because wrong size leads to wrong
bounds and false bounds violations. Following patch uses DECL_SIZE_UNIT
instead of type size and works well for me. Does it look OK?
There is a bug about this in bugzilla somewhere.
I looked through bugzilla and found two trackers with similar problem.
The first one is 57180 which is still open but with comment that
problem is fixed on the trunk. I checked it and it really passes for
the trunk (should tracker be closed then?).
Another one is 28865 (and a set of its duplicates). Interesting thing
here is that original testcase uses array of integers but testcases
which were added with commit use arrays of chars. Original test is
still compiled wrongly. I also see that a patch very similar to what I
posted was proposed as a solution but it was reported to cause a
problem with glibc/nss/nss_files/files-init.c. There is a
corresponding testcase in the tracker which results wrong padding when
patch is applied but it seems to be another problem because I do not
see any problem when use mpx compiler branch for this testcase.
It looks ok to me - did you test with all languages? In particular did
you test Ada?
I configure compiler with no language disabling and then run bootstrap
and make check. Does it mean all languages are covered? Will make more
testing if required.
Thanks,
Ilya
Thanks,
Richard.
Bootstrapped and tested on linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-06-11 Ilya Enkovich ilya.enkov...@intel.com
* config/elfos.h (ASM_DECLARE_OBJECT_NAME): Use decl size
instead of type size.
(ASM_FINISH_DECLARE_OBJECT): Likewise.
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index c1d5553..7929708 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -313,7 +313,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
(DECL) DECL_SIZE (DECL))\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, NAME, size); \
} \
\
@@ -341,7 +341,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
!size_directive_output)\
{ \
size_directive_output = 1;\
- size = int_size_in_bytes (TREE_TYPE (DECL)); \
+ size = tree_to_uhwi (DECL_SIZE_UNIT (DECL)); \
ASM_OUTPUT_SIZE_DIRECTIVE (FILE, name, size); \
} \
} \
