Bug 1799189 - bip: FTBFS in Fedora rawhide/f32
Summary: bip: FTBFS in Fedora rawhide/f32
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: bip
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Adam Williamson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F32FTBFS
TreeView+ depends on / blocked
 
Reported: 2020-02-06 16:07 UTC by Fedora Release Engineering
Modified: 2020-05-21 21:19 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-02-07 20:01:02 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
build.log (13.30 KB, text/plain)
2020-02-06 16:07 UTC, Fedora Release Engineering
no flags Details
root.log (32.00 KB, text/plain)
2020-02-06 16:07 UTC, Fedora Release Engineering
no flags Details
state.log (1.05 KB, text/plain)
2020-02-06 16:07 UTC, Fedora Release Engineering
no flags Details

Description Fedora Release Engineering 2020-02-06 16:07:38 UTC
bip failed to build from source in Fedora rawhide/f32

https://koji.fedoraproject.org/koji/taskinfo?taskID=41315989


For details on the mass rebuild see:

https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild
Please fix bip at your earliest convenience and set the bug's status to
ASSIGNED when you start fixing it. If the bug remains in NEW state for 8 weeks,
bip will be orphaned. Before branching of Fedora 33,
bip will be retired, if it still fails to build.

For more details on the FTBFS policy, please visit:
https://fedoraproject.org/wiki/Fails_to_build_from_source

Comment 1 Fedora Release Engineering 2020-02-06 16:07:47 UTC
Created attachment 1658507 [details]
build.log

Comment 2 Fedora Release Engineering 2020-02-06 16:07:50 UTC
Created attachment 1658508 [details]
root.log

file root.log too big, will only attach last 32768 bytes

Comment 3 Fedora Release Engineering 2020-02-06 16:07:53 UTC
Created attachment 1658509 [details]
state.log

Comment 4 Adam Williamson 2020-02-07 18:57:31 UTC
So, it failed only on s390x - which I think means "only on big-endian" - and it fails with a warning that's treated as an error since we're doing -Wall:

gcc -DHAVE_CONFIG_H -I.     -Wall -Wextra -Werror -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=zEC12 -mtune=z13 -fasynchronous-unwind-tables -fstack-clash-protection -fPIE -Wno-unused-result -Wno-error=format-truncation -c -o libbip_a-irc.o `test -f 'irc.c' || echo './'`irc.c
In file included from /usr/include/string.h:495,
                 from irc.c:16:
In function 'strncpy',
    inlined from 'get_str_elem' at irc.c:646:3,
    inlined from 'irc_cli_startup.constprop' at irc.c:730:9:
/usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
irc.c: In function 'irc_cli_startup.constprop':
irc.c:642:13: note: length computed here
  642 |   c = str + strlen(str);
      |             ^~~~~~~~~~~
cc1: all warnings being treated as errors

I'm not enough of a C guy to understand what's going on there. I can just disable -Wall to get a build through, of course, but it'd be nice to understand the problem. CCing DJ and Patsy in case they can help, thanks.

Comment 5 DJ Delorie 2020-02-07 19:07:42 UTC
it seems to me that you could replace these two lines:

                strncpy(ret, cur, c - cur);
                ret[c - cur] = 0;

with this:

Comment 6 DJ Delorie 2020-02-07 19:09:37 UTC
stupid browser.

with this:

  strncpy (ret, cur, c-cur+1);

or even:

  memcpy (ret, cur, c-cur+1);

The warning is correct; the code is copying up to the NUL on purpose, then copying the NUL in a separate command.  Not sure why.

Comment 7 Adam Williamson 2020-02-07 19:18:18 UTC
Thanks DJ! So, change that in both places it occurs (within and after the `while` block)?

Comment 8 DJ Delorie 2020-02-07 19:22:18 UTC
looks like it.  Of course, I'm just now looking at that code for the first time ;-)

Comment 9 Adam Williamson 2020-02-07 20:01:02 UTC
Well, it built, so I'm sure it'll be fine!

https://koji.fedoraproject.org/koji/buildinfo?buildID=1458002

Thanks again.

Comment 10 Tom Hughes 2020-04-23 10:59:16 UTC
That patch has completely broken bip because the terminating colon is no longer changed to a nul so the returned string is not nul terminated and it fails to authenticate as the password (no longer being nul terminated) doesn't match.

I think the correct solution was to use memcpy instead of strncpy in the original code as we always know exactly how much we are copying and there is no question of hitting a nul. Going to try that now...

Comment 11 Tom Hughes 2020-04-23 11:08:04 UTC
Confirmed - that has fixed it.

Comment 12 Tom Hughes 2020-04-23 11:11:26 UTC
Note the original fix probably does work for the final string (the second instance, outside the loop) but not for the version inside the loop.

Comment 13 Tom Hughes 2020-04-23 11:17:50 UTC
Opened https://src.fedoraproject.org/rpms/bip/pull-request/1.

Comment 14 Adam Williamson 2020-04-23 15:24:49 UTC
Thanks, Tom. I took the original patch on trust as I'm no expert in this kind of C stuff, I only maintain this package as I use it and no-one else wants it...I think I meant to do a scratch build and test it on my server (which is F31) but forgot to :/

Comment 15 Adam Williamson 2020-04-23 16:45:46 UTC
Hmm, your patch isn't good either, it seems: it fails to build on s390x. Is it not safe for big-endian?

https://kojipkgs.fedoraproject.org//work/tasks/3813/43693813/build.log

Comment 16 Tom Hughes 2020-04-23 18:08:29 UTC
Sorry It seems I managed to introduce some noise in the prevous patch which meant that the second hunk silently failed to apply... New PR now open.


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