[llvm-dev] [RFC] Usage of NDEBUG as a guard for non-assert debug code

Thu Apr 9 10:34:29 PDT 2020

On Thu, Apr 9, 2020 at 10:24 AM Chris Tetreault <ctetreau at quicinc.com>
wrote:

> All,
>
>
>
>    I guess I stand corrected on my statement that nobody uses Release+no
> debug info+ asserts. Honestly, I find this strange since Debug builds in
> Visual Studio work perfectly fine and I use the debugger daily, but I’ll
> not judge. I have noticed that RelWithDebInfo is heinously slow on Linux, I
> recently switched to Release there and I’m dreading the next time when I
> have to actually debug a Linux specific issue. Maybe I’ll try this config
> as a “better than nothing” option.
>

There are things to improve debug info performance/cost on linux - but,
yes, the distribution model is different (no pdb files being built
on-the-side during compilation - all the debug info goes in the .o files
and gets linked into the final executable usually). Split DWARF helps
somewhat, but there's still a lot of duplication that's deferred to the
linker to cleanup, due to the lack of the compile time inter-compile
communication with pdb generation that deduplicates at that time. (this
helps MSVC builds locally, but the model doesn't distribute well - so it's
not all upside, FWIW)

>
>
> > This seems fairly orthogonal to the rest of this discussion, to me at
> least - it's an improvement over the existing/common
>
>
>
>    Possibly. I only mentioned it because, in my opinion, David Truby was
> proposing a band-aid for a bigger issue. That said, if people are actually
> using -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=TRUE, then my
> plan breaks apart. Regardless, I guess I agree that we need two macros.
>
>
>
> Thanks,
>
>    Christopher Tetreault
>
>
>
> *From:* David Blaikie <dblaikie at gmail.com>
> *Sent:* Thursday, April 9, 2020 10:10 AM
> *To:* Chris Tetreault <ctetreau at quicinc.com>
> *Cc:* David Truby <David.Truby at arm.com>; llvm-dev at lists.llvm.org
> *Subject:* [EXT] Re: [llvm-dev] [RFC] Usage of NDEBUG as a guard for
> non-assert debug code
>
>
>
> On Thu, Apr 9, 2020 at 9:59 AM Chris Tetreault via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> David,
>
>
>
>    In my opinion, NDEBUG is one of those gross old C things that everybody
> complains about. It’s called “Not Debug”, but really it means “Assert
> Disabled”. I think one could be forgiven for actually using it as a
> heuristic of whether or not a build is a debug build, especially since no
> other options are provided. I appreciate your desire, but I think it’d be
> unfortunate if the build system grew yet another flag to control debugness.
>
>
>
>    As far as I can tell, as it currently works, LLVM_ENABLE_ASSERTIONS
> just makes sure that NDEBUG is not defined, even in release builds. So if I
> do -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=TRUE, I’ll get an
> optimized build with no debug symbols but with asserts enabled, which in my
> mind isn’t a terribly useful thing to have.
>
>
> FWIW, I believe quite a few people use that mode & don't use debuggers
> much - faster link/compile times, etc.
>
>
> Furthermore, none of this works on Visual Studio because it has a UI menu
> to control the build type. I personally would be very disappointed to see
> Visual Studio’s build type dropdown break.
>
>
>
>    Since we C’s assert, it is intrinsically tied to NDEBUG. What we need
> is proper custom asserts. In codebases I’ve seen in my travels that have
> this it usually looks like:
>
>
>
> // If asserts are enabled, evaluate and assert that expr is truthy. If it
> is not, complain with msg.
>
> LLVM_ASSERT(expr, msg)
>
>
>
> // If asserts are enabled, evaluate and assert that expr is truthy. If it
> is not, complain with msg.
>
> // If asserts are disabled, evaluate expr, do not assert.
>
> // either way, return expr
>
> LLVM_VERIFY(expr, msg)
>
>
>
>    The first one is useful as a traditional assert. The second one is
> useful if you are calling a function, and want to assert that it succeeds,
> but still need it to be evaluated in release builds:
>
>
>
> auto *Foo = LLVM_VERIFY(ReturnsAPointerThatShouldNeverActuallyBeNull(),
> “this should never return null”);
>
>
>
> This seems fairly orthogonal to the rest of this discussion, to me at
> least - it's an improvement over the existing/common:
>
> auto *Foo = ReturnsAPointer...;
> assert(Foo && "this should never be null");
>
> But not a fundamentally new thing, etc. (& could be proposed independent
> of renaming 'assert' to LLVM_ASSERT)
>
>
>
>
>
>    If we have custom asserts, then we can have custom assert guard macros:
>
>
>
> // true if this is any sort of debug build
>
> LLVM_DEBUG_BUILD
>
>
>
> // true if asserts are turned on (Debug build on Windows,
>
> // Debug build or -DLLVM_ASSERTIONS_ENABLED=TRUE on other platforms)
>
> LLVM_ASSERTS_ENABLED
>
>
>
>    These flags could be derived from just CMAKE_BUILD_TYPE, and
> LLVM_ENABLE_ASSERTIONS can go away (assuming we agree that an asserting
> build with optimizations and no debug info is worse than useless).
>
>
>
> I don't think people would agree to that - I believe at least a few
> regular LLVM developers use that mode regularly.
>
>
>
> Custom asserts also have the advantage of having a proper message
> parameter and not needing to rely on the truthiness of string literals.
> Obviously this is a much more invasive change than what you are proposing,
> but in my opinion it’s the correct thing to do.
>
>
>
> Thanks,
>
>    Christopher Tetreault
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of *David
> Truby via llvm-dev
> *Sent:* Thursday, April 9, 2020 7:26 AM
> *To:* llvm-dev at lists.llvm.org
> *Subject:* [EXT] [llvm-dev] [RFC] Usage of NDEBUG as a guard for
> non-assert debug code
>
>
>
> Hi all,
>
>
>
> During discussions about assertions in the Flang project, we noticed that
> there are a lot of cases in LLVM that #ifndef NDEBUG is used as a guard for
> non-assert code that we want enabled in debug builds.
>
> This works fine on its own, however it affects the behaviour of
> LLVM_ENABLE_ASSERTIONS;  since NDEBUG controls whether assertions are
> enabled or not, a lot of debug code gets enabled in addition to asserts if
> you specify this flag. This goes contrary to the name of the flag I believe
> also its intention. Specifically in Flang we have a case where someone
> wants to ship a build with assertions enabled, but doesn't want to drag in
> all the extra things that are controlled by NDEBUG in LLVM.
>
>
>
> In my opinion we ideally want LLVM_ENABLE_ASSERTIONS to _only_ enable
> assertions and do nothing else. I don't think this is possible without
> changing the use of NDEBUG elsewhere as NDEBUG controls whether assert is
> enabled.
>
> I propose we should be using another macro (something like
> LLVM_DEBUG_CHECKS ?) that is enabled in Debug builds, and possibly
> controlled by another cmake flag (LLVM_ENABLE_DEBUG_CHECKS ?) for code that
> we want enabled for debugging but not in releases. This would allow
> LLVM_ENABLE_ASSERTIONS to do what it says on the tin and actually enable
> assertions only.
>
>
>
> Does anyone else have any thoughts on this?
>
>
>
> Thanks
>
> David Truby
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200409/b839a384/attachment.html>