[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] Fix building error



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@xxxxxxxxxxxxxx>. 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@xxxxxxxxxxxxxx> (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.

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.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.