gcc: honour -ffile-prefix-map in ASM_MAP [PR93371]

  -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
and -fdebug-prefix-map. However, when building .S or .s files, gas is
not called with the appropriate --debug-prefix-map option when
-ffile-prefix-map is used.

While the user can specify -fdebug-prefix-map when building assembly
files via gcc, it's more ergonomic to also support -ffile-prefix-map;
especially since for .S files that could contain the __FILE__ macro,
one would then also have to specify -fmacro-prefix-map.

gcc:
	PR driver/93371
	* gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
---

I've tested that this works as expected, both by looking at how gas is
now invoked, and by running 'strings' on the generated .o file. But I
don't know how to add something to the testsuite for this.

I stumbled on this since it came up on the U-Boot mailing list:
https://lore.kernel.org/u-boot/4ed9f811-5244-54ef-b58e-83dba51510e4@prevas.dk/
.

 gcc/gcc.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On 29/08/2022 11.29, Rasmus Villemoes wrote:
> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
> and -fdebug-prefix-map. However, when building .S or .s files, gas is
> not called with the appropriate --debug-prefix-map option when
> -ffile-prefix-map is used.
> 
> While the user can specify -fdebug-prefix-map when building assembly
> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
> especially since for .S files that could contain the __FILE__ macro,
> one would then also have to specify -fmacro-prefix-map.
> 
> gcc:
> 	PR driver/93371
> 	* gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
> ---
> 
> I've tested that this works as expected, both by looking at how gas is
> now invoked, and by running 'strings' on the generated .o file. But I
> don't know how to add something to the testsuite for this.

Is this ok for trunk? If so, how about older maintained branches?

And does anyone have ideas for how I could add a test case?


> 
> I stumbled on this since it came up on the U-Boot mailing list:
> https://lore.kernel.org/u-boot/4ed9f811-5244-54ef-b58e-83dba51510e4@prevas.dk/
> .
> 
>  gcc/gcc.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index b6d562a92f0..44eafc60187 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -878,7 +878,7 @@ proper position among the other output files.  */
>  #endif
>  
>  #ifdef HAVE_AS_DEBUG_PREFIX_MAP
> -#define ASM_MAP " %{fdebug-prefix-map=*:--debug-prefix-map %*}"
> +#define ASM_MAP " %{ffile-prefix-map=*:--debug-prefix-map %*} %{fdebug-prefix-map=*:--debug-prefix-map %*}"
>  #else
>  #define ASM_MAP ""
>  #endif
  

On 12/09/2022 11.46, Rasmus Villemoes wrote:
> On 29/08/2022 11.29, Rasmus Villemoes wrote:
>> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
>> and -fdebug-prefix-map. However, when building .S or .s files, gas is
>> not called with the appropriate --debug-prefix-map option when
>> -ffile-prefix-map is used.
>>
>> While the user can specify -fdebug-prefix-map when building assembly
>> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
>> especially since for .S files that could contain the __FILE__ macro,
>> one would then also have to specify -fmacro-prefix-map.
>>
>> gcc:
>> 	PR driver/93371
>> 	* gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
>> ---
>>
>> I've tested that this works as expected, both by looking at how gas is
>> now invoked, and by running 'strings' on the generated .o file. But I
>> don't know how to add something to the testsuite for this.
> 
> Is this ok for trunk? If so, how about older maintained branches?
> 
> And does anyone have ideas for how I could add a test case?

ping.

> 
>>
>> I stumbled on this since it came up on the U-Boot mailing list:
>> https://lore.kernel.org/u-boot/4ed9f811-5244-54ef-b58e-83dba51510e4@prevas.dk/
>> .
>>
>>  gcc/gcc.cc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
>> index b6d562a92f0..44eafc60187 100644
>> --- a/gcc/gcc.cc
>> +++ b/gcc/gcc.cc
>> @@ -878,7 +878,7 @@ proper position among the other output files.  */
>>  #endif
>>  
>>  #ifdef HAVE_AS_DEBUG_PREFIX_MAP
>> -#define ASM_MAP " %{fdebug-prefix-map=*:--debug-prefix-map %*}"
>> +#define ASM_MAP " %{ffile-prefix-map=*:--debug-prefix-map %*} %{fdebug-prefix-map=*:--debug-prefix-map %*}"
>>  #else
>>  #define ASM_MAP ""
>>  #endif
>
  

On 27/09/2022 08.54, Rasmus Villemoes wrote:
> On 12/09/2022 11.46, Rasmus Villemoes wrote:
>> On 29/08/2022 11.29, Rasmus Villemoes wrote:
>>> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
>>> and -fdebug-prefix-map. However, when building .S or .s files, gas is
>>> not called with the appropriate --debug-prefix-map option when
>>> -ffile-prefix-map is used.
>>>
>>> While the user can specify -fdebug-prefix-map when building assembly
>>> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
>>> especially since for .S files that could contain the __FILE__ macro,
>>> one would then also have to specify -fmacro-prefix-map.
>>>
>>> gcc:
>>> 	PR driver/93371
>>> 	* gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
>>> ---
>>>
>>> I've tested that this works as expected, both by looking at how gas is
>>> now invoked, and by running 'strings' on the generated .o file. But I
>>> don't know how to add something to the testsuite for this.
>>
>> Is this ok for trunk? If so, how about older maintained branches?
>>
>> And does anyone have ideas for how I could add a test case?
> 
> ping.
> 

ping^2
  

On 8/29/22 03:29, Rasmus Villemoes wrote:
> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
> and -fdebug-prefix-map. However, when building .S or .s files, gas is
> not called with the appropriate --debug-prefix-map option when
> -ffile-prefix-map is used.
>
> While the user can specify -fdebug-prefix-map when building assembly
> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
> especially since for .S files that could contain the __FILE__ macro,
> one would then also have to specify -fmacro-prefix-map.
>
> gcc:
> 	PR driver/93371
> 	* gcc.cc (ASM_MAP): Honour -ffile-prefix-map.

OK.  Sorry for the long delay.

jeff
  

On 01/11/2022 21.11, Jeff Law wrote:
> 
> On 8/29/22 03:29, Rasmus Villemoes wrote:
>> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
>> and -fdebug-prefix-map. However, when building .S or .s files, gas is
>> not called with the appropriate --debug-prefix-map option when
>> -ffile-prefix-map is used.
>>
>> While the user can specify -fdebug-prefix-map when building assembly
>> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
>> especially since for .S files that could contain the __FILE__ macro,
>> one would then also have to specify -fmacro-prefix-map.
>>
>> gcc:
>>     PR driver/93371
>>     * gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
> 
> OK.  Sorry for the long delay.

Thanks, and no problem.

However, when I try to push the new master branch I get

$ git push origin master
fatal: remote error: service not enabled: /git/gcc.git

I do gcc patches sufficiently rare that I may have forgotten the right
procedure, but this is what I think I've done previously (along with
running a "git gcc-verify HEAD" to ensure there's a proper changelog
fragment to extract, with gcc-verify being a suitable alias).

Have I simply lost by commit bit?

Rasmus
  

On 11/2/22 06:35, Rasmus Villemoes wrote:
> On 01/11/2022 21.11, Jeff Law wrote:
>> On 8/29/22 03:29, Rasmus Villemoes wrote:
>>> -ffile-prefix-map is supposed to be a superset of -fmacro-prefix-map
>>> and -fdebug-prefix-map. However, when building .S or .s files, gas is
>>> not called with the appropriate --debug-prefix-map option when
>>> -ffile-prefix-map is used.
>>>
>>> While the user can specify -fdebug-prefix-map when building assembly
>>> files via gcc, it's more ergonomic to also support -ffile-prefix-map;
>>> especially since for .S files that could contain the __FILE__ macro,
>>> one would then also have to specify -fmacro-prefix-map.
>>>
>>> gcc:
>>>      PR driver/93371
>>>      * gcc.cc (ASM_MAP): Honour -ffile-prefix-map.
>> OK.  Sorry for the long delay.
> Thanks, and no problem.
>
> However, when I try to push the new master branch I get
>
> $ git push origin master
> fatal: remote error: service not enabled: /git/gcc.git
>
> I do gcc patches sufficiently rare that I may have forgotten the right
> procedure, but this is what I think I've done previously (along with
> running a "git gcc-verify HEAD" to ensure there's a proper changelog
> fragment to extract, with gcc-verify being a suitable alias).
>
> Have I simply lost by commit bit?

No idea what that error means.  If I had to guess, it'd be that you've 
got an anonymous checkout tree which is obviously unsuitable for pushing 
or something of that nature.

It's probably just faster/easier for me to push it for you.  I'll take 
care of it momentarily.

Jeff
  

On 02/11/2022 16.45, Jeff Law wrote:
> 
> On 11/2/22 06:35, Rasmus Villemoes wrote:

>> However, when I try to push the new master branch I get
>>
>> $ git push origin master
>> fatal: remote error: service not enabled: /git/gcc.git
>>
>> I do gcc patches sufficiently rare that I may have forgotten the right
>> procedure, but this is what I think I've done previously (along with
>> running a "git gcc-verify HEAD" to ensure there's a proper changelog
>> fragment to extract, with gcc-verify being a suitable alias).
>>
>> Have I simply lost by commit bit?
> 
> No idea what that error means.  If I had to guess, it'd be that you've
> got an anonymous checkout tree which is obviously unsuitable for pushing
> or something of that nature.
> 
> It's probably just faster/easier for me to push it for you.  I'll take
> care of it momentarily.

Thanks.

I think I found out what was wrong (though I know it has worked for me
previously): My remote url was git://gcc.gnu.org/git/gcc.git , and I
used to rely on my .ssh/config specifying "villemoes" as username when
accessing gcc.gnu.org. For some reason that no longer worked, but
updating the remote url to git+ssh://villemoes@gcc.gnu.org/git/gcc.git
seemed to do the trick.

What do you think about applying this to older branches? IMO it's a
defect from when the umbrella -ffile-prefix-map was introduced, and the
potential for regressions should be very low, but I can also see how it
might not really qualify as a bug fix.

Rasmus
  

On 11/3/22 07:29, Rasmus Villemoes wrote:
> On 02/11/2022 16.45, Jeff Law wrote:
>> On 11/2/22 06:35, Rasmus Villemoes wrote:
>>> However, when I try to push the new master branch I get
>>>
>>> $ git push origin master
>>> fatal: remote error: service not enabled: /git/gcc.git
>>>
>>> I do gcc patches sufficiently rare that I may have forgotten the right
>>> procedure, but this is what I think I've done previously (along with
>>> running a "git gcc-verify HEAD" to ensure there's a proper changelog
>>> fragment to extract, with gcc-verify being a suitable alias).
>>>
>>> Have I simply lost by commit bit?
>> No idea what that error means.  If I had to guess, it'd be that you've
>> got an anonymous checkout tree which is obviously unsuitable for pushing
>> or something of that nature.
>>
>> It's probably just faster/easier for me to push it for you.  I'll take
>> care of it momentarily.
> Thanks.
>
> I think I found out what was wrong (though I know it has worked for me
> previously): My remote url was git://gcc.gnu.org/git/gcc.git , and I
> used to rely on my .ssh/config specifying "villemoes" as username when
> accessing gcc.gnu.org. For some reason that no longer worked, but
> updating the remote url to git+ssh://villemoes@gcc.gnu.org/git/gcc.git
> seemed to do the trick.

Good to know you got it sorted out.


>
> What do you think about applying this to older branches? IMO it's a
> defect from when the umbrella -ffile-prefix-map was introduced, and the
> potential for regressions should be very low, but I can also see how it
> might not really qualify as a bug fix.

I'd probably lean against backporting.  Generally we try to limit 
backporting to regression fixes, incorrect code generation issues and 
the like.  This seems much less serious.


Jeff