CppunitTest_sw_ooxmlsdrexport: division by zero in chart2/source/controller/chartapiwrapper/LegendWrapper.cxx

[Stephan Bergmann]

On 01/08/2015 09:45 AM, Stephan Bergmann wrote:
> Running CppunitTest_sw_ooxmlsdrexport with
> -fsanitize=float-divide-by-zero fails with
[...]

ensuing IRC conversation:

> <sberg> vmiklos, maybe you have some idea about the
>  "CppunitTest_sw_ooxmlsdrexport: division by zero in
>  chart2/source/controller/chartapiwrapper/LegendWrapper.cxx" mail I just sent to
>  the list?
> <vmiklos> sberg: i'm not entirely sure what is the proper level to fix it; i
>  know that at various places we do "if height is 0, let it 1", and doing the
>  same at the lowest level (chart2) would solve the problem; but there might be
>  a better fix.
> <sberg> vmiklos, would that height=width=100 special case be relevant in this
>  scenario, or is it a red herring?
> <vmiklos> sberg: i think the height=width=100 case is for ole objects embedded
>  as an icon, which is not relevant for this bugdoc. but even if that happens,
>  that should be handled, i guess LegendWrapper::setPosition() can be invoked by
>  a user basic macro as well, so not dividing by 0 for any user input there is
>  necessary.
> <sberg> vmiklos, in LegendWrapper::setPosition() it's not the (user-controlled)
>  position that caused div by zero, but the model's page size;  anyway, will see
>  to hack-fix it at the SvxOle2Shape::createObject level then, maybe not calling
>  xObj->setVisualAreaSize if GetLogicRect returns an empty rect and see how that
>  works out
> <vmiklos> sberg: ah, i see. yes, i would just work it around in the lower level,
>  then ask moggi if he thinks there is a more proper way. i don't have too much
>  clue about chart2, either.
> <moggi> vmiklos: sberg: I have no idea, I would have some if it would be odf
>  import but I have no idea how docx import handles chart init (there are quite
>  a few corner cases that you can only hit in the import code)
> <vmiklos> moggi: it's the same as chart-in-xlsx, OOXML handling for charts is
>  all in oox, so i guess it's not specific to Writer/docx
> <moggi> vmiklos: at least the second backtrace in sberg's mail is writer
>  specific
> <vmiklos> moggi: hmm, writerfilter just sends all SAX events of the shape to
>  oox, and then at the end it calls oox::shape::ShapeContextHandler::getShape()
>  to get the shape. so it doesn't know too much if the resulting shape will be a
>  chart or a ellipse or something else.
> <vmiklos> moggi: of the resulting*
> <moggi> vmiklos: the ChartModel::setVisualArea that sets the 0 size is from
>  parsing the document.xml file and not the chart file, the containing
>  application is responsible for the size of the shape and not the chart stream
> <vmiklos> moggi: aha, indeed. writerfilter first imports the chart, and once it
>  has an uno reference to the chart, it sets the size -- is that unexpected?
> <moggi> vmiklos: let me check what calc is doing
> <moggi> vmiklos: I checked and at least in calc we call setVisualAreaSize only
>  with non zero values, in some places we have some code to set a default size if
>  we requested a zero sized chart
> <moggi> vmiklos: e.g. fuins2.cxx:338
> <vmiklos> moggi: so you first create the chart uno object, set its size, and
>  only then do the real import?
> <moggi> vmiklos: yes
> <vmiklos> moggi: OK, thanks
> <moggi> vmiklos: I think one problem if you go the other way is that some
>  positions/sizes might not be correct if the chart size is not correct
> <moggi> e.g. places where we need to convert from relative to absolute
>  positioning
> <vmiklos> i see.
> <moggi> and in calc the whole spreadsheet data needs to be imported as otherwise
>  we might switch from calc data provider to internal data provider

worked around for now with 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=8eb880a04d17c888c2c986426e935e02ae309e7a> 
"HACK to avoid empty page size/div by 0 in chart2 
LegendWrapper::setPosition"