[llvm] [llvm-debuginfo-analyzer] Fix ODR violation in llvm::logicalview::LVObject (PR #140265)

Carlos Alberto Enciso via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 13 02:33:14 PDT 2025


CarlosAlbertoEnciso wrote:

> > The changes for `dump` I think make sense. I'm not fully convinced about the remainder of the change. I think that we do want the ID checking in the debug builds (it is meant to be a debugging tool). If we are building without assertions, then we can enable it with ABI breaking changes. That is, I think that `#if !defined(NDEBUG) || ENABLE_ABI_BREAKING_CHANGES` is the right approach.
> 
> Thanks for the review, @compnerd!
> 
> Note that, although the pull-request mentions `[llvm-debuginfo-analyzer]`, the issue is really in `DebugInfoLogicalView` (the underlying library used by `llvm-debuginfo-analyzer`). As described in #139098, the `LVObject` class layout may differ if the `NDEBUG` PP macro has different definition (other than it had when LLVM was built) in a downstream project that consumes the library. Because in a downstream project, `NDEBUG` is usually used to indicate a Release build, using `#if !defined(NDEBUG) || ENABLE_ABI_BREAKING_CHANGES`, may still cause issues from the PoV of the consumer project.

Looking at the other `--internal=xxx` options, the only one that is not available in non-debug builds is `--internal=id`, which introduce an inconsistence and restriction to the options. I understand what @compnerd mention about the `ID`, but as @jalopezg-git states, the issue is with the `DebugInfoLogicalView` library.

The `LVObject::ScopeLevel` is declared as `uint32_t`. Maybe we can reduce it and allow `ID` for all builds.

https://github.com/llvm/llvm-project/pull/140265


More information about the llvm-commits mailing list