[Bioc-devel] R CMD check without WARNING: g++ and STL issue for xcms and mzR

Neumann, Steffen sneumann at ipb-halle.de
Wed Apr 19 20:52:50 CEST 2017


Hi,

just a quick report, I followed this up to r-devel,
and a fix adding -O2 back to default CXXFLAGS 
has been committed (r72549, see [1]) to R by Martyn Plummer, 
thus "fixing" the abort() related WARNING in mzR/xcms 
once this gets into the R version used on malbec2.

Yours,
Steffen

[1] http://developer.r-project.org/R_svnlog_2013

On Mi, 2017-04-12 at 22:07 -0400, Martin Morgan wrote:
> On 04/12/2017 11:31 AM, Neumann, Steffen wrote:
> > 
> > Hi,
> > 
> > certainly these are good suggestions, but I would tend
> > to simply add some whitelisting functionality if the
> > cause is beyond the package's control.
> > 
> > In this case I doubt it is a handler for SIGABRT,
> > since that would not go away with -O2, or would it ?
> > 
> > For now, just adding -O2 on Linux would fix this issue.
> The code is compiled on the user machine, so twiddling with the
> build 
> system masks the problem from the developer while leaving the user 
> vulnerable.
> 
> Calling abort() is certainly a serious problem, abruptly ending the
> user 
> session.
> 
> Optimization could either eliminate a dead code path, or it could 
> compile the call to abort in a way that R no longer recognizes it --
>> is doing the equivalent of nm *.o |grep abort.
> 
> It would be appropriate to ignore the warning about abort() if it
> were 
> never reached; one would only know this by careful code analysis
> rather 
> than adjusting optimization flags.
> 
> Rsamtools re-defines abort (before including offending headers), and 
> avoids the warning (and crash), with the equivalent of
> 
>    PKG_CFLAGS=-D__builtin_abort=_samtools_abort
> 
> where _samtools_abort is my own implementation to signal an R error 
> telling the user that an unrecoverable error has occurred and they 
> should stop what they are doing. Unfortunately this requires 
> -U_FORTIFY_SOURCE and doesn't actually address the reason for the
> abort, 
> and is hardly ideal.
> 
> I don't think the developer can set the optimization flag in a way
> that 
> overrides the R or user setting (the PKG_CXXFLAGS is inserted before
> the 
> R flags). And it doesn't address the underlying problem anyway.
> 
> Kasper's pragmatic approach (it's a very unlikely situation, and 
> difficult to fix, so live with the warning) seems like it would often
> be 
> a reasonable one.
> 
> FWIW this little bit of C++ seems to be enough to include abort
> 
> tmp.cpp:
> -------
> #include <list>
> 
> int foo(int argc, const char *argv[]) {
>      std::list<int> l1, l2;
>      std::list<int>::iterator it;
> 
>      it = l1.begin();
>      l1.splice (it, l2); // mylist1: 1 10 20 30 2 3 4
> 
>      return 0;
> }
> -------
> 
> Test with
> 
>    rm -f tmp.o && R CMD SHLIB tmp.cpp && nm tmp.o | grep abort
> 
> with compiler settings in
> 
> ~/.R/Makevars
> -------------
> CXXFLAGS = -g -O0
> -------------
> 
> 
> Martin
> 
> > 
> > 
> > Yours,
> > Steffen
> > 
> > 
> > On Mi, 2017-04-12 at 10:07 -0400, Vincent Carey wrote:
> > > 
> > > 
> > > Suppose you had a handler for SIGABRT in your code.  Could CMD
> > > check
> > > check for that and, if found, refrain from warning?  That is
> > > somewhat
> > > involved and goes beyond Bioc but it seems a principled way of
> > > dealing with operations in binary infrastructure whose behavior
> > > we
> > > cannot control.  The problem will surely arise in other settings.
> > > Could BiocCheck prototype the approach?
> > > 
> > > On Wed, Apr 12, 2017 at 9:12 AM, Kasper Daniel Hansen
> > > <kasperdanielha
> > > nsen at gmail.com> wrote:
> > > > 
> > > > 
> > > > I think "we" have to appreciate that the warning about
> > > > abort/etc
> > > > and others
> > > > is really hard to deal with when you're including (large)
> > > > external
> > > > source
> > > > as you do in mzR and for example affxparser /
> > > > Rgraphviz.  Generally
> > > > fixing
> > > > those in external code requires a lot of effort, and the
> > > > further
> > > > effort to
> > > > document that the included sources in the package are different
> > > > from the
> > > > official sources.  I have had warnings about this in affxparser
> > > > for
> > > > a long,
> > > > long time and no-one has bothered me over it.
> > > > 
> > > > What might be nice is (somehow) setting a flag saying this
> > > > should
> > > > be
> > > > ignored in the check process, but of course we don't want that
> > > > flag
> > > > to be
> > > > set by the developer, because usually, these issues should be
> > > > dealt
> > > > with.
> > > > So perhaps there is nothing to do and I always have to click on
> > > > each build
> > > > report to verify that there is only 1 warning from this issues
> > > > and
> > > > not
> > > > others.
> > > > 
> > > > Best,
> > > > Kasper
> > > > 
> > > > On Wed, Apr 12, 2017 at 7:22 AM, Neumann, Steffen <sneumann at ipb
> > > > -hal
> > > > le.de>
> > > > wrote:
> > > > 
> > > > > 
> > > > > 
> > > > > Hi Martin and malbec2 admin(s):
> > > > > 
> > > > > Some more digging revealed that malbec2
> > > > > http://bioconductor.org/checkResults/devel/bioc-
> > > > > LATEST/malbec2-NodeInfo.html
> > > > > is using "CFLAGS=-g -O2 -Wall" but only "CXXFLAGS=-Wall"
> > > > > while e.g. tokay2 uses "CXXFLAGS=-O2 -Wall -mtune=core2"
> > > > > 
> > > > > The -O2 optimisation is getting rid of the abort() symbol,
> > > > > as shown in https://github.com/sneumann/xcms/issues/150#
> > > > > issuecomment-293545521
> > > > > 
> > > > > => Is there a way to get -O2 back on the BioC build machines
> > > > > ?
> > > > >     This should fix our WARNING. Bonus: will fix the same
> > > > > issue
> > > > > in
> > > > mzR :-)
> > > > > 
> > > > > 
> > > > > 
> > > > > Yours,
> > > > > Steffen
> > > > > 
> > > > > On So, 2017-04-02 at 10:01 -0400, Martin Morgan wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 04/02/2017 06:52 AM, Neumann, Steffen wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > [...]
> > > > > > > in preparation for the release, we are hunting down
> > > > > > > WARNINGS
> > > > > > > "Found ‘abort’, possibly from ‘abort’ (C)" in xcms/mzR.
> > > > > > > The abort() call is not coming from XCMS, but rather
> > > > > > > from the C++ code in the STL, and we have no idea
> > > > > [...]
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > We are tracking this in:
> > > > > > > https://github.com/sneumann/xcms/issues/150
> > > > > [...]
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > I don't think Bioconductor can help with this; maybe the
> > > > > > Rcpp
> > > > or R-
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > devel mailing lists?
> > > > > --
> > > > > 3rd Leibniz Plant Biochemistry Symposium, 22.-23. of June
> > > > > 2017
> > > > > http://www.ipb-halle.de/en/public-relations/3rd-leibniz-plant
> > > > > -bio
> > > > chemis
> > > > > 
> > > > > 
> > > > > try-symposium/
> > > > > --
> > > > > IPB Halle                    AG Massenspektrometrie &
> > > > > Bioinformatik
> > > > > Dr. Steffen Neumann          http://www.IPB-Halle.DE
> > > > > Weinberg 3                   Tel. +49 (0) 345 5582 - 1470
> > > > > 06120 Halle                       +49 (0) 345 5582 - 0
> > > > > sneumann(at)IPB-Halle.DE     Fax. +49 (0) 345 5582 - 1409
> > > > > 
> > > > > --
> > > > > 3rd Leibniz Plant Biochemistry Symposium, 22.-23. of June
> > > > > 2017
> > > > > http://www.ipb-halle.de/en/public-relations/3rd-leibniz-plant
> > > > > -bio
> > > > chemis
> > > > > 
> > > > > 
> > > > > try-symposium/
> > > > > --
> > > > > IPB Halle                    AG Massenspektrometrie &
> > > > > Bioinformatik
> > > > > Dr. Steffen Neumann          http://www.IPB-Halle.DE
> > > > > Weinberg 3                   Tel. +49 (0) 345 5582 - 1470
> > > > > 06120 Halle                       +49 (0) 345 5582 - 0
> > > > > sneumann(at)IPB-Halle.DE     Fax. +49 (0) 345 5582 - 1409
> > > > > 
> > > > > _______________________________________________
> > > > > Bioc-devel at r-project.org mailing list
> > > > > https://stat.ethz.ch/mailman/listinfo/bioc-devel
> > > > > 
> > > >         [[alternative HTML version deleted]]
> > > > 
> > > > _______________________________________________
> > > > Bioc-devel at r-project.org mailing list
> > > > https://stat.ethz.ch/mailman/listinfo/bioc-devel
> > > > 
> 
> This email message may contain legally privileged and/or confidential
> information.  If you are not the intended recipient(s), or the
> employee or agent responsible for the delivery of this message to the
> intended recipient(s), you are hereby notified that any disclosure,
> copying, distribution, or use of this email message is
> prohibited.  If you have received this message in error, please
> notify the sender immediately by e-mail and delete this email message
> from your computer. Thank you.
-- 
3rd Leibniz Plant Biochemistry Symposium, 22.-23. of June 2017
http://www.ipb-halle.de/en/public-relations/3rd-leibniz-plant-biochemis
try-symposium/
--
IPB Halle                    AG Massenspektrometrie &
Bioinformatik
Dr. Steffen Neumann          http://www.IPB-Halle.DE
Weinberg 3                   Tel. +49 (0) 345 5582 - 1470
06120 Halle                       +49 (0) 345 5582 - 0           
sneumann(at)IPB-Halle.DE     Fax. +49 (0) 345 5582 - 1409



More information about the Bioc-devel mailing list