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 27 Oct 2011 04:25:41 PM UTC  
 
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.0

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 07 Jun 2014 09:10:13 PM UTC, 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 07 Jun 2014 09:10:08 PM UTC, 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 07 Jun 2014 09:10:00 PM UTC, 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 05 Jun 2014 09:59:45 PM UTC, comment #13:

Patch

(file #20932)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 05 Jun 2014 09:51:46 PM UTC, 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 15 Feb 2014 01:03:50 AM UTC, 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 11 Jan 2014 06:25:29 AM UTC, 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 16 Nov 2011 10:58:25 PM UTC, 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 06 Nov 2011 04:54:17 PM UTC, 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 06 Nov 2011 12:54:41 AM UTC, 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 06 Nov 2011 12:52:30 AM UTC, 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 06 Nov 2011 12:39:25 AM UTC, 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 29 Oct 2011 04:39:08 AM UTC, 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 27 Oct 2011 11:25:43 PM UTC, 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 27 Oct 2011 06:16:26 PM UTC, 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 27 Oct 2011 04:32:23 PM UTC, 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 27 Oct 2011 04:25:41 PM UTC, 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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 07 Jun 2014 09:10:25 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Thu 05 Jun 2014 09:59:45 PM UTCcazfiAttached File-=>Added ClangMd5.patch, #20932
      StatusIn Progress=>Ready For Test
      Planned Release=>2.4.3, 2.5.0, 2.6.0
    Sun 16 Mar 2014 01:22:32 AM UTCcazfiSummaryFreeciv gives warnings with clang toolchain=>md5.c gives warnings with clang toolchain
    Sat 15 Feb 2014 01:03:50 AM UTCcazfiOperating SystemMac OS=>Any
    Wed 16 Nov 2011 10:58:25 PM UTCcazfiCategoryNone=>bootstrap
      StatusNone=>In Progress
    Sun 06 Nov 2011 12:39:25 AM UTCjtnSummaryFreeciv does not compile with clang=>Freeciv gives warnings with clang toolchain
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup