[llvm-debuginfo-analyzer] Use LLVM_BUILD_DEBUG in class definitions
#140265
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-debuginfo Author: Javier Lopez-Gomez (jalopezg-git) ChangesSome data members are only part of a class definition in a Debug build, e.g. Fix this situation by introducing See #139098 for details. FYI, @CarlosAlbertoEnciso. I think this is ready for review - let me know what you think. Full diff: https://github.com/llvm/llvm-project/pull/140265.diff 17 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 91bedba8a548d..bb85b9c11da73 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -492,6 +492,11 @@ set(LLVM_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) # --libdir
set(LLVM_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR} ) # --src-root
set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include ) # --includedir
set(LLVM_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR} ) # --prefix
+if( uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+ set(LLVM_BUILD_DEBUG ON)
+else()
+ set(LLVM_BUILD_DEBUG OFF)
+endif()
# Note: LLVM_CMAKE_DIR does not include generated files
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index dbc882937b4f4..3e1ed42e5735c 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -104,6 +104,9 @@
/* Define to 1 if you have the <sysexits.h> header file. */
#cmakedefine HAVE_SYSEXITS_H ${HAVE_SYSEXITS_H}
+/* Define if this LLVM build is a Debug build */
+#cmakedefine LLVM_BUILD_DEBUG
+
/* Define if building libLLVM shared library */
#cmakedefine LLVM_BUILD_LLVM_DYLIB
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVCompare.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVCompare.h
index 4019ea6f17448..f881d1cccaa78 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVCompare.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVCompare.h
@@ -76,7 +76,7 @@ class LVCompare final {
void printItem(LVElement *Element, LVComparePass Pass);
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLine.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLine.h
index c335c34e372b9..c196fefd95db4 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLine.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLine.h
@@ -105,7 +105,7 @@ class LVLine : public LVElement {
void print(raw_ostream &OS, bool Full = true) const override;
void printExtra(raw_ostream &OS, bool Full = true) const override {}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLocation.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLocation.h
index 3b556f9927832..c50924cdc6540 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLocation.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVLocation.h
@@ -49,7 +49,7 @@ class LVOperation final {
void print(raw_ostream &OS, bool Full = true) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() { print(dbgs()); }
#endif
};
@@ -159,7 +159,7 @@ class LVLocation : public LVObject {
void print(raw_ostream &OS, bool Full = true) const override;
void printExtra(raw_ostream &OS, bool Full = true) const override;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVObject.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVObject.h
index efc8db12a6972..18f5b707f428d 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVObject.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVObject.h
@@ -15,6 +15,7 @@
#define LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVOBJECT_H
#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/Config/llvm-config.h"
#include "llvm/DebugInfo/CodeView/CodeView.h"
#include "llvm/DebugInfo/CodeView/TypeIndex.h"
#include "llvm/DebugInfo/LogicalView/Core/LVSupport.h"
@@ -154,7 +155,7 @@ class LVObject {
// copy constructor to create that object; it is used to print a reference
// to another object and in the case of templates, to print its encoded args.
LVObject(const LVObject &Object) {
-#ifndef NDEBUG
+#ifdef LLVM_BUILD_DEBUG
incID();
#endif
Properties = Object.Properties;
@@ -165,7 +166,7 @@ class LVObject {
Parent = Object.Parent;
}
-#ifndef NDEBUG
+#ifdef LLVM_BUILD_DEBUG
// This is an internal ID used for debugging logical elements. It is used
// for cases where an unique offset within the binary input file is not
// available.
@@ -193,7 +194,7 @@ class LVObject {
public:
LVObject() {
-#ifndef NDEBUG
+#ifdef LLVM_BUILD_DEBUG
incID();
#endif
};
@@ -311,13 +312,13 @@ class LVObject {
// (class attributes, debug ranges, files, directories, etc).
virtual void printExtra(raw_ostream &OS, bool Full = true) const {}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
virtual void dump() const { print(dbgs()); }
#endif
uint64_t getID() const {
return
-#ifndef NDEBUG
+#ifdef LLVM_BUILD_DEBUG
ID;
#else
0;
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h
index ac0dfba0f1ede..f9a9580bf7909 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h
@@ -435,7 +435,7 @@ class LVOptions {
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
@@ -632,7 +632,7 @@ class LVPatterns final {
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVRange.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVRange.h
index 3ec0ccb31168f..6790ebc396249 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVRange.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVRange.h
@@ -87,7 +87,7 @@ class LVRange final : public LVObject {
void print(raw_ostream &OS, bool Full = true) const override;
void printExtra(raw_ostream &OS, bool Full = true) const override {}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
index 9ce26398e48df..f6c96588a0bea 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h
@@ -325,7 +325,7 @@ class LVReader {
void print(raw_ostream &OS) const;
virtual void printRecords(raw_ostream &OS) const {}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
index 1b3c377cd7dbb..8cf92811f64cd 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h
@@ -317,7 +317,7 @@ class LVScope : public LVElement {
virtual void printWarnings(raw_ostream &OS, bool Full = true) const {}
virtual void printMatchedElements(raw_ostream &OS, bool UseMatchedElements) {}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h
index 8ce751a56c590..432dcc32d55df 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h
@@ -80,7 +80,7 @@ class LVStringPool {
}
}
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h
index 25bfa9eb77d8a..df14a7699a658 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h
@@ -183,7 +183,7 @@ class LVSymbol final : public LVElement {
void print(raw_ostream &OS, bool Full = true) const override;
void printExtra(raw_ostream &OS, bool Full = true) const override;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
index 28881b3c95b17..9226fec6eb718 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Core/LVType.h
@@ -139,7 +139,7 @@ class LVType : public LVElement {
void print(raw_ostream &OS, bool Full = true) const override;
void printExtra(raw_ostream &OS, bool Full = true) const override;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const override { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/LVReaderHandler.h b/llvm/include/llvm/DebugInfo/LogicalView/LVReaderHandler.h
index bf30501d00c1f..23c0e317f771c 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/LVReaderHandler.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/LVReaderHandler.h
@@ -88,7 +88,7 @@ class LVReaderHandler {
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h
index 9cda64e33ddf7..bb7d837647650 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h
@@ -232,7 +232,7 @@ class LVBinaryReader : public LVReader {
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewReader.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewReader.h
index 4dd7c967ddc17..285e58a8f63e9 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewReader.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewReader.h
@@ -225,7 +225,7 @@ class LVCodeViewReader final : public LVBinaryReader {
LogicalVisitor.printRecords(OS);
};
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
diff --git a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVDWARFReader.h b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVDWARFReader.h
index aa47bd9cd2cdd..8c7278c5f326e 100644
--- a/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVDWARFReader.h
+++ b/llvm/include/llvm/DebugInfo/LogicalView/Readers/LVDWARFReader.h
@@ -146,7 +146,7 @@ class LVDWARFReader final : public LVBinaryReader {
void print(raw_ostream &OS) const;
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#if defined(LLVM_BUILD_DEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const { print(dbgs()); }
#endif
};
|
0301469 to
246f933
Compare
|
I think we already have a separate macro for this - ABI_BREAKING_CHECKS, perhaps? |
f237c52 to
11bec1f
Compare
Some LLVM libraries may conditionally define some class members depending on whether LLVM build type == Debug. However, relying on `NDEBUG` for this may create situations in which the consumer of a header in a downstream project has a different definition for `NDEBUG`, resulting in an ODR violation. The use of `LLVM_BUILD_DEBUG` should be preferred in these cases.
Some data members are only part of a class definition in a Debug build, e.g. `LVObject::ID`. If `debuginfologicalview` is used as a library, `NDEBUG` cannot be used for this purpose, as this PP macro may have a different definition in a downstream project, which in turn triggers an ODR violation. See llvm#139098 for details. Instead, rely on `LLVM_BUILD_DEBUG` for class definitions.
11bec1f to
0f6d6ea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some data members are only part of a class definition in a Debug build, e.g.
LVObject::ID. Ifdebuginfologicalviewis used as a library,NDEBUGcannot be used for this purpose, as this PP macro may have a different definition in a downstream project, which in turn triggers an ODR violation.Fix this situation by introducing
LLVM_BUILD_DEBUGin llvm-config.h, which will be defined only if LLVM itself was built in Debug mode.Fix also all other potentially problematic places; specifically, the
LVObject::dump()function isvirtual, and hence appears in the vtable for classLVObject. Thus, it participates also in class layout and is subject to the same issue.FYI, @CarlosAlbertoEnciso. I think this is ready for review - let me know what you think.
Fixes #139098.