Makeconfig (ASFLAGS): Always append required assembler flags

Message ID 20180704142954.4F54743994575@oldenburg.str.redhat.com
State New
Headers show
Series
  • Makeconfig (ASFLAGS): Always append required assembler flags
Related show

Commit Message

Florian Weimer July 4, 2018, 2:29 p.m.
Otherwise, it is impossible to set ASFLAGS differently from CFLAGS
without also overriding essential flags such as -Wa,--noexecstack.

2018-07-04  Florian Weimer  <fweimer@redhat.com>

	* Makeconfig (ASFLAGS): Always append required assembler flags.

Comments

Florian Weimer Aug. 14, 2018, 3:57 p.m. | #1
On 07/04/2018 04:29 PM, Florian Weimer wrote:
> Otherwise, it is impossible to set ASFLAGS differently from CFLAGS
> without also overriding essential flags such as -Wa,--noexecstack.
> 
> 2018-07-04  Florian Weimer  <fweimer@redhat.com>
> 
> 	* Makeconfig (ASFLAGS): Always append required assembler flags.
> 
> diff --git a/Makeconfig b/Makeconfig
> index 608ffe648c..099f184088 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -1047,7 +1047,7 @@ endif
>   ifndef ASFLAGS
>   ASFLAGS := $(filter -g% -fdebug-prefix-map=%,$(CFLAGS))
>   endif
> -ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
> +override ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
>   
>   ifndef BUILD_CC
>   BUILD_CC = $(CC)

Ping?

Thanks,
Florian
Gabriel F. T. Gomes Aug. 16, 2018, 1 p.m. | #2
On Tue, 14 Aug 2018, Florian Weimer wrote:

>On 07/04/2018 04:29 PM, Florian Weimer wrote:
>> Otherwise, it is impossible to set ASFLAGS differently from CFLAGS
>> without also overriding essential flags such as -Wa,--noexecstack.

I understood that this patch will let us set ASFLAGS from the
command-line.  You mentioned CFLAGS because that's how we could indirectly
set ASFLAGS, right?

If so, looks good to me.

>> --- a/Makeconfig
>> +++ b/Makeconfig
>> @@ -1047,7 +1047,7 @@ endif
>>   ifndef ASFLAGS
>>   ASFLAGS := $(filter -g% -fdebug-prefix-map=%,$(CFLAGS))
>>   endif
>> -ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
>> +override ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
>>   
>>   ifndef BUILD_CC
>>   BUILD_CC = $(CC)  
>
>Ping?
Florian Weimer Aug. 16, 2018, 1:32 p.m. | #3
On 08/16/2018 03:00 PM, Gabriel F. T. Gomes wrote:
> On Tue, 14 Aug 2018, Florian Weimer wrote:
> 
>> On 07/04/2018 04:29 PM, Florian Weimer wrote:
>>> Otherwise, it is impossible to set ASFLAGS differently from CFLAGS
>>> without also overriding essential flags such as -Wa,--noexecstack.
> 
> I understood that this patch will let us set ASFLAGS from the
> command-line.  You mentioned CFLAGS because that's how we could indirectly
> set ASFLAGS, right?

Correct, but it also affects compilation of C sources, so it is not 
always desirable to use this.

> If so, looks good to me.

Thanks for the review!

Florian
Carlos O'Donell Aug. 16, 2018, 3:53 p.m. | #4
On 08/14/2018 11:57 AM, Florian Weimer wrote:
> On 07/04/2018 04:29 PM, Florian Weimer wrote:
>> Otherwise, it is impossible to set ASFLAGS differently from CFLAGS
>> without also overriding essential flags such as -Wa,--noexecstack.
>>
>> 2018-07-04  Florian Weimer  <fweimer@redhat.com>
>>
>>     * Makeconfig (ASFLAGS): Always append required assembler flags.
>>
>> diff --git a/Makeconfig b/Makeconfig
>> index 608ffe648c..099f184088 100644
>> --- a/Makeconfig
>> +++ b/Makeconfig
>> @@ -1047,7 +1047,7 @@ endif
>>   ifndef ASFLAGS
>>   ASFLAGS := $(filter -g% -fdebug-prefix-map=%,$(CFLAGS))
>>   endif
>> -ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
>> +override ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)

This looks correct to me.

>>     ifndef BUILD_CC
>>   BUILD_CC = $(CC)

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Patch

diff --git a/Makeconfig b/Makeconfig
index 608ffe648c..099f184088 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -1047,7 +1047,7 @@  endif
 ifndef ASFLAGS
 ASFLAGS := $(filter -g% -fdebug-prefix-map=%,$(CFLAGS))
 endif
-ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
+override ASFLAGS += -Werror=undef $(ASFLAGS-config) $(asflags-cpu)
 
 ifndef BUILD_CC
 BUILD_CC = $(CC)