Patchwork [buildrobot] Re: [PATCH] Split -fisolate-erroneous-paths into two options

login
register
mail settings
Submitter Ian Taylor
Date Dec. 5, 2013, 4:41 p.m.
Message ID <CAKOQZ8zzNhh62Zk4BOL--vrDsaC2ODuQErxH5w7k=bq0Hki+Rg@mail.gmail.com>
Download mbox | patch
Permalink /patch/297198/
State New
Headers show

Comments

Ian Taylor - Dec. 5, 2013, 4:41 p.m.
On Thu, Dec 5, 2013 at 12:18 AM, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> On Wed, 2013-12-04 20:19:29 -0700, Jeff Law <law@redhat.com> wrote:
>> This patch splits up the erroneous path optimization into two
>> pieces. One which detects NULL dereferences and isolates those paths
>> and a second which detects passing/returning a NULL pointer in cases
>> where an attribute says a non-NULL value is required.
> [...]
>
> This seems to break Go, see eg.
> http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 :
>
> g++ -c  -DDEFAULT_TARGET_VERSION=\"4.9.0\" -DDEFAULT_TARGET_MACHINE=\"i686-pc-linux-gnu\" -DIN_GCC_FRONTEND -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c
> ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’:
> ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’
>    if (!global_options_set.x_flag_isolate_erroneous_paths)
>                            ^
> ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’
>      global_options.x_flag_isolate_erroneous_paths = 0;
>                     ^
> make[2]: *** [go/go-lang.o] Error 1


I've committed this patch to mainline to fix this problem.  I could
have removed this code earlier but hadn't gotten around to it.

Ian


2013-12-05  Ian Lance Taylor  <iant@google.com>

	Revert this change; no longer required.
	2013-11-06  Ian Lance Taylor  <iant@google.com>

	* go-lang.c (go_langhook_post_options): If
	-fisolate-erroneous-paths was turned on by an optimization option,
	turn it off.
Jeff Law - Dec. 5, 2013, 5:03 p.m.
On 12/05/13 09:41, Ian Lance Taylor wrote:
> On Thu, Dec 5, 2013 at 12:18 AM, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
>> On Wed, 2013-12-04 20:19:29 -0700, Jeff Law <law@redhat.com> wrote:
>>> This patch splits up the erroneous path optimization into two
>>> pieces. One which detects NULL dereferences and isolates those paths
>>> and a second which detects passing/returning a NULL pointer in cases
>>> where an attribute says a non-NULL value is required.
>> [...]
>>
>> This seems to break Go, see eg.
>> http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 :
>>
>> g++ -c  -DDEFAULT_TARGET_VERSION=\"4.9.0\" -DDEFAULT_TARGET_MACHINE=\"i686-pc-linux-gnu\" -DIN_GCC_FRONTEND -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c
>> ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’:
>> ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’
>>     if (!global_options_set.x_flag_isolate_erroneous_paths)
>>                             ^
>> ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’
>>       global_options.x_flag_isolate_erroneous_paths = 0;
>>                      ^
>> make[2]: *** [go/go-lang.o] Error 1
>
>
> I've committed this patch to mainline to fix this problem.  I could
> have removed this code earlier but hadn't gotten around to it.
Sorry, I should have remembered that GO was going to need updating.

Q. Now that we're in stage3, presumably GO imports onto the trunk are 
stopping?  If so would it make sense to enable GO by default?


jeff
Ian Taylor - Dec. 5, 2013, 7:04 p.m.
On Thu, Dec 5, 2013 at 9:03 AM, Jeff Law <law@redhat.com> wrote:
>
> Q. Now that we're in stage3, presumably GO imports onto the trunk are
> stopping?  If so would it make sense to enable GO by default?

The Go library will no longer be updated except for bug fixes.  If
there is a Go 1.2.1 release, and if it is released before GCC 4.9.0,
then I plan to bring those library changes in before the GCC release.
However, those changes, if any, would be small.  Go has a fairly
strict policy on x.y.z patches: only those that address critical bugs
with no workaround.

However, I do still plan to update the files in gcc/testsuite/go.test
to the 1.2 set of tests, and that may require some follow-on patches
to the Go frontend.  This is to pursue the goal of having solid Go 1.2
support in the GCC 4.9.0 release.  So while enabling Go by default is
an interesting idea I think it is premature.

Ian

Patch

Index: gcc/go/go-lang.c
===================================================================
--- gcc/go/go-lang.c	(revision 205710)
+++ gcc/go/go-lang.c	(working copy)
@@ -270,12 +270,6 @@  go_langhook_post_options (const char **p
   if (flag_excess_precision_cmdline == EXCESS_PRECISION_DEFAULT)
     flag_excess_precision_cmdline = EXCESS_PRECISION_STANDARD;
 
-  /* The isolate_erroneous_paths optimization can change a nil
-     dereference from a panic to a trap, so we have to disable it for
-     Go, even though it is normally enabled by -O2.  */
-  if (!global_options_set.x_flag_isolate_erroneous_paths)
-    global_options.x_flag_isolate_erroneous_paths = 0;
-
   /* Returning false means that the backend should be used.  */
   return false;
 }