Bug 216206 - editors/openoffice-4 and editors/openoffice-devel: fails to build with clang 4.0
Summary: editors/openoffice-4 and editors/openoffice-devel: fails to build with clang 4.0
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-office mailing list
URL:
Keywords: needs-patch
Depends on:
Blocks: 216008
  Show dependency treegraph
 
Reported: 2017-01-18 00:58 UTC by Jan Beich
Modified: 2017-01-31 00:04 UTC (History)
2 users (show)

See Also:


Attachments
WIP patch (1.41 KB, patch)
2017-01-20 18:22 UTC, Pedro F. Giffuni
no flags Details | Diff
Minimalistic patch (probably incomplete) (1.18 KB, patch)
2017-01-20 20:10 UTC, Pedro F. Giffuni
no flags Details | Diff
patch using NULL (1.18 KB, patch)
2017-01-21 03:06 UTC, Pedro F. Giffuni
no flags Details | Diff
Updated with new case (3 total) (1.71 KB, patch)
2017-01-21 21:59 UTC, Pedro F. Giffuni
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2017-01-18 00:58:51 UTC
main/desktop/source/deployment/misc/dp_misc.cxx:106:16: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
    if (digest <= 0) {
        ~~~~~~ ^  ~
main/sd/source/ui/view/viewshe3.cxx:229:48: error: ordered comparison between pointer and zero ('SdPage *' and 'int')
        if (pDocument->GetSdPage(0, ePageKind) > 0)
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~

regressed by: https://github.com/llvm-mirror/clang/commit/4b6ad14285f3
Comment 1 Pedro F. Giffuni freebsd_committer 2017-01-20 18:22:24 UTC
Created attachment 179155 [details]
WIP patch

It is likely there are more bugs like that: the codebase is really old.
Comment 2 Pedro F. Giffuni freebsd_committer 2017-01-20 20:10:41 UTC
Created attachment 179161 [details]
Minimalistic patch (probably incomplete)

I am getting some weird issue on udkapi using clang-devel:

ERROR: error 65280 occurred while making /usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/udkapi/com/sun/star/uno

There's no clang-4 port available so I can't test more.

I will upstream the very minimal patch for now.
Comment 3 Jan Beich freebsd_committer 2017-01-20 20:22:49 UTC
Comment on attachment 179161 [details]
Minimalistic patch (probably incomplete)

I'd prefer s/nullptr/NULL/ as only libc++ exposes nullptr in pre-C++11 mode (i.e. always). And in C++11 mode it'd be converted to nullptr by base r228918, anyway.
Comment 4 Pedro F. Giffuni freebsd_committer 2017-01-21 03:06:15 UTC
Created attachment 179172 [details]
patch using NULL
Comment 5 Pedro F. Giffuni freebsd_committer 2017-01-21 03:07:19 UTC
(In reply to Jan Beich (mail not working) from comment #3)

Ugh, in C++, nullptr is the way to go. Unfortunately you are right and MSVC 8, still used in upstream OpenOffice, doesn't support nullptr, so NULL or perhaps just 0 is more "portable".
Comment 6 Jan Beich freebsd_committer 2017-01-21 19:03:38 UTC
Comment on attachment 179172 [details]
patch using NULL

Not enough to unbreak.

main/desktop/source/app/officeipcthread.cxx:228:14: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
        if ( handle > 0 )
             ~~~~~~ ^ ~
Comment 7 Pedro F. Giffuni freebsd_committer 2017-01-21 19:43:37 UTC
(In reply to Jan Beich (mail not working) from comment #6)
> Not enough to unbreak.

I know .. I just can't reproduce because clang-5.0 doesn't get that far :(.
They are pretty easy to fix though, keep them coming :-/.

> main/desktop/source/app/officeipcthread.cxx:228:14: error: ordered
> comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
>        if ( handle > 0 )
>             ~~~~~~ ^ ~

Should be:
       if ( handle != NULL )
 
(will fix upstream)
Comment 8 Jan Beich freebsd_committer 2017-01-21 19:54:42 UTC
(In reply to Pedro F. Giffuni from comment #7)
> They are pretty easy to fix though, keep them coming :-/.

Keep in mind these sometimes uncover real bugs e.g., bug 216253, bug 216019, bug 216159 or bug 216160.
Comment 9 Pedro F. Giffuni freebsd_committer 2017-01-21 21:59:15 UTC
Created attachment 179197 [details]
Updated with new case (3 total)

Oh I am so glad clang is finding these.
Comment 10 Jan Beich freebsd_committer 2017-01-22 02:00:47 UTC
At least editors/openoffice-4 builds fine now. I haven't tested -devel yet.

(In reply to Pedro F. Giffuni from comment #9)
> Oh I am so glad clang is finding these.

The error itself isn't new. GCC (even 4.2) supports it as -W + -Werror. Clang 4.0 is pedantic enough to reject such code in C++ but doesn't even warn in C.
Comment 11 Pedro F. Giffuni freebsd_committer 2017-01-22 03:16:21 UTC
(In reply to Jan Beich (mail not working) from comment #10)

The differences between the the stable version and -devel are not huge so hopefully it will build fine.

Thank you for taking the time to build the monster(s).
Comment 12 commit-hook freebsd_committer 2017-01-22 08:35:58 UTC
A commit references this bug:

Author: jbeich
Date: Sun Jan 22 08:35:12 UTC 2017
New revision: 432098
URL: https://svnweb.freebsd.org/changeset/ports/432098

Log:
  editors/openoffice-devel: unbreak with clang 4.0

  main/desktop/source/app/officeipcthread.cxx:228:14: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
          if ( handle > 0 )
               ~~~~~~ ^ ~
  main/desktop/source/deployment/misc/dp_misc.cxx:106:16: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
      if (digest <= 0) {
          ~~~~~~ ^  ~
  main/sd/source/ui/view/viewshe3.cxx:229:48: error: ordered comparison between pointer and zero ('SdPage *' and 'int')
          if (pDocument->GetSdPage(0, ePageKind) > 0)
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~

  PR:		216206
  Submitted by:	pfg
  Obtained from:	upstream

Changes:
  head/editors/openoffice-4/files/patch-clang40
  head/editors/openoffice-devel/files/patch-clang40
Comment 13 commit-hook freebsd_committer 2017-01-22 08:39:03 UTC
A commit references this bug:

Author: jbeich
Date: Sun Jan 22 08:38:13 UTC 2017
New revision: 432099
URL: https://svnweb.freebsd.org/changeset/ports/432099

Log:
  MFH: r432098

  editors/openoffice-devel: unbreak with clang 4.0

  main/desktop/source/app/officeipcthread.cxx:228:14: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
          if ( handle > 0 )
               ~~~~~~ ^ ~
  main/desktop/source/deployment/misc/dp_misc.cxx:106:16: error: ordered comparison between pointer and zero ('rtlDigest' (aka 'void *') and 'int')
      if (digest <= 0) {
          ~~~~~~ ^  ~
  main/sd/source/ui/view/viewshe3.cxx:229:48: error: ordered comparison between pointer and zero ('SdPage *' and 'int')
          if (pDocument->GetSdPage(0, ePageKind) > 0)
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~

  PR:		216206
  Submitted by:	pfg
  Obtained from:	upstream
  Approved by:	ports-secteam blanket

Changes:
_U  branches/2017Q1/
  branches/2017Q1/editors/openoffice-4/files/patch-clang40
  branches/2017Q1/editors/openoffice-devel/files/patch-clang40
Comment 14 Don Lewis freebsd_committer 2017-01-26 09:56:00 UTC
(In reply to Pedro F. Giffuni from comment #2)

I used a poudriere jail running the /projects/clang400-import/ branch to track down some more openoffice build problems with clang 4.0.

In all cases the code looked like:
  somestruct *obj = new somestruct ();

It turns out that clang 4.0 tries to use SSE instructions to zero the memory block returned by new, which requires that the memory be 16-byte aligned, but our malloc implementation only does 8-byte alignment.  If the memory isn't sufficiently aligned, then we get a bus error core dump.

I wasn't able to provoke our malloc() implementation into returning an unaligned pointer for objects of 16 bytes or larger, but I see that AOO redefines new and uses its own memory allocator.
Comment 15 Pedro F. Giffuni freebsd_committer 2017-01-26 14:01:51 UTC
(In reply to Don Lewis from comment #14)
Hi Don;

Try the --with-alloc=system configure option. I will see if I can rescue an old patch that may help but this is likely to require a lot more.
Comment 16 Pedro F. Giffuni freebsd_committer 2017-01-26 17:18:36 UTC
(In reply to Pedro F. Giffuni from comment #15)
For the record ...

I just noticed editors/libreoffice uses the system alloc:

https://svnweb.freebsd.org/ports/head/editors/libreoffice/Makefile?revision=431796&view=markup#l229
Comment 17 Don Lewis freebsd_committer 2017-01-26 21:03:04 UTC
(In reply to Pedro F. Giffuni from comment #16)

system alloc turns out not to help.  It just replaces the guts of rtl_allocateMemory() and friends.  I previously tried changing that to do the necessary alignment and it didn't help.

There is also two other memory allocation mechanisms, rtl_cache_alloc() and rtl_arena_alloc().  The former is an object cache and I think it carves up larger allocations.  It doesn't know about anything bigger than 8 byte alignment.  Fixing it cleanly looks complicated.  I haven't looked at the arena stuff.
Comment 18 Pedro F. Giffuni freebsd_committer 2017-01-26 22:07:43 UTC
(In reply to Don Lewis from comment #17)
Bummer ...

The logical option seems to be to disable the SSE optimizations somehow(?).

I still think we should use the native alloc; running it I see no difference though.
Comment 19 Don Lewis freebsd_committer 2017-01-27 08:01:59 UTC
(In reply to Pedro F. Giffuni from comment #18)

Disabling SSE just hides the problem, one of these unaligned pointers could be passed to code that doesn't have SSE disabled.

I dug into rtl_cache_alloc() and friends and discovered these use mmap() instead of calling malloc().  Their interfaces are very unlike malloc(), so replacing them with system malloc() would be difficult.  I did hack on the implementation to try to get them to do the proper alignment.

Using system malloc() I'm still seeing idlc core dump in the offapi module build.  This is a case where new has been overridden to call allocate(), which calls rtl_allocateMemory(), which calls malloc().  The problem is that it appears to be calling the malloc() implementation built into rtld instead of the libc implemenation.

reakpoint 1 (malloc) pending.
(gdb) run @/tmp/r
Starting program: /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/bin/idlc @/tmp/r
[New LWP 101741]
Breakpoint 2 at 0x8006a5f01: file /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c, line 163.
Pending breakpoint "malloc" resolved
Trace 67695/1: "Min Prioriy for policy '2' == '0'
"
Trace 67695/1: "Max Prioriy for policy '2' == '103'
"
/wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/bin/idlc: compiling 1 source files ... 
Compiling: /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/offapi/com/sun/star/i18n/KParseTokens.idl
[New Thread 802616000 (LWP 101741/idlc)]
[Switching to Thread 802616000 (LWP 101741/idlc)]

Breakpoint 2, malloc (nbytes=343)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	/var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c: No such file or directory.
	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
Current language:  auto; currently minimal
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=32)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.
Trace 67962/2: "ChildStatusProc : starting '/wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/bin/ucpp'"
[New Thread 802616500 (LWP 100242/idlc)]
[Switching to Thread 802616500 (LWP 100242/idlc)]

Breakpoint 2, malloc (nbytes=19)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=34)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=16)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=16)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=16)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.

Breakpoint 2, malloc (nbytes=16)
    at /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c:163
163	in /var/poudriere/jails/clang400amd64/usr/src/libexec/rtld-elf/malloc.c
(gdb) cont
Continuing.
sizeof(AstExprValue)=16

Program received signal SIGBUS, Bus error.
[Switching to Thread 802616000 (LWP 101741/idlc)]
0x0000000000478cc2 in AstExpression::eval_bit_op (this=0x802633dc8, 
    ek=EK_const) at astexpression.cxx:1001
1001	    std::auto_ptr< AstExprValue	> retval(new AstExprValue());

This is occurring even though idlc is linked to libc.so:

LD_LIBRARY_PATH=/wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/lib ldd solver/420/unxfbsdx.pro/bin/idlc
solver/420/unxfbsdx.pro/bin/idlc:
	libreg.so.3 => /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/lib/libreg.so.3 (0x8008b9000)
	libuno_sal.so.3 => /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/lib/libuno_sal.so.3 (0x800c00000)
	libuno_salhelpergcc3.so.3 => /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/lib/libuno_salhelpergcc3.so.3 (0x801040000)
	libm.so.5 => /lib/libm.so.5 (0x801244000)
	libc++.so.1 => /usr/lib/libc++.so.1 (0x80146e000)
	libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x801735000)
	libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x801953000)
	libthr.so.3 => /lib/libthr.so.3 (0x801b69000)
	libc.so.7 => /lib/libc.so.7 (0x801d91000)
	libstore.so.3 => /wrkdirs/usr/ports/editors/openoffice-devel/work/aoo-4.2.0/main/solver/420/unxfbsdx.pro/lib/libstore.so.3 (0x802152000)

I'm unable to reproduce this oddity with a simple test program.
Comment 20 Don Lewis freebsd_committer 2017-01-27 18:14:25 UTC
Another piece of the puzzle is --enable-debug.  The new operator is overridden to call allocate(), which in turn calls rtl_allocateMemory(), which either calls malloc() or uses the internal allocator code, depending on the --with-alloc setting.

Inside allocate() the allocation size (n) is adjusted by n = rTraits.size (n).  If --enable-debug is set, then the size is increased by 8 bytes.  Then the pointer returned by rtl_allocateMemory() is adjusted by rTraits.init (p), which writes a signature to the first 8 bytes of the memory block and increments the pointer by 8.

When the memory is freed, the pointer is decremented by 8 bytes and the signature is checked before calling rtl_freeMemory().

Basically even if rtl_allocateMemory() always returns a properly aligned block of memory, --enable-debug will break things if 16 byte alignment is needed.