On 21 July 2016 at 17:38, Andrew Fish <af...@apple.com> wrote: > >> On Jul 21, 2016, at 4:00 AM, Ard Biesheuvel <ard.biesheu...@linaro.org> >> wrote: >> >> On 20 July 2016 at 23:09, Andrew Fish <af...@apple.com> wrote: >>> It looks like clang has added a warning to detect infinite recursion and >>> I'm seeing the failure here: >>> >>> https://github.com/tianocore/edk2/blob/master/StdLib/LibC/StdLib/Environs.c#L123 >>> void >>> _Exit(int status) >>> { >>> gMD->ExitValue = status; // Save our exit status. Allows a >>> status of 0. >>> longjmp(gMD->MainExit, 0x55); // Get out of here. longjmp can't >>> return 0. Use 0x55 for a non-zero value. >>> >>> #ifdef __GNUC__ >>> _Exit(status); /* Keep GCC happy - never reached */ >>> #endif >>> } >>> >>> This is the compiler warning. >>> >>> error: all paths through this function will call itself >>> [-Werror,-Winfinite-recursion] >>> >>> I fixed by replacing the infinite recursion with UNREACHABLE() but I'm not >>> sure what the rules are changing this chunk of code? >>> >> >> UNREACHABLE() resolves to an empty string on GCC44 [or it will as soon >> as my patch that fixes that is merged, since 4.4 does not implement >> __builtin_unreachable()] >> >> The correct thing would be to set __attribute__((noreturn)) on the >> longjmp() prototype [if __GNUC__], but it would be interesting to >> understand what GCC was unhappy about to begin with. >> > > Ard, > > It looks like _Exit() is __noreturn and longjmp() is not. If I remove the > UNREACHABLE() then clang warns: > error: function declared 'noreturn' should not return > [-Werror,-Winvalid-noreturn] >
OK, thanks for clearing that up. > I noticed that a while (1); also fixes the issue. I wonder if UNREACHABLE() > should map to while(1) if __builtin_unreachable() does not exist. > AFAIU it amounts to the same thing, i.e., the compiler will infer that the code after the while (1) is not reachable. The only question is if that is always the case, for each version of each compiler that #defines __GNUC__ > I'm not sure longjmp() is noreturn as it is really a non local goto. Maybe it > is a legacy issue as marking it as noreturn could break existing code (You > could start getting unreachable code warnings)? > longjmp() is the mentioned in the __noreturn documentation, and it seems to me that 'returning' in this context is defined in relation to whether the stack frame is reused after the call. For the same reason, setjmp() is mentioned in the docs of the returns_twice attribute. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
