Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mesa build failure: warning: "DEBUG" redefined #24002

Closed
vinsonlee opened this issue May 22, 2015 · 7 comments
Closed

Mesa build failure: warning: "DEBUG" redefined #24002

vinsonlee opened this issue May 22, 2015 · 7 comments
Labels
bugzilla Issues migrated from bugzilla build-problem confirmed Verified by a second party regression

Comments

@vinsonlee
Copy link

Bugzilla Link 23628
Version trunk
OS All
CC @asl,@jvesely,@Keno

Extended Description

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)
^
:0:0: note: this is the location of the previous definition
:0:7: error: expected identifier before numeric constant
llvm/include/llvm/Support/COFF.h:541:5: note: in expansion of macro ‘DEBUG’
DEBUG,
^
:0:7: error: expected ‘}’ before numeric constant
llvm/include/llvm/Support/COFF.h:541:5: note: in expansion of macro ‘DEBUG’
DEBUG,
^
: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.
^

b6976af is the first bad commit
commit b6976af
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

@asl
Copy link
Collaborator

asl commented May 22, 2015

Looks like mesa fault, since it redefines DEBUG in the command line to something unusable

@llvmbot
Copy link
Collaborator

llvmbot commented May 26, 2015

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.

@vinsonlee
Copy link
Author

Ping. Mesa llvmpipe build is still failing with llvm-3.7.0svn.

@Keno
Copy link
Member

Keno commented May 28, 2015

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.

@llvmbot
Copy link
Collaborator

llvmbot commented May 28, 2015

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.

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.)

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 14, 2016

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.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@arsenm
Copy link
Contributor

arsenm commented Aug 10, 2023

Old build issue, LLVM uses LLVM_DEBUG now

@arsenm arsenm closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
Chaotic-Temeraire pushed a commit to chaotic-cx/mesa-mirror that referenced this issue Mar 22, 2024
rationale:
DEBUG is easily conflict with third-party headers, for example LLVM llvm/llvm-project#24002 (comment)
And amdcommon's addrlib

And according to llvm/llvm-project#24002 (comment)
LLVM is already switched to LLVM_DEBUG, I think it's time for mesa switch to MESA_DEBUG for #ifdef DEBUG.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Acked-by: David Heidelberg <david.heidelberg@collabora.com>
Reviewed-by: Eric Engestrom <eric@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28092>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla build-problem confirmed Verified by a second party regression
Projects
None yet
Development

No branches or pull requests

5 participants