View Issue Details [ Jump to Notes ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013781VTK(No Category)public2013-01-11 10:402014-10-02 12:27
Reporterkentwilliams 
Assigned ToDave DeMarle 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version5.10.0 
Target VersionFixed in Version5.10.dev 
Summary0013781: CLang 3.3 prerelease confused by use of alloca in vtkVRML
DescriptionThis is a problem I detected using the CLang trunk revision (Version 3.3 pre-release), though the fix I'm suggesting will work with any CLang version. The problem stems from CLang's impersonation of GCC -- in other words CLang defines __GNUC__ even though it diverges from GNUC behavior in several ways.

The compile error is this, and is followed by a patch that allows compilation to complete.

/usr/include/alloca.h:36:7: error: declaration of '__builtin_alloca' has a different language linkage
void *alloca(size_t); /* built-in for gcc */
         ^
/Volumes/scratch/kent/BRAINSStandalone/clangbuild/VTK/Hybrid/vtkVRMLImporter.cxx:14:16: note:
      expanded from macro 'alloca'
#define alloca __builtin_alloca
               ^
/usr/include/alloca.h:36:7: note: previous implicit declaration is here
/Volumes/scratch/kent/BRAINSStandalone/clangbuild/VTK/Hybrid/vtkVRMLImporter.cxx:14:16: note:
      expanded from macro 'alloca'
#define alloca __builtin_alloca
               ^
1 error generated.




diff --git a/Hybrid/vtkVRMLImporter.cxx b/Hybrid/vtkVRMLImporter.cxx
index ae6f239..2fd15a4 100644
--- a/Hybrid/vtkVRMLImporter.cxx
+++ b/Hybrid/vtkVRMLImporter.cxx
@@ -10,8 +10,10 @@
 
 
 #ifdef __GNUC__
+#ifndef __clang__
 #undef alloca
 #define alloca __builtin_alloca
+#endif
 #else /* not __GNUC__ */
 #if HAVE_ALLOCA_H
 #include <alloca.h>
Tagshackaton
ProjectTBD
Typeincorrect functionality
Attached Files

 Relationships

  Notes
(0030147)
Sean McBride (developer)
2013-01-16 14:01

 Strange that my clang trunk dashboard (Rogue7) does not have any errors. It's using clang version 3.3 (trunk 170355). I'm updating it now to be even newer. What version are you using exactly? And on what OS? I'm on OS X. Perhaps it's less the compiler and more your /usr/include/alloca.h?

Anyway... what a ghastly file... :) Comments about MSDOS, wow! It seems to be at least partly autogenerated...

A better fix might be to remove the "#ifdef __GNUC__" case entirely... are there are VTK 6 supported systems that use GCC but don't have alloca.h? I doubt it these days... we could try such a patch on gerrit...
(0030148)
kentwilliams (reporter)
2013-01-16 14:05

 Sean that is the output of Bison/BYacc -- it would make more sense to me if the original parser source file was in VTK with the option to re-generate it.

The problem comes down to this: CLang defined the GCC preprocessor symbols, and that works great to the extent that it also implements GCC features. This is a case where pretending to be GCC is a fail.
(0030149)
Sean McBride (developer)
2013-01-16 14:14

 I understand, but the bigger fail, IMHO, is:

#ifdef __GNUC__
#undef alloca
#define alloca __builtin_alloca

What's this for? Why assume gcc has #defined alloca? Why #undefine it? Why redefine it? Major code smell. We should just #include <alloca.h>. If it does not exist, then do the hacks, not vice versa.

No?
(0030150)
kentwilliams (reporter)
2013-01-16 14:52

 Sean, couldn't agree more. Simpler is better.

#if HAVE_ALLOCA_H
#include <alloca.h>
#else
#define alloca __builtin_alloca
#endif
(0030151)
Sean McBride (developer)
2013-01-16 14:55

 Cool. I wasn't thinking to reduce it to that small, because I don't know how many of those other funky cases are actually needed...

Want me to match gerrit patch, or you will?
(0030152)
kentwilliams (reporter)
2013-01-16 16:32

 Sean, go ahead. I've avoided VTK gerrit so far. The gobbledegook about alloca is there because it's part of the standard byacc boilerplate. All I know is that my original patch seemed to change what was already there the minimal amount.

It's actually interesting (for extremely nerdy values of 'nerdy') why this causes a problem: In LLVM/CLang the C Preprocessor seems to do more than textual substitution; macros get syntax checked. Not sure that's an improvement.
(0030176)
Sean McBride (developer)
2013-01-17 14:52

 So the HAVE_ALLOCA_H define comes from a few lines above, not some CMake check as I assumed... do you know how/where I could actually use CMake to find that it exists or not?
(0030177)
kentwilliams (reporter)
2013-01-17 15:28
edited on: 2013-01-17 15:30

 http://www.cmake.org/Wiki/CMake:How_To_Write_Platform_Checks [^]

include(CheckIncludeFiles)
CHECK_INCLUDE_FILES(alloca.h HAVE_ALLOCA_H)

Then use CONFIGURE_FILE to generate a header (VTK prob. does this already somewhere), as described in the link above.
(0030235)
Sean McBride (developer)
2013-01-23 17:53

 http://review.source.kitware.com/9448 [^]
(0030281)
Sean McBride (developer)
2013-01-30 10:51

 Fixed here: http://review.source.kitware.com/9471 [^]

Marcus, can you merge this into the 5.x branch?
(0030282)
Marcus D. Hanwell (developer)
2013-01-30 11:28

 Only Dave DeMarle (AFAIK) has privileges to do that.
(0031240)
Dave DeMarle (administrator)
2013-07-22 20:02

 moving all "tabled" bugs into "backlog" category since "tabled" is no longer used.
(0031950)
Dave DeMarle (administrator)
2013-12-16 14:13

 Will put this onto release-5.10 branch shortly
(0033458)
Dave DeMarle (administrator)
2014-10-02 11:05

 backported fix with http://review.source.kitware.com/#/c/17326/ [^]

 Issue History
Date Modified Username Field Change
2013-01-11 10:40 kentwilliams New Issue
2013-01-16 14:01 Sean McBride Note Added: 0030147
2013-01-16 14:05 kentwilliams Note Added: 0030148
2013-01-16 14:14 Sean McBride Note Added: 0030149
2013-01-16 14:52 kentwilliams Note Added: 0030150
2013-01-16 14:55 Sean McBride Note Added: 0030151
2013-01-16 16:32 kentwilliams Note Added: 0030152
2013-01-17 14:52 Sean McBride Note Added: 0030176
2013-01-17 15:28 kentwilliams Note Added: 0030177
2013-01-17 15:28 Sean McBride Assigned To => Sean McBride
2013-01-17 15:28 Sean McBride Status backlog => tabled
2013-01-17 15:30 kentwilliams Note Edited: 0030177
2013-01-23 17:53 Sean McBride Note Added: 0030235
2013-01-30 10:51 Sean McBride Assigned To Sean McBride => Marcus D. Hanwell
2013-01-30 10:51 Sean McBride Note Added: 0030281
2013-01-30 11:28 Marcus D. Hanwell Assigned To Marcus D. Hanwell => Dave DeMarle
2013-01-30 11:28 Marcus D. Hanwell Note Added: 0030282
2013-07-22 20:02 Dave DeMarle Status tabled => backlog
2013-07-22 20:02 Dave DeMarle Note Added: 0031240
2013-12-16 14:13 Dave DeMarle Note Added: 0031950
2014-09-30 10:55 Dave DeMarle Tag Attached: hackaton
2014-10-02 11:05 Dave DeMarle Note Added: 0033458
2014-10-02 11:05 Dave DeMarle Status backlog => gerrit review
2014-10-02 12:27 Dave DeMarle Status gerrit review => closed
2014-10-02 12:27 Dave DeMarle Resolution open => fixed
2014-10-02 12:27 Dave DeMarle Fixed in Version => 5.10.dev

Copyright © 2000 - 2018 MantisBT Team