#306 assert() should have __noreturn__ attribute

v1.0 (example)
closed-invalid
nobody
header (101)
5
2015-02-05
2012-07-04
Khairul Kasmiran
No

The assert() function and its kind should have the __noreturn__ attribute (through __MINGW_ATTRIB_NORETURN). The reason is illustrated below.

The following program:

--------------------------------------

#include <assert.h>

int main(void) {
assert(0);
}

--------------------------------------

when compiled under -Wreturn-type or -Wall generates the following spurious warning:

--------------------------------------

noreturn.c: In function 'main':
noreturn.c:5:1: warning: control reaches end of non-void function

--------------------------------------

even though control will never reach the end of main().

Discussion

  • Jonathan Yong
    Jonathan Yong
    2012-07-04

    These are runtime asserts, GCC can't know if the assert expression is true or not.
    Besides, noreturn is false for assert, as it might actually return if the expression is true.

     
  • _assert()/_wassert() noreturn patch

     
    Attachments
  • I agree with you, except that the truthness of some assert expressions can be determined at compile time like in the trivial case above.

    Maybe I should have been more specific. What I actually meant is the _assert() function called by the assert macro. The attached patch made against assert.h (revision 5161) at /trunk/mingw-w64-headers/crt/ implements what I wanted.

     
  • Jonathan Yong
    Jonathan Yong
    2012-07-05

    From the GCC manual:
    "Some programs define their own functions that never return. You can declare them noreturn to tell the compiler this fact.."

    How is this even correct for _assert if it can return without aborting the program?

    I have to reiterate again these are runtime assertions, they are not in anyway evaluated at compile time, even if it was obvious.

    I think you are looking for C++ static_assert instead.

     
  • Testing done with the following program:

    --------------------------------------

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

    int main(void) {
    _assert("1", __FILE__, __LINE__);
    puts("After _assert()");

    return 0;
    }

    --------------------------------------

    strongly suggests that the MinGW-w64 implementation of _assert() does not return no matter what I do.

     
  • Kai Tietz
    Kai Tietz
    2012-07-06

    Yes, it would be a failure to add to assert noreturn as it can return dependent to its boolean-expression-value.

    So this bug is invalid in my opinion.

     
  • Kai Tietz
    Kai Tietz
    2012-07-06

    • status: open --> pending-invalid
     
  • Kai Tietz
    Kai Tietz
    2012-07-06

    Err, sorry about the "expression is true". I mixed it up with assert ....

    There is a chance that _assert/_wassert can return. In GUI mode user has here the option to choose ignore in dialog-box, which means _assert/_wassert returns. Therefore the attribute of noreturn would be wrong.

    Cheers,
    Kai

     
  • Hmm, didn't know that. Thanks Kai for the explanation.

     
    • status: pending-invalid --> open-invalid
     
  • Ozkan Sezer
    Ozkan Sezer
    2012-07-06

    • status: open-invalid --> closed-invalid
     
  • With the current QEMU source with mingw you hit the following warning only if the assert is there:

    qemu-world3/hw/pci/pci.c: In function 'pci_register_bar':
    qemu-world3/hw/pci/pci.c:932:29: warning: array subscript is above array bounds [-Warray-bounds]
    r = &pci_dev->io_regions[region_num];

    void pci_register_bar(PCIDevice pci_dev, int region_num,
    uint8_t type, MemoryRegion
    memory)
    {
    PCIIORegion *r;
    uint32_t addr;
    uint64_t wmask;
    pcibus_t size = memory_region_size(memory);

    assert(region_num >= 0);
    assert(region_num < PCI_NUM_REGIONS);
    if (size & (size-1)) {
        fprintf(stderr, "ERROR: PCI region size must be pow2 "
                    "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
        exit(1);
    }
    
    r = &pci_dev->io_regions[region_num];
    

    This only happens on mingw, not on Linux gcc; and the preprocessed line is:

    (void) ((!!(        region_num >= 0)) || (_assert("region_num >= 0","qemu-world3/hw/pci/pci.c",924),0));
    (void) ((!!(region_num < 7)) || (_assert("region_num < PCI_NUM_REGIONS","qemu-world3/hw/pci/pci.c",925),0));
    

    My reading of this is:
    1) Without the assert, the compiler knows nothing about region_num, and so doesn't warn you because it would get too many false triggers.
    2) The comparison in the assert says to the compiler that region_num might be bigger than 7; and therefore it's worth warning you about the array operation.
    3) If _assert was noreturn then it would know that the array operation was unreachable under that condition and you wouldn't get the warning

    So having an _assert that can return is a bad idea, because the gcc will cause warnings because of the comparisons that are generated as part of the assert.
    It seems a bad idea to discourage people putting asserts in.

    (mingw64-gcc-4.9.2-1.fc21.x86_64)

     
  • Kai Tietz
    Kai Tietz
    2015-02-05

    • Group: --> v1.0 (example)
     
  • Kai Tietz
    Kai Tietz
    2015-02-05

    Sorry, but this isn't true. assert is just noreturn for console-application. For GUI variant assert might return on user's input of pushing ignore-button on assert-messagebox.
    As this is the same routine, and there is no way to determine if function is compiled for GUI or CUI mode (this is a runtime-thing), this routine can't be set as noreturn.

     
    • Is the fact that mingw's assert() might return breaking C99 ? My reading of the C99 spec is that assert() calls abort() and 'The abort function does not return to its caller'.

      However, if mingw is wiring it through a gui I can see the problem.

       
  • Kai Tietz
    Kai Tietz
    2015-02-05

    It is not that much that mingw actively breaking C99, it is more the point that mingw depends on msvcrt.dll (produced by Microsoft). Later does this, and we need to play nice to be compatible.

     
    • Ah ok, that's fair enough (on your part, but what were they thinking...)
      Hmm, I can't think of a way to stop gcc trying to pick up on the evaluation and causing bounds warnings on it.
      It may be worth adding a comment to your assert.h to explain that weirdness to stop people like me coming here and asking again.