Project

General

Profile

Actions

Feature #15304

closed

assert() could provide hints to compilers

Added by Andy Fiddaman 5 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:
External Bug:

Description

TL;TR: assert() - or more specifically the functions called on failure, __assert_c99() and __assert() could be annotated so that the compiler knows that they do not return.
This is what at least Linux and FreeBSD have, and I have encountered one software package that relies on the compiler being able to optimise out any following code for an assertion that will always fail (that's a dubious as it sounds - "one NDEBUG away from a bad day" - but it's in a real software package - qemu, more details below - that appears to build everywhere except illumos).

The following short program exhibits this:

#include <stdio.h>
#include <assert.h>

#define BOB 0

__attribute__((error("elided")))
void
f(void)
{
        printf("elided\n");
}

int
main(void)
{
        assert(BOB);
        f();
}

This compiles on a Linux system I have to hand, and on FreeBSD, and both elide all of the code following the assertion in main, for example:

000000000040058d <main>:
  40058d:       55                      push   %rbp
  40058e:       48 89 e5                mov    %rsp,%rbp
  400591:       b9 52 06 40 00          mov    $0x400652,%ecx
  400596:       ba 10 00 00 00          mov    $0x10,%edx
  40059b:       be 47 06 40 00          mov    $0x400647,%esi
  4005a0:       bf 50 06 40 00          mov    $0x400650,%edi
  4005a5:       e8 b6 fe ff ff          callq  400460 <__assert_fail@plt>
  4005aa:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)

On OmniOS, both gcc and clang abort with an error:

build% gcc-12 -o assert assert.c
assert.c: In function 'main':
assert.c:17:9: error: call to 'f' declared with attribute error: elided
   17 |         f();
      |         ^~~

build% clang-14 -o assert assert.c
assert.c:17:2: error: call to f declared with 'error' attribute: elided
        f();
        ^

FreeBSD introduced a _dead2 attribute on __assert() in 2011:

commit 62779d80fb85de470823a8ff2902218a9a5e715e
Author: Ed Schouten <ed@FreeBSD.org>
Date:   Sun Jan 9 21:39:46 2011 +0000

    Add missing __dead2 to __assert().

    __assert() is called when an assertion fails. After printing an error
    message, it will call abort(). abort() never returns, hence it has the
    __dead2 attribute. Also add this attribute to __assert().

illumos should do the same .

qemu

The software package that I came across that led me to look into this is qemu 7.2.0. It is using compile time assertions to ensure that code which should be unreachable is removed by the compiler, which relies on the compiler knowing that an assert() call with a value that is always false will not return.

The commit which introduced the specific build problem on illumos is https://github.com/qemu/qemu/commit/8ed558ec0cbcc29ecf490e93c54dd65d276e8e69#diff-92d5901063822825a1eff26216a272ffb3af74c0854665dc8363897f9b3f150eR459 - and the addition of the assert(!TARGET_TB_PCREL); at that line is supposed to ensure that tb_pc() (two lines below) is never called if TARGET_TB_PCREL is defined as 0.

Whatever you may think of this, it's in the code base and it apparently works as intended on mainstream platforms.

I opened https://gitlab.com/qemu-project/qemu/-/issues/1402 for this originally, which has since been closed with Either your compiler or system headers for <assert.h> are broken.

Actions #1

Updated by Andy Fiddaman 5 months ago

  • Description updated (diff)
Actions #2

Updated by Andy Fiddaman 5 months ago

  • Description updated (diff)
Actions #3

Updated by Electric Monk 5 months ago

  • Gerrit CR set to 2587
Actions #4

Updated by Andy Fiddaman 5 months ago

I've tested the change in the associated gerrit review by doing full bulk builds of OmniOS core (289 packages) and extra (255 packages); these were all successful.
I've also confirmed that qemu 7.2.0 builds with this fix in place.

Actions #5

Updated by Electric Monk 5 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 6d4184838d392397f06dc2496b61d5e256c39ef7

commit  6d4184838d392397f06dc2496b61d5e256c39ef7
Author: Andy Fiddaman <illumos@fiddaman.net>
Date:   2023-01-13T09:59:25.000Z

    15304 assert() could provide hints to compilers
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Reviewed by: Marco van Wieringen <mvw@planets.elm.net>
    Reviewed by: Jason King <jason.brian.king+illumos@gmail.com>
    Approved by: Rich Lowe <richlowe@richlowe.net>

Actions

Also available in: Atom PDF