MantisBT - VTK
View Issue Details
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 
ProjectTBD
Typeincorrect functionality
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>
Steps To Reproduce
Additional Information
Tagshackaton
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2013-01-11 10:40kentwilliamsNew Issue
2013-01-16 14:01Sean McBrideNote Added: 0030147
2013-01-16 14:05kentwilliamsNote Added: 0030148
2013-01-16 14:14Sean McBrideNote Added: 0030149
2013-01-16 14:52kentwilliamsNote Added: 0030150
2013-01-16 14:55Sean McBrideNote Added: 0030151
2013-01-16 16:32kentwilliamsNote Added: 0030152
2013-01-17 14:52Sean McBrideNote Added: 0030176
2013-01-17 15:28kentwilliamsNote Added: 0030177
2013-01-17 15:28Sean McBrideAssigned To => Sean McBride
2013-01-17 15:28Sean McBrideStatusbacklog => tabled
2013-01-17 15:30kentwilliamsNote Edited: 0030177bug_revision_view_page.php?bugnote_id=30177#r551
2013-01-23 17:53Sean McBrideNote Added: 0030235
2013-01-30 10:51Sean McBrideAssigned ToSean McBride => Marcus D. Hanwell
2013-01-30 10:51Sean McBrideNote Added: 0030281
2013-01-30 11:28Marcus D. HanwellAssigned ToMarcus D. Hanwell => Dave DeMarle
2013-01-30 11:28Marcus D. HanwellNote Added: 0030282
2013-07-22 20:02Dave DeMarleStatustabled => backlog
2013-07-22 20:02Dave DeMarleNote Added: 0031240
2013-12-16 14:13Dave DeMarleNote Added: 0031950
2014-09-30 10:55Dave DeMarleTag Attached: hackaton
2014-10-02 11:05Dave DeMarleNote Added: 0033458
2014-10-02 11:05Dave DeMarleStatusbacklog => gerrit review
2014-10-02 12:27Dave DeMarleStatusgerrit review => closed
2014-10-02 12:27Dave DeMarleResolutionopen => fixed
2014-10-02 12:27Dave DeMarleFixed in Version => 5.10.dev

Notes
(0030147)
Sean McBride   
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   
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   
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   
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   
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   
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   
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   
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   
2013-01-23 17:53   
http://review.source.kitware.com/9448 [^]
(0030281)
Sean McBride   
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   
2013-01-30 11:28   
Only Dave DeMarle (AFAIK) has privileges to do that.
(0031240)
Dave DeMarle   
2013-07-22 20:02   
moving all "tabled" bugs into "backlog" category since "tabled" is no longer used.
(0031950)
Dave DeMarle   
2013-12-16 14:13   
Will put this onto release-5.10 branch shortly
(0033458)
Dave DeMarle   
2014-10-02 11:05   
backported fix with http://review.source.kitware.com/#/c/17326/ [^]