Bug 1631 - SuSE 9.1 compile with -O2 generates buggy code
SuSE 9.1 compile with -O2 generates buggy code
Status: CLOSED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: Build environment
3.0.5
x86 Linux
: P3 critical
: none
Assigned To: Lars Müller
Samba QA Contact
:
: 1720 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-19 07:22 UTC by Luke Howard
Modified: 2005-08-24 10:20 UTC (History)
4 users (show)

See Also:


Attachments
patch to compile MD4 with O2 (1.04 KB, patch)
2004-11-12 10:12 UTC, Daniel Beschorner
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Howard 2004-08-19 07:22:48 UTC
This is not your fault, I expect it is a C compiler problem, but it might be worthwhile knowing about. I 
just upgraded to SuSE Linux Professional 9.1 and re-built 3.0.6rc2 with gcc 3.3.3.

Our RPM build script was compiling SAMBA with -O2, and I noticed that the built rpcclient did not work 
(whilst the version built from my sandbox, ie. with debugging symbolbs worked).

Although I haven't gone so far as to look at the assembly, it turns out that compiling SAMBA with -O2 
on SuSE 9.1 generates buggy code in the mdfour() routine. Compiling with -O or without optimization 
works fine. I noticed this by stepping through rpcclient and looking at the resultant OWF generated by 
mdfour() -- only when compiled without -O2 did the output match the OWF in the directory (ie. MD4 of 
the UCS2-LE password).

System/gcc specs follow:

lukeh@off/power-station[75]% uname -a
Linux power-station 2.6.5-7.104-default #1 Wed Jul 28 16:42:13 UTC 2004 i686 i686 i386 GNU/Linux
lukeh@off/power-station[76]% gcc -v
Reading specs from /usr/lib/gcc-lib/i586-suse-linux/3.3.3/specs
Configured with: ../configure --enable-threads=posix --prefix=/usr --with-local-prefix=/usr/local --
infodir=/usr/share/info --mandir=/usr/share/man --enable-languages=c,c++,f77,objc,java,ada --
disable-checking --libdir=/usr/lib --enable-libgcj --with-gxx-include-dir=/usr/include/g++ --with-
slibdir=/lib --with-system-zlib --enable-shared --enable-__cxa_atexit i586-suse-linux
Thread model: posix
gcc version 3.3.3 (SuSE Linux)
Comment 1 Andrew Bartlett 2004-08-23 17:54:56 UTC
This has been reproduced by others on the mailing list.
Comment 2 Gerald (Jerry) Carter 2004-08-23 18:21:22 UTC
why is this our bug ?
Comment 3 Lars Müller 2004-08-24 00:49:43 UTC
We've seen this already with 9.0.  For the builds of current Samba packages for
this version I've added -O to the CFLAGS to get a proper smbpasswd binary.

While discussing thie issue the last time with one of our compiler developers,
he argued that this is more likely a Samba and not a gcc problem.  I've informed
him about this bug.
Comment 4 Gerald (Jerry) Carter 2004-08-24 05:42:04 UTC
Lars,  if you can let us know what to change to work around this, 
we will make the change in our code.
Comment 5 Jörn Nettingsmeier 2004-10-19 13:08:37 UTC
lars, i have run into the same problem. using suse 9.1 on a dual-p4 with ht
enabled with your samba-3.0.7 rpms. 

when trying to change a password from a win2k client:

[2004/10/19 21:50:16, 0] libsmb/smbencrypt.c:decode_pw_buffer(539)
  decode_pw_buffer: incorrect password length (-727309796).
[2004/10/19 21:50:16, 0] libsmb/smbencrypt.c:decode_pw_buffer(540)
  decode_pw_buffer: check that 'encrypt passwords = yes'
[2004/10/19 21:51:00, 1] smbd/service.c:make_connection_snum(648)

here's what happens when using smbpaswd on the command line:

nettings@pol-serv1:~> smbpasswd
Old SMB password:
New SMB password:
Retype new SMB password:
read_socket_with_timeout: timeout read. read error = Connection reset by peer.
machine 127.0.0.1 rejected the tconX on the IPC$ share. Error was : Read error:
Connection reset by peer.
Failed to change password for nettings

the log says:

[2004/10/19 22:05:39, 0] lib/access.c:check_access(328)
  Denied connection from  (127.0.0.1)
[2004/10/19 22:05:39, 1] smbd/process.c:process_smb(1085)
  Connection denied from 127.0.0.1

is there a quick workaround?
Comment 6 Lars Müller 2004-10-19 13:40:26 UTC
The Samba 3 RPMs from Samba.org and ftp.SuSE.com are build with -O and should
not have this problem.

Are you using the following package:
Name        : samba                 Relocations: (not relocatable)
Version     : 3.0.7                 Vendor: SuSE Linux AG, Nuernberg, Germany
Release     : 1.1                   Build Date: Do 16 Sep 2004 12:01:27 CEST
Install date: (not installed)       Build Host: prokofjieff.suse.de
Comment 7 Lars Müller 2004-10-19 13:42:43 UTC
Jörn: Is your smbd listening to localhost?
Comment 8 Sven Strickroth 2004-10-19 14:01:13 UTC
*** Bug 1720 has been marked as a duplicate of this bug. ***
Comment 9 Jörn Nettingsmeier 2004-10-19 14:13:05 UTC
(In reply to comment #6)

yes.
Comment 10 Jörn Nettingsmeier 2004-10-19 14:13:34 UTC
(In reply to comment #7)
> Jörn: Is your smbd listening to localhost?

yes again.
Comment 11 Daniel Beschorner 2004-10-20 02:08:12 UTC
Just for the sake of completeness:

As I mentioned on mailing list with CFLAGS="-O2" and SuSE's GCC 3.3.1 our 
clients can't join the Samba PDC domain successfully.

The smbd ends up with:

[2004/06/03 19:29:58, 0] libsmb/smbencrypt.c:decode_pw_buffer(521)
  decode_pw_buffer: incorrect password length (-1061250291).
[2004/06/03 19:29:58, 0] libsmb/smbencrypt.c:decode_pw_buffer(522)
  decode_pw_buffer: check that 'encrypt passwords = yes'

and "incorrect user name" is displayed by the client.
Comment 12 Daniel Beschorner 2004-11-12 10:12:34 UTC
Created attachment 769 [details]
patch to compile MD4 with O2

works for me, don't ask my why now hash results are correct.
Comment 13 Lars Müller 2004-11-12 14:17:55 UTC
DB thanks for the patch. I've informed one of our compiler guys and hope to get
an explanation why your changes fix it.

I'll also test your patch in our build system and keep ypu up to date.
Comment 14 Lars Müller 2004-11-15 08:33:39 UTC
Comment from a compiler guy:

No.  The changed code doesn't "fix" any obvious problems which would lead 
to miscompilations (if it's still equivalent, which I can't determine from 
just the patch).  So if it works now, then only by accident, because of 
changed order of the code.  That could very well hide the bug.
# end comment

So we have to investigate further.
Comment 15 Daniel Beschorner 2004-11-16 07:38:04 UTC
Lars, could you please forward this simple piece of test code I developed from 
the md4 routine to your compiler guy?
Code seems ok, but output with SuSE 3.3.1 and 3.3.3 (i386/SuSE 8.2/9.1) differs 
between -O and -O2!!!
Some people with non-SuSE GCCs confirmed me that it compiles to same result 
with or without -O2, I only have SuSE gcc for testing :-)

So maybe an SuSE issue with one of your vendor patches?

#include "stdio.h"
unsigned int A,B,C;
unsigned int i;
unsigned int Q[1];

unsigned int H(unsigned int X, unsigned int Y)
{
        return X*Y;
}

int main()
{
        A=B=C=100000;

        A+=H(B,C);
        C+=H(A,B);
        A+=H(B,C);
        C+=H(A,B);

        for (i=0;i<1;i++)
                Q[i] = 0;

        printf("%u\n",A);
        return 0;
}
Comment 16 Lars Müller 2004-11-17 06:10:41 UTC
Michael Matz answered:

This is a very nice example.  The problem is, that the second call to "H(B,C)" 
is removed.  The SuSE GCC correctly determines that H is a const function 
(i.e. doesn't access global memory, and only depends on its arguments), which 
the mainline GCC 3.3.x is not able to. 
 
But then it somehow thinks that C is not changed in between the two calls, 
so (because H is const) this second call to H(B,C) is not needed, because 
it would result in the same value, which already was computed, so it's 
optimized away.  Of course C _does_ change between the two calls, and hence 
such argumentation can't be applied. 
 
This does not happen on the 9.2 3.3.4 based compiler, but I need to 
investigate if we really fixed it by chance, or if the bug is just hidden.
Comment 17 Lars Müller 2004-11-30 07:17:40 UTC
This is fixed in all SuSE Linux post 9.1 products.  The bug is indeed not only
hidden, but fixed for good in the SuSE Linux 9.2 gcc.

Therefore I'm now using -O for pre 9.2 products and the default -O2 for all post
9.1 in our spec file.
Comment 18 Gerald (Jerry) Carter 2005-02-07 09:39:30 UTC
originally against 3.0.6rc2
Comment 19 Gerald (Jerry) Carter 2005-08-24 10:20:52 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.