Merge lp:~yao-codesourcery/gcc-linaro/4.4-fix-647597 into lp:gcc-linaro/4.4

[Yao Qi]

The cause of this bug is that DECL_SOURCE_LOCATION (decl) is not set properly, and this patch is to address this problem.

No regression on x86_64. arm cross compiler can compile WAbstractArea.ii without any errors. Regression test is not run on ARM.

[Michael Hope (michaelh1)]

Hi Yao. Could you ask someone in Linaro or CSL to review this please? Ulrich is a good candidate.

Why does this problem not appear in 4.5?

[Yao Qi (yao-codesourcery)]

> Hi Yao. Could you ask someone in Linaro or CSL to review this please? Ulrich
> is a good candidate.

I'll ping Ulrich or Julian to review it, when they get up.

>
> Why does this problem not appear in 4.5?

The problem is only on linaro 4.4. FSF 4.4 and 4.5 works well. Problem is introduced by backporting this patch http://gcc.gnu.org/viewcvs?view=revision&revision=157124 . Segmentation fault goes away once I revert this path. I am building FSF GCC r157124, to see if segmentation fault is there also.

Front-end is changed a lot between 4.4 and 4.5.

[Ulrich Weigand (uweigand)]

I'm wondering why you need the "if" here:

24 + if (DECL_SOURCE_LOCATION (decl) == UNKNOWN_LOCATION)
25 + DECL_SOURCE_LOCATION (decl) = loc;

Given that immediately before the call to (*make_fname_decl), input_location is unconditionally set to UNKNOWN_LOCATION, DECL_SOURCE_LOCATION (decl) should *always* be UNKNOWN_LOCATION here ...

Do you have any case where this DECL_SOURCE_LOCATION is something different here? If not, it seems to me it would be less confusing to leave the "if" out and unconditionally set the location.

Note that in current mainline, make_fname_decl is changed to take a location_t argument, which is then used to set the decl's location. This was done as part of the big diagnostics branch merge:
http://gcc.gnu.org/ml/gcc-cvs/2009-06/msg00422.html
So it seems your patch is doing the right thing.

[Yao Qi (yao-codesourcery)]

> I'm wondering why you need the "if" here:
>
> 24 + if (DECL_SOURCE_LOCATION (decl) == UNKNOWN_LOCATION)
> 25 + DECL_SOURCE_LOCATION (decl) = loc;
>
> Given that immediately before the call to (*make_fname_decl), input_location
> is unconditionally set to UNKNOWN_LOCATION, DECL_SOURCE_LOCATION (decl) should
> *always* be UNKNOWN_LOCATION here ...
>
> Do you have any case where this DECL_SOURCE_LOCATION is something different
> here? If not, it seems to me it would be less confusing to leave the "if" out
> and unconditionally set the location.
>

No, I don't have such case. The reason I put "if" here is that I am little worried
that input_location is changed somewhere else, after it is set to UNKNOWN_LOCATION.
Because global variable input_location is modified in many places in current c/cpp
front end.

I've removed that "if" statement. Regression tested again on x86_64. Rebuild ARM cross compiler, no segmentation fault. I'll check in.

> Note that in current mainline, make_fname_decl is changed to take a location_t
> argument, which is then used to set the decl's location. This was done as
> part of the big diagnostics branch merge:
> http://gcc.gnu.org/ml/gcc-cvs/2009-06/msg00422.html
> So it seems your patch is doing the right thing.

I planed to backport this big patch before, but gave up after some hours, since it is not
as easy as I firstly imagined.

[Yao Qi (yao-codesourcery)]

Ulrich, please approve this merge request, then, I can check it in to linaro 4.4 tree.

[Ulrich Weigand (uweigand)]

This is OK, thanks.

review: Approve

=== modified file 'ChangeLog.linaro'
--- ChangeLog.linaro	2010-09-25 04:58:28 +0000
+++ ChangeLog.linaro	2010-10-01 13:29:26 +0000
@@ -1,3 +1,9 @@
[phone]  Yao Qi  <[e-mail]>
+
+	Fix LP:#647597
+	gcc/
+	* c-common.c (fname_decl): Update decl's source location.
+
 2010-09-24  Yao Qi  <[e-mail]>
 
 	Backport from FSF to fix ICE found in LP:635409:

=== modified file 'gcc/c-common.c'
--- gcc/c-common.c	2010-09-07 14:49:43 +0000
+++ gcc/c-common.c	2010-10-01 13:29:26 +0000
@@ -1025,6 +1025,8 @@
       if (!IS_EMPTY_STMT (stmts))
 	saved_function_name_decls
 	  = tree_cons (decl, stmts, saved_function_name_decls);
+      DECL_SOURCE_LOCATION (decl) = loc;
+
       *fname_vars[ix].decl = decl;
       input_location = saved_location;
     }