bugFreeciv - Bugs: bug #18872, md5.c gives warnings with clang...

 
 
Show feedback again

bug #18872: md5.c gives warnings with clang toolchain

Submitted by:  David Lowe <doctorjlowe>
Submitted on:  Thu Oct 27 16:25:41 2011  
 
Category: bootstrapSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: 2.3.0Operating System: Any
Planned Release: 2.4.3, 2.5.0, 2.6.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat Jun 7 21:10:13 2014, SVN revision 25084:

Dropped the special optimized version of md5 code for some platforms where alignment would
cause no problems. This fixes clang warning about how that optimized version would be unportable
to other platforms.

Reported by David Lowe

See bug #18872

(Browse SVN revision 25084)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jun 7 21:10:08 2014, SVN revision 25083:

Dropped the special optimized version of md5 code for some platforms where alignment would
cause no problems. This fixes clang warning about how that optimized version would be unportable
to other platforms.

Reported by David Lowe

See bug #18872

(Browse SVN revision 25083)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jun 7 21:10:00 2014, SVN revision 25082:

Dropped the special optimized version of md5 code for some platforms where alignment would
cause no problems. This fixes clang warning about how that optimized version would be unportable
to other platforms.

Reported by David Lowe

See bug #18872

(Browse SVN revision 25082)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu Jun 5 21:59:45 2014, comment #13:

Patch

(file #20932)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu Jun 5 21:51:46 2014, comment #12:

Actually, this warning about code portability comes from a code that is only used on platforms where it works. This is about the optimized version for platforms where alignment causes no problems (but clang warns that it would not work on platforms where it's never used...)

Code comment:
" * The check for little-endian architectures that tolerate unaligned

  • memory accesses is just an optimization. Nothing will break if it
  • doesn't work."

As md5 code is not used in a time-critical paths, I think we can simply drop the optimized version and use the portable version always.

Of cource jtn already mentioned that part of code in comment #5, but I didn't fully understood what was going on back then.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Feb 15 01:03:50 2014, comment #11:

md5.c update in patch #4505, doesn't help with the clang compilation.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jan 11 06:25:29 2014, comment #10:

> Rats; that's the new MD5 code I pulled in for bug #18170.
> Checking upstream finds no new version


There has been some upstream updates last year, but I haven't checked if they fix this problem.
Could jtn create patch for updating our codebase to match current upstream? (I can do it later, but you know this code way better than I do)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed Nov 16 22:58:25 2011, comment #9:

Somewhat related to this ticket: I just installed clang for myself and have been testing various freeciv compilations with it. So far I've been testing with trunk and S2_4 only, but may look at S2_3 issues at some point. To sum: Freeciv compiles, but with numerous warnings. I will need to make changes both to configure stuff (adjust setting of compiler flags) and code changes (warnings that indicate problems) In general I will not target to warningless clang build in S2_3 as some changes may have potential to break other currently working builds, but any real bugs found by clang diagnostic should be fixed in S2_3 too.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Nov 6 16:54:17 2011, comment #8:

It's actually not my setup - it's a user of my Freeciv package in the 'fink' environment. But yeah, i'm fairly certain that he's on x86_64. I'll send him a new patch and see what happens. It won't happen right away though, as i've got some calculus homework coming due. If the patch suppresses the warning, upstream probably should be notified that their assumptions as to what will tolerated are out of date...

Judging by the new title of the ticket, the other two warnings treated as errors are still going to be handled on this ticket. In that case, here is more detail on the third error he saw:

/bin/sh ../libtool --preserve-dup-deps --silent --tag=CC --mode=compile
llvm-gcc-4.2 -DHAVE_CONFIG_H -I. -I.. -I/sw/include
-DLOCALEDIR="\"/sw/share/locale\"" -DBINDIR="\"/sw/bin\""
-DDEFAULT_DATA_PATH="\".:data:~/.freeciv/2.3:/sw/share/freeciv\""
-DDEFAULT_SAVES_PATH="\"\""
-DDEFAULT_SCENARIO_PATH="\".:data/scenario:~/.freeciv/2.3/scenarios:~/.freeciv/scenarios:/sw/share/freeciv/scenario\""
-Wall -Wpointer-arith -Wcast-align -Wmissing-prototypes
-Wmissing-declarations -Werror -g -O2 -c -o registry.lo registry.c
cc1: warnings being treated as errors
registry.c: In function 'secfile_entry_delete':
registry.c:1713: warning: return makes integer from pointer without a cast
registry.c: In function 'secfile_lookup_plain_enum_full':
registry.c:2116: warning: return makes integer from pointer without a cast
registry.c:2117: warning: return makes integer from pointer without a cast
registry.c:2118: warning: return makes integer from pointer without a cast
registry.c:2119: warning: return makes integer from pointer without a cast
registry.c: In function 'secfile_lookup_bitwise_enum_full':
registry.c:2258: warning: return makes integer from pointer without a cast
registry.c:2259: warning: return makes integer from pointer without a cast
registry.c:2260: warning: return makes integer from pointer without a cast
registry.c:2261: warning: return makes integer from pointer without a cast
registry.c: In function 'secfile_lookup_enum_data':
registry.c:2440: warning: return makes integer from pointer without a cast
registry.c:2441: warning: return makes integer from pointer without a cast
registry.c:2442: warning: return makes integer from pointer without a cast

David Lowe <doctorjlowe>
Sun Nov 6 00:54:41 2011, comment #7:

Or we could just disable the optimisation -- we're only hashing passwords, not bulk data, so we hardly need it.

Jacob Nevins <jtn>
Project Administrator
Sun Nov 6 00:52:30 2011, comment #6:

> If so, we could avoid the warning by adding some sort of clang
> detection in that #if. (Alternatively, by turning off
> -Wcast-align for clang, but that's likely to be fiddlier and may
> hide issues we're interested in.)


The hyper proper way would be to make configure check instead of listing environments with known properties in that #if.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Nov 6 00:39:25 2011, comment #5:

Re the md5.c warning: I suspect it's running into this bit of code (which is from Solar Designer's original):

Presumably one of those symbols is defined on doctorjlowe's setup. Does the warning go away if you change this to "#if 0"?

If so, we could avoid the warning by adding some sort of clang detection in that #if. (Alternatively, by turning off -Wcast-align for clang, but that's likely to be fiddlier and may hide issues we're interested in.)

Jacob Nevins <jtn>
Project Administrator
Sat Oct 29 04:39:08 2011, comment #4:

Neither the shared.c error or the md5.c error appear without --enable-debug, so perhaps i named this ticket incorrectly. Actually i've just collected a third warning that pops up in this environment. Should i split out the new one and the 'comparison of unsigned expression' problem into separate tickets?

David Lowe <doctorjlowe>
Thu Oct 27 23:25:43 2011, comment #3:

Rats; that's the new MD5 code I pulled in for bug #18170. Checking upstream finds no new version. I suppose we're going to have to work out if there's a bug intrinsic to that code or just in the way I integrated it into Freeciv.

Jacob Nevins <jtn>
Project Administrator
Thu Oct 27 18:16:26 2011, comment #2:

My package does set --enable-debug, but i thought the user in question had deleted that line. I don't currently have access to 10.7, so i'm awaiting his response.

David Lowe <doctorjlowe>
Thu Oct 27 16:32:23 2011, comment #1:

The warning you show is a minor bug perhaps, but the real question here is why is compilation using -Werror? You didn't pass --enable-debug=yes or --enable-debug=checks to the autogen / configure script? Or is the non-presence of gcc confusing it somehow?

Jason Dorje Short <jdorje>
Project Member
Thu Oct 27 16:25:41 2011, original submission:

Clang is now the default compiler on OSX 10.7 A.K.A Lion, and also OSX 10.6 A.K.A Snow Leopard with the optional XCode 4.2 update. On those systems, an ordinary make fails like so:

/bin/sh ../libtool --preserve-dup-deps --silent --tag=CC --mode=compile
gcc -DHAVE_CONFIG_H -I. -I.. -I/sw/include
-DLOCALEDIR="\"/sw/share/locale\"" -DBINDIR="\"/sw/bin\""
-DDEFAULT_DATA_PATH="\".:data:~/.freeciv/2.3:/sw/share/freeciv\""
-DDEFAULT_SAVES_PATH="\"\""
-DDEFAULT_SCENARIO_PATH="\".:data/scenario:~/.freeciv/2.3/scenarios:~/.freeciv/scenarios:/sw/share/freeciv/scenario\""
-Wall -Wpointer-arith -Wcast-align -Wmissing-prototypes
-Wmissing-declarations -Wno-unused-but-set-variable -Werror -g -O2
-fsigned-char -c -o shared.lo shared.c
shared.c:370:36: error: comparison of unsigned expression >= 0 is always
true [-Werror,-Wtautological-compare]
while (mantissa != 0 && exponent >= 0) {
~~~~~~~~ ^ ~
/bin/sh ../libtool --preserve-dup-deps --silent --tag=CC --mode=compile
gcc -DHAVE_CONFIG_H -I. -I.. -I/sw/include
-DLOCALEDIR="\"/sw/share/locale\"" -DBINDIR="\"/sw/bin\""
-DDEFAULT_DATA_PATH="\".:data:~/.freeciv/2.3:/sw/share/freeciv\""
-DDEFAULT_SAVES_PATH="\"\""
-DDEFAULT_SCENARIO_PATH="\".:data/scenario:~/.freeciv/2.3/scenarios:~/.freeciv/scenarios:/sw/share/freeciv/scenario\""
-Wall -Wpointer-arith -Wcast-align -Wmissing-prototypes
-Wmissing-declarations -Wno-unused-but-set-variable -Werror -g -O2
-fsigned-char -c -o md5.lo md5.c
md5.c:148:5: error: cast from 'const unsigned char ' to 'MD5_u32plus '
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Werror,-Wcast-align]
STEP(F, a, b, c, d, SET(0), 0xd76aa478, 7)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The workaround for now is to force usage of the less strict llvm compiler, though i expect clang usage is going to become more widespread.

David Lowe <doctorjlowe>

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #20932:  ClangMd5.patch added by cazfi (962B - text/x-diff)

 

Depends on the following items: None found

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by jdorje (Posted a comment)
  • -unavailable- added by doctorjlowe (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Error: not logged in

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat Jun 7 21:10:25 2014cazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Thu Jun 5 21:59:45 2014cazfiAttached File-=>Added ClangMd5.patch, #20932
      StatusIn Progress=>Ready For Test
      Planned Release=>2.4.3, 2.5.0, 2.6.0
    Sun Mar 16 01:22:32 2014cazfiSummaryFreeciv gives warnings with clang toolchain=>md5.c gives warnings with clang toolchain
    Sat Feb 15 01:03:50 2014cazfiOperating SystemMac OS=>Any
    Wed Nov 16 22:58:25 2011cazfiCategoryNone=>bootstrap
      StatusNone=>In Progress
    Sun Nov 6 00:39:25 2011jtnSummaryFreeciv does not compile with clang=>Freeciv gives warnings with clang toolchain
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup