Bug 1003986 - FTBFS: libcomps-0.1.3-2.fc20 tests fail on power & s390
Summary: FTBFS: libcomps-0.1.3-2.fc20 tests fail on power & s390
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: libcomps
Version: 20
Hardware: powerpc
OS: Linux
urgent
urgent
Target Milestone: ---
Assignee: Jindrich Luza
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: ZedoraTracker F20PPCAlpha
TreeView+ depends on / blocked
 
Reported: 2013-09-03 16:17 UTC by David Aquilina
Modified: 2014-06-24 00:19 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-09-06 13:45:08 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
libcomps-0.1.3-2.fc20 build log (99.20 KB, text/x-log)
2013-09-03 16:17 UTC, David Aquilina
no flags Details

Description David Aquilina 2013-09-03 16:17:21 UTC
Created attachment 793269 [details]
libcomps-0.1.3-2.fc20 build log

Description of problem:

Some libcomps tests fail when trying to build on power, e.g.: 

in comps_parse test: 

WARNING: Unknown elemenet 'O$`�' at line:-7209592
'OX' found between elements at line:-7209560 column :-7209564
No declaration for element packageFOO
*** Error in `./test_parse': free(): invalid next size (fast): 0x104f5698 ***

Attaching the full build log (and also it's available from http://ppc.koji.fedoraproject.org/kojifiles/work/tasks/114/1390114/build.log

Comment 1 Gustavo Luiz Duarte 2013-09-05 15:29:33 UTC
1.
"test_default" fails because the variable "def" is not initialized on libcomps/src/python/pycomps_gids.c:421.

From http://docs.python.org/dev/c-api/arg.html:

|: Indicates that the remaining arguments in the Python argument list are optional. The C variables corresponding to optional arguments should be initialized to their default value — when an optional argument is not specified, PyArg_ParseTuple() does not touch the contents of the corresponding C variable(s).

Non-static local variables are not automatically initialized. Please initialize def with the default value (which I believe to be zero, inferring from the test case).


2.
"test_fedora" is failing due to misuse of sprintf() on ./libcomps/src/comps_logger.c:226.

The type of the arguments passed to sprintf() must match the corresponding specifier character in the format string, otherwise the result is non-portable and has undefined behavior.
On comps_logger.c:226, the arguments passed to sprintf() have the type union, even though the format string expects a pointer to char and a few integers.

We need to rework the logic on comps_log_entry_str() to make it pass the arguments to sprintf() with the correct type.

A quick workaround is to change the type of pitem.i from int to intptr_t. Even though this workaround makes the test pass on ppc64, it is still non-portable and I have not tested on x86 or arm.

Another thing to note is that this code is always passing 4 arguments to sprintf(), even though there might be only 3 specifier characters. AFAIK, passing the wrong number of arguments to sprintf() also has undefined behavior and should be avoided, even though it doesn't cause any issue on ppc64 with current glibc.


Even though we might go with the workaround I mentioned to unblock the composes on ppc64, I strongly suggest we properly fix this issue ASAP by reworking comps_log_entry_str() to make it pass sprintf() arguments appropriately.

Comment 2 Gustavo Luiz Duarte 2013-09-05 15:34:27 UTC
Forgot to mention, there are still a couple of tests failing on ppc64: test_xml and test_union3. I will look at it later.

Comment 3 Gustavo Luiz Duarte 2013-09-05 20:13:18 UTC
3.
The test_xml and test_union3 test failures seem to be caused by another misuse of PyArg_ParseTuple() on libcomps/src/python/pycomps_groups.c:1097.
The format string passed to PyArg_ParseTuple() is "|si", indicating two optional arguments, a pointer to char and an integer. But instead of an integer, a long argument has been passed.
Please change either the type of the variable or the format string so that they match.

Comment 4 Gustavo Luiz Duarte 2013-09-05 20:31:41 UTC
With fixes 1, 2 (workaround) and 3 in place I was able to build libcomps successfully on ppc64. I can try to come up with a proper fix for item #2, though anyone with more familiarity with libcomps code is very welcome to jump in here and write it.

Comment 5 Jindrich Luza 2013-09-06 13:45:08 UTC
I fixed problems above by Gustavo Luiz Duarte guidelines (except 2. - no workaround) and build on ppc passed. Here is task info: 
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=1401994
And I just submited package update, so fix will be in libcomps-0.1.3-3.fc20.

Issue no.2 is fixed by rewriting comps_log_entry_str function.I passing all strings to sprintf now, and I also changed all format string specifiers to %s, so it shouldn't be problem in passing differ type as sprintf arguments anymore.
Wrong argument number passing to sprintf is solved by counting argument usage and then calling sprintf with appropriate arguments (not much elegant but works for now - whole libcomps c-python binding architecture is designated to rewrite according to https://github.com/midnightercz/libcomps/issues/11 anyway).

And also big thanks to Gustavo Luiz Duarte. Because within his investigations, I probably had to wait for ppc arch reservation (I haven't any ppc machine avaible) and whole proccess took much more time.

Comment 6 Gustavo Luiz Duarte 2013-09-06 14:26:06 UTC
Great! Thanks Jindrich.


Note You need to log in before you can comment on or make changes to this bug.