LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 23628 - Mesa build failure: warning: "DEBUG" redefined
Summary: Mesa build failure: warning: "DEBUG" redefined
Status: CONFIRMED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC All
: P normal
Assignee: Vedran Miletic
URL: https://bugs.freedesktop.org/show_bug...
Keywords: regression
Depends on:
Blocks:
 
Reported: 2015-05-21 22:43 PDT by Vinson Lee
Modified: 2017-02-09 09:03 PST (History)
7 users (show)

See Also:
Fixed By Commit(s):


Attachments
Rename DEBUG macro to MESA_DEBUG (129.08 KB, patch)
2016-07-14 15:55 PDT, Vedran Miletic
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vinson Lee 2015-05-21 22:43:43 PDT
Mesa fails to build with llvm-3.7.0svn.


  Compiling src/gallium/auxiliary/gallivm/lp_bld_misc.cpp ...
In file included from llvm/include/llvm/Object/RelocVisitor.h:23:0,
                 from llvm/include/llvm/DebugInfo/DIContext.h:21,
                 from llvm/include/llvm/ExecutionEngine/RuntimeDyld.h:21,
                 from llvm/include/llvm/ExecutionEngine/ExecutionEngine.h:18,
                 from src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:56:
llvm/include/llvm/Support/Debug.h:92:0: warning: "DEBUG" redefined
 #define DEBUG(X) DEBUG_WITH_TYPE(DEBUG_TYPE, X)
 ^
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:7: error: expected identifier before numeric constant
llvm/include/llvm/Support/COFF.h:541:5: note: in expansion of macro ‘DEBUG’
     DEBUG,
     ^
<command-line>:0:7: error: expected ‘}’ before numeric constant
llvm/include/llvm/Support/COFF.h:541:5: note: in expansion of macro ‘DEBUG’
     DEBUG,
     ^
<command-line>:0:7: error: expected unqualified-id before numeric constant
llvm/include/llvm/Support/COFF.h:541:5: note: in expansion of macro ‘DEBUG’
     DEBUG,
     ^
In file included from llvm/include/llvm/Object/COFF.h:19:0,
                 from llvm/include/llvm/Object/RelocVisitor.h:20,
                 from llvm/include/llvm/DebugInfo/DIContext.h:21,
                 from llvm/include/llvm/ExecutionEngine/RuntimeDyld.h:21,
                 from llvm/include/llvm/ExecutionEngine/ExecutionEngine.h:18,
                 from src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:56:
llvm/include/llvm/Support/COFF.h:684:1: error: expected declaration before ‘}’ token
 } // End namespace llvm.
 ^


b6976af3cd6650da1e46127fc890fa256d0add31 is the first bad commit
commit b6976af3cd6650da1e46127fc890fa256d0add31
Author: Keno Fischer <kfischer@college.harvard.edu>
Date:   Thu May 21 21:24:32 2015 +0000

    Make it easier to use DwarfContext with MCJIT
    
    Summary:
    This supersedes http://reviews.llvm.org/D4010, hopefully properly
    dealing with the JIT case and also adds an actual test case.
    DwarfContext was basically already usable for the JIT (and back when
    we were overwriting ELF files it actually worked out of the box by
    accident), but in order to resolve relocations correctly it needs
    to know the load address of the section.
    Rather than trying to get this out of the ObjectFile or requiring
    the user to create a new ObjectFile just to get some debug info,
    this adds the capability to pass in that info directly.
    As part of this I separated out part of the LoadedObjectInfo struct
    from RuntimeDyld, since it is now required at a higher layer.
    
    Reviewers: lhames, echristo
    
    Reviewed By: echristo
    
    Subscribers: vtjnash, friss, rafael, llvm-commits
    
    Differential Revision: http://reviews.llvm.org/D6961
    
    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237961 91177308-0d34-0410-b5e6-96231b3b80d8

:040000 040000 dacc067efc36838238854773830812292ab666ac d62ce735614fd84e5ce0c186d35c6852261b49a6 M	include
:040000 040000 8230e332555950f27a239bec10048ff41212c4fa 6fd01c676a9d9d933c97187ad5259c3001d7684b M	lib
:040000 040000 131a6578fbd9c5227b43006895298144a0f07ff6 b7a25dcbc4547d4b4d353b5e3188d414955ffbd1 M	test
:040000 040000 8a0a777c2e3f1c41ea5bb46f4a540d78c1dd017d 29decdce85fcb43d18d4d5be11a2bd5ec403dc58 M	tools
bisect run success
Comment 1 Anton Korobeynikov 2015-05-22 02:44:25 PDT
Looks like mesa fault, since it redefines DEBUG in the command line to something unusable
Comment 2 Jose Fonseca 2015-05-26 08:14:01 PDT
(In reply to comment #1)
> Looks like mesa fault, since it redefines DEBUG in the command line to
> something unusable

Mesa uses DEBUG=1 macro to mean a debug build. (ie., the opposite of NDEBUG)

Several Microsoft Windows SDKs do the same (ie, #define DEBUG=1 for debug builds.) (Just grep DEBUG on Microsoft Visual Studio / Windows SDK headers and you'll see.)

We can easily fix Mesa to not rely on the DEBUG macro.  (We did it mostly for interoperability with the Microsoft headers. Not a biggie either way.)


But I think that LLVM (at least the parts of it that are meant to be consumed by other applications) is a tad too late in the party to claim ownership for such a generic macro name as "DEBUG".

In fact, I suspect you'll hit this over and over again...  In particular on Windows.
Comment 3 Vinson Lee 2015-05-27 19:19:39 PDT
Ping. Mesa llvmpipe build is still failing with llvm-3.7.0svn.
Comment 4 Keno Fischer 2015-05-27 19:34:41 PDT
LLVM claiming the DEBUG name is a well known bug, but I actually see that this is more serious, because COFF defines DEBUG in an enum. That should probably just be renamed. In any case, even if LLVM didn't define DEBUG as a macro, this would still be broken, because the usage in COFF is not one of the macro.
Comment 5 Jose Fonseca 2015-05-28 04:22:06 PDT
(In reply to comment #3)
> Ping. Mesa llvmpipe build is still failing with llvm-3.7.0svn.

I've worked around this Mesa with commit 
http://cgit.freedesktop.org/mesa/mesa/commit/?id=09d6243aed016eed4518435c9885275dbb6d2aa9

So there's no more need to rush on our account.

(In reply to comment #4)
> LLVM claiming the DEBUG name is a well known bug, but I actually see that
> this is more serious, because COFF defines DEBUG in an enum. That should
> probably just be renamed. In any case, even if LLVM didn't define DEBUG as a
> macro, this would still be broken, because the usage in COFF is not one of
> the macro.

Yep.  Both llvm/include/llvm/Support/Debug.h and llvm/include/llvm/Object/COFF.h should avoid it.

It's a bit sad that C++ namespaces can't protect against C-pre-processor defines, but that's the world we live in.  (Maybe not the case here, but it's even worse with code that needs to include windows.h, as Microsoft claimed lots of all-caps single words names with macros.)  .

You could use the push/pop_macro pragma trick like Mesa's workaround, but that would defeat the purpose. It would make the code complex. More than using some sort of unique prefix.

A better approach would be to change the coding style to use a different capitalization for enums (leaving ALL_CAPS_NAMES to macros alone.)
Comment 6 Vedran Miletic 2016-07-14 15:55:10 PDT
Created attachment 16745 [details]
Rename DEBUG macro to MESA_DEBUG

This patch should rename all occurrences of the DEBUG macro and fix this bug. I have removed all the instances of pop/push_macro().

I will submit it to the mailing list as well for review.