Re: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On 11/14/14 10:29, Zamyatin, Igor wrote:
On Fri, Nov 14, 2014 at 05:05:57PM +, Zamyatin, Igor wrote:
It is not that easy, -fsanitize=address is not supported everywhere.
Better if you stick it into testsuite/gcc.dg/asan/ No point adding effective-
target ia32/fpic, there is nothing i?86 specific, not even ix86/x86_64 specific,
nor pic specific in the test, just use
/* { dg-additional-options -fPIC { target fpic } } */
Thanks, updated.
gcc/Changelog
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
gcc/testsuite/Changelog
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* gcc.dg/asan/pr63845.c: New test.
Ok.
Jeff
RE: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
ChangeLog:
2014-10-30 Igor Zamyatin igor.zamya...@intel.com
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
The patch is probably fine. However, it would be good to have the
analysis why you want to move initialization of the PIC register earlier.
Asan (and anybody else can) emits global variable(s) in expand_used_vars
during function expanding while pic reg is currently initialized later, during
expand_function_start in assign_parms thus to be late in asan case in PIC
mode.
So to avoid such cases we put pic reg initialization in the beginning of
expand_used_vars. This seems to be early enough.
Please mention PR in ChangeLog and add a few testcases so that the fix will
be tested on Linux.
Bootstrapped and regtested on x86_64 and i686 incl pic mode.
Is it ok?
Thanks,
Igor
gcc/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
gcc/testsuite/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* gcc.target/i386/pr63845.c: New test.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 15d7638..bcd3b35 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1722,6 +1722,9 @@ expand_used_vars (void)
init_vars_expansion ();
+ if (targetm.use_pseudo_pic_reg ())
+pic_offset_table_rtx = gen_reg_rtx (Pmode);
+
hash_maptree, tree ssa_name_decls;
for (i = 0; i SA.map-num_partitions; i++)
{
diff --git a/gcc/function.c b/gcc/function.c
index ef98091..97e0b79 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3679,11 +3679,6 @@ assign_parms (tree fndecl)
fnargs.release ();
- /* Initialize pic_offset_table_rtx with a pseudo register
- if required. */
- if (targetm.use_pseudo_pic_reg ())
-pic_offset_table_rtx = gen_reg_rtx (Pmode);
-
/* Output all parameter conversion instructions (possibly including calls)
now that all parameters have been copied out of hard registers. */
emit_insn (all.first_conversion_insn);
diff --git a/gcc/testsuite/gcc.target/i386/pr63845.c
b/gcc/testsuite/gcc.target/i386/pr63845.c
new file mode 100644
index 000..4b675e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr63845.c
@@ -0,0 +1,20 @@
+/* PR sanitizer/63845 */
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-require-effective-target fpic } */
+/* { dg-skip-if No Windows PIC { *-*-mingw* *-*-cygwin } { * } { } } */
+/* { dg-options -fPIC } */
+
+int __attribute__ ((noinline, noclone))
+foo (void *p)
+{
+ return *(int*)p;
+}
+
+int main ()
+{
+ char a = 0;
+ foo (a);
+ return 0;
+}
+
Re: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On Fri, Nov 14, 2014 at 8:59 AM, Zamyatin, Igor igor.zamya...@intel.com wrote:
ChangeLog:
2014-10-30 Igor Zamyatin igor.zamya...@intel.com
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
The patch is probably fine. However, it would be good to have the
analysis why you want to move initialization of the PIC register earlier.
Asan (and anybody else can) emits global variable(s) in expand_used_vars
during function expanding while pic reg is currently initialized later,
during
expand_function_start in assign_parms thus to be late in asan case in PIC
mode.
So to avoid such cases we put pic reg initialization in the beginning of
expand_used_vars. This seems to be early enough.
Please mention PR in ChangeLog and add a few testcases so that the fix will
be tested on Linux.
Bootstrapped and regtested on x86_64 and i686 incl pic mode.
Is it ok?
Thanks,
Igor
gcc/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
gcc/testsuite/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* gcc.target/i386/pr63845.c: New test.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 15d7638..bcd3b35 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1722,6 +1722,9 @@ expand_used_vars (void)
init_vars_expansion ();
+ if (targetm.use_pseudo_pic_reg ())
+pic_offset_table_rtx = gen_reg_rtx (Pmode);
+
hash_maptree, tree ssa_name_decls;
for (i = 0; i SA.map-num_partitions; i++)
{
diff --git a/gcc/function.c b/gcc/function.c
index ef98091..97e0b79 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3679,11 +3679,6 @@ assign_parms (tree fndecl)
fnargs.release ();
- /* Initialize pic_offset_table_rtx with a pseudo register
- if required. */
- if (targetm.use_pseudo_pic_reg ())
-pic_offset_table_rtx = gen_reg_rtx (Pmode);
-
/* Output all parameter conversion instructions (possibly including calls)
now that all parameters have been copied out of hard registers. */
emit_insn (all.first_conversion_insn);
diff --git a/gcc/testsuite/gcc.target/i386/pr63845.c
b/gcc/testsuite/gcc.target/i386/pr63845.c
new file mode 100644
index 000..4b675e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr63845.c
@@ -0,0 +1,20 @@
+/* PR sanitizer/63845 */
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-require-effective-target fpic } */
+/* { dg-skip-if No Windows PIC { *-*-mingw* *-*-cygwin } { * } { } }
*/
+/* { dg-options -fPIC } */
+
+int __attribute__ ((noinline, noclone))
+foo (void *p)
+{
+ return *(int*)p;
+}
+
+int main ()
+{
+ char a = 0;
+ foo (a);
+ return 0;
+}
+
Will this test fail on Linux without your fix? Doesn't testcase
need -fsanitize=address to fail?
--
H.J.
RE: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On Fri, Nov 14, 2014 at 8:59 AM, Zamyatin, Igor igor.zamya...@intel.com
wrote:
ChangeLog:
2014-10-30 Igor Zamyatin igor.zamya...@intel.com
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
The patch is probably fine. However, it would be good to have the
analysis why you want to move initialization of the PIC register
earlier.
Asan (and anybody else can) emits global variable(s) in
expand_used_vars
during function expanding while pic reg is currently initialized
later, during expand_function_start in assign_parms thus to be late
in asan case in PIC mode.
So to avoid such cases we put pic reg initialization in the
beginning of
expand_used_vars. This seems to be early enough.
Please mention PR in ChangeLog and add a few testcases so that the
fix will be tested on Linux.
Bootstrapped and regtested on x86_64 and i686 incl pic mode.
Is it ok?
Thanks,
Igor
gcc/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
gcc/testsuite/Changelog:
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* gcc.target/i386/pr63845.c: New test.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 15d7638..bcd3b35
100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1722,6 +1722,9 @@ expand_used_vars (void)
init_vars_expansion ();
+ if (targetm.use_pseudo_pic_reg ())
+pic_offset_table_rtx = gen_reg_rtx (Pmode);
+
hash_maptree, tree ssa_name_decls;
for (i = 0; i SA.map-num_partitions; i++)
{
diff --git a/gcc/function.c b/gcc/function.c index ef98091..97e0b79
100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3679,11 +3679,6 @@ assign_parms (tree fndecl)
fnargs.release ();
- /* Initialize pic_offset_table_rtx with a pseudo register
- if required. */
- if (targetm.use_pseudo_pic_reg ())
-pic_offset_table_rtx = gen_reg_rtx (Pmode);
-
/* Output all parameter conversion instructions (possibly including
calls)
now that all parameters have been copied out of hard registers. */
emit_insn (all.first_conversion_insn); diff --git
a/gcc/testsuite/gcc.target/i386/pr63845.c
b/gcc/testsuite/gcc.target/i386/pr63845.c
new file mode 100644
index 000..4b675e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr63845.c
@@ -0,0 +1,20 @@
+/* PR sanitizer/63845 */
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-require-effective-target fpic } */
+/* { dg-skip-if No Windows PIC { *-*-mingw* *-*-cygwin } { * } {
+ } } */
+/* { dg-options -fPIC } */
+
+int __attribute__ ((noinline, noclone)) foo (void *p) {
+ return *(int*)p;
+}
+
+int main ()
+{
+ char a = 0;
+ foo (a);
+ return 0;
+}
+
Will this test fail on Linux without your fix? Doesn't testcase need -
fsanitize=address to fail?
Sure, you're right, will add it
Thanks,
Igor
--
H.J.
Re: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On Fri, Nov 14, 2014 at 05:05:57PM +, Zamyatin, Igor wrote:
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr63845.c
@@ -0,0 +1,20 @@
+/* PR sanitizer/63845 */
+/* { dg-do compile } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-require-effective-target fpic } */
+/* { dg-skip-if No Windows PIC { *-*-mingw* *-*-cygwin } { * } {
+ } } */
+/* { dg-options -fPIC } */
+
+int __attribute__ ((noinline, noclone)) foo (void *p) {
+ return *(int*)p;
+}
+
+int main ()
+{
+ char a = 0;
+ foo (a);
+ return 0;
+}
+
Will this test fail on Linux without your fix? Doesn't testcase need -
fsanitize=address to fail?
Sure, you're right, will add it
It is not that easy, -fsanitize=address is not supported everywhere.
Better if you stick it into testsuite/gcc.dg/asan/
No point adding effective-target ia32/fpic, there is nothing i?86 specific,
not even ix86/x86_64 specific, nor pic specific in the test, just use
/* { dg-additional-options -fPIC { target fpic } } */
Jakub
RE: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On Fri, Nov 14, 2014 at 05:05:57PM +, Zamyatin, Igor wrote:
It is not that easy, -fsanitize=address is not supported everywhere.
Better if you stick it into testsuite/gcc.dg/asan/ No point adding effective-
target ia32/fpic, there is nothing i?86 specific, not even ix86/x86_64
specific,
nor pic specific in the test, just use
/* { dg-additional-options -fPIC { target fpic } } */
Thanks, updated.
gcc/Changelog
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* function.c (assign_parms): Move init of pic_offset_table_rtx
from here to...
* cfgexpand.c (expand_used_vars): ...here.
gcc/testsuite/Changelog
2014-11-14 Igor Zamyatin igor.zamya...@intel.com
PR sanitizer/63845
* gcc.dg/asan/pr63845.c: New test.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 15d7638..bcd3b35 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1722,6 +1722,9 @@ expand_used_vars (void)
init_vars_expansion ();
+ if (targetm.use_pseudo_pic_reg ())
+pic_offset_table_rtx = gen_reg_rtx (Pmode);
+
hash_maptree, tree ssa_name_decls;
for (i = 0; i SA.map-num_partitions; i++)
{
diff --git a/gcc/function.c b/gcc/function.c
index ef98091..97e0b79 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3679,11 +3679,6 @@ assign_parms (tree fndecl)
fnargs.release ();
- /* Initialize pic_offset_table_rtx with a pseudo register
- if required. */
- if (targetm.use_pseudo_pic_reg ())
-pic_offset_table_rtx = gen_reg_rtx (Pmode);
-
/* Output all parameter conversion instructions (possibly including calls)
now that all parameters have been copied out of hard registers. */
emit_insn (all.first_conversion_insn);
diff --git a/gcc/testsuite/gcc.dg/asan/pr63845.c
b/gcc/testsuite/gcc.dg/asan/pr63845.c
new file mode 100644
index 000..a3fbe06
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr63845.c
@@ -0,0 +1,17 @@
+/* PR sanitizer/63845 */
+/* { dg-do compile } */
+/* { dg-options -fPIC { target fpic } } */
+
+int __attribute__ ((noinline, noclone))
+foo (void *p)
+{
+ return *(int*)p;
+}
+
+int main ()
+{
+ char a = 0;
+ foo (a);
+ return 0;
+}
Igor
Jakub
Re: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On Wed, Nov 5, 2014 at 7:17 AM, Zamyatin, Igor igor.zamya...@intel.com wrote: Hi! Following patch (moving initialization of pic_offset_table_rtx earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 Bootstrapped/regtested on x86_64, i686 Is it ok for trunk? ChangeLog: 2014-10-30 Igor Zamyatin igor.zamya...@intel.com * function.c (assign_parms): Move init of pic_offset_table_rtx from here to... * cfgexpand.c (expand_used_vars): ...here. The patch is probably fine. However, it would be good to have the analysis why you want to move initialization of the PIC register earlier. Asan (and anybody else can) emits global variable(s) in expand_used_vars during function expanding while pic reg is currently initialized later, during expand_function_start in assign_parms thus to be late in asan case in PIC mode. So to avoid such cases we put pic reg initialization in the beginning of expand_used_vars. This seems to be early enough. Please mention PR in ChangeLog and add a few testcases so that the fix will be tested on Linux. -- H.J.
RE: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
Hi! Following patch (moving initialization of pic_offset_table_rtx earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 Bootstrapped/regtested on x86_64, i686 Is it ok for trunk? ChangeLog: 2014-10-30 Igor Zamyatin igor.zamya...@intel.com * function.c (assign_parms): Move init of pic_offset_table_rtx from here to... * cfgexpand.c (expand_used_vars): ...here. The patch is probably fine. However, it would be good to have the analysis why you want to move initialization of the PIC register earlier. Asan (and anybody else can) emits global variable(s) in expand_used_vars during function expanding while pic reg is currently initialized later, during expand_function_start in assign_parms thus to be late in asan case in PIC mode. So to avoid such cases we put pic reg initialization in the beginning of expand_used_vars. This seems to be early enough. Thanks, Igor
Re: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode
On 10/31/14 09:34, Zamyatin, Igor wrote: Hi! Following patch (moving initialization of pic_offset_table_rtx earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 Bootstrapped/regtested on x86_64, i686 Is it ok for trunk? ChangeLog: 2014-10-30 Igor Zamyatin igor.zamya...@intel.com * function.c (assign_parms): Move init of pic_offset_table_rtx from here to... * cfgexpand.c (expand_used_vars): ...here. The patch is probably fine. However, it would be good to have the analysis why you want to move initialization of the PIC register earlier. You should also reference the PR in the ChangeLog like this: PR target/63634 So get the analysis posted, and I'll likely approve after reviewing the background info. Thanks, Jeff
