Re: [PATCH v2] Fix building error

[Wen Congyang <wency@cn.fujitsu.com>]

On 01/20/2015 06:18 PM, Ian Campbell wrote:
> On Tue, 2015-01-20 at 10:21 +0800, Wen Congyang wrote:
>> On 01/19/2015 11:23 PM, Ian Campbell wrote:
>>> On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
>>>> Wen Congyang writes ("[PATCH v2] Fix building error"):
>>>>>  ifeq ($(debug),y)
>>>>>  # Disable optimizations and enable debugging information for macros
>>>>>  CFLAGS += -O0 -g3
>>>>> +# _FORTIFY_SOURCE requires compiling with optimization
>>>>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
>>>>
>>>> I'm not entirely convinced about this.  I have two kinds of concern:
>>>>
>>>> One is practical, which is that ATM AIUI a debug build, while not
>>>> intended for production use, is not any less secure.  (Leaving aside
>>>> the ability of guests to perform a DoS with copious debugging output.)
>>>>
>>>> The other is that we seem to be entering into a battle of escalation
>>>> between various Makefiles.  Specifically, this seems to be a
>>>> workaround for a bug in some other Makefiles we are using: the bug
>>>> being that these other Makefiles can't cope with -O0.  And
>>>> unconditionally squashing the other Makefiles' options seems like a
>>>> big hammer.
>>>>
>>>> The fact that Fortify doesn't support -O0 is a property of Fortify
>>>> which ought to be encoded in Fortify (or the places where it is
>>>> enabled).
>>>>
>>>> Assuming that the underlying bug is intractible
>>>
>>> I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
>>> enter into a dialogue with Fedora about the practice of enabling these
>>> flags for all Python modules built on a Fedora system vs. just those
>>> built via RPM etc.
>>>
>>>>  I think the right
>>>> answer is for an affected developer
>>>
>>> Which will be all developers using Fedora AFAICT.
>>>
>>>>  to do one of the following, as a
>>>> workaround: either, manually override Fortify when requesting a debug
>>>> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
>>>> -O0 setting.
>>>>
>>>> To make this easier to do without editing tools/Rules.mk I would
>>>> support a patch to Rules.mk which has it honour a variable containing
>>>> a debug-specific set of CFLAGS.
>>
>> I don't understand your suggestion.
> 
> He means for the build system to honour a new
> EXTRA_FLAGS_XEN_TOOLS_DEBUG (or similar) iff debug=y.
> 
>>> This seems reasonable enough to me.
>>>
>>> The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
>>> applied the workaround only to the Python parts of the build
>>> (tools/{python,pygrub}) whereas this v2 and your suggestion would affect
>>> all of tools/*. That seems like a reasonable compromise under the
>>> circumstances (the alternative being special overrides for Python or
>>> something, no nice).
>>
>> Building problems only affect developers, so I think we should fix this problem.
>>
>> If the developers add extra flags to define the macro _FORTIFY_SOURCE, it also
>> breaks building.
> 
> So can adding all manner of random macros to the build. If a developer
> adds such flags then they should be prepared to deal with the
> consequences, including dealing with any breakage which results.

Agree with it.

> 
> The only reason we aren't taking this hard line with the situation you
> find yourself in is that a major distro has seen fit to define these
> extra macros into the default Python build system on that distro, hence
> we are trying to find a reasonable compromise for users/developers of
> that distro without making things worse for users/developers of other
> distros which don't do this.
> 
>>  We can disable debug for such case, or undefine the macro
>> in Rules.mk. If the macro _FORTIFY_SOURCE is not defined, undefining it is harmless.
> 
> On the other hand a user who has a fixed version of fortify which is
> compatible with -O0 then cannot enable it if we do this.

Yes. We can detect it in the configure. And only undefine it when fortify cannot work with -O0

Thanks
Wen Congyang

> 
> Ian.
> 
> .
>